Validate copy sources with HEAD instead of downloading the blob - #2680
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes blob copy-source validation in Azurite by switching from a metadata GET (which Azurite currently serves as a full blob download) to a bodiless HEAD request, avoiding unnecessary data transfer and preventing axios decompression failures when the source declares Content-Encoding: gzip over non-gzip bytes.
Changes:
- Update copy-source validation to use
HEAD(Get Blob Properties) and retry withGET ?comp=metadataonly when needed, with axios decompression disabled on the retry. - Add a SAS-based regression test ensuring copy succeeds when the source declares
Content-Encoding: gzipbut the payload is not gzipped. - Document the behavior change in
ChangeLog.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/blob/sas.test.ts | Adds regression coverage for copy with misleading Content-Encoding: gzip on the source blob. |
| src/blob/handlers/BlobHandler.ts | Switches copy-source validation to HEAD, with a conditional GET ?comp=metadata retry to preserve error details and avoid decompression issues. |
| ChangeLog.md | Notes the optimization/fix in the upcoming release section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await blob2.beginCopyFromURL(blob1.url); | ||
|
|
||
| const properties = await blob2.getProperties(); |
|
Andrew Gaul (@gaul) , could you please refresh your PR with main, and address the review comments if any to move this PR forward. |
e601d9a to
8d754f9
Compare
Done. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/blob/handlers/BlobHandler.ts:744
- The archive-tier special-case here forces a GET
?comp=metadataeven when the HEAD succeeds. This validation helper doesn’t have the destinationtieroption available, so it can incorrectly block scenarios that the metadata store explicitly allows (e.g., same-account copy from an Archive blob when a destination tier is specified), and it also relies on?comp=metadatabehaving like a download to fail (butBlobHandler.getProperties()handlescomp=metadataas a 200 and does not enforce Archive restrictions).
Consider limiting the retry GET to non-200 responses (auth/404/etc.) and let the actual copy path enforce Archive semantics based on the real copy options.
if (
validationResponse.status !== 200 ||
validationResponse.headers["x-ms-access-tier"] === "Archive"
) {
// Error details live only in response bodies, and reading an
Re: the suppressed comment on
So the archive-tier check is load-bearing rather than redundant. HEAD returns 200 for an archived source, so without the special case an archived source would pass validation where it previously failed. On the concern that this can block a same-account copy from an Archive blob when a destination tier is specified: that's an accurate description of the behavior, but it is pre-existing rather than introduced by this PR. There is a genuine inconsistency in that area worth noting separately: |
8d754f9 to
f1a258f
Compare
|
Andrew Gaul (@gaul), to avoid merge commit conflicts, you can add the changelog line at 2 different places in the section. |
startCopyFromURL and copyFromURL validate the copy source by fetching <source>?comp=metadata through axios. Azurite serves that request as a full Blob_Download (issue Azure#646), so every validated copy downloaded the entire source only to discard it, and a source blob declaring Content-Encoding: gzip over bytes that are not really gzip made axios decompression fail and turned every copy of that blob into a 500. Validate with a bodiless HEAD (Get Blob Properties) request instead. Error details live only in response bodies, and reading an archived source must fail the copy like the download did, so those cases repeat the request as the previous comp=metadata GET, now with axios decompression disabled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Done. |
f1a258f to
675f66b
Compare
9144a4b
into
Azure:main
The azureblob store copies a part with Put Block From URL, which no Azurite implemented: the first attempt came back 501, the store remembered that, and every UploadPartCopy afterwards fell back to streaming the range through s3proxy. Azure/Azurite#2681 implements the operation, source range and x-ms-source-if-* conditions included, and it sits past the startFrom commit this pin already named, so move the pin on to it rather than build twice. The lane takes the native path now: the multipart copy tests stage their blocks from a URL, and a condition the source does not meet comes back 412 SourceConditionNotMet, which is the 412 s3proxy owes the client. The bump also picks up Azure/Azurite#2680, which authorizes a classic copy's source with a HEAD instead of downloading the whole blob only to discard it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
startCopyFromURL and copyFromURL validate the copy source by fetching?comp=metadata through axios. Azurite serves that request as a full Blob_Download (issue #646), so every validated copy downloaded the entire source only to discard it, and a source blob declaring Content-Encoding: gzip over bytes that are not really gzip made axios decompression fail and turned every copy of that blob into a 500.
Validate with a bodiless HEAD (Get Blob Properties) request instead. Error details live only in response bodies, and reading an archived source must fail the copy like the download did, so those cases repeat the request as the previous comp=metadata GET, now with axios decompression disabled.