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 blobCtx = new BlobStorageContext(context); | ||
| const accountName = blobCtx.account!; | ||
| const containerName = blobCtx.container!; | ||
| const blobName = blobCtx.blob!; | ||
| const date = blobCtx.startTime!; | ||
|
|
| const sourceConditions = options.sourceModifiedAccessConditions || {}; | ||
| if (sourceConditions.sourceIfMatch !== undefined) { | ||
| headers["if-match"] = sourceConditions.sourceIfMatch; | ||
| } | ||
| if (sourceConditions.sourceIfNoneMatch !== undefined) { | ||
| headers["if-none-match"] = sourceConditions.sourceIfNoneMatch; | ||
| } | ||
| if (sourceConditions.sourceIfModifiedSince !== undefined) { | ||
| headers["if-modified-since"] = new Date( | ||
| sourceConditions.sourceIfModifiedSince | ||
| ).toUTCString(); | ||
| } | ||
| if (sourceConditions.sourceIfUnmodifiedSince !== undefined) { | ||
| headers["if-unmodified-since"] = new Date( | ||
| sourceConditions.sourceIfUnmodifiedSince | ||
| ).toUTCString(); | ||
| } |
| 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:
|
| const scheme = "encrypted" in rawRequest.socket ? "https" : "http"; | ||
| const pinnedUrl = | ||
| `${scheme}://127.0.0.1:${rawRequest.socket.localPort}` + | ||
| `${url.pathname}${url.search}`; |
| if (options.sourceRange !== undefined) { | ||
| headers.range = options.sourceRange; | ||
| } |
|
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
});
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. Supplied x-ms-source-content-md5 and x-ms-source-content-crc64 values are validated against the fetched bytes through computeAndValidateTransactionalChecksums, the same path Put Block uses, so malformed values return InvalidMd5 or InvalidHeaderValue, mismatches return Md5Mismatch or Crc64Mismatch, and supplying both returns BothCrc64AndMd5HeaderPresent. 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. 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>
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
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.