From 81c1288dc6f9cbebf7ffec580c8fa230ece1d0ab Mon Sep 17 00:00:00 2001 From: compiler-segfault <2671715549@qq.com> Date: Sun, 8 Nov 2026 20:32:18 +0800 Subject: [PATCH] Reject Acquire Lease without a usable duration x-ms-lease-duration is mandatory for acquire, but a missing header parsed as undefined and slipped through the range check: undefined compares false against both bounds. The blob or container was then stored with leaseState Leased, no leaseDurationSeconds, and an invalid leaseExpireTime, so every later request failed with 500 and the resource stayed unusable until the emulator data was discarded. Require the header in both acquire handlers, returning 400 MissingRequiredHeader, and validate the duration in the lease state builders so a missing or non-integer value can never be persisted. Fixes #2855 --- ChangeLog.md | 1 + src/blob/errors/StorageErrorFactory.ts | 21 ++++++++++++++++++ src/blob/handlers/BlobHandler.ts | 9 ++++++++ src/blob/handlers/ContainerHandler.ts | 14 ++++++++++-- src/blob/lease/LeaseAvailableState.ts | 4 +--- src/blob/lease/LeaseBrokenState.ts | 4 +--- src/blob/lease/LeaseExpiredState.ts | 4 +--- src/blob/lease/LeaseLeasedState.ts | 4 +--- src/blob/lease/LeaseStateBase.ts | 16 ++++++++++++++ src/blob/utils/constants.ts | 1 + tests/blob/apis/blob.test.ts | 30 ++++++++++++++++++++++++++ tests/blob/apis/container.test.ts | 24 +++++++++++++++++++++ 12 files changed, 118 insertions(+), 14 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 900ec37a2..2a34fb168 100644 --- a/ChangeLog.md +++ b/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: diff --git a/src/blob/errors/StorageErrorFactory.ts b/src/blob/errors/StorageErrorFactory.ts index 71ac5013b..a5e3b7807 100644 --- a/src/blob/errors/StorageErrorFactory.ts +++ b/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 { diff --git a/src/blob/handlers/BlobHandler.ts b/src/blob/handlers/BlobHandler.ts index acbbe5c23..f42d1d05a 100644 --- a/src/blob/handlers/BlobHandler.ts +++ b/src/blob/handlers/BlobHandler.ts @@ -398,6 +398,15 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { ): Promise { 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!; diff --git a/src/blob/handlers/ContainerHandler.ts b/src/blob/handlers/ContainerHandler.ts index 48f699304..6b2bd43c9 100644 --- a/src/blob/handlers/ContainerHandler.ts +++ b/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 { 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!; diff --git a/src/blob/lease/LeaseAvailableState.ts b/src/blob/lease/LeaseAvailableState.ts index 7943ea7f1..eec22c7ff 100644 --- a/src/blob/lease/LeaseAvailableState.ts +++ b/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 diff --git a/src/blob/lease/LeaseBrokenState.ts b/src/blob/lease/LeaseBrokenState.ts index 5dd748164..8a79db5e0 100644 --- a/src/blob/lease/LeaseBrokenState.ts +++ b/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 diff --git a/src/blob/lease/LeaseExpiredState.ts b/src/blob/lease/LeaseExpiredState.ts index 5baa90fc0..8c7642c99 100644 --- a/src/blob/lease/LeaseExpiredState.ts +++ b/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 diff --git a/src/blob/lease/LeaseLeasedState.ts b/src/blob/lease/LeaseLeasedState.ts index 242e592e8..aacef4ea6 100644 --- a/src/blob/lease/LeaseLeasedState.ts +++ b/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 diff --git a/src/blob/lease/LeaseStateBase.ts b/src/blob/lease/LeaseStateBase.ts index 6f92ae40a..f1ebd0e46 100644 --- a/src/blob/lease/LeaseStateBase.ts +++ b/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(syncer: ILeaseSyncer): T { return syncer.sync(this.lease); } diff --git a/src/blob/utils/constants.ts b/src/blob/utils/constants.ts index 12d0ab34c..77f6d254e 100644 --- a/src/blob/utils/constants.ts +++ b/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", diff --git a/tests/blob/apis/blob.test.ts b/tests/blob/apis/blob.test.ts index 902fc7503..1e7568862 100644 --- a/tests/blob/apis/blob.test.ts +++ b/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; diff --git a/tests/blob/apis/container.test.ts b/tests/blob/apis/container.test.ts index 418102417..180335884 100644 --- a/tests/blob/apis/container.test.ts +++ b/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;