Prevents invalid metadata names in Blob APIs (#2453)

* do not allow invalid metadata names in blob APIs * update chanelog * more test coverage
Azure · Aug 28, 2024 · 76f6262 · 76f6262
1 parent 331dff2
commit 76f6262
Show file tree

Hide file tree

Showing 13 changed files with 151 additions and 19 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -4,6 +4,10 @@
 
 ## Upcoming Release
 
+Blob:
+
+- Fixed an issue where all blob APIs allowed metadata names which were not valid C# identifiers.
+
 ## 2024.08 Version 3.32.0
 
 General:

diff --git a/src/blob/handlers/AppendBlobHandler.ts b/src/blob/handlers/AppendBlobHandler.ts
@@ -45,7 +45,7 @@ export default class AppendBlobHandler extends BaseHandler
 
     // Preserve metadata key case
     const metadata = convertRawHeadersToMetadata(
-      blobCtx.request!.getRawHeaders()
+      blobCtx.request!.getRawHeaders(), context.contextId!
     );
 
     const blob: BlobModel = {

diff --git a/src/blob/handlers/BlobHandler.ts b/src/blob/handlers/BlobHandler.ts
@@ -327,17 +327,9 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler {
 
     // Preserve metadata key case
     const metadata = convertRawHeadersToMetadata(
-      blobCtx.request!.getRawHeaders()
+      blobCtx.request!.getRawHeaders(), context.contextId!
     );
 
-    if (metadata != undefined) {
-      Object.entries(metadata).forEach(([key, value]) => {
-        if (key.includes("-")) {
-          throw StorageErrorFactory.getInvalidMetadata(context.contextId!);
-        }
-      });
-    }
-
     const res = await this.metadataStore.setBlobMetadata(
       context,
       account,
@@ -597,7 +589,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler {
 
     // Preserve metadata key case
     const metadata = convertRawHeadersToMetadata(
-      blobCtx.request!.getRawHeaders()
+      blobCtx.request!.getRawHeaders(), context.contextId!
     );
 
     const res = await this.metadataStore.createSnapshot(
@@ -669,7 +661,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler {
 
     // Preserve metadata key case
     const metadata = convertRawHeadersToMetadata(
-      blobCtx.request!.getRawHeaders()
+      blobCtx.request!.getRawHeaders(), context.contextId!
     );
 
     const res = await this.metadataStore.startCopyFromURL(
@@ -872,7 +864,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler {
 
     // Preserve metadata key case
     const metadata = convertRawHeadersToMetadata(
-      blobCtx.request!.getRawHeaders()
+      blobCtx.request!.getRawHeaders(), context.contextId!
     );
 
     const res = await this.metadataStore.copyFromURL(

diff --git a/src/blob/handlers/BlockBlobHandler.ts b/src/blob/handlers/BlockBlobHandler.ts
@@ -97,7 +97,7 @@ export default class BlockBlobHandler
     const blob: BlobModel = {
       deleted: false,
       // Preserve metadata key case
-      metadata: convertRawHeadersToMetadata(blobCtx.request!.getRawHeaders()),
+      metadata: convertRawHeadersToMetadata(blobCtx.request!.getRawHeaders(), context.contextId!),
       accountName,
       containerName,
       name: blobName,
@@ -345,7 +345,7 @@ export default class BlockBlobHandler
     blob.properties.blobType = Models.BlobType.BlockBlob;
     blob.metadata = convertRawHeadersToMetadata(
       // Preserve metadata key case
-      blobCtx.request!.getRawHeaders()
+      blobCtx.request!.getRawHeaders(), context.contextId!
     );
     blob.properties.accessTier = Models.AccessTier.Hot;
     blob.properties.cacheControl = options.blobHTTPHeaders.blobCacheControl;

diff --git a/src/blob/handlers/ContainerHandler.ts b/src/blob/handlers/ContainerHandler.ts
@@ -64,7 +64,7 @@ export default class ContainerHandler extends BaseHandler
 
     // Preserve metadata key case
     const metadata = convertRawHeadersToMetadata(
-      blobCtx.request!.getRawHeaders()
+      blobCtx.request!.getRawHeaders(), context.contextId!
     );
 
     await this.metadataStore.createContainer(context, {
@@ -205,7 +205,7 @@ export default class ContainerHandler extends BaseHandler
 
     // Preserve metadata key case
     const metadata = convertRawHeadersToMetadata(
-      blobCtx.request!.getRawHeaders()
+      blobCtx.request!.getRawHeaders(), context.contextId!
     );
 
     await this.metadataStore.setContainerMetadata(

diff --git a/src/blob/handlers/PageBlobHandler.ts b/src/blob/handlers/PageBlobHandler.ts
@@ -100,7 +100,7 @@ export default class PageBlobHandler extends BaseHandler
 
     // Preserve metadata key case
     const metadata = convertRawHeadersToMetadata(
-      blobCtx.request!.getRawHeaders()
+      blobCtx.request!.getRawHeaders(), context.contextId!
     );
 
     const etag = newEtag();

diff --git a/src/common/utils/constants.ts b/src/common/utils/constants.ts
@@ -58,3 +58,5 @@ export const EMULATOR_ACCOUNT_KEY = Buffer.from(
   "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==",
   "base64"
 );
+
+export const VALID_CSHARP_IDENTIFIER_REGEX = /^[a-zA-Z_][a-zA-Z0-9_]*$/;
diff --git a/src/common/utils/utils.ts b/src/common/utils/utils.ts
@@ -2,6 +2,8 @@ import { createHash, createHmac } from "crypto";
 import rimraf = require("rimraf");
 import { parse } from "url";
 import { promisify } from "util";
+import StorageErrorFactory from "../../blob/errors/StorageErrorFactory";
+import { VALID_CSHARP_IDENTIFIER_REGEX } from "./constants";
 
 // LokiFsStructuredAdapter
 // tslint:disable-next-line:no-var-requires
@@ -21,7 +23,7 @@ export function convertDateTimeStringMsTo7Digital(
 }
 
 export function convertRawHeadersToMetadata(
-  rawHeaders: string[] = []
+  rawHeaders: string[] = [], contextId: string = ""
 ): { [propertyName: string]: string } | undefined {
   const metadataPrefix = "x-ms-meta-";
   const res: { [propertyName: string]: string } = {};
@@ -34,6 +36,9 @@ export function convertRawHeadersToMetadata(
       header.length > metadataPrefix.length
     ) {
       const key = header.substr(metadataPrefix.length);
+      if (!key.match(VALID_CSHARP_IDENTIFIER_REGEX)) {
+        throw StorageErrorFactory.getInvalidMetadata(contextId);
+      }
       let value = rawHeaders[i + 1] || "";
       if (res[key] !== undefined) {
         value = `${res[key]},${value}`;

diff --git a/tests/blob/apis/appendblob.test.ts b/tests/blob/apis/appendblob.test.ts
@@ -133,6 +133,32 @@ describe("AppendBlobAPIs", () => {
     assert.deepStrictEqual(properties.blobCommittedBlockCount, 0);
   });
 
+  it("Create append blob should fail when metadata names are invalid C# identifiers @loki @sql", async () => {
+    let invalidNames = [
+      "1invalid",
+      "invalid.name",
+      "invalid-name",
+    ]
+    for (let i = 0; i < invalidNames.length; i++) {
+      const metadata = {
+        [invalidNames[i]]: "value"
+      };
+      let hasError = false;
+      try {
+        await appendBlobClient.create({
+          metadata: metadata
+        });
+      } catch (error) {
+        assert.deepStrictEqual(error.statusCode, 400);
+        assert.strictEqual(error.code, 'InvalidMetadata');
+        hasError = true;
+      }
+      if (!hasError) {
+        assert.fail();
+      }
+    }
+  });
+
   it("Delete append blob should work @loki", async () => {
     await appendBlobClient.create();
     await appendBlobClient.delete();

diff --git a/tests/blob/apis/blob.test.ts b/tests/blob/apis/blob.test.ts
@@ -496,6 +496,31 @@ describe("BlobAPIs", () => {
 
   });
 
+  it("should fail when upload has metadata names that are invalid C# identifiers @loki @sql", async () => {
+    let invalidNames = [
+      "1invalid",
+      "invalid.name",
+      "invalid-name",
+    ]
+    for (let i = 0; i < invalidNames.length; i++) {
+      const metadata = {
+        [invalidNames[i]]: "value"
+      };
+      let hasError = false;
+      try {
+        await blockBlobClient.upload(content, content.length, { metadata });
+      } catch (error) {
+        assert.deepStrictEqual(error.statusCode, 400);
+        assert.strictEqual(error.code, 'InvalidMetadata');
+        hasError = true;
+      }
+      if (!hasError)
+      {
+        assert.fail();
+      }
+    }
+  });
+
   it("acquireLease_available_proposedLeaseId_fixed @loki @sql", async () => {
     const guid = "ca761232ed4211cebacd00aa0057b223";
     const duration = 30;

diff --git a/tests/blob/apis/blockblob.test.ts b/tests/blob/apis/blockblob.test.ts
@@ -151,6 +151,32 @@ describe("BlockBlobAPIs", () => {
     );
   });
 
+  it("upload should fail when metadata names are invalid C# identifiers @loki @sql", async () => {
+    let invalidNames = [
+      "1invalid",
+      "invalid.name",
+      "invalid-name",
+    ]
+    for (let i = 0; i < invalidNames.length; i++) {
+      const metadata = {
+        [invalidNames[i]]: "value"
+      };
+      let hasError = false;
+      try {
+        await blockBlobClient.upload('b', 1, {
+          metadata: metadata
+        });
+      } catch (error) {
+        assert.deepStrictEqual(error.statusCode, 400);
+        assert.strictEqual(error.code, 'InvalidMetadata');
+        hasError = true;
+      }
+      if (!hasError) {
+        assert.fail();
+      }
+    }
+  });
+
   it("stageBlock @loki @sql", async () => {
     const body = "HelloWorld";
     const result_stage = await blockBlobClient.stageBlock(

diff --git a/tests/blob/apis/container.test.ts b/tests/blob/apis/container.test.ts
@@ -239,6 +239,32 @@ describe("ContainerAPIs", () => {
     done();
   });
 
+  it("create should fail when metadata names are invalid C# identifiers @loki @sql", async () => {
+    let invalidNames = [
+      "1invalid",
+      "invalid.name",
+      "invalid-name",
+    ]
+    for (let i = 0; i < invalidNames.length; i++) {
+      const metadata = {
+        [invalidNames[i]]: "value"
+      };
+      let hasError = false;
+      try {
+        const cURL = serviceClient.getContainerClient(getUniqueName(containerName));
+        const access = "container";
+        await cURL.create({ metadata, access });
+      } catch (error) {
+        assert.deepStrictEqual(error.statusCode, 400);
+        assert.strictEqual(error.code, 'InvalidMetadata');
+        hasError = true;
+      }
+      if (!hasError) {
+        assert.fail();
+      }
+    }
+  });
+
   it("listBlobHierarchySegment with default parameters @loki @sql", async () => {
     const blobClients = [];
     for (let i = 0; i < 3; i++) {

diff --git a/tests/blob/apis/pageblob.test.ts b/tests/blob/apis/pageblob.test.ts
@@ -145,6 +145,32 @@ describe("PageBlobAPIs", () => {
     );
   });
 
+  it("create should fail when metadata names are invalid C# identifiers @loki @sql", async () => {
+    let invalidNames = [
+      "1invalid",
+      "invalid.name",
+      "invalid-name",
+    ]
+    for (let i = 0; i < invalidNames.length; i++) {
+      const metadata = {
+        [invalidNames[i]]: "value"
+      };
+      let hasError = false;
+      try {
+        await pageBlobClient.create(512, {
+          metadata: metadata
+        });
+      } catch (error) {
+        assert.deepStrictEqual(error.statusCode, 400);
+        assert.strictEqual(error.code, 'InvalidMetadata');
+        hasError = true;
+      }
+      if (!hasError) {
+        assert.fail();
+      }
+    }
+  });
+
   it("download page blob with partial ranges @loki", async () => {
     const length = 512 * 10;
     await pageBlobClient.create(length);