fix: put presigned transfer work inside one owned abort scope
TR-RR-05. The shared abortable-operation primitive now distinguishes VALUE, REJECTED and TERMINAL, so a collaborator's own rejection is no longer forged into a cancellation and race() always names the same first owner terminal() reports. The caller signal and scheduler are captured once, so replacing a method after construction cannot change how an in-flight operation is bounded. A scheduler that cannot install the deadline is itself terminal: previously it released the caller listener and left no owner, which made every later abort invisible. Late values are compensated exactly once. Both presigned subsystems, which each carried their own copy of these mechanics, are now projections of that primitive — giving it real production importers rather than a shared helper nobody used. TR-RR-01. close() on an active download aborts the scope instead of only dropping listeners, so a fetch or read already in flight actually stops. The consumer's stream signal joins the operation's ownership before any I/O begins, so an already-aborted consumer no longer causes one network request first. TR-RR-02. The upload abort scope is created before the digest, and the digest races the caller and the deadline like every other step. A non-settling hash can no longer hold put() open, and the vault claim and the network call happen only after the owner is re-checked. TR-RR-03. A capability registration is a versioned exact union validated at registration time: the protocol version, HTTPS only, exact own-data fields, a 2xx expected status, and no ambient credential or cookie header — a presigned URL carries its own authorization, and a session header alongside it would send the user's credentials to that origin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
c0f53d1855
commit
46e067e555
@@ -8,6 +8,7 @@ import type {
|
||||
PresignedUploadPartOutcome,
|
||||
PresignedUploadPartPort,
|
||||
} from "../../../application/ports/browser-transfer/presigned-transfer.ts";
|
||||
import { createAbortableOperation } from "../../platform/abortable-operation.ts";
|
||||
import { RESUMABLE_UPLOAD_PROTOCOL } from "../../../application/ports/browser-transfer/resumable-upload.ts";
|
||||
import type {
|
||||
BrowserDataFailureCode,
|
||||
@@ -171,7 +172,13 @@ export function createPresignedTransferExecutor(
|
||||
}),
|
||||
validateResponse: (response) =>
|
||||
validateDownloadResponse(response, binding),
|
||||
createScope: () => createAbortScope(signal, timeoutMs, scheduler),
|
||||
createScope: (consumerSignal?: AbortSignal) =>
|
||||
createAbortScope(
|
||||
signal,
|
||||
timeoutMs,
|
||||
scheduler,
|
||||
consumerSignal ? [consumerSignal] : [],
|
||||
),
|
||||
recheckExpiry: () =>
|
||||
validateExpiry(capability, minimumRemainingLifetimeMs, now()),
|
||||
binding,
|
||||
@@ -266,37 +273,51 @@ export function createPresignedTransferExecutor(
|
||||
) {
|
||||
return browserDataFailure("POLICY_REJECTED", "PRESIGNED_TRANSFER");
|
||||
}
|
||||
let actualDigest: string;
|
||||
try {
|
||||
actualDigest = normalizedSha256(await digestBytes(bytes));
|
||||
} catch {
|
||||
return browserDataFailure(
|
||||
"INTEGRITY_FAILED",
|
||||
"PRESIGNED_TRANSFER",
|
||||
);
|
||||
}
|
||||
if (actualDigest !== capability.expectedSha256) {
|
||||
return browserDataFailure(
|
||||
"INTEGRITY_FAILED",
|
||||
"PRESIGNED_TRANSFER",
|
||||
);
|
||||
}
|
||||
if (request.signal.aborted) {
|
||||
return browserDataFailure("ABORTED", "PRESIGNED_TRANSFER");
|
||||
}
|
||||
const active = validateExpiry(
|
||||
capability,
|
||||
minimumRemainingLifetimeMs,
|
||||
now(),
|
||||
);
|
||||
if (!active.ok) return active;
|
||||
const claimed = claim(capability);
|
||||
if (!claimed.ok) return claimed;
|
||||
const consumed = consume(capability);
|
||||
if (!consumed.ok) return consumed;
|
||||
|
||||
// TR-RR-02. The abort scope is created first, so the digest — which can be
|
||||
// a long or non-settling computation over a large buffer — is owned by the
|
||||
// caller signal and the deadline like every other step. Computing it before
|
||||
// the scope existed meant an abort or a deadline could not reach it, and a
|
||||
// hash that never settled held the whole `put` open.
|
||||
const scope = createAbortScope(request.signal, timeoutMs, scheduler);
|
||||
try {
|
||||
const digested = await scope.race(
|
||||
Promise.resolve().then(async () => await digestBytes(bytes)),
|
||||
);
|
||||
if (digested === SCOPE_ENDED) {
|
||||
return transferFailure(request.signal, scope.timedOut());
|
||||
}
|
||||
let actualDigest: string;
|
||||
try {
|
||||
actualDigest = normalizedSha256(digested);
|
||||
} catch {
|
||||
return browserDataFailure(
|
||||
"INTEGRITY_FAILED",
|
||||
"PRESIGNED_TRANSFER",
|
||||
);
|
||||
}
|
||||
if (actualDigest !== capability.expectedSha256) {
|
||||
return browserDataFailure(
|
||||
"INTEGRITY_FAILED",
|
||||
"PRESIGNED_TRANSFER",
|
||||
);
|
||||
}
|
||||
// The owner is re-checked after the wait: a claim and a network call may
|
||||
// only follow a digest that finished while this operation still held the
|
||||
// execution.
|
||||
if (scope.signal.aborted || request.signal.aborted) {
|
||||
return transferFailure(request.signal, scope.timedOut());
|
||||
}
|
||||
const active = validateExpiry(
|
||||
capability,
|
||||
minimumRemainingLifetimeMs,
|
||||
now(),
|
||||
);
|
||||
if (!active.ok) return active;
|
||||
const claimed = claim(capability);
|
||||
if (!claimed.ok) return claimed;
|
||||
const consumed = consume(capability);
|
||||
if (!consumed.ok) return consumed;
|
||||
|
||||
const raced = await scope.race(
|
||||
fetcher(binding.href, {
|
||||
method: "PUT",
|
||||
@@ -409,7 +430,9 @@ function createDownloadSource(input: Readonly<{
|
||||
scope: ReturnType<typeof createAbortScope>,
|
||||
) => Promise<Response>;
|
||||
validateResponse: (response: Response) => BrowserDataResult<unknown>;
|
||||
createScope: () => ReturnType<typeof createAbortScope>;
|
||||
createScope: (
|
||||
consumerSignal?: AbortSignal,
|
||||
) => ReturnType<typeof createAbortScope>;
|
||||
recheckExpiry: () => BrowserDataResult<unknown>;
|
||||
binding: PresignedCapabilityBinding;
|
||||
capability: PresignedDownloadCapability;
|
||||
@@ -425,7 +448,13 @@ function createDownloadSource(input: Readonly<{
|
||||
let activeScope: ReturnType<typeof createAbortScope> | undefined;
|
||||
let activeResponse: Response | undefined;
|
||||
|
||||
// TR-RR-01. Releasing the scope only dropped listeners and the timer, so a
|
||||
// fetch or read already in flight kept running after `close()`. The scope is
|
||||
// aborted here, which is what actually ends the physical I/O.
|
||||
const releaseActive = () => {
|
||||
if (activeScope) {
|
||||
activeScope.abort();
|
||||
}
|
||||
if (activeResponse) {
|
||||
cancelBody(activeResponse);
|
||||
activeResponse = undefined;
|
||||
@@ -486,8 +515,22 @@ function createDownloadSource(input: Readonly<{
|
||||
yield stillActive as BrowserDataResult<never>;
|
||||
return;
|
||||
}
|
||||
const scope = input.createScope();
|
||||
// TR-RR-01. The consumer's stream signal is part of this operation's
|
||||
// ownership from the start. Composing it only after the fetch had begun
|
||||
// meant an already-aborted consumer still caused one network request.
|
||||
const scope = input.createScope(consumerSignal);
|
||||
activeScope = scope;
|
||||
if (scope.signal.aborted) {
|
||||
state = "CLOSED";
|
||||
releaseActive();
|
||||
const failure = transferFailure(
|
||||
input.externalSignal,
|
||||
scope.timedOut(),
|
||||
);
|
||||
observeTransferResult(input.observer, "DOWNLOAD", failure, 0);
|
||||
yield failure;
|
||||
return;
|
||||
}
|
||||
let response: Response;
|
||||
try {
|
||||
// BT-PRE-03. A fetch that ignores its signal cannot outlive the scope.
|
||||
@@ -524,9 +567,6 @@ function createDownloadSource(input: Readonly<{
|
||||
yield validated as BrowserDataResult<never>;
|
||||
return;
|
||||
}
|
||||
let combined:
|
||||
| ReturnType<typeof combineConsumerAbort>
|
||||
| undefined;
|
||||
let reader:
|
||||
| ReadableStreamDefaultReader<Uint8Array>
|
||||
| undefined;
|
||||
@@ -544,10 +584,6 @@ function createDownloadSource(input: Readonly<{
|
||||
return browserDataFailure(code, "PRESIGNED_TRANSFER", options);
|
||||
};
|
||||
try {
|
||||
combined = combineConsumerAbort(
|
||||
scope,
|
||||
consumerSignal,
|
||||
);
|
||||
const verifier = input.createVerifier(
|
||||
input.capability.expectedSha256,
|
||||
);
|
||||
@@ -676,7 +712,6 @@ function createDownloadSource(input: Readonly<{
|
||||
});
|
||||
}
|
||||
} finally {
|
||||
combined?.release();
|
||||
if (!completed) {
|
||||
if (reader) cancelReader(reader);
|
||||
else cancelBody(response);
|
||||
@@ -1016,88 +1051,74 @@ function compensateLateResponse(task: Promise<unknown>): void {
|
||||
.catch(() => undefined);
|
||||
}
|
||||
|
||||
/**
|
||||
* TR-RR-01 / TR-RR-05. A projection of the shared `createAbortableOperation`
|
||||
* primitive into this subsystem's vocabulary, replacing a second hand-written
|
||||
* copy of the same mechanics.
|
||||
*
|
||||
* `additionalSignals` lets a consumer's stream signal join the operation's
|
||||
* ownership before any I/O begins, and `abort()` ends the physical work rather
|
||||
* than only releasing bookkeeping.
|
||||
*/
|
||||
function createAbortScope(
|
||||
external: AbortSignal,
|
||||
timeoutMs: number,
|
||||
scheduler: Scheduler,
|
||||
additionalSignals: readonly AbortSignal[] = [],
|
||||
) {
|
||||
const controller = new AbortController();
|
||||
let timedOut = false;
|
||||
let released = false;
|
||||
const onAbort = () => controller.abort(external.reason);
|
||||
const releaseListener = () => {
|
||||
try {
|
||||
external.removeEventListener("abort", onAbort);
|
||||
} catch {
|
||||
// A hostile signal facade cannot block the rest of cleanup.
|
||||
}
|
||||
};
|
||||
external.addEventListener("abort", onAbort, { once: true });
|
||||
if (external.aborted) onAbort();
|
||||
let timer: unknown;
|
||||
try {
|
||||
timer = scheduler.setTimeout(() => {
|
||||
timedOut = true;
|
||||
controller.abort("timeout");
|
||||
releaseListener();
|
||||
}, timeoutMs);
|
||||
} catch {
|
||||
// BT-PRE-03. A scheduler that throws must not leave the external listener
|
||||
// attached; the scope stays bounded by the caller signal alone.
|
||||
releaseListener();
|
||||
timer = undefined;
|
||||
}
|
||||
return Object.freeze({
|
||||
signal: controller.signal,
|
||||
timedOut: () => timedOut,
|
||||
/**
|
||||
* BT-PRE-03. Aborting the controller does not settle a fetch that ignores
|
||||
* its signal, so the caller-facing wait is raced against the scope and a
|
||||
* late `Response` has its body cancelled.
|
||||
*/
|
||||
async race<Value>(task: Promise<Value>): Promise<Value | typeof SCOPE_ENDED> {
|
||||
task.catch(() => {});
|
||||
if (controller.signal.aborted) {
|
||||
compensateLateResponse(task);
|
||||
return SCOPE_ENDED;
|
||||
}
|
||||
const ended = new Promise<typeof SCOPE_ENDED>((resolve) => {
|
||||
controller.signal.addEventListener(
|
||||
"abort",
|
||||
() => resolve(SCOPE_ENDED),
|
||||
{ once: true },
|
||||
);
|
||||
});
|
||||
const raced = await Promise.race([task, ended]);
|
||||
if (raced === SCOPE_ENDED) compensateLateResponse(task);
|
||||
return raced;
|
||||
},
|
||||
abort(reason?: unknown) {
|
||||
controller.abort(reason);
|
||||
},
|
||||
release() {
|
||||
if (released) return;
|
||||
released = true;
|
||||
try {
|
||||
if (timer !== undefined) scheduler.clearTimeout(timer as never);
|
||||
} catch {
|
||||
// Cleanup failures cannot change the classified result.
|
||||
}
|
||||
releaseListener();
|
||||
const operation = createAbortableOperation({
|
||||
signal: external,
|
||||
timeoutMs,
|
||||
setTimer: (callback, delayMs) => scheduler.setTimeout(callback, delayMs),
|
||||
clearTimer: (handle) => {
|
||||
scheduler.clearTimeout(handle as never);
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function combineConsumerAbort(
|
||||
scope: ReturnType<typeof createAbortScope>,
|
||||
consumer: AbortSignal,
|
||||
) {
|
||||
const onAbort = () => scope.abort(consumer.reason);
|
||||
consumer.addEventListener("abort", onAbort, { once: true });
|
||||
if (consumer.aborted) onAbort();
|
||||
const releases: (() => void)[] = [];
|
||||
for (const extra of additionalSignals) {
|
||||
if (!extra) continue;
|
||||
if (extra.aborted) {
|
||||
operation.close();
|
||||
continue;
|
||||
}
|
||||
const onAbort = () => operation.close();
|
||||
try {
|
||||
extra.addEventListener("abort", onAbort, { once: true });
|
||||
releases.push(() => {
|
||||
try {
|
||||
extra.removeEventListener("abort", onAbort);
|
||||
} catch {
|
||||
// A hostile signal facade cannot block the rest of cleanup.
|
||||
}
|
||||
});
|
||||
} catch {
|
||||
operation.close();
|
||||
}
|
||||
}
|
||||
const releaseExtras = () => {
|
||||
for (const release of releases.splice(0)) release();
|
||||
};
|
||||
return Object.freeze({
|
||||
signal: operation.signal,
|
||||
timedOut: () => operation.terminal() === "DEADLINE",
|
||||
async race<Value>(task: Promise<Value>): Promise<Value | typeof SCOPE_ENDED> {
|
||||
const outcome = await operation.race(task, (value) => {
|
||||
const body = (
|
||||
value as { body?: { cancel(): Promise<void> } | null } | null
|
||||
)?.body;
|
||||
void body?.cancel().catch(() => undefined);
|
||||
});
|
||||
if (outcome.kind === "VALUE") return outcome.value;
|
||||
if (outcome.kind === "REJECTED") throw outcome.reason;
|
||||
return SCOPE_ENDED;
|
||||
},
|
||||
abort() {
|
||||
operation.close();
|
||||
releaseExtras();
|
||||
},
|
||||
release() {
|
||||
consumer.removeEventListener("abort", onAbort);
|
||||
operation.close();
|
||||
releaseExtras();
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user