controllers/krate/publish: Validate JSON metadata before reading tarball

[Turbo87]

Previously, the BytesRequest allocated memory for the full tarball (and the JSON metadata blob) before even validating that the name and vers fields of the JSON metadata correspond to a crate and version that the user has publish access too. This commit changes the code to first read the JSON metadata from the request body stream, validate it, and then read the tarball bytes afterwards.

Unfortunately, this means that we still buffer the full tarball in memory, but since we only want to upload the tarball to S3 once we validated the content of the tarball, this is somewhat hard to fix. Ideas are welcome! 😉

[codecov]

Codecov Report

Attention: Patch coverage is 91.17647% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.49%. Comparing base (154e004) to head (1fb5bed).

Files with missing lines	Patch %	Lines
src/controllers/krate/publish.rs	87.75%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10069      +/-   ##
==========================================
- Coverage   89.50%   89.49%   -0.01%     
==========================================
  Files         295      295              
  Lines       31257    31309      +52     
==========================================
+ Hits        27976    28021      +45     
- Misses       3281     3288       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[eth3lbert]

nice, I'll have a look tomorrow :D

[eth3lbert]

LGTM 👍

[eth3lbert]

Unfortunately, this means that we still buffer the full tarball in memory, but since we only want to upload the tarball to S3 once we validated the content of the tarball, this is somewhat hard to fix. Ideas are welcome!

Yeah, it would be great if we could stream uploads to S3 and abort them if they're invalid. But this might incur additional costs(?).

[Turbo87]

But this might incur additional costs(?).

yeah, that's the part that I'm slightly worried about. not sure if we can protect against that in a reasonable way.