diff --git a/ChangeLog.md b/ChangeLog.md index 900ec37a2..bd9c231f2 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -27,6 +27,7 @@ Blob: - Fixed service- and container-level Filter Blobs requests failing when the optional `where` query parameter is omitted. - Fixed blob operations hanging when a client disconnects before the operation queue processes the request. (issue #2575) - Implement `PutBlobFromUrl` (`Put Blob From URL`), which previously returned 501. The source is fetched over loopback, as `PutBlockFromURL` already does, so that SAS authentication and the `x-ms-source-if-*` conditions are enforced by the existing download path. Standard blob properties are copied from the source unless `x-ms-copy-source-blob-properties` is false, request blob content headers override them either way, request metadata replaces the source's rather than adding to it, and `x-ms-copy-source-tag-option: COPY` reads the source's tags over that same authorized path. An `x-ms-source-content-md5`, `x-ms-blob-content-md5`, `Content-MD5`, or `x-ms-content-crc64` header is checked against the copied content, and the response reports the MD5 and CRC64 of that content. A SAS needs Create or Write to create the blob, Write to overwrite it, and Tag as well when the request sets tags with `x-ms-tags` or copies the source's. As with `CopyBlobFromURL`, only sources on the same Azurite instance are supported. +- Fixed content writes addressed to a blob snapshot URL so they now fail with `400 InvalidOperation` instead of silently modifying the base blob. A snapshot is read-only, so `Put Blob`, `Put Block`, `Put Block List`, `Put Page`, `Clear Page`, `Append Block`, page blob resize and sequence number updates, and `Copy Blob` no longer fall through to the base blob when the request URL names a snapshot. (issue #2854) Queue: diff --git a/src/blob/handlers/AppendBlobHandler.ts b/src/blob/handlers/AppendBlobHandler.ts index 20a7627c2..328da2b20 100644 --- a/src/blob/handlers/AppendBlobHandler.ts +++ b/src/blob/handlers/AppendBlobHandler.ts @@ -15,7 +15,11 @@ import { MAX_APPEND_BLOB_BLOCK_COUNT, MAX_APPEND_BLOB_BLOCK_SIZE } from "../utils/constants"; -import { computeAndValidateTransactionalChecksums, getTagsFromString } from "../utils/utils"; +import { + computeAndValidateTransactionalChecksums, + getTagsFromString, + validateWriteTargetIsNotSnapshot +} from "../utils/utils"; import BaseHandler from "./BaseHandler"; export default class AppendBlobHandler extends BaseHandler @@ -25,6 +29,7 @@ export default class AppendBlobHandler extends BaseHandler options: Models.AppendBlobCreateOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; @@ -108,6 +113,7 @@ export default class AppendBlobHandler extends BaseHandler options: Models.AppendBlobAppendBlockOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; @@ -224,6 +230,7 @@ export default class AppendBlobHandler extends BaseHandler options: Models.AppendBlobSealOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; diff --git a/src/blob/handlers/BlobHandler.ts b/src/blob/handlers/BlobHandler.ts index acbbe5c23..2ef5264b6 100644 --- a/src/blob/handlers/BlobHandler.ts +++ b/src/blob/handlers/BlobHandler.ts @@ -32,6 +32,7 @@ import { isValidBlobTimestamp, validateBlobTag, validateProposedLeaseId, + validateWriteTargetIsNotSnapshot, } from "../utils/utils"; import BaseHandler from "./BaseHandler"; import IPageBlobRangesManager from "./IPageBlobRangesManager"; @@ -666,6 +667,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { options: Models.BlobStartCopyFromURLOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const account = blobCtx.account!; const container = blobCtx.container!; @@ -895,6 +897,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { options: Models.BlobCopyFromURLOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const account = blobCtx.account!; const container = blobCtx.container!; diff --git a/src/blob/handlers/BlockBlobHandler.ts b/src/blob/handlers/BlockBlobHandler.ts index 43d28fd0d..ef25d8646 100644 --- a/src/blob/handlers/BlockBlobHandler.ts +++ b/src/blob/handlers/BlockBlobHandler.ts @@ -23,7 +23,8 @@ import BaseHandler from "./BaseHandler"; import { computeAndValidateTransactionalChecksums, getTagsFromString, - validateTransactionalChecksumHeaders + validateTransactionalChecksumHeaders, + validateWriteTargetIsNotSnapshot } from "../utils/utils"; /** @@ -86,6 +87,7 @@ export default class BlockBlobHandler context: Context ): Promise { // TODO: Check Lease status, and set to available if it's expired, see sample in BlobHandler.setMetadata() + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; @@ -215,6 +217,7 @@ export default class BlockBlobHandler options: Models.BlockBlobPutBlobFromUrlOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; @@ -442,6 +445,7 @@ export default class BlockBlobHandler options: Models.BlockBlobStageBlockOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; @@ -540,6 +544,7 @@ export default class BlockBlobHandler options: Models.BlockBlobStageBlockFromURLOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; @@ -693,6 +698,7 @@ export default class BlockBlobHandler options: Models.BlockBlobCommitBlockListOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; diff --git a/src/blob/handlers/PageBlobHandler.ts b/src/blob/handlers/PageBlobHandler.ts index 16e8e0230..bc57af7de 100644 --- a/src/blob/handlers/PageBlobHandler.ts +++ b/src/blob/handlers/PageBlobHandler.ts @@ -16,7 +16,8 @@ import { BLOB_API_VERSION, HeaderConstants } from "../utils/constants"; import { computeAndValidateTransactionalChecksums, deserializePageBlobRangeHeader, - getTagsFromString + getTagsFromString, + validateWriteTargetIsNotSnapshot } from "../utils/utils"; import BaseHandler from "./BaseHandler"; import IPageBlobRangesManager from "./IPageBlobRangesManager"; @@ -58,6 +59,7 @@ export default class PageBlobHandler extends BaseHandler options: Models.PageBlobCreateOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; @@ -175,6 +177,7 @@ export default class PageBlobHandler extends BaseHandler options: Models.PageBlobUploadPagesOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; @@ -295,6 +298,7 @@ export default class PageBlobHandler extends BaseHandler options: Models.PageBlobClearPagesOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; @@ -439,6 +443,7 @@ export default class PageBlobHandler extends BaseHandler options: Models.PageBlobResizeOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; @@ -481,6 +486,7 @@ export default class PageBlobHandler extends BaseHandler options: Models.PageBlobUpdateSequenceNumberOptionalParams, context: Context ): Promise { + validateWriteTargetIsNotSnapshot(context); const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; diff --git a/src/blob/utils/utils.ts b/src/blob/utils/utils.ts index 023bf5396..73050bde6 100644 --- a/src/blob/utils/utils.ts +++ b/src/blob/utils/utils.ts @@ -4,6 +4,7 @@ import StorageErrorFactory from "../errors/StorageErrorFactory"; import { HeaderConstants, USERDELEGATIONKEY_SIGNING_SEED } from "./constants"; import { BlobTag, BlobTags } from "@azure/storage-blob"; import { TagContent } from "../persistence/QueryInterpreter/QueryNodes/IQueryNode"; +import Context from "../generated/Context"; import { computeTransactionalChecksums } from "../../common/utils/utils"; const GUID_HEX = "[0-9a-fA-F]"; @@ -531,3 +532,25 @@ export function toBlobTags(input: TagContent[]): BlobTag[] { } }); } + +/** + * Rejects a request that would modify blob content through a snapshot URL. + * + * A snapshot is an immutable copy of a blob, so the `snapshot` query parameter + * is only meaningful for operations that read a specific snapshot (or delete + * it). Content-write operations used to ignore the parameter, which meant that + * a Put Blob addressing a snapshot silently replaced the base blob instead of + * failing. + * + * @export + * @param {Context} context + */ +export function validateWriteTargetIsNotSnapshot(context: Context): void { + const snapshot = context.request?.getQuery("snapshot"); + if (snapshot !== undefined && snapshot !== "") { + throw StorageErrorFactory.getInvalidOperation( + context.contextId, + "Invalid operation against a blob snapshot." + ); + } +} diff --git a/tests/blob/apis/blob.test.ts b/tests/blob/apis/blob.test.ts index 902fc7503..5dc27ef11 100644 --- a/tests/blob/apis/blob.test.ts +++ b/tests/blob/apis/blob.test.ts @@ -44,6 +44,30 @@ import { // Set true to enable debug log configLogger(false); +/** + * A snapshot is read-only for blob content, so a write addressed to a snapshot + * URL has to fail instead of landing on the base blob. Assert the request was + * rejected the way Azure rejects it and never reached the base blob. + */ +async function assertSnapshotWriteRejected( + write: Promise, + operation: string +): Promise { + let error: any; + try { + await write; + } catch (err) { + error = err; + } + assert.ok(error, `${operation} to a snapshot URL should fail`); + assert.strictEqual(error.statusCode, 400, `${operation} should return 400`); + assert.strictEqual( + error.code, + "InvalidOperation", + `${operation} should report InvalidOperation` + ); +} + describe("BlobAPIs", () => { const factory = new BlobTestServerFactory(); const server = factory.createServer(); @@ -887,6 +911,61 @@ describe("BlobAPIs", () => { } }); + it("should reject content writes addressed to a snapshot and keep the base blob intact @loki @sql", async () => { + const snapshot = await blockBlobClient.createSnapshot(); + const snapshotClient = blockBlobClient.withSnapshot(snapshot.snapshot!); + + await assertSnapshotWriteRejected( + snapshotClient.upload("replaced", 8), + "Put Blob" + ); + await assertSnapshotWriteRejected( + snapshotClient.stageBlock("block-1", "x", 1), + "Put Block" + ); + await assertSnapshotWriteRejected( + snapshotClient.commitBlockList(["block-1"]), + "Put Block List" + ); + await assertSnapshotWriteRejected( + snapshotClient.syncCopyFromURL(blobClient.url), + "Copy From URL" + ); + + const pageBlobClient = containerClient.getPageBlobClient( + getUniqueName("pageblob") + ); + await pageBlobClient.create(512); + const pageSnapshot = await pageBlobClient.createSnapshot(); + await assertSnapshotWriteRejected( + pageBlobClient + .withSnapshot(pageSnapshot.snapshot!) + .uploadPages(Buffer.alloc(512), 0, 512), + "Put Page" + ); + + const appendBlobClient = containerClient.getAppendBlobClient( + getUniqueName("appendblob") + ); + await appendBlobClient.create(); + const appendSnapshot = await appendBlobClient.createSnapshot(); + await assertSnapshotWriteRejected( + appendBlobClient + .withSnapshot(appendSnapshot.snapshot!) + .appendBlock("x", 1), + "Append Block" + ); + + // None of the rejected writes may touch the base blob. + const properties = await blobClient.getProperties(); + assert.strictEqual(properties.contentLength, content.length); + const downloaded = await blobClient.download(0); + assert.strictEqual( + await bodyToString(downloaded, content.length), + content + ); + }); + it("acquireLease_available_proposedLeaseId_fixed @loki @sql", async () => { const guid = "ca761232ed4211cebacd00aa0057b223"; const duration = 30;