diff --git a/ChangeLog.md b/ChangeLog.md index 71f381dd4..c5ceb9d93 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -14,6 +14,7 @@ General: Blob: - Fixed block blob uploads with `If-None-Match: *` returning `BlobAlreadyExists` before validating an active lease, matching Azure Storage's `LeaseIdMissing` and lease mismatch error precedence. (issue #2637) +- Validate proposed blob and container lease IDs during acquire and change lease operations, returning `InvalidHeaderValue` for malformed GUID values to match Azure Storage. (issue #2367) - 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. diff --git a/src/blob/handlers/BlobHandler.ts b/src/blob/handlers/BlobHandler.ts index ca5206566..897c5fb9a 100644 --- a/src/blob/handlers/BlobHandler.ts +++ b/src/blob/handlers/BlobHandler.ts @@ -29,7 +29,8 @@ import { deserializePageBlobRangeHeader, deserializeRangeHeader, getBlobTagsCount, - validateBlobTag + validateBlobTag, + validateProposedLeaseId } from "../utils/utils"; import BaseHandler from "./BaseHandler"; import IPageBlobRangesManager from "./IPageBlobRangesManager"; @@ -371,6 +372,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { options: Models.BlobAcquireLeaseOptionalParams, context: Context ): Promise { + validateProposedLeaseId(options.proposedLeaseId, context.contextId); + const blobCtx = new BlobStorageContext(context); const account = blobCtx.account!; const container = blobCtx.container!; @@ -505,6 +508,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { options: Models.BlobChangeLeaseOptionalParams, context: Context ): Promise { + validateProposedLeaseId(proposedLeaseId, context.contextId); + const blobCtx = new BlobStorageContext(context); const account = blobCtx.account!; const container = blobCtx.container!; diff --git a/src/blob/handlers/ContainerHandler.ts b/src/blob/handlers/ContainerHandler.ts index 7563261b2..0db5d7f3f 100644 --- a/src/blob/handlers/ContainerHandler.ts +++ b/src/blob/handlers/ContainerHandler.ts @@ -16,7 +16,11 @@ import { EMULATOR_ACCOUNT_SKUNAME } from "../utils/constants"; import { DEFAULT_LIST_BLOBS_MAX_RESULTS } from "../utils/constants"; -import { getBlobTagsCount, removeQuotationFromListBlobEtag } from "../utils/utils"; +import { + getBlobTagsCount, + removeQuotationFromListBlobEtag, + validateProposedLeaseId +} from "../utils/utils"; import BaseHandler from "./BaseHandler"; import { BlobBatchHandler } from "./BlobBatchHandler"; @@ -423,6 +427,8 @@ export default class ContainerHandler extends BaseHandler options: Models.ContainerAcquireLeaseOptionalParams, context: Context ): Promise { + validateProposedLeaseId(options.proposedLeaseId, context.contextId); + const blobCtx = new BlobStorageContext(context); const accountName = blobCtx.account!; const containerName = blobCtx.container!; @@ -589,6 +595,8 @@ export default class ContainerHandler extends BaseHandler options: Models.ContainerChangeLeaseOptionalParams, context: Context ): Promise { + validateProposedLeaseId(proposedLeaseId, context.contextId); + 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 eb7a1c4a9..128117543 100644 --- a/src/blob/utils/utils.ts +++ b/src/blob/utils/utils.ts @@ -6,6 +6,20 @@ import { BlobTag, BlobTags } from "@azure/storage-blob"; import { TagContent } from "../persistence/QueryInterpreter/QueryNodes/IQueryNode"; import { computeTransactionalChecksums } from "../../common/utils/utils"; +const GUID_HEX = "[0-9a-fA-F]"; +const GUID_DASHED = `${GUID_HEX}{8}-${GUID_HEX}{4}-${GUID_HEX}{4}-${GUID_HEX}{4}-${GUID_HEX}{12}`; +const GUID_X_PREFIX = "0[xX]"; +const GUID_X_FORMAT = `\\{${GUID_X_PREFIX}${GUID_HEX}{8},${GUID_X_PREFIX}${GUID_HEX}{4},${GUID_X_PREFIX}${GUID_HEX}{4},\\{${GUID_X_PREFIX}${GUID_HEX}{2}(,${GUID_X_PREFIX}${GUID_HEX}{2}){7}\\}\\}`; +const AZURE_GUID_REGEX = new RegExp( + "^(" + + `${GUID_HEX}{32}` + + `|${GUID_DASHED}` + + `|\\{${GUID_DASHED}\\}` + + `|\\(${GUID_DASHED}\\)` + + `|${GUID_X_FORMAT}` + + ")$" +); + function decodeBase64HeaderValue(value: string): Buffer | undefined { if (value.length === 0) { return Buffer.alloc(0); @@ -100,6 +114,27 @@ export function validateTransactionalChecksumHeaders( return { md5, crc64 }; } +/** + * Validates x-ms-proposed-lease-id against the Azure accepted GUID string + * forms: 32 hex digits, dashed GUID, braced dashed GUID, parenthesized dashed + * GUID, and X-format GUID. Throws InvalidHeaderValue with header details when + * the supplied value is malformed. + */ +export function validateProposedLeaseId( + proposedLeaseId: string | undefined, + contextId: string | undefined +): void { + if ( + proposedLeaseId !== undefined && + !AZURE_GUID_REGEX.test(proposedLeaseId) + ) { + throw StorageErrorFactory.getInvalidHeaderValue(contextId, { + HeaderName: "x-ms-proposed-lease-id", + HeaderValue: proposedLeaseId + }); + } +} + /** * Computes MD5 and/or CRC-64/NVME from a stream in a single pass and validates * against the request-supplied values. Throws Md5Mismatch / Crc64Mismatch diff --git a/tests/blob/apis/appendblob.test.ts b/tests/blob/apis/appendblob.test.ts index dbe72e78d..da665fed5 100644 --- a/tests/blob/apis/appendblob.test.ts +++ b/tests/blob/apis/appendblob.test.ts @@ -774,7 +774,7 @@ describe("AppendBlobAPIs", () => { it("Append block lease condition should work @loki", async () => { await appendBlobClient.create(); - const leaseId = "abcdefg"; + const leaseId = "ca761232-ed42-11ce-bacd-00aa0057b223"; const blobLeaseClient = await appendBlobClient.getBlobLeaseClient(leaseId); await blobLeaseClient.acquireLease(20); @@ -802,7 +802,7 @@ describe("AppendBlobAPIs", () => { it("Append block should refresh lease state @loki", async () => { await appendBlobClient.create(); - const leaseId = "abcdefg"; + const leaseId = "3c7e72eb-b430-4526-bc53-d8ecef03798f"; const blobLeaseClient = await appendBlobClient.getBlobLeaseClient(leaseId); await blobLeaseClient.acquireLease(20); diff --git a/tests/blob/apis/blob.test.ts b/tests/blob/apis/blob.test.ts index fcc52a712..9ea5834b9 100644 --- a/tests/blob/apis/blob.test.ts +++ b/tests/blob/apis/blob.test.ts @@ -22,6 +22,14 @@ import { } from "../../testutils"; import CustomHeaderPolicyFactory from "../RequestPolicy/CustomHeaderPolicyFactory"; import RangePolicyFactory from "../RequestPolicy/RangePolicyFactory"; +import { + assertInvalidProposedLeaseId, + bracedGuid, + parenthesizedGuid, + xFormatGuid, + xFormatGuidUppercasePrefix, + xFormatGuidExtraClosingBrace +} from "./leaseTestUtils"; // Set true to enable debug log configLogger(false); @@ -744,6 +752,35 @@ describe("BlobAPIs", () => { ); }); + it("acquireLease_available_proposedLeaseId_guidFormats @loki @sql", async () => { + const duration = 30; + for (const guid of [ + bracedGuid, + parenthesizedGuid, + xFormatGuid, + xFormatGuidUppercasePrefix + ]) { + blobLeaseClient = blobClient.getBlobLeaseClient(guid); + const result = await blobLeaseClient.acquireLease(duration); + assert.equal(result.leaseId, guid); + + await blobLeaseClient.releaseLease(); + } + }); + + it("acquireLease_malformed_proposedLeaseId @loki @sql", async () => { + blobLeaseClient = blobClient.getBlobLeaseClient( + xFormatGuidExtraClosingBrace + ); + + try { + await blobLeaseClient.acquireLease(30); + assert.fail("Should not reach here"); + } catch (error) { + assertInvalidProposedLeaseId(error, xFormatGuidExtraClosingBrace); + } + }); + it("acquireLease_available_NoproposedLeaseId_infinite @loki @sql", async () => { const leaseResult = await blobLeaseClient.acquireLease(-1); const leaseId = leaseResult.leaseId; @@ -916,6 +953,43 @@ describe("BlobAPIs", () => { await blobLeaseClient.releaseLease(); }); + it("changeLease_malformed_proposedLeaseId @loki @sql", async () => { + const guid = "ca761232ed4211cebacd00aa0057b223"; + const invalidGuid = "not-a-guid"; + blobLeaseClient = blobClient.getBlobLeaseClient(guid); + await blobLeaseClient.acquireLease(30); + + try { + await blobLeaseClient.changeLease(invalidGuid); + assert.fail("Should not reach here"); + } catch (error) { + assertInvalidProposedLeaseId(error, invalidGuid); + } finally { + await blobLeaseClient.releaseLease(); + } + }); + + it("changeLease_available_proposedLeaseId_guidFormats @loki @sql", async () => { + const guid = "ca761232ed4211cebacd00aa0057b223"; + blobLeaseClient = blobClient.getBlobLeaseClient(guid); + await blobLeaseClient.acquireLease(30); + + try { + for (const proposedGuid of [ + bracedGuid, + parenthesizedGuid, + xFormatGuid, + xFormatGuidUppercasePrefix + ]) { + const result = await blobLeaseClient.changeLease(proposedGuid); + assert.equal(result.leaseId, proposedGuid); + blobLeaseClient = blobClient.getBlobLeaseClient(proposedGuid); + } + } finally { + await blobLeaseClient.releaseLease(); + } + }); + it("breakLease @loki @sql", async () => { const guid = "ca761232ed4211cebacd00aa0057b223"; const duration = 15; diff --git a/tests/blob/apis/blockblob.test.ts b/tests/blob/apis/blockblob.test.ts index b9b04ee18..0dfd0072c 100644 --- a/tests/blob/apis/blockblob.test.ts +++ b/tests/blob/apis/blockblob.test.ts @@ -110,7 +110,7 @@ describe("BlockBlobAPIs", () => { it("Block blob upload should refresh lease state @loki @sql", async () => { await blockBlobClient.upload('a', 1); - const leaseId = "abcdefg"; + const leaseId = "ca761232-ed42-11ce-bacd-00aa0057b223"; const blobLeaseClient = await blockBlobClient.getBlobLeaseClient(leaseId); await blobLeaseClient.acquireLease(20); diff --git a/tests/blob/apis/container.test.ts b/tests/blob/apis/container.test.ts index 5900269ba..e93679605 100644 --- a/tests/blob/apis/container.test.ts +++ b/tests/blob/apis/container.test.ts @@ -24,6 +24,14 @@ import { sleep } from "../../testutils"; import QueryRequestPolicyFactory from "../RequestPolicy/QueryRequestPolicyFactory"; +import { + assertInvalidProposedLeaseId, + bracedGuid, + parenthesizedGuid, + xFormatGuid, + xFormatGuidUppercasePrefix, + xFormatGuidExtraClosingBrace +} from "./leaseTestUtils"; // Set to true enable debug log configLogger(false); @@ -446,6 +454,35 @@ describe("ContainerAPIs", () => { ); }); + it("acquireLease_available_proposedLeaseId_guidFormats @loki @sql", async () => { + const duration = 30; + for (const guid of [ + bracedGuid, + parenthesizedGuid, + xFormatGuid, + xFormatGuidUppercasePrefix + ]) { + blobLeaseClient = containerClient.getBlobLeaseClient(guid); + const result = await blobLeaseClient.acquireLease(duration); + assert.equal(result.leaseId, guid); + + await blobLeaseClient.releaseLease(); + } + }); + + it("acquireLease_malformed_proposedLeaseId @loki @sql", async () => { + blobLeaseClient = containerClient.getBlobLeaseClient( + xFormatGuidExtraClosingBrace + ); + + try { + await blobLeaseClient.acquireLease(30); + assert.fail("Should not reach here"); + } catch (error) { + assertInvalidProposedLeaseId(error, xFormatGuidExtraClosingBrace); + } + }); + it("acquireLease_available_NoproposedLeaseId_infinite @loki @sql", async () => { const leaseResult = await blobLeaseClient.acquireLease(-1); const leaseId = leaseResult.leaseId; @@ -525,6 +562,43 @@ describe("ContainerAPIs", () => { await blobLeaseClient.releaseLease(); }); + it("changeLease_malformed_proposedLeaseId @loki @sql", async () => { + const guid = "ca761232ed4211cebacd00aa0057b223"; + const invalidGuid = "not-a-guid"; + blobLeaseClient = containerClient.getBlobLeaseClient(guid); + await blobLeaseClient.acquireLease(30); + + try { + await blobLeaseClient.changeLease(invalidGuid); + assert.fail("Should not reach here"); + } catch (error) { + assertInvalidProposedLeaseId(error, invalidGuid); + } finally { + await blobLeaseClient.releaseLease(); + } + }); + + it("changeLease_available_proposedLeaseId_guidFormats @loki @sql", async () => { + const guid = "ca761232ed4211cebacd00aa0057b223"; + blobLeaseClient = containerClient.getBlobLeaseClient(guid); + await blobLeaseClient.acquireLease(30); + + try { + for (const proposedGuid of [ + bracedGuid, + parenthesizedGuid, + xFormatGuid, + xFormatGuidUppercasePrefix + ]) { + const result = await blobLeaseClient.changeLease(proposedGuid); + assert.equal(result.leaseId, proposedGuid); + blobLeaseClient = containerClient.getBlobLeaseClient(proposedGuid); + } + } finally { + await blobLeaseClient.releaseLease(); + } + }); + it("breakLease @loki @sql", async () => { const guid = "ca761232ed4211cebacd00aa0057b223"; const duration = 15; diff --git a/tests/blob/apis/leaseTestUtils.ts b/tests/blob/apis/leaseTestUtils.ts new file mode 100644 index 000000000..154659069 --- /dev/null +++ b/tests/blob/apis/leaseTestUtils.ts @@ -0,0 +1,26 @@ +import * as assert from "assert"; + +export const bracedGuid = "{ca761232-ed42-11ce-bacd-00aa0057b223}"; +export const parenthesizedGuid = "(ca761232-ed42-11ce-bacd-00aa0057b223)"; +export const xFormatGuid = + "{0xca761232,0xed42,0x11ce,{0xba,0xcd,0x00,0xaa,0x00,0x57,0xb2,0x23}}"; +export const xFormatGuidUppercasePrefix = + "{0Xca761232,0Xed42,0X11ce,{0Xba,0Xcd,0X00,0Xaa,0X00,0X57,0Xb2,0X23}}"; +export const xFormatGuidExtraClosingBrace = `${xFormatGuid}}`; + +export function assertInvalidProposedLeaseId( + error: any, + headerValue: string +): void { + assert.deepStrictEqual(error.statusCode, 400); + assert.deepStrictEqual(error.code, "InvalidHeaderValue"); + assert.deepStrictEqual(error.details.errorCode, "InvalidHeaderValue"); + assert.deepStrictEqual( + /([^<]*)([^<]*)