Implement Put Block From URL - #2681
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for Block Blob Stage Block From URL (Put Block From URL) in Azurite by downloading the source content via the existing blob download path (so SAS/range/source conditions can be applied) and persisting the staged data through the normal extent + uncommitted-block flow.
Changes:
- Implements
stageBlockFromURLinBlockBlobHandlerby fetching the source via HTTP and staging it as an uncommitted block. - Introduces a new
SourceConditionNotMet(412) storage error for unmet source conditional headers during staging. - Expands the block blob API test suite with stageBlockFromURL coverage (range, full copy, unmet condition, missing source).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/blob/apis/blockblob.test.ts | Adds tests validating stageBlockFromURL behavior (ranges, full copy, 412 on unmet source condition, 404 on missing source). |
| src/blob/handlers/BlockBlobHandler.ts | Implements the stageBlockFromURL handler: validates input, downloads source data, persists it, and returns MD5. |
| src/blob/errors/StorageErrorFactory.ts | Adds getSourceConditionNotMet() (412) error factory helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const currentServer = blobCtx.request!.getHeader("Host") || ""; | ||
| if (currentServer !== url.host) { | ||
| this.logger.error( | ||
| `BlockBlobHandler:stageBlockFromURL() Source ${url} is not on the same Azurite instance as target account ${accountName}`, | ||
| context.contextId | ||
| ); | ||
| throw StorageErrorFactory.getCannotVerifyCopySource( | ||
| context.contextId!, | ||
| 404, | ||
| "The specified resource does not exist" | ||
| ); | ||
| } |
|
Addressed the review in aa00b52:
|
|
Second round addressed in 381b126: the source fetch now pins to the local address the request arrived on (bracketing IPv6 literals) instead of hard-coded 127.0.0.1, so non-loopback |
|
|
||
| // Fetch the source range over loopback so that SAS authentication, | ||
| // range handling, and source conditions reuse the download path. | ||
| const headers: { [key: string]: string } = {}; |
|
Addressed in 8847aec: the pinned loopback fetch now sends the original source URL host as the |
| const currentServer = blobCtx.request!.getHeader("Host") || ""; | ||
| if (currentServer !== url.host) { |
|
Addressed in e665103: the same-instance comparison now lowercases the client-supplied Host header before comparing against the already-lowercased URL host, with a mixed-case Host regression test. |
e665103 to
d2654cb
Compare
|
Squashed after AI review succeeded. |
|
Andrew Gaul (@gaul) , could you please refresh your PR with main, and address the review comments if any to move this PR forward. |
d2654cb to
33059bb
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/BlockBlobHandler.ts:431
sourceContentMD5validation currently compares raw bytes and throwsInvalidOperationon mismatch. This diverges from other checksum validation paths in the codebase and fails to return the expectedInvalidMd5(wrong length) /Md5Mismatch(value mismatch) errors.
Consider validating that sourceContentMD5 is exactly 16 bytes and, on mismatch, throwing StorageErrorFactory.getMd5Mismatch(...) with base64-encoded values (same behavior as computeAndValidateTransactionalChecksums).
if (options.sourceContentMD5 !== undefined) {
if (
!Buffer.from(options.sourceContentMD5).equals(calculatedContentMD5)
) {
throw StorageErrorFactory.getInvalidOperation(
33059bb to
4f9ee08
Compare
Done. Added MD5 validation. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/blob/handlers/BlockBlobHandler.ts:426
- stageBlockFromURL validates only sourceContentMD5. If a client supplies x-ms-source-content-crc64 (options.sourceContentcrc64), it is currently ignored, and the request can incorrectly succeed even when CRC64 mismatches or when both MD5 and CRC64 are provided (Azure rejects both via BothCrc64AndMd5HeaderPresent). Pass both expected checksums into computeAndValidateTransactionalChecksums so header validation + mismatch behavior matches the existing transactional checksum semantics.
const { md5: calculatedContentMD5 } =
await computeAndValidateTransactionalChecksums(
stream,
{ md5: options.sourceContentMD5 },
context.contextId,
4f9ee08 to
a912e9d
Compare
Done. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/blob/handlers/BlockBlobHandler.ts:387
axios.get(pinnedUrl, ...)can throw (e.g., TLS certificate validation failures when Azurite is running with HTTPS + a self-signed cert, connection errors, etc.). Right now that exception will bypass the status-based mapping below and likely surface as an unhandled 500 rather than aCannotVerifyCopySource/SourceConditionNotMetStorageError. Consider wrapping the request in try/catch and translating failures into a deterministic StorageError (and logging the underlying error) so callers get a consistent Azure-like response.
const sourceResponse: AxiosResponse = await axios.get(pinnedUrl, {
headers,
responseType: "stream",
validateStatus: () => true
});
a912e9d to
01b187a
Compare
Done. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/blob/handlers/BlockBlobHandler.ts:399
- The internal axios GET may transparently decompress the response when the source blob has a
Content-Encoding(e.g., gzip) or when a proxy applies transfer compression based onAccept-Encoding. That would stage different bytes than the source and can also cause unexpected decompression errors. For byte-for-byte correctness, force identity transfer encoding and disable axios decompression for this loopback download.
sourceResponse = await axios.get(pinnedUrl, {
headers,
responseType: "stream",
validateStatus: () => true,
// The connection is pinned above to the address and port this very
01b187a to
8288a90
Compare
Done. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/blob/handlers/BlockBlobHandler.ts:412
- A new https.Agent is constructed for every stageBlockFromURL call. Creating a fresh Agent per request can add overhead and can retain sockets/listeners longer than necessary under load. Prefer reusing a shared Agent instance for these loopback self-requests (e.g., a module-level constant or a static class field).
httpsAgent:
scheme === "https"
? new Agent({ rejectUnauthorized: false })
: undefined
src/blob/handlers/BlockBlobHandler.ts:453
- If the internal loopback GET returns 200/206 but the body stream errors while being read (connection reset mid-stream, socket error, etc.), extentStore.appendExtent will reject and the error will currently escape without being mapped to an Azure-shaped CannotVerifyCopySource response (and without ensuring the source stream is destroyed). Wrapping the appendExtent call in a try/catch keeps stageBlockFromURL consistent with the earlier transport-error handling.
const persistency = await this.extentStore.appendExtent(
sourceResponse.data,
context.contextId
);
Done. |
8288a90 to
7eca854
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/blob/handlers/BlockBlobHandler.ts:494
- The extent read stream returned by extentStore.readExtent() is not cleaned up if computeAndValidateTransactionalChecksums() throws before consuming the stream (e.g., invalid sourceContentMD5/sourceContentcrc64). That can leave an FS read stream open. Also, invalid sourceContentcrc64 currently reports the header name as x-ms-content-crc64 via the shared helper, which doesn’t match the request header x-ms-source-content-crc64.
Wrap checksum computation in a try/finally that always destroys the stream, and pre-validate sourceContentcrc64 length (<8) here so InvalidHeaderValue references x-ms-source-content-crc64.
const stream = await this.extentStore.readExtent(
persistency,
context.contextId
);
const { md5: calculatedContentMD5 } =
Stage the block by fetching the copy source over loopback so that SAS authentication, x-ms-source-range, and the x-ms-source-if-* conditions are enforced by the existing download path, then persist it through the same extent flow as Put Block. Only sources on the same Azurite instance are supported, matching copyFromURL. The response carries the MD5 of the staged content and source condition failures return 412 SourceConditionNotMet as on the real service. Reject malformed x-ms-source-content-md5 and x-ms-source-content-crc64 up front so the errors name the header the caller sent, then compare the surviving values against the fetched bytes with computeAndValidateTransactionalChecksums, the same path Put Block uses. Mismatches return Md5Mismatch or Crc64Mismatch, and supplying both returns BothCrc64AndMd5HeaderPresent. The response reports the computed CRC64 when no MD5 was supplied, as stageBlock does. Request the source body verbatim and never decompress it. A blob's Content-Encoding is stored metadata rather than a description of the wire framing, so the download echoes it back over the raw stored bytes; decoding here would stage the decompressed content instead of what the source holds, and would fail outright when the property does not match the bytes (issue Azure#646). The source fetch is pinned to the address and port the request arrived on, so over HTTPS it presents whatever certificate Azurite was started with. Skip verification for that self-request, which would otherwise reject the self-signed certificates Azurite is normally run with and make the operation unusable under --cert/--key, and translate transport-level failures into CannotVerifyCopySource rather than letting them escape as a bodiless 500. The same applies to a body that fails partway through being read. Validated with the blockblob test suite and end to end with S3Proxy's native multipart part copy, which previously fell back to streamed emulation on Azurite's 501. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
7eca854 to
b23178e
Compare
Done. |
Stage the block by fetching the copy source over loopback so that SAS authentication, x-ms-source-range, and the x-ms-source-if-* conditions are enforced by the existing download path, then persist it through the same extent flow as Put Block. Only sources on the same Azurite instance are supported, matching copyFromURL. The response carries the MD5 of the staged content and source condition failures return 412 SourceConditionNotMet as on the real service.
Validated with the blockblob test suite and end to end with S3Proxy's native multipart part copy, which previously fell back to streamed emulation on Azurite's 501.