Validate proposed Blob and Container lease IDs

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.

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<Models.BlobAcquireLeaseResponse> {
+    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<Models.BlobChangeLeaseResponse> {
+    validateProposedLeaseId(proposedLeaseId, context.contextId);
+
     const blobCtx = new BlobStorageContext(context);
     const account = blobCtx.account!;
     const container = blobCtx.container!;

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<Models.ContainerAcquireLeaseResponse> {
+    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<Models.ContainerChangeLeaseResponse> {
+    validateProposedLeaseId(proposedLeaseId, context.contextId);
+
     const blobCtx = new BlobStorageContext(context);
     const accountName = blobCtx.account!;
     const containerName = blobCtx.container!;

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

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);
 

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;

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);
 

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;

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(
+    /<HeaderName>([^<]*)</.exec(error.response?.bodyAsText ?? "")?.[1],
+    "x-ms-proposed-lease-id"
+  );
+  assert.deepStrictEqual(
+    /<HeaderValue>([^<]*)</.exec(error.response?.bodyAsText ?? "")?.[1],
+    headerValue
+  );
+}