fix(blob): return a snapshot from LokiBlobMetadataStore.downloadBlob so a read racing commitBlockList can't pair a new ETag with old content

ChangeLog.md

@@ -26,6 +26,7 @@ Blob:
 - 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)
+- Fixed a blob download that races a `Put Block List` on the same blob returning the new ETag with the old content. The default Loki metadata store now returns a snapshot of the blob's metadata for a download, so the ETag, properties, and block list in one response always come from the same blob version. (issue #2845)
 - 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.
 
 Queue:

src/blob/handlers/AppendBlobHandler.ts

@@ -131,7 +131,10 @@ export default class AppendBlobHandler extends BaseHandler
       accountName,
       containerName,
       blobName,
-      {}
+      {},
+      undefined,
+      undefined,
+      false // This write precheck does not need a copy of the block list.
     );
 
     if (blob.properties.blobType !== Models.BlobType.AppendBlob) {

src/blob/handlers/PageBlobHandler.ts

@@ -194,7 +194,9 @@ export default class PageBlobHandler extends BaseHandler
       containerName,
       blobName,
       {},
-      options.leaseAccessConditions
+      options.leaseAccessConditions,
+      undefined,
+      false // This write precheck does not need a copy of the page ranges.
     );
 
     if (blob.properties.blobType !== Models.BlobType.PageBlob) {
@@ -314,7 +316,9 @@ export default class PageBlobHandler extends BaseHandler
       containerName,
       blobName,
       {},
-      options.leaseAccessConditions
+      options.leaseAccessConditions,
+      undefined,
+      false // This write precheck does not need a copy of the page ranges.
     );
 
     if (blob.properties.blobType !== Models.BlobType.PageBlob) {

src/blob/persistence/IBlobMetadataStore.ts

@@ -615,6 +615,8 @@ export interface IBlobMetadataStore
   /**
    * Gets a blob item from metadata store by account name, container name and blob name.
    * Will return block list or page list as well for downloading.
+   * Write prechecks may disable the snapshot to avoid copying content metadata.
+   * Without a snapshot, the result may reference live metadata and must not be mutated.
    *
    * @param {Context} context
    * @param {string} account
@@ -623,6 +625,7 @@ export interface IBlobMetadataStore
    * @param {BlobSelector} selector Snapshot or version to target; empty targets the current blob
    * @param {Models.LeaseAccessConditions} [leaseAccessConditions] Optional. Will validate lease if provided
    * @param {Models.ModifiedAccessConditions} [modifiedAccessConditions]
+   * @param {boolean} [returnSnapshot=true] Isolate the result from subsequent writes.
    * @returns {Promise<BlobModel>}
    * @memberof IBlobMetadataStore
    */
@@ -633,7 +636,8 @@ export interface IBlobMetadataStore
     blob: string,
     selector: BlobSelector,
     leaseAccessConditions?: Models.LeaseAccessConditions,
-    modifiedAccessConditions?: Models.ModifiedAccessConditions
+    modifiedAccessConditions?: Models.ModifiedAccessConditions,
+    returnSnapshot?: boolean
   ): Promise<BlobModel>;
 
   /**

src/blob/persistence/LokiBlobMetadataStore.ts

@@ -1497,7 +1497,8 @@ export default class LokiBlobMetadataStore
     blob: string,
     selector: BlobSelector = {},
     leaseAccessConditions?: Models.LeaseAccessConditions,
-    modifiedAccessConditions?: Models.ModifiedAccessConditions
+    modifiedAccessConditions?: Models.ModifiedAccessConditions,
+    returnSnapshot: boolean = true
   ): Promise<BlobModel> {
     const snapshot = selector.snapshot ?? "";
     const versionId = selector.versionId ?? "";
@@ -1523,7 +1524,11 @@ export default class LokiBlobMetadataStore
       context
     );
 
-    return doc;
+    // Downloads need a snapshot. The handlers await between reading the block
+    // list and the ETag, and commitBlockList mutates this live Loki document in
+    // place, so a shared reference can pair a new ETag with old content.
+    // Write prechecks can skip the copy because persistence re-fetches the blob.
+    return returnSnapshot ? structuredClone(doc) : doc;
   }
 
   /**

src/blob/persistence/SqlBlobMetadataStore.ts

@@ -1190,8 +1190,10 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {
     blob: string,
     selector: BlobSelector = {},
     leaseAccessConditions?: Models.LeaseAccessConditions,
-    modifiedAccessConditions?: Models.ModifiedAccessConditions
+    modifiedAccessConditions?: Models.ModifiedAccessConditions,
+    returnSnapshot: boolean = true
   ): Promise<BlobModel> {
+    // SQL queries already return independent models, regardless of returnSnapshot.
     const snapshot = selector.snapshot ?? "";
     this.assertNoVersionId(context, selector.versionId);
     return this.sequelize.transaction(async (t) => {

tests/blob/handlers/AppendBlobHandler.test.ts

@@ -61,7 +61,10 @@ describe("AppendBlobHandler", () => {
       blobCtx.account,
       blobCtx.container,
       blobCtx.blob,
-      deepEqual({})
+      deepEqual({}),
+      undefined,
+      undefined,
+      false
     )
   ).thenResolve({
     name: blobCtx.blob,

tests/blob/unit/lokiBlobMetadataStore.unit.test.ts

@@ -0,0 +1,147 @@
+import * as assert from "assert";
+
+import Context from "../../../src/blob/generated/Context";
+import * as Models from "../../../src/blob/generated/artifacts/models";
+import {
+  BlobModel,
+  BlockModel
+} from "../../../src/blob/persistence/IBlobMetadataStore";
+import LokiBlobMetadataStore from "../../../src/blob/persistence/LokiBlobMetadataStore";
+
+const context = { contextId: "test", startTime: new Date() } as Context;
+const account = "devstoreaccount1";
+const container = "container";
+const blobName = "blob";
+
+describe("LokiBlobMetadataStore @loki", () => {
+  let store: LokiBlobMetadataStore;
+
+  beforeEach(async () => {
+    store = new LokiBlobMetadataStore("lokiBlobMetadataStore.unit.test", true);
+    await store.init();
+    await store.createContainer(context, {
+      accountName: account,
+      name: container,
+      properties: {
+        etag: "container-etag",
+        lastModified: new Date()
+      }
+    });
+  });
+
+  afterEach(async () => {
+    await store.close();
+  });
+
+  async function commitVersion(blockName: string, etag: string, size: number) {
+    const block: BlockModel = {
+      accountName: account,
+      containerName: container,
+      blobName,
+      isCommitted: false,
+      name: blockName,
+      size,
+      persistency: { id: `extent-${blockName}`, offset: 0, count: size }
+    };
+    await store.stageBlock(context, block);
+
+    const date = new Date();
+    const blob: BlobModel = {
+      accountName: account,
+      containerName: container,
+      name: blobName,
+      snapshot: "",
+      properties: {
+        creationTime: date,
+        lastModified: date,
+        etag,
+        blobType: Models.BlobType.BlockBlob
+      },
+      isCommitted: true
+    };
+    await store.commitBlockList(context, blob, [
+      { blockName, blockCommitType: "Latest" }
+    ]);
+  }
+
+  it("downloadBlob returns a snapshot that a later commitBlockList does not change", async () => {
+    await commitVersion("block-v1", "etag-v1", 10);
+
+    // A download handler holds this model across awaits while it streams the
+    // body. A commit in that window must not change the version it describes.
+    const downloaded = await store.downloadBlob(
+      context,
+      account,
+      container,
+      blobName
+    );
+
+    await commitVersion("block-v2", "etag-v2", 20);
+
+    assert.strictEqual(downloaded.properties.etag, "etag-v1");
+    assert.strictEqual(downloaded.properties.contentLength, 10);
+    assert.deepStrictEqual(
+      (downloaded.committedBlocksInOrder || []).map((block) => block.name),
+      ["block-v1"]
+    );
+
+    const current = await store.downloadBlob(
+      context,
+      account,
+      container,
+      blobName
+    );
+    assert.strictEqual(current.properties.etag, "etag-v2");
+    assert.strictEqual(current.properties.contentLength, 20);
+    assert.deepStrictEqual(
+      (current.committedBlocksInOrder || []).map((block) => block.name),
+      ["block-v2"]
+    );
+  });
+
+  for (const blobType of [Models.BlobType.AppendBlob, Models.BlobType.PageBlob]) {
+    it(`${blobType} write prechecks do not traverse content metadata`, async () => {
+      let extentReads = 0;
+      const persistency = {
+        get id() {
+          extentReads++;
+          return "extent";
+        },
+        offset: 0,
+        count: 512
+      };
+      await store.createBlob(context, {
+        accountName: account,
+        containerName: container,
+        name: blobName,
+        snapshot: "",
+        properties: {
+          blobType,
+          etag: "etag",
+          lastModified: new Date(),
+          contentLength: 512
+        },
+        isCommitted: true,
+        ...(blobType === Models.BlobType.AppendBlob
+          ? { committedBlocksInOrder: [{ name: "", size: 512, persistency }] }
+          : { pageRangesInOrder: [{ start: 0, end: 511, persistency }] })
+      });
+      extentReads = 0;
+
+      const blob = await store.downloadBlob(
+        context,
+        account,
+        container,
+        blobName,
+        undefined,
+        undefined,
+        undefined,
+        false
+      );
+
+      assert.strictEqual(blob.properties.blobType, blobType);
+      assert.strictEqual(blob.properties.contentLength, 512);
+      assert.strictEqual(extentReads, 0);
+    });
+  }
+});