Reject Acquire Lease without a usable duration

ChangeLog.md

@@ -27,6 +27,7 @@ Blob:
 - 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.
+- Fixed `Acquire Lease` accepting a request without `x-ms-lease-duration`. The lease was stored with an invalid expiry, and every later request for the blob or container returned `500`. Acquire now returns `400 MissingRequiredHeader`, and the lease state builders reject a duration that is missing or not an integer before any lease state is persisted. (issue #2855)
 
 Queue:
 

src/blob/errors/StorageErrorFactory.ts

@@ -282,6 +282,27 @@ export default class StorageErrorFactory {
     );
   }
 
+  public static getMissingRequiredHeader(
+    contextID: string = DefaultID,
+    headerName?: string
+  ): StorageError {
+    const additionalMessages: {
+      [key: string]: string;
+    } = {};
+
+    if (headerName) {
+      additionalMessages.HeaderName = headerName;
+    }
+
+    return new StorageError(
+      400,
+      "MissingRequiredHeader",
+      "A required HTTP header wasn't specified for the request.",
+      contextID,
+      additionalMessages
+    );
+  }
+
   public static getInvalidLeaseBreakPeriod(
     contextID: string = DefaultID
   ): StorageError {

src/blob/handlers/BlobHandler.ts

@@ -398,6 +398,15 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler {
   ): Promise<Models.BlobAcquireLeaseResponse> {
     validateProposedLeaseId(options.proposedLeaseId, context.contextId);
 
+    // x-ms-lease-duration is mandatory for acquire; without it the lease would
+    // be persisted with no usable expiry and every later request would fail.
+    if (options.duration === undefined) {
+      throw StorageErrorFactory.getMissingRequiredHeader(
+        context.contextId,
+        HeaderConstants.X_MS_LEASE_DURATION
+      );
+    }
+
     const blobCtx = new BlobStorageContext(context);
     const account = blobCtx.account!;
     const container = blobCtx.container!;

src/blob/handlers/ContainerHandler.ts

@@ -6,16 +6,18 @@ import IExtentStore from "../../common/persistence/IExtentStore";
 import { convertRawHeadersToMetadata, newEtag } from "../../common/utils/utils";
 import BlobStorageContext from "../context/BlobStorageContext";
 import NotImplementedError from "../errors/NotImplementedError";
+import StorageErrorFactory from "../errors/StorageErrorFactory";
 import * as Models from "../generated/artifacts/models";
 import Context from "../generated/Context";
 import IContainerHandler from "../generated/handlers/IContainerHandler";
 import IBlobMetadataStore from "../persistence/IBlobMetadataStore";
 import {
   BLOB_API_VERSION,
+  DEFAULT_LIST_BLOBS_MAX_RESULTS,
   EMULATOR_ACCOUNT_KIND,
-  EMULATOR_ACCOUNT_SKUNAME
+  EMULATOR_ACCOUNT_SKUNAME,
+  HeaderConstants
 } from "../utils/constants";
-import { DEFAULT_LIST_BLOBS_MAX_RESULTS } from "../utils/constants";
 import {
   getBlobTagsCount,
   removeQuotationFromListBlobEtag,
@@ -431,6 +433,14 @@ export default class ContainerHandler extends BaseHandler
   ): Promise<Models.ContainerAcquireLeaseResponse> {
     validateProposedLeaseId(options.proposedLeaseId, context.contextId);
 
+    // x-ms-lease-duration is mandatory for acquire, the same as for blobs.
+    if (options.duration === undefined) {
+      throw StorageErrorFactory.getMissingRequiredHeader(
+        context.contextId,
+        HeaderConstants.X_MS_LEASE_DURATION
+      );
+    }
+
     const blobCtx = new BlobStorageContext(context);
     const accountName = blobCtx.account!;
     const containerName = blobCtx.container!;

src/blob/lease/LeaseAvailableState.ts

@@ -97,9 +97,7 @@ export default class LeaseAvailableState extends LeaseStateBase {
   }
 
   public acquire(duration: number, proposedLeaseId: string = ""): ILeaseState {
-    if ((duration < 15 || duration > 60) && duration !== -1) {
-      throw StorageErrorFactory.getInvalidLeaseDuration(this.context.contextId);
-    }
+    this.validateLeaseDuration(duration);
 
     // TODO: Validate proposedLeaseId follows GUID format
 

src/blob/lease/LeaseBrokenState.ts

@@ -137,9 +137,7 @@ export default class LeaseBrokenState extends LeaseStateBase {
   }
 
   public acquire(duration: number, proposedLeaseId: string = ""): ILeaseState {
-    if ((duration < 15 || duration > 60) && duration !== -1) {
-      throw StorageErrorFactory.getInvalidLeaseDuration(this.context.contextId);
-    }
+    this.validateLeaseDuration(duration);
 
     // TODO: Validate proposedLeaseId follows GUID format
 

src/blob/lease/LeaseExpiredState.ts

@@ -144,9 +144,7 @@ export default class LeaseExpiredState extends LeaseStateBase {
   }
 
   public acquire(duration: number, proposedLeaseId: string = ""): ILeaseState {
-    if ((duration < 15 || duration > 60) && duration !== -1) {
-      throw StorageErrorFactory.getInvalidLeaseDuration(this.context.contextId);
-    }
+    this.validateLeaseDuration(duration);
 
     // TODO: Validate proposedLeaseId follows GUID format
 

src/blob/lease/LeaseLeasedState.ts

@@ -91,9 +91,7 @@ export default class LeaseLeasedState extends LeaseStateBase {
       throw StorageErrorFactory.getLeaseAlreadyPresent(this.context.contextId);
     }
 
-    if ((duration < 15 || duration > 60) && duration !== -1) {
-      throw StorageErrorFactory.getInvalidLeaseDuration(this.context.contextId);
-    }
+    this.validateLeaseDuration(duration);
 
     // TODO: Validate proposedLeaseId follows GUID format
 

src/blob/lease/LeaseStateBase.ts

@@ -1,3 +1,4 @@
+import StorageErrorFactory from "../errors/StorageErrorFactory";
 import Context from "../generated/Context";
 import {
   ILease,
@@ -19,6 +20,21 @@ export default abstract class LeaseStateBase implements ILeaseState {
   public abstract renew(leaseId: string): ILeaseState;
   public abstract change(leaseId: string, proposedLeaseId: string): ILeaseState;
   public abstract release(leaseId: string): ILeaseState;
+  /**
+   * Azure accepts a fixed lease duration between 15 and 60 seconds, or -1 for
+   * an infinite lease. A missing x-ms-lease-duration parses as undefined and
+   * used to slip through the comparison below, persisting a lease whose expiry
+   * time was invalid and making every later request fail.
+   */
+  protected validateLeaseDuration(duration: number): void {
+    if (
+      !Number.isInteger(duration) ||
+      ((duration < 15 || duration > 60) && duration !== -1)
+    ) {
+      throw StorageErrorFactory.getInvalidLeaseDuration(this.context.contextId);
+    }
+  }
+
   public sync<T>(syncer: ILeaseSyncer<T>): T {
     return syncer.sync(this.lease);
   }

src/blob/utils/constants.ts

@@ -67,6 +67,7 @@ export const HeaderConstants = {
   X_MS_ENCRYPTION_KEY: "x-ms-encryption-key",
   X_MS_ENCRYPTION_KEY_SHA256: "x-ms-encryption-key-sha256",
   X_MS_ENCRYPTION_ALGORITHM: "x-ms-encryption-algorithm",
+  X_MS_LEASE_DURATION: "x-ms-lease-duration",
   PREFIX_FOR_STORAGE: "x-ms-",
   RANGE: "Range",
   USER_AGENT: "User-Agent",

tests/blob/apis/blob.test.ts

@@ -887,6 +887,36 @@ describe("BlobAPIs", () => {
     }
   });
 
+  it("acquireLease_without_duration_is_rejected_and_leaves_the_blob_usable @loki @sql", async () => {
+    // x-ms-lease-duration is mandatory for acquire; the SDK omits the header
+    // when the duration is undefined.
+    let err;
+    try {
+      await blobLeaseClient.acquireLease(undefined as any);
+    } catch (error) {
+      err = error;
+    }
+    assert.deepStrictEqual((err as any).statusCode, 400);
+    assert.deepStrictEqual((err as any).code, "MissingRequiredHeader");
+
+    // A rejected acquire must not persist a half built lease, which used to
+    // make every later request against the blob fail with 500.
+    const properties = await blobClient.getProperties();
+    assert.equal(properties.leaseStatus, "unlocked");
+    assert.equal(properties.contentLength, content.length);
+    const downloaded = await blobClient.download(0);
+    assert.equal(await bodyToString(downloaded, content.length), content);
+    const listed = (
+      await containerClient.listBlobsFlat().byPage().next()
+    ).value;
+    assert.equal(listed.segment.blobItems!.length, 1);
+
+    // A well formed acquire still succeeds afterwards.
+    const acquired = await blobLeaseClient.acquireLease(15);
+    assert.ok(acquired.leaseId);
+    await blobClient.getBlobLeaseClient(acquired.leaseId!).releaseLease();
+  });
+
   it("acquireLease_available_proposedLeaseId_fixed @loki @sql", async () => {
     const guid = "ca761232ed4211cebacd00aa0057b223";
     const duration = 30;

tests/blob/apis/container.test.ts

@@ -429,6 +429,30 @@ describe("ContainerAPIs", () => {
     }
   });
 
+  it("acquireLease_without_duration_is_rejected_and_leaves_the_container_usable @loki @sql", async () => {
+    let err;
+    try {
+      await blobLeaseClient.acquireLease(undefined as any);
+    } catch (error) {
+      err = error;
+    }
+    assert.deepStrictEqual((err as any).statusCode, 400);
+    assert.deepStrictEqual((err as any).code, "MissingRequiredHeader");
+
+    // The container must stay usable, and a rejected acquire must not persist
+    // a lease with no usable expiry.
+    const properties = await containerClient.getProperties();
+    assert.equal(properties.leaseStatus, "unlocked");
+    const listed = (
+      await containerClient.listBlobsFlat().byPage().next()
+    ).value;
+    assert.deepStrictEqual(listed.segment.blobItems!.length, 0);
+
+    const acquired = await blobLeaseClient.acquireLease(15);
+    assert.ok(acquired.leaseId);
+    await containerClient.getBlobLeaseClient(acquired.leaseId!).releaseLease();
+  });
+
   it("acquireLease_available_proposedLeaseId_fixed @loki @sql", async () => {
     const guid = "ca761232-ed42-11ce-bacd-00aa0057b223";
     const duration = 30;