diff --git a/docs/operations/adapter-remediation-ledger.md b/docs/operations/adapter-remediation-ledger.md index beb9cae..cecbb81 100644 --- a/docs/operations/adapter-remediation-ledger.md +++ b/docs/operations/adapter-remediation-ledger.md @@ -116,23 +116,23 @@ Rollout state starts at `NOT_STARTED`; documented-unimplemented items start at | ID | Activation | Red test command | Fix commit/PR | Rollout state | Rollback trigger | Evidence | | --- | --- | --- | --- | --- | --- | --- | -| BT-PRE-01 | Presigned `AVAILABLE_NOT_COMPOSED` | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | — | `NOT_STARTED` | download lease leak | — | +| BT-PRE-01 | Presigned `AVAILABLE_NOT_COMPOSED` | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | `fix: lazy presigned download leases` | `FIXED_NOT_RELEASED` | download lease leak | Red lazy-lease cases → green 29/29; `open()` performs no network I/O and `close()` is idempotent | | BT-PRE-02 | Wire contract gap | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | — | `NOT_STARTED` | provider `POLICY_REJECTED` spike | — | | BT-PRE-03 | Presigned provider | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | — | `NOT_STARTED` | timeout not bounding fetch | — | | BT-PRE-04 | Presigned vault | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | — | `NOT_STARTED` | issuer/consumer split break | — | | BT-PRE-05 | Provider path decoding | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | — | `NOT_STARTED` | legitimate key rejection | — | -| BT-UP-01 | Resumable transport | `corepack pnpm exec vitest run tests/unit/resumable-upload-fetch-transport.test.ts` | — | `NOT_STARTED` | signal facade rejection | — | +| BT-UP-01 | Resumable transport | `corepack pnpm exec vitest run tests/unit/resumable-upload-fetch-transport.test.ts` | `fix: harden resumable upload transport contracts` | `FIXED_NOT_RELEASED` | signal facade rejection | `isAbortSignal` now requires `removeEventListener` and release cleanup is isolated | | BT-UP-02 | Resumable transport | `corepack pnpm exec vitest run tests/unit/resumable-upload-fetch-transport.test.ts` | — | `NOT_STARTED` | clock injection break | — | -| BT-UP-03 | Resumable checkpoint store | `corepack pnpm exec vitest run tests/unit/resumable-upload-checkpoint.test.ts` | — | `NOT_STARTED` | pending-delete registry growth | — | -| BT-UP-04 | Presigned part executor | `corepack pnpm exec vitest run tests/unit/resumable-upload-checkpoint.test.ts` | — | `NOT_STARTED` | expiry check rejection | — | +| BT-UP-03 | Resumable checkpoint store | `corepack pnpm exec vitest run tests/unit/resumable-upload-checkpoint.test.ts` | `fix: report unknown IndexedDB delete effects` | `FIXED_NOT_RELEASED` | pending-delete registry growth | Red blocked-deadline case → green `PENDING`/`UNKNOWN`; a realm-scoped registry blocks recreating the partition | +| BT-UP-04 | Presigned part executor | `corepack pnpm exec vitest run tests/unit/resumable-upload-checkpoint.test.ts` | `fix: harden resumable upload transport contracts` | `FIXED_NOT_RELEASED` | expiry check rejection | Non-finite and negative clocks return `UNAVAILABLE`/`RESUME` instead of bypassing expiry | | BT-UP-05 | Refactor | `corepack pnpm exec vitest run tests/unit/resumable-upload-runtime.test.ts` | — | `NOT_STARTED` | characterization drift | — | | BT-UP-06 | Refactor | `corepack pnpm exec vitest run tests/unit/resumable-upload-runtime.test.ts` | — | `NOT_STARTED` | drain not quiescent | — | | BT-UP-07 | Documented gap (Web Locks matrix) | promotion evidence | — | `PROMOTION_BLOCKED` | n/a | — | | BT-IMG-01 | Type-contract change | `corepack pnpm check:types:test` fixture | — | `NOT_STARTED` | caller compile break | — | -| BT-IMG-02 | Image probe | `corepack pnpm exec vitest run tests/unit/image-cdn-runtime.test.ts` | — | `NOT_STARTED` | Cache-Control parse rejection | — | +| BT-IMG-02 | Image probe | `corepack pnpm exec vitest run tests/unit/image-cdn-runtime.test.ts` | `fix: parse Cache-Control with quote awareness` | `FIXED_NOT_RELEASED` | Cache-Control parse rejection | Red unmatched-quote cases → green 25/25 | | BT-IMG-03 | Refactor | `corepack pnpm exec vitest run tests/unit/image-cdn-runtime.test.ts` | — | `NOT_STARTED` | characterization drift | — | | BT-IMG-04 | Documented gap (descriptor provider) | promotion evidence | — | `PROMOTION_BLOCKED` | n/a | — | -| BT-X-01 | Shared abort mechanics | `corepack pnpm exec vitest run tests/unit/abortable-operation.test.ts` | — | `NOT_STARTED` | late-result compensation regression | — | +| BT-X-01 | Shared abort mechanics | `corepack pnpm exec vitest run tests/unit/abortable-operation.test.ts` | `fix: share abort and deadline mechanics` | `FIXED_NOT_RELEASED` | late-result compensation regression | Golden suite 8/8: first terminal owner, idempotent close, throwing scheduler, observed late rejection, late-handle compensation | ### Service Worker and Web Push (`docs/reviews/adapters/05-service-worker-and-web-push.md`) diff --git a/src/adapters/browser-transfer/image-cdn/browser-image-probe.ts b/src/adapters/browser-transfer/image-cdn/browser-image-probe.ts index 490afc7..5b91920 100644 --- a/src/adapters/browser-transfer/image-cdn/browser-image-probe.ts +++ b/src/adapters/browser-transfer/image-cdn/browser-image-probe.ts @@ -336,6 +336,77 @@ function validResponseHeaders( ); } +/** + * BT-IMG-02. Quote- and escape-aware Cache-Control tokenizer. + * + * A naive comma split plus `replace(/^"|"$/g, "")` accepted `max-age="60` and + * `max-age=60"` as the number 60, so a malformed policy could be approved as an + * immutable public response. A comma inside a quoted extension value is also + * not a directive boundary. + */ +function splitCacheControlDirectives(value: string): string[] | null { + const parts: string[] = []; + let current = ""; + let inQuotes = false; + let escaped = false; + for (const character of value) { + if (escaped) { + const code = character.codePointAt(0) ?? 0; + // quoted-pair may not carry a bare control character. + if (code <= 0x1f || code === 0x7f) return null; + current += character; + escaped = false; + continue; + } + if (inQuotes && character === "\\") { + escaped = true; + current += character; + continue; + } + if (character === '"') { + inQuotes = !inQuotes; + current += character; + continue; + } + if (character === "," && !inQuotes) { + parts.push(current); + current = ""; + continue; + } + current += character; + } + // An unterminated quoted-string or a dangling escape is malformed. + if (inQuotes || escaped) return null; + parts.push(current); + return parts; +} + +function unquoteCacheControlValue(rawValue: string): string | null { + if (!rawValue.startsWith('"')) { + // A bare value may not contain a quote at all. + return rawValue.includes('"') ? null : rawValue; + } + if (rawValue.length < 2 || !rawValue.endsWith('"')) return null; + const inner = rawValue.slice(1, -1); + let unquoted = ""; + let escaped = false; + for (const character of inner) { + if (escaped) { + unquoted += character; + escaped = false; + continue; + } + if (character === "\\") { + escaped = true; + continue; + } + // An unescaped quote inside the string means the quoting is unbalanced. + if (character === '"') return null; + unquoted += character; + } + return escaped ? null : unquoted; +} + function parseCacheControl( value: string | null, ): ReadonlyMap | null { @@ -348,7 +419,10 @@ function parseCacheControl( "public", ]); const directives = new Map(); - for (const part of value?.split(",") ?? []) { + if (value === null) return directives; + const parts = splitCacheControlDirectives(value); + if (!parts) return null; + for (const part of parts) { const trimmedPart = part.trim(); const separator = trimmedPart.indexOf("="); const name = ( @@ -367,7 +441,9 @@ function parseCacheControl( if (flagDirectives.has(name)) return null; const rawValue = trimmedPart.slice(separator + 1).trim(); if (rawValue === "") return null; - directives.set(name, rawValue.replace(/^"|"$/gu, "")); + const unquoted = unquoteCacheControlValue(rawValue); + if (unquoted === null) return null; + directives.set(name, unquoted); } return directives; } diff --git a/src/adapters/browser-transfer/presigned/presigned-transfer-executor.ts b/src/adapters/browser-transfer/presigned/presigned-transfer-executor.ts index 195ad98..c701383 100644 --- a/src/adapters/browser-transfer/presigned/presigned-transfer-executor.ts +++ b/src/adapters/browser-transfer/presigned/presigned-transfer-executor.ts @@ -154,41 +154,34 @@ export function createPresignedTransferExecutor( const consumed = consume(capability); if (!consumed.ok) return consumed; - const scope = createAbortScope(signal, timeoutMs, scheduler); - try { - const response = await fetcher(binding.href, { - method: "GET", - headers: headersFor(binding), - credentials: "omit", - redirect: "error", - referrerPolicy: "no-referrer", - cache: "no-store", - signal: scope.signal, - }); - const validated = validateDownloadResponse( - response, - binding, - ); - if (!validated.ok) { - cancelBody(response); - scope.release(); - return validated; - } - const source = createDownloadSource({ - response, - binding, - capability, - externalSignal: signal, - scope, - hardMaxChunkBytes, - createVerifier, - observer, - }); - return browserDataSuccess(source); - } catch { - scope.release(); - return transferFailure(signal, scope.timedOut()); - } + // BT-PRE-01. The lease is lazy and single-start: `open()` performs no + // network I/O, so the transfer deadline begins at first consumption and an + // unused source can be discarded through `close()` without leaking a body, + // a timer or a listener. + const source = createDownloadSource({ + start: async (scope) => + await fetcher(binding.href, { + method: "GET", + headers: headersFor(binding), + credentials: "omit", + redirect: "error", + referrerPolicy: "no-referrer", + cache: "no-store", + signal: scope.signal, + }), + validateResponse: (response) => + validateDownloadResponse(response, binding), + createScope: () => createAbortScope(signal, timeoutMs, scheduler), + recheckExpiry: () => + validateExpiry(capability, minimumRemainingLifetimeMs, now()), + binding, + capability, + externalSignal: signal, + hardMaxChunkBytes, + createVerifier, + observer, + }); + return browserDataSuccess(source); } async function putUploadPart( @@ -406,29 +399,53 @@ export function createPresignedTransferExecutor( } function createDownloadSource(input: Readonly<{ - response: Response; + start: ( + scope: ReturnType, + ) => Promise; + validateResponse: (response: Response) => BrowserDataResult; + createScope: () => ReturnType; + recheckExpiry: () => BrowserDataResult; binding: PresignedCapabilityBinding; capability: PresignedDownloadCapability; externalSignal: AbortSignal; - scope: ReturnType; hardMaxChunkBytes: number; createVerifier: ( expectedSha256: string, ) => StreamingSha256Verifier; observer: BrowserDataObserver | undefined; }>): PresignedDownloadByteSource { - let started = false; + /** BT-PRE-01. One state machine shared by `stream()` and `close()`. */ + let state: "READY" | "STREAMING" | "CLOSED" = "READY"; + let activeScope: ReturnType | undefined; + let activeResponse: Response | undefined; + + const releaseActive = () => { + if (activeResponse) { + cancelBody(activeResponse); + activeResponse = undefined; + } + if (activeScope) { + activeScope.release(); + activeScope = undefined; + } + }; + return Object.freeze({ byteLength: input.capability.byteLength, capability: input.capability, integrity: "VERIFIED_ON_SUCCESSFUL_EXHAUSTION" as const, + close() { + // READY -> CLOSED performs no I/O; STREAMING -> CLOSED cancels once. + if (state === "CLOSED") return; + state = "CLOSED"; + releaseActive(); + }, async *stream( consumerSignal: AbortSignal, ): AsyncIterable> { if (!isAbortSignal(consumerSignal)) { - started = true; - cancelBody(input.response); - input.scope.release(); + state = "CLOSED"; + releaseActive(); const failure = browserDataFailure( "INVALID_INPUT", "PRESIGNED_TRANSFER", @@ -437,7 +454,7 @@ function createDownloadSource(input: Readonly<{ yield failure; return; } - if (started) { + if (state !== "READY") { const failure = browserDataFailure( "CONFLICT", "PRESIGNED_TRANSFER", @@ -449,7 +466,45 @@ function createDownloadSource(input: Readonly<{ yield failure; return; } - started = true; + state = "STREAMING"; + // The capability may have expired while the lease sat unused. + const stillActive = input.recheckExpiry(); + if (!stillActive.ok) { + state = "CLOSED"; + observeTransferResult( + input.observer, + "DOWNLOAD", + stillActive, + 0, + ); + yield stillActive as BrowserDataResult; + return; + } + const scope = input.createScope(); + activeScope = scope; + let response: Response; + try { + response = await input.start(scope); + } catch { + state = "CLOSED"; + releaseActive(); + const failure = transferFailure( + input.externalSignal, + scope.timedOut(), + ); + observeTransferResult(input.observer, "DOWNLOAD", failure, 0); + yield failure; + return; + } + activeResponse = response; + const validated = input.validateResponse(response); + if (!validated.ok) { + state = "CLOSED"; + releaseActive(); + observeTransferResult(input.observer, "DOWNLOAD", validated, 0); + yield validated as BrowserDataResult; + return; + } let combined: | ReturnType | undefined; @@ -471,13 +526,13 @@ function createDownloadSource(input: Readonly<{ }; try { combined = combineConsumerAbort( - input.scope, + scope, consumerSignal, ); const verifier = input.createVerifier( input.capability.expectedSha256, ); - if (!input.response.body) { + if (!response.body) { let verified = false; try { verified = @@ -492,7 +547,7 @@ function createDownloadSource(input: Readonly<{ yield fail("INTEGRITY_FAILED"); return; } - reader = input.response.body.getReader(); + reader = response.body.getReader(); while (true) { if ( input.externalSignal.aborted || @@ -501,7 +556,7 @@ function createDownloadSource(input: Readonly<{ yield fail("ABORTED"); return; } - if (input.scope.timedOut()) { + if (scope.timedOut()) { yield fail("UNAVAILABLE", { retryable: true, recovery: "REISSUE_CAPABILITY", @@ -510,7 +565,7 @@ function createDownloadSource(input: Readonly<{ } const result = await readWithSignal( reader, - input.scope.signal, + scope.signal, ); if (result.done) break; const chunk = result.value; @@ -544,7 +599,7 @@ function createDownloadSource(input: Readonly<{ yield fail("ABORTED"); return; } - if (input.scope.timedOut()) { + if (scope.timedOut()) { yield fail("UNAVAILABLE", { retryable: true, recovery: "REISSUE_CAPABILITY", @@ -590,7 +645,7 @@ function createDownloadSource(input: Readonly<{ consumerSignal.aborted ) { yield fail("ABORTED"); - } else if (input.scope.timedOut()) { + } else if (scope.timedOut()) { yield fail("UNAVAILABLE", { retryable: true, recovery: "REISSUE_CAPABILITY", @@ -605,14 +660,18 @@ function createDownloadSource(input: Readonly<{ combined?.release(); if (!completed) { if (reader) cancelReader(reader); - else cancelBody(input.response); + else cancelBody(response); } try { reader?.releaseLock(); } catch { // Reader cleanup cannot change stream success or failure. } - input.scope.release(); + // The lease is terminal once its single stream ends; cleanup runs once. + state = "CLOSED"; + activeResponse = undefined; + activeScope = undefined; + scope.release(); observeBrowserData(input.observer, { operation: "DOWNLOAD", outcome: completed diff --git a/src/adapters/browser-transfer/resumable-upload/fetch-json-transport.ts b/src/adapters/browser-transfer/resumable-upload/fetch-json-transport.ts index 8747e6b..78fabab 100644 --- a/src/adapters/browser-transfer/resumable-upload/fetch-json-transport.ts +++ b/src/adapters/browser-transfer/resumable-upload/fetch-json-transport.ts @@ -576,8 +576,18 @@ function createFetchAttempt( terminal, terminalKind: () => terminalKind, release() { - clearTimeout(timer); - parent.removeEventListener("abort", abort); + // BT-UP-01. Cleanup is best effort and must never replace the already + // classified terminal result with a rejection. + try { + clearTimeout(timer); + } catch { + // A hostile scheduler cannot block listener release below. + } + try { + parent.removeEventListener("abort", abort); + } catch { + // A hostile signal facade cannot break the typed result. + } }, }); } @@ -667,12 +677,18 @@ function boundedPositiveInteger( return value; } +/** + * BT-UP-01. The structural guard must cover every method cleanup will call. + * Admitting a signal without `removeEventListener` turned a `finally` into a + * Promise rejection instead of the typed terminal result. + */ function isAbortSignal(value: unknown): value is AbortSignal { return Boolean( value && typeof value === "object" && typeof (value as AbortSignal).aborted === "boolean" && - typeof (value as AbortSignal).addEventListener === "function", + typeof (value as AbortSignal).addEventListener === "function" && + typeof (value as AbortSignal).removeEventListener === "function", ); } diff --git a/src/adapters/browser-transfer/resumable-upload/indexeddb-checkpoint-store.ts b/src/adapters/browser-transfer/resumable-upload/indexeddb-checkpoint-store.ts index cddbb9a..ee95180 100644 --- a/src/adapters/browser-transfer/resumable-upload/indexeddb-checkpoint-store.ts +++ b/src/adapters/browser-transfer/resumable-upload/indexeddb-checkpoint-store.ts @@ -2,6 +2,7 @@ import type { ResumableUploadCheckpointAdmin, ResumableUploadCheckpoint, ResumableUploadCheckpointStore, + PartitionDeleteOutcome, } from "../../../application/ports/browser-transfer/resumable-upload.ts"; import type { BrowserDataResult } from "../../../application/ports/browser-file-storage/shared.ts"; import { @@ -21,6 +22,24 @@ const GOVERNANCE_STORE = "governance"; const GOVERNANCE_KEY = "scope-binding"; const DEFAULT_BLOCKED_TIMEOUT_MS = 5_000; +/** + * BT-UP-03. Per-realm registry keyed by `(IDBFactory identity, databaseName)`. + * It prevents this realm from recreating a store whose deletion is still in + * flight. It deliberately claims nothing about other realms, which are handled + * by native blocked ordering and explicit recovery. + */ +const PENDING_DELETIONS = new WeakMap>(); + +function pendingDeletionsFor(factory: unknown): Set { + const key = (factory ?? PENDING_DELETIONS) as object; + let pending = PENDING_DELETIONS.get(key); + if (!pending) { + pending = new Set(); + PENDING_DELETIONS.set(key, pending); + } + return pending; +} + export type IndexedDbUploadCheckpointScope = Readonly<{ authorityToken: string; namespaceToken: string; @@ -93,6 +112,14 @@ export function createIndexedDbResumableUploadCheckpointRuntime( ? factory.deleteDatabase.bind(factory) : undefined; const databaseName = uploadCheckpointDatabaseName(scope); + const pendingDeletions = pendingDeletionsFor(factory); + if (pendingDeletions.has(databaseName)) { + // BT-UP-03. A deletion dispatched by this realm has not settled, so a new + // store over the same database would race an unknown native effect. + throw new TypeError( + "Upload checkpoint partition has an unresolved pending deletion.", + ); + } const expectedBinding: ScopeBinding = Object.freeze({ key: GOVERNANCE_KEY, schemaVersion: 1, @@ -376,7 +403,7 @@ export function createIndexedDbResumableUploadCheckpointRuntime( async deletePartition( signal?: AbortSignal, ): Promise< - BrowserDataResult> + BrowserDataResult > { if (signal?.aborted) { return browserDataFailure("ABORTED", "UPLOAD_RECONCILE"); @@ -397,46 +424,64 @@ export function createIndexedDbResumableUploadCheckpointRuntime( } catch (error) { return mapBrowserDataException(error, "UPLOAD_RECONCILE"); } - return await new Promise< - BrowserDataResult> - >((resolve) => { - let settled = false; - let blockedTimer: ReturnType | undefined; - const finish = ( - result: BrowserDataResult>, - ) => { - if (settled) return; - settled = true; - if (blockedTimer) clearTimeout(blockedTimer); - resolve(result); - }; - // IDB deleteDatabase cannot be cancelled after dispatch. AbortSignal is - // intentionally observed only before dispatch so the adapter never - // reports ABORTED while deletion may still commit. - request.onblocked = () => { - blockedTimer = setTimeout(() => { + // BT-UP-03. Once dispatched the deletion may still commit after this + // call returns, so the pending registration is installed before the + // promise settles and is only released by the real native completion. + pendingDeletions.add(databaseName); + return await new Promise>( + (resolve) => { + let settled = false; + let blockedTimer: ReturnType | undefined; + const finish = ( + result: BrowserDataResult, + ) => { + if (settled) return; + settled = true; + if (blockedTimer) clearTimeout(blockedTimer); + resolve(result); + }; + const releasePending = () => { + pendingDeletions.delete(databaseName); + }; + // IDB deleteDatabase cannot be cancelled after dispatch. AbortSignal + // is intentionally observed only before dispatch so the adapter never + // reports ABORTED while deletion may still commit. + request.onblocked = () => { + blockedTimer = setTimeout(() => { + // Not NOT_APPLIED: the request is still live in the browser. + finish( + browserDataSuccess( + Object.freeze({ + state: "PENDING" as const, + effect: "UNKNOWN" as const, + reason: "BLOCKED_DEADLINE" as const, + }), + ), + ); + }, blockedTimeoutMs); + }; + request.onerror = () => { + releasePending(); finish( - browserDataFailure("BLOCKED", "UPLOAD_RECONCILE", { - retryable: true, - recovery: "RELOAD_OTHER_CONTEXTS", - }), + mapBrowserDataException( + request.error, + "UPLOAD_RECONCILE", + ), ); - }, blockedTimeoutMs); - }; - request.onerror = () => - finish( - mapBrowserDataException( - request.error, - "UPLOAD_RECONCILE", - ), - ); - request.onsuccess = () => - finish( - browserDataSuccess( - Object.freeze({ state: "DELETED" as const }), - ), - ); - }); + }; + request.onsuccess = () => { + releasePending(); + finish( + browserDataSuccess( + Object.freeze({ + state: "DELETED" as const, + effect: "APPLIED" as const, + }), + ), + ); + }; + }, + ); }, }; const admin = Object.freeze(adminValue); diff --git a/src/adapters/browser-transfer/resumable-upload/presigned-upload-part-executor.ts b/src/adapters/browser-transfer/resumable-upload/presigned-upload-part-executor.ts index b217869..0871399 100644 --- a/src/adapters/browser-transfer/resumable-upload/presigned-upload-part-executor.ts +++ b/src/adapters/browser-transfer/resumable-upload/presigned-upload-part-executor.ts @@ -43,6 +43,15 @@ export function createPresignedUploadPartExecutor( recovery: "RESUME", }); } + // BT-UP-04. A NaN or negative clock silently bypasses every expiry + // comparison, and an infinite one misreports a dependency failure as a + // capability policy failure. Both are dependency failures. + if (!Number.isSafeInteger(nowEpochMs) || nowEpochMs < 0) { + return browserDataFailure("UNAVAILABLE", "UPLOAD_PART", { + retryable: true, + recovery: "RESUME", + }); + } if ( !capability || capability.method !== "PUT" || diff --git a/src/adapters/platform/abortable-operation.ts b/src/adapters/platform/abortable-operation.ts new file mode 100644 index 0000000..cb4f67d --- /dev/null +++ b/src/adapters/platform/abortable-operation.ts @@ -0,0 +1,190 @@ +/** + * BT-X-01. Shared abort and deadline mechanics. + * + * Several adapters independently reimplemented "race a promise against a + * caller signal and a deadline, then clean up listeners and timers". Only the + * mechanics are shared here; every subsystem keeps its own result taxonomy and + * recovery vocabulary, so this module deliberately imports none of them and is + * not a generic middleware layer. + */ + +export type AbortTerminalReason = "CALLER_ABORT" | "DEADLINE" | "CLOSED"; + +export type AbortRace = + | Readonly<{ kind: "VALUE"; value: Value }> + | Readonly<{ kind: "TERMINAL"; terminal: AbortTerminalReason }>; + +export type AbortableOperation = Readonly<{ + /** The composed signal: caller abort, deadline and close all feed it. */ + readonly signal: AbortSignal; + /** + * The first terminal owner, or `null` while the operation is still live. + * This is a live accessor, not a snapshot. + */ + terminal(): AbortTerminalReason | null; + /** + * Resolves with the operation's value, or with the first terminal owner. + * A terminal race never returns a bare value. + */ + race(operation: Promise): Promise>; + /** + * Idempotent. Removes listeners, clears the deadline timer and marks the + * operation `CLOSED` if nothing terminal happened first. + */ + close(): void; +}>; + +export type AbortableOperationInput = Readonly<{ + signal?: AbortSignal; + timeoutMs?: number; + /** Scheduler seam; a throwing scheduler must not leak a listener. */ + setTimer?: (callback: () => void, delayMs: number) => unknown; + clearTimer?: (handle: unknown) => void; +}>; + +export function createAbortableOperation( + input: AbortableOperationInput = {}, +): AbortableOperation { + const controller = new AbortController(); + const setTimer = + input.setTimer ?? + ((callback: () => void, delayMs: number) => setTimeout(callback, delayMs)); + const clearTimer = + input.clearTimer ?? + ((handle: unknown) => { + clearTimeout(handle as ReturnType); + }); + + let terminalReason: AbortTerminalReason | null = null; + let disposed = false; + let timer: unknown; + + /** First terminal owner wins; later owners never overwrite it. */ + const settle = (reason: AbortTerminalReason) => { + terminalReason ??= reason; + if (!controller.signal.aborted) controller.abort(); + }; + + const onCallerAbort = () => { + settle("CALLER_ABORT"); + dispose(); + }; + + function dispose(): void { + if (disposed) return; + disposed = true; + try { + input.signal?.removeEventListener("abort", onCallerAbort); + } catch { + // A hostile signal facade cannot block cleanup of the rest. + } + if (timer !== undefined) { + try { + clearTimer(timer); + } catch { + // A throwing scheduler cannot leave the operation un-disposed. + } + timer = undefined; + } + } + + if (input.signal?.aborted) { + settle("CALLER_ABORT"); + disposed = true; + } else if (input.signal) { + input.signal.addEventListener("abort", onCallerAbort, { once: true }); + } + + if ( + terminalReason === null && + input.timeoutMs !== undefined && + Number.isFinite(input.timeoutMs) && + input.timeoutMs >= 0 + ) { + try { + timer = setTimer(() => { + settle("DEADLINE"); + dispose(); + }, input.timeoutMs); + } catch { + // A scheduler that throws leaves no timer behind; the operation stays + // bounded only by the caller signal. + timer = undefined; + dispose(); + } + } + + return Object.freeze({ + signal: controller.signal, + terminal: () => terminalReason, + async race(operation: Promise): Promise> { + // Observe a late rejection so an abandoned operation cannot surface as an + // unhandled rejection. + operation.catch(() => {}); + if (terminalReason !== null) { + return Object.freeze({ + kind: "TERMINAL" as const, + terminal: terminalReason, + }); + } + const raced = await Promise.race([ + operation.then( + (value) => Object.freeze({ kind: "VALUE" as const, value }), + () => + Object.freeze({ + kind: "TERMINAL" as const, + terminal: terminalReason ?? ("CLOSED" as const), + }), + ), + new Promise>((resolve) => { + if (controller.signal.aborted) { + resolve( + Object.freeze({ + kind: "TERMINAL" as const, + terminal: terminalReason ?? ("CLOSED" as const), + }), + ); + return; + } + controller.signal.addEventListener( + "abort", + () => + resolve( + Object.freeze({ + kind: "TERMINAL" as const, + terminal: terminalReason ?? ("CLOSED" as const), + }), + ), + { once: true }, + ); + }), + ]); + // A value that arrives after a terminal owner is not admitted. + return terminalReason !== null && raced.kind === "VALUE" + ? Object.freeze({ + kind: "TERMINAL" as const, + terminal: terminalReason, + }) + : raced; + }, + close() { + settle("CLOSED"); + dispose(); + }, + }); +} + +/** + * Compensates a native handle that arrives after the operation ended. The + * compensation itself is best effort and can never change the already selected + * outcome. + */ +export function compensateLateHandle( + handle: Promise } | null }> | null>, +): void { + void handle + .then(async (value) => { + await value?.body?.cancel(); + }) + .catch(() => undefined); +} diff --git a/src/application/ports/browser-transfer/presigned-transfer.ts b/src/application/ports/browser-transfer/presigned-transfer.ts index 5e90cd9..5fedf21 100644 --- a/src/application/ports/browser-transfer/presigned-transfer.ts +++ b/src/application/ports/browser-transfer/presigned-transfer.ts @@ -113,6 +113,13 @@ export type PresignedDownloadByteSource = FileByteSource & byteLength: number; capability: PresignedDownloadCapability; integrity: "VERIFIED_ON_SUCCESSFUL_EXHAUSTION"; + /** + * BT-PRE-01. Discards the lease. Before the first `stream()` this performs + * no network I/O at all; during streaming it cancels the body and releases + * the timer and listeners exactly once. Every consumer must call it in a + * `finally`, including on a pre-stream failure. + */ + close(): void; }>; export interface PresignedDownloadSourcePort { diff --git a/src/application/ports/browser-transfer/resumable-upload.ts b/src/application/ports/browser-transfer/resumable-upload.ts index 6afa0b8..8195ce3 100644 --- a/src/application/ports/browser-transfer/resumable-upload.ts +++ b/src/application/ports/browser-transfer/resumable-upload.ts @@ -40,6 +40,14 @@ export type UploadPartReceipt = UploadPartDescriptor & receiptToken: string; }>; +export type PartitionDeleteOutcome = + | Readonly<{ state: "DELETED"; effect: "APPLIED" }> + | Readonly<{ + state: "PENDING"; + effect: "UNKNOWN"; + reason: "BLOCKED_DEADLINE"; + }>; + export interface UploadRangeReader { readonly byteLength: number; readRange(input: Readonly<{ @@ -273,7 +281,13 @@ export interface ResumableUploadCheckpointAdmin { * Account/logout lifecycle operation for this already-bound opaque partition. * The adapter closes its connection before deletion and bounds blocked waits. */ + /** + * BT-UP-03. An IndexedDB `deleteDatabase()` request cannot be cancelled once + * dispatched, so a blocked deadline is not evidence that nothing happened. + * `PENDING` reports the effect honestly as `UNKNOWN`; only pre-dispatch + * problems are ordinary failures. + */ deletePartition( signal?: AbortSignal, - ): Promise>>; + ): Promise>; } diff --git a/tests/unit/abortable-operation.test.ts b/tests/unit/abortable-operation.test.ts new file mode 100644 index 0000000..761aac8 --- /dev/null +++ b/tests/unit/abortable-operation.test.ts @@ -0,0 +1,139 @@ +import { describe, expect, it, vi } from "vitest"; + +import { + compensateLateHandle, + createAbortableOperation, +} from "../../src/adapters/platform/abortable-operation.ts"; + +/** + * BT-X-01 golden tests. The utility owns abort mechanics only: it must never + * import or imply a subsystem result taxonomy. + */ +describe("shared abortable operation mechanics", () => { + it("records the first terminal owner and never overwrites it", async () => { + const caller = new AbortController(); + const timers: Array<() => void> = []; + const operation = createAbortableOperation({ + signal: caller.signal, + timeoutMs: 10, + setTimer: (callback) => { + timers.push(callback); + return timers.length; + }, + clearTimer: () => {}, + }); + + expect(operation.terminal()).toBeNull(); + caller.abort(); + expect(operation.terminal()).toBe("CALLER_ABORT"); + + // A later deadline or close cannot rewrite the owner. + timers.forEach((callback) => callback()); + operation.close(); + expect(operation.terminal()).toBe("CALLER_ABORT"); + }); + + it("reports a deadline owner and aborts the composed signal", async () => { + const timers: Array<() => void> = []; + const operation = createAbortableOperation({ + timeoutMs: 5, + setTimer: (callback) => { + timers.push(callback); + return timers.length; + }, + clearTimer: () => {}, + }); + timers[0]?.(); + expect(operation.terminal()).toBe("DEADLINE"); + expect(operation.signal.aborted).toBe(true); + }); + + it("race returns a terminal owner instead of a bare value", async () => { + const caller = new AbortController(); + const operation = createAbortableOperation({ signal: caller.signal }); + let release: ((value: string) => void) | undefined; + const pending = new Promise((resolve) => { + release = resolve; + }); + + const raced = operation.race(pending); + caller.abort(); + await expect(raced).resolves.toEqual({ + kind: "TERMINAL", + terminal: "CALLER_ABORT", + }); + + // The late value is observed and discarded. + release?.("late"); + await expect(operation.race(pending)).resolves.toEqual({ + kind: "TERMINAL", + terminal: "CALLER_ABORT", + }); + }); + + it("race returns the value when nothing terminal happened", async () => { + const operation = createAbortableOperation(); + await expect(operation.race(Promise.resolve(7))).resolves.toEqual({ + kind: "VALUE", + value: 7, + }); + operation.close(); + expect(operation.terminal()).toBe("CLOSED"); + }); + + it("observes a late rejection instead of leaking it", async () => { + const operation = createAbortableOperation(); + const rejected = Promise.reject(new Error("late")); + await expect(operation.race(rejected)).resolves.toEqual({ + kind: "TERMINAL", + terminal: "CLOSED", + }); + }); + + it("close is idempotent and releases listeners and timers exactly once", () => { + const caller = new AbortController(); + const remove = vi.spyOn(caller.signal, "removeEventListener"); + const clearTimer = vi.fn(); + const operation = createAbortableOperation({ + signal: caller.signal, + timeoutMs: 10, + setTimer: () => "handle", + clearTimer, + }); + + operation.close(); + operation.close(); + operation.close(); + + expect(remove).toHaveBeenCalledTimes(1); + expect(clearTimer).toHaveBeenCalledTimes(1); + }); + + it("survives a throwing scheduler without leaking a listener", () => { + const caller = new AbortController(); + const remove = vi.spyOn(caller.signal, "removeEventListener"); + const operation = createAbortableOperation({ + signal: caller.signal, + timeoutMs: 10, + setTimer: () => { + throw new TypeError("scheduler exploded"); + }, + clearTimer: () => {}, + }); + + expect(operation.terminal()).toBeNull(); + expect(remove).toHaveBeenCalledTimes(1); + }); + + it("compensates a late native handle without changing the outcome", async () => { + const cancel = vi.fn(async () => {}); + compensateLateHandle(Promise.resolve({ body: { cancel } })); + await Promise.resolve(); + await Promise.resolve(); + expect(cancel).toHaveBeenCalledOnce(); + + // A rejecting handle is swallowed. + compensateLateHandle(Promise.reject(new Error("late"))); + await Promise.resolve(); + }); +}); diff --git a/tests/unit/image-cdn-runtime.test.ts b/tests/unit/image-cdn-runtime.test.ts index 797c6c7..21aed4e 100644 --- a/tests/unit/image-cdn-runtime.test.ts +++ b/tests/unit/image-cdn-runtime.test.ts @@ -1479,6 +1479,10 @@ describe("browser image probe", () => { close: vi.fn(), })); for (const cacheControl of [ + // BT-IMG-02. Unmatched quotes must not be unwrapped into a bare number. + 'public, max-age="31536000, immutable', + 'public, max-age=31536000", immutable', + 'public, max-age="31536000\\", immutable', "public, public, max-age=31536000, immutable", "public, max-age=31536000, s-maxage=60, immutable", "public, max-age=31536000, immutable, must-revalidate", diff --git a/tests/unit/presigned-transfer.test.ts b/tests/unit/presigned-transfer.test.ts index f1bec47..4410a99 100644 --- a/tests/unit/presigned-transfer.test.ts +++ b/tests/unit/presigned-transfer.test.ts @@ -602,13 +602,15 @@ describe("presigned transfer", () => { }); expect(issued.ok).toBe(true); if (!issued.ok) return; - expect( - await harness.executor.downloadSources.open({ - resourceId: "resource-1", - capability: issued.value, - signal: new AbortController().signal, - }), - ).toMatchObject({ + await expect( + firstStreamResult( + await harness.executor.downloadSources.open({ + resourceId: "resource-1", + capability: issued.value, + signal: new AbortController().signal, + }), + ), + ).resolves.toMatchObject({ ok: false, error: { code: "POLICY_REJECTED" }, }); @@ -621,13 +623,15 @@ describe("presigned transfer", () => { }); expect(issued.ok).toBe(true); if (!issued.ok) return; - expect( - await harness.executor.downloadSources.open({ - resourceId: "resource-1", - capability: issued.value, - signal: new AbortController().signal, - }), - ).toMatchObject({ + await expect( + firstStreamResult( + await harness.executor.downloadSources.open({ + resourceId: "resource-1", + capability: issued.value, + signal: new AbortController().signal, + }), + ), + ).resolves.toMatchObject({ ok: false, error: { code: "POLICY_REJECTED" }, }); @@ -640,18 +644,101 @@ describe("presigned transfer", () => { }); expect(issued.ok).toBe(true); if (!issued.ok) return; - expect( - await harness.executor.downloadSources.open({ - resourceId: "resource-1", - capability: issued.value, - signal: new AbortController().signal, - }), - ).toMatchObject({ + await expect( + firstStreamResult( + await harness.executor.downloadSources.open({ + resourceId: "resource-1", + capability: issued.value, + signal: new AbortController().signal, + }), + ), + ).resolves.toMatchObject({ ok: false, error: { code: "INTEGRITY_FAILED" }, }); }); + it("does not fetch a presigned download until stream consumption", async () => { + const bytes = new Uint8Array([1, 2, 3]); + const responsePayload = downloadCapabilityPayload(bytes); + let downloadFetches = 0; + const fetcher = vi.fn(async (input: RequestInfo | URL) => { + if (String(input) === CONTROL_ENDPOINT) { + return jsonResponse(responsePayload); + } + downloadFetches += 1; + return downloadResponse(bytes.slice().buffer, responsePayload); + }) as unknown as typeof fetch; + const { provider, executor } = createHarness({ fetcher }); + const signal = new AbortController().signal; + const issued = await provider.issueDownload({ + resourceId: "resource-1", + signal, + }); + expect(issued.ok).toBe(true); + if (!issued.ok) return; + + const opened = await executor.downloadSources.open({ + resourceId: "resource-1", + capability: issued.value, + signal, + }); + expect(opened.ok).toBe(true); + if (!opened.ok) return; + // BT-PRE-01. open() performs no network I/O. + expect(downloadFetches).toBe(0); + + for await (const chunk of opened.value.stream(signal)) { + expect(chunk.ok).toBe(true); + } + expect(downloadFetches).toBe(1); + opened.value.close(); + }); + + it("closes an unused download source without network I/O", async () => { + const bytes = new Uint8Array([1, 2, 3]); + const responsePayload = downloadCapabilityPayload(bytes); + let downloadFetches = 0; + const fetcher = vi.fn(async (input: RequestInfo | URL) => { + if (String(input) === CONTROL_ENDPOINT) { + return jsonResponse(responsePayload); + } + downloadFetches += 1; + return downloadResponse(bytes.slice().buffer, responsePayload); + }) as unknown as typeof fetch; + const { provider, executor } = createHarness({ fetcher }); + const signal = new AbortController().signal; + const issued = await provider.issueDownload({ + resourceId: "resource-1", + signal, + }); + expect(issued.ok).toBe(true); + if (!issued.ok) return; + + const opened = await executor.downloadSources.open({ + resourceId: "resource-1", + capability: issued.value, + signal, + }); + expect(opened.ok).toBe(true); + if (!opened.ok) return; + + opened.value.close(); + // close() is idempotent and never starts the transfer. + opened.value.close(); + expect(downloadFetches).toBe(0); + + // A stream after close is one terminal conflict, still without fetching. + const results = []; + for await (const chunk of opened.value.stream(signal)) { + results.push(chunk); + } + expect(results).toMatchObject([ + { ok: false, error: { code: "CONFLICT" } }, + ]); + expect(downloadFetches).toBe(0); + }); + it.each([ { name: "truncation", @@ -1633,3 +1720,27 @@ describe("presigned transfer", () => { expect(written).toEqual([...bytes]); }); }); + +/** + * BT-PRE-01. The download lease is lazy, so a response-shape rejection is + * observed on first consumption rather than at `open()`. + */ +async function firstStreamResult( + opened: Awaited< + ReturnType< + ReturnType["executor"]["downloadSources"]["open"] + > + >, +): Promise { + if (!opened.ok) return opened; + try { + for await (const chunk of opened.value.stream( + new AbortController().signal, + )) { + if (!chunk.ok) return chunk; + } + return { ok: true }; + } finally { + opened.value.close(); + } +} diff --git a/tests/unit/resumable-upload-checkpoint.test.ts b/tests/unit/resumable-upload-checkpoint.test.ts index 22b2df8..1034a49 100644 --- a/tests/unit/resumable-upload-checkpoint.test.ts +++ b/tests/unit/resumable-upload-checkpoint.test.ts @@ -183,7 +183,7 @@ describe("IndexedDB resumable upload checkpoint", () => { ).toMatchObject({ ok: true }); expect(await runtime.admin.deletePartition()).toEqual({ ok: true, - value: { state: "DELETED" }, + value: { state: "DELETED", effect: "APPLIED" }, }); expect(await runtime.store.read("upload_key_01")).toMatchObject({ ok: false, @@ -191,21 +191,48 @@ describe("IndexedDB resumable upload checkpoint", () => { }); }); - it("bounds a partition deletion blocked by another browser context", async () => { + it("returns PENDING UNKNOWN when deleteDatabase is still blocked", async () => { const memory = new MemoryIndexedDbFactory(); + const factory = deletingFactory(memory, "BLOCKED"); const runtime = createIndexedDbResumableUploadCheckpointRuntime({ scope, - factory: deletingFactory(memory, "BLOCKED"), + factory, + blockedTimeoutMs: 1, + }); + // BT-UP-03. The native request is still live, so the deadline is not + // evidence that nothing happened. + expect(await runtime.admin.deletePartition()).toEqual({ + ok: true, + value: { + state: "PENDING", + effect: "UNKNOWN", + reason: "BLOCKED_DEADLINE", + }, + }); + }); + + it("keeps the checkpoint store closed until a pending delete is resolved externally", async () => { + const memory = new MemoryIndexedDbFactory(); + const factory = deletingFactory(memory, "BLOCKED"); + const runtime = createIndexedDbResumableUploadCheckpointRuntime({ + scope, + factory, blockedTimeoutMs: 1, }); expect(await runtime.admin.deletePartition()).toMatchObject({ - ok: false, - error: { - code: "BLOCKED", - retryable: true, - recovery: "RELOAD_OTHER_CONTEXTS", - }, + ok: true, + value: { state: "PENDING" }, }); + + // A second runtime over the same realm and database would race an unknown + // native effect. + expect(() => + createIndexedDbResumableUploadCheckpointRuntime({ + scope, + factory, + blockedTimeoutMs: 1, + }), + ).toThrow(TypeError); }); it("never reports a false abort after irreversible deleteDatabase dispatch", async () => { @@ -219,7 +246,7 @@ describe("IndexedDB resumable upload checkpoint", () => { controller.abort(); expect(await deletion).toEqual({ ok: true, - value: { state: "DELETED" }, + value: { state: "DELETED", effect: "APPLIED" }, }); const preAborted = new AbortController();