Fix issue 1954 & issue 1955 (#2178)

Azure · Sep 22, 2023 · 47b0e90 · 47b0e90
1 parent 808c655
commit 47b0e90
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 4 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -9,6 +9,8 @@ Blob:
 - Fix validation of Blob SAS token when using the second key for an account in `AZURITE_ACCOUNTS`
 - Set accessTierInferred to false after upload blob with accessTier (issue #2038)
 - Support blob new access tier Cold
+- Fixed startCopyFromURL, copyFromURL API to return 400 (InvalidHeaderValue) when copy source has invalid format. (issue #1954)
+- Fixed CommitBlockList API to return 400 (InvalidXmlDocument) when the request is sent with JSON body. (issue #1955)
 - Added "x-ms-is-hns-enabled" header in x-ms-is-hns-enabled API responds (issue #1810)
 
 Queue:

diff --git a/src/blob/errors/StorageErrorFactory.ts b/src/blob/errors/StorageErrorFactory.ts
@@ -807,5 +807,16 @@ export default class StorageErrorFactory {
       "The tags specified are invalid. It contains characters that are not permitted.",
       contextID
     );
+  }  
+
+  public static getInvaidXmlDocument(
+    contextID: string = ""
+  ): StorageError {
+    return new StorageError(
+      400,
+      "InvaidXmlDocument",
+      `XML specified is not syntactically valid.`,
+      contextID
+    );
   }
 }
diff --git a/src/blob/handlers/BlobHandler.ts b/src/blob/handlers/BlobHandler.ts
@@ -648,7 +648,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler {
     const blob = blobCtx.blob!;
 
     // TODO: Check dest Lease status, and set to available if it's expired, see sample in BlobHandler.setMetadata()
-    const url = new URL(copySource);
+    const url = this.NewUriFromCopySource(copySource, context);
     const [
       sourceAccount,
       sourceContainer,
@@ -709,7 +709,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler {
     const blobCtx = new BlobStorageContext(context);
 
     const currentServer = blobCtx.request!.getHeader("Host") || "";
-    const url = new URL(copySource)
+    const url = this.NewUriFromCopySource(copySource, context);
     if (currentServer !== url.host) {
       this.logger.error(
         `BlobHandler:startCopyFromURL() Source account ${url} is not on the same Azurite instance as target account ${blobCtx.account}`,
@@ -846,7 +846,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler {
     const blob = blobCtx.blob!;
 
     // TODO: Check dest Lease status, and set to available if it's expired, see sample in BlobHandler.setMetadata()
-    const url = new URL(copySource);
+    const url = this.NewUriFromCopySource(copySource, context);
     const [
       sourceAccount,
       sourceContainer,
@@ -1314,4 +1314,19 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler {
 
     return response;
   }
+
+  private NewUriFromCopySource(copySource: string, context: Context): URL{
+      try{
+        return new URL(copySource)
+      }
+      catch
+      {
+        throw StorageErrorFactory.getInvalidHeaderValue(
+          context.contextId,
+          {
+            HeaderName: "x-ms-copy-source",
+            HeaderValue: copySource
+          })
+      }
+  }
 }
diff --git a/src/blob/handlers/BlockBlobHandler.ts b/src/blob/handlers/BlockBlobHandler.ts
@@ -298,7 +298,14 @@ export default class BlockBlobHandler
     if (rawBody === undefined) {
       throw badRequestError;
     }
-    const parsed = await parseXML(rawBody, true);
+
+    let parsed;
+    try {
+      parsed = await parseXML(rawBody, true);
+    } catch (err) {
+      // return the 400(InvalidXmlDocument) error for issue 1955
+      throw StorageErrorFactory.getInvaidXmlDocument(context.contextId);
+    }
 
     // Validate selected block list
     const commitBlockList = [];

diff --git a/tests/blob/apis/blob.test.ts b/tests/blob/apis/blob.test.ts
@@ -1146,6 +1146,23 @@ describe("BlobAPIs", () => {
     assert.deepStrictEqual(err.statusCode, 400);
   });
 
+  it("Copy blob should fail with 400 when copy source is invalid @loki", async () => {
+    const destBlob = getUniqueName("blob");
+
+    const destBlobClient = containerClient.getBlockBlobClient(destBlob);
+
+    try {
+      await destBlobClient.beginCopyFromURL('/devstoreaccount1/container78/blob125')
+    } 
+    catch (error) 
+    {
+      assert.deepStrictEqual(error.statusCode, 400);
+      assert.deepStrictEqual(error.code, 'InvalidHeaderValue');      
+      return;
+    }
+    assert.fail();
+  });
+
   it("Copy blob should not work with  ifNoneMatch * when dest exist @loki", async () => {
     const sourceBlob = getUniqueName("blob");
     const destBlob = getUniqueName("blob");