diff --git a/ChangeLog.md b/ChangeLog.md index 4053d41ac..ff49c163a 100644 --- 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 index d52bd788f..9731b7c71 100644 --- 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 index 981f9a278..ef3625cee 100644 --- 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 index 025809987..c3252161d 100644 --- 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 index 9fde0d7b4..c8d82d786 100644 --- 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 index 51ec16eaa..429d860ef 100644 --- 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 index 4ed88ae2d..c4fa903f5 100644 --- 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 index 8df02200f..3debf4f56 100644 --- 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 index 64ff23667..c0ec6b97f 100644 --- 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 index ac07bc396..5574a4f81 100644 --- 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 index b483a2e7a..a23a09b13 100644 --- 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 index ca9aa039a..3a51cb75c 100644 --- 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 index ac6ed07d2..e66ec5756 100644 --- 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);