feat: let a module route stream an image-sized upload, explicitly and bounded
ADR 0017 capped a module route at 1 MiB and said large media belonged to a future storage design. That design arrived — ADR 0030 gave modules a bucket, ADR 0031 took Ekklesia out of the read path — and it did not solve the write path: signed write URLs were deliberately declined, so the only way bytes reach a bucket is a module's own route, and a photograph is not 1 MiB. Extend the route contract rather than adding a second HTTP system. A route declares `bodyMode`: `buffered` (the default, and what every existing route already meant) keeps the 1 MiB ceiling and materializes the body before the handler runs; `streamed` raises the ceiling to 32 MiB and counts bytes as the handler consumes them, holding nothing. The mode is explicit rather than inferred from the size, because inferring it would mean an author who raised a limit silently changed how their own handler receives its body and when a 413 can reach their own code. 32 MiB holds one 25 MiB image plus bounded overhead and is also the platform's absolute ceiling: no route in any mode may declare more, and no unbounded option exists in the contract. The unit is one file per request — twenty images are twenty individually bounded requests, and nothing here can express a 500 MiB aggregate whose failure would waste everything already sent. Replace the buffering dependency with the platform's own limiter. A declared Content-Length over the limit is refused synchronously, before authorization and before a byte is read. Buffered mode now counts what actually arrives rather than trusting the header, which closes a gap the previous implementation had. Streamed mode wraps the body in a counting transform; on overflow the stream errors, the source is cancelled, and the response is replaced with the same sanitized 413 even if the handler swallowed the error — a limit a handler can swallow is not a limit. An unread body is cancelled so an abandoned upload releases its socket, and an abort stays an ordinary failed request rather than becoming a 413. Ordering is unchanged and now strictly better: in streamed mode no byte is read until the handler reads it, and the handler runs only after authorization succeeds, so an unauthorized caller cannot push an upload through at all. The live suite uploads a real 25 MiB body over a real socket end to end. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
ekklesia
co-authored by
Claude Opus 5
Claude Opus 5
parent
c444ff570b
commit
e36485bf99
@@ -58,14 +58,17 @@ function fakeStore(): AuthorizationStore {
|
||||
};
|
||||
}
|
||||
|
||||
/** Defaults `maxBodyBytes` to a generous value for tests that are not themselves exercising the
|
||||
/** Defaults `maxBodyBytes` and `bodyMode` for tests that are not themselves exercising the
|
||||
* body-size contract, so the many pre-existing routing/authorization/CORS scenarios below do not
|
||||
* each need to restate an unrelated body-size choice; the dedicated "module route body limits"
|
||||
* suite further down overrides it explicitly to prove the contract itself. */
|
||||
* suite further down overrides them explicitly to prove the contract itself. */
|
||||
function moduleRoute(
|
||||
fields: Omit<RegisteredModuleHttpRoute, "maxBodyBytes"> & { maxBodyBytes?: number },
|
||||
fields: Omit<RegisteredModuleHttpRoute, "bodyMode" | "maxBodyBytes"> & {
|
||||
bodyMode?: RegisteredModuleHttpRoute["bodyMode"];
|
||||
maxBodyBytes?: number;
|
||||
},
|
||||
): RegisteredModuleHttpRoute {
|
||||
return Object.freeze({ maxBodyBytes: 1024, ...fields });
|
||||
return Object.freeze({ bodyMode: "buffered", maxBodyBytes: 1024, ...fields });
|
||||
}
|
||||
|
||||
/** A small typed fake `ModuleRuntimeServices` — these tests exercise route mounting,
|
||||
@@ -1634,3 +1637,375 @@ describe("module route overlap can never reach mounting", () => {
|
||||
expect(moduleRoutesOverlap(paramRoute, staticRoute)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Explicit, bounded streaming uploads (ADR 0042).
|
||||
*
|
||||
* The distinction being proven throughout is *when* the platform knows a body is too large, and what
|
||||
* it holds while finding out. In `buffered` mode the answer is "before the handler runs, holding at
|
||||
* most the route's own declared allowance". In `streamed` mode it is "as the handler consumes it,
|
||||
* holding nothing" — and the `413` still wins, even when the handler tried to answer anyway.
|
||||
*/
|
||||
describe("module route streamed body limits", () => {
|
||||
const oneMebibyte = 1024 * 1024;
|
||||
|
||||
/** A body of `size` bytes delivered in realistic chunks, with no declared `Content-Length` — the
|
||||
* chunked-transfer shape a browser `fetch` of a `ReadableStream` actually produces. */
|
||||
function chunkedBody(size: number, chunkSize = 64 * 1024): ReadableStream<Uint8Array> {
|
||||
let sent = 0;
|
||||
return new ReadableStream<Uint8Array>({
|
||||
pull(controller) {
|
||||
if (sent >= size) {
|
||||
controller.close();
|
||||
return;
|
||||
}
|
||||
const next = Math.min(chunkSize, size - sent);
|
||||
sent += next;
|
||||
controller.enqueue(new Uint8Array(next).fill(97));
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function streamingApp(options: {
|
||||
readonly handler: ModuleRouteHandler;
|
||||
readonly maxBodyBytes: number;
|
||||
readonly evaluator?: AuthorizationEvaluator;
|
||||
}) {
|
||||
return buildModuleApp({
|
||||
...(options.evaluator ? { evaluator: options.evaluator } : {}),
|
||||
registry: permissionRegistry([{ id: writePermission, baseline: "public" }]),
|
||||
routes: [
|
||||
moduleRoute({
|
||||
bodyMode: "streamed",
|
||||
handler: options.handler,
|
||||
maxBodyBytes: options.maxBodyBytes,
|
||||
method: "POST",
|
||||
moduleId: "calendar",
|
||||
path: "/uploads",
|
||||
permission: writePermission,
|
||||
}),
|
||||
],
|
||||
});
|
||||
}
|
||||
|
||||
/** Counts what the handler actually received, which is the only honest way to tell "streamed" from
|
||||
* "buffered then handed over". */
|
||||
async function countingHandler(received: { bytes: number }): Promise<ModuleRouteHandler> {
|
||||
return async (request) => {
|
||||
const body = request.body;
|
||||
if (!body) {
|
||||
return Response.json({ bytes: 0 });
|
||||
}
|
||||
const reader = body.getReader();
|
||||
for (;;) {
|
||||
const { done, value } = await reader.read();
|
||||
if (done) {
|
||||
break;
|
||||
}
|
||||
received.bytes += value.byteLength;
|
||||
}
|
||||
return Response.json({ bytes: received.bytes });
|
||||
};
|
||||
}
|
||||
|
||||
it("accepts a body of exactly the declared limit", async () => {
|
||||
const received = { bytes: 0 };
|
||||
const { app } = streamingApp({
|
||||
handler: await countingHandler(received),
|
||||
maxBodyBytes: oneMebibyte,
|
||||
});
|
||||
|
||||
const response = await app.request("/api/v1/modules/calendar/uploads", {
|
||||
body: chunkedBody(oneMebibyte),
|
||||
duplex: "half",
|
||||
headers: { origin: trustedOrigin },
|
||||
method: "POST",
|
||||
} as RequestInit);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(await response.json()).toEqual({ bytes: oneMebibyte });
|
||||
});
|
||||
|
||||
it("rejects a declared Content-Length over the limit before the handler runs", async () => {
|
||||
const received = { bytes: 0 };
|
||||
const counting = await countingHandler(received);
|
||||
const handler = vi.fn<ModuleRouteHandler>(counting);
|
||||
const { app } = streamingApp({ handler, maxBodyBytes: 1024 });
|
||||
|
||||
const response = await app.request("/api/v1/modules/calendar/uploads", {
|
||||
body: chunkedBody(2048, 16),
|
||||
duplex: "half",
|
||||
headers: { "content-length": String(1025), origin: trustedOrigin },
|
||||
method: "POST",
|
||||
} as RequestInit);
|
||||
|
||||
expect(response.status).toBe(413);
|
||||
expect(await response.json()).toEqual({ code: "REQUEST_BODY_TOO_LARGE" });
|
||||
// The cheap rejection: the declared size alone decided it, so no handler work happened and no
|
||||
// byte of the upload reached module code.
|
||||
expect(handler).not.toHaveBeenCalled();
|
||||
expect(received.bytes).toBe(0);
|
||||
});
|
||||
|
||||
it("terminates a body whose actual size exceeds a smaller declared Content-Length", async () => {
|
||||
// A misleading `Content-Length` buys nothing: the running byte count is what bounds the request,
|
||||
// so a header claiming 8 bytes cannot smuggle a megabyte past the limit.
|
||||
const received = { bytes: 0 };
|
||||
const { app } = streamingApp({
|
||||
handler: await countingHandler(received),
|
||||
maxBodyBytes: 64,
|
||||
});
|
||||
|
||||
const response = await app.request("/api/v1/modules/calendar/uploads", {
|
||||
body: chunkedBody(4096, 32),
|
||||
duplex: "half",
|
||||
headers: { "content-length": "8", origin: trustedOrigin },
|
||||
method: "POST",
|
||||
} as RequestInit);
|
||||
|
||||
expect(response.status).toBe(413);
|
||||
expect(await response.json()).toEqual({ code: "REQUEST_BODY_TOO_LARGE" });
|
||||
// It stopped promptly rather than draining the whole upload first.
|
||||
expect(received.bytes).toBeLessThanOrEqual(96);
|
||||
});
|
||||
|
||||
it("answers 413 even when the handler swallowed the stream error and tried to respond", async () => {
|
||||
// A limit a handler can swallow is not a limit.
|
||||
const handler = vi.fn<ModuleRouteHandler>(async (request) => {
|
||||
try {
|
||||
await request.arrayBuffer();
|
||||
} catch {
|
||||
// Deliberately ignored, exactly as a careless module might.
|
||||
}
|
||||
return Response.json({ ok: "handler won" });
|
||||
});
|
||||
const { app } = streamingApp({ handler, maxBodyBytes: 64 });
|
||||
|
||||
const response = await app.request("/api/v1/modules/calendar/uploads", {
|
||||
body: chunkedBody(4096, 32),
|
||||
duplex: "half",
|
||||
headers: { origin: trustedOrigin },
|
||||
method: "POST",
|
||||
} as RequestInit);
|
||||
|
||||
expect(handler).toHaveBeenCalledTimes(1);
|
||||
expect(response.status).toBe(413);
|
||||
expect(await response.json()).toEqual({ code: "REQUEST_BODY_TOO_LARGE" });
|
||||
});
|
||||
|
||||
it("never buffers the body: the handler sees the first bytes before the last are sent", async () => {
|
||||
let released: (() => void) | undefined;
|
||||
const finished = new Promise<void>((resolve) => {
|
||||
released = resolve;
|
||||
});
|
||||
let firstChunkSeen = false;
|
||||
let secondChunkQueued = false;
|
||||
const body = new ReadableStream<Uint8Array>({
|
||||
async pull(controller) {
|
||||
if (!secondChunkQueued) {
|
||||
secondChunkQueued = true;
|
||||
controller.enqueue(new Uint8Array(16).fill(97));
|
||||
return;
|
||||
}
|
||||
// Only produce the rest once the handler has demonstrably consumed the first chunk. A
|
||||
// buffering platform would deadlock here rather than reach the assertion below.
|
||||
await finished;
|
||||
controller.close();
|
||||
},
|
||||
});
|
||||
const handler = vi.fn<ModuleRouteHandler>(async (request) => {
|
||||
const reader = (request.body as ReadableStream<Uint8Array>).getReader();
|
||||
const first = await reader.read();
|
||||
firstChunkSeen = !first.done;
|
||||
released?.();
|
||||
for (;;) {
|
||||
const next = await reader.read();
|
||||
if (next.done) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
return Response.json({ ok: true });
|
||||
});
|
||||
const { app } = streamingApp({ handler, maxBodyBytes: oneMebibyte });
|
||||
|
||||
const response = await app.request("/api/v1/modules/calendar/uploads", {
|
||||
body,
|
||||
duplex: "half",
|
||||
headers: { origin: trustedOrigin },
|
||||
method: "POST",
|
||||
} as RequestInit);
|
||||
|
||||
expect(firstChunkSeen).toBe(true);
|
||||
expect(response.status).toBe(200);
|
||||
});
|
||||
|
||||
it("releases an aborted upload without taking the platform down", async () => {
|
||||
const aborted = new ReadableStream<Uint8Array>({
|
||||
start(controller) {
|
||||
controller.enqueue(new Uint8Array(16).fill(97));
|
||||
controller.error(new Error("client went away"));
|
||||
},
|
||||
});
|
||||
const handler = vi.fn<ModuleRouteHandler>(async (request) => {
|
||||
await request.arrayBuffer();
|
||||
return Response.json({ ok: true });
|
||||
});
|
||||
const { app } = streamingApp({ handler, maxBodyBytes: oneMebibyte });
|
||||
|
||||
const response = await app.request("/api/v1/modules/calendar/uploads", {
|
||||
body: aborted,
|
||||
duplex: "half",
|
||||
headers: { origin: trustedOrigin },
|
||||
method: "POST",
|
||||
} as RequestInit);
|
||||
|
||||
// An abort is a failed request, never an unhandled rejection or a 413: nothing exceeded a limit.
|
||||
expect(response.status).toBe(500);
|
||||
expect(await response.json()).toEqual({ code: "MODULE_HANDLER_FAILED" });
|
||||
});
|
||||
|
||||
it("releases a body the handler never read", async () => {
|
||||
let cancelled = false;
|
||||
const body = new ReadableStream<Uint8Array>({
|
||||
cancel() {
|
||||
cancelled = true;
|
||||
},
|
||||
pull(controller) {
|
||||
controller.enqueue(new Uint8Array(16).fill(97));
|
||||
},
|
||||
});
|
||||
const { app } = streamingApp({
|
||||
handler: async () => Response.json({ ok: true }),
|
||||
maxBodyBytes: oneMebibyte,
|
||||
});
|
||||
|
||||
const response = await app.request("/api/v1/modules/calendar/uploads", {
|
||||
body,
|
||||
duplex: "half",
|
||||
headers: { origin: trustedOrigin },
|
||||
method: "POST",
|
||||
} as RequestInit);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(cancelled).toBe(true);
|
||||
});
|
||||
|
||||
it("refuses a declared oversize before authorization, and never streams for a denied caller", async () => {
|
||||
const evaluator: AuthorizationEvaluator = {
|
||||
authorize: vi.fn().mockResolvedValue({ allowed: false }),
|
||||
};
|
||||
const handler = vi.fn<ModuleRouteHandler>(async () => Response.json({ ok: true }));
|
||||
const { app } = buildModuleApp({
|
||||
evaluator,
|
||||
registry: permissionRegistry([{ id: writePermission, baseline: "restricted" }]),
|
||||
routes: [
|
||||
moduleRoute({
|
||||
bodyMode: "streamed",
|
||||
handler,
|
||||
maxBodyBytes: 64,
|
||||
method: "POST",
|
||||
moduleId: "calendar",
|
||||
path: "/uploads",
|
||||
permission: writePermission,
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
const declaredOversize = await app.request("/api/v1/modules/calendar/uploads", {
|
||||
body: "a".repeat(65),
|
||||
headers: { "content-length": "65", origin: trustedOrigin },
|
||||
method: "POST",
|
||||
});
|
||||
expect(declaredOversize.status).toBe(413);
|
||||
expect(evaluator.authorize).not.toHaveBeenCalled();
|
||||
|
||||
// A within-limit body from a denied caller is refused by authorization, and the handler — the
|
||||
// only thing that ever reads a byte in streamed mode — never runs.
|
||||
const response = await app.request("/api/v1/modules/calendar/uploads", {
|
||||
body: chunkedBody(32, 8),
|
||||
duplex: "half",
|
||||
headers: { origin: trustedOrigin },
|
||||
method: "POST",
|
||||
} as RequestInit);
|
||||
expect(response.status).toBe(403);
|
||||
expect(handler).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still rejects an untrusted Origin before any body handling", async () => {
|
||||
const handler = vi.fn<ModuleRouteHandler>(async () => Response.json({ ok: true }));
|
||||
const { app } = streamingApp({ handler, maxBodyBytes: oneMebibyte });
|
||||
|
||||
const response = await app.request("/api/v1/modules/calendar/uploads", {
|
||||
body: chunkedBody(4096),
|
||||
duplex: "half",
|
||||
headers: { origin: untrustedOrigin },
|
||||
method: "POST",
|
||||
} as RequestInit);
|
||||
|
||||
expect(response.status).toBe(403);
|
||||
expect(await response.json()).toEqual({ code: "UNTRUSTED_ORIGIN" });
|
||||
expect(handler).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps every rejection sanitized, naming no limit, size, or body content", async () => {
|
||||
const { app } = streamingApp({
|
||||
handler: async () => Response.json({ ok: true }),
|
||||
maxBodyBytes: 64,
|
||||
});
|
||||
|
||||
const response = await app.request("/api/v1/modules/calendar/uploads", {
|
||||
body: "secret-payload-".repeat(64),
|
||||
headers: { "content-length": String(15 * 64), origin: trustedOrigin },
|
||||
method: "POST",
|
||||
});
|
||||
const body = await response.text();
|
||||
|
||||
expect(response.status).toBe(413);
|
||||
expect(body).toBe('{"code":"REQUEST_BODY_TOO_LARGE"}');
|
||||
expect(body).not.toContain("secret-payload");
|
||||
expect(body).not.toContain("64");
|
||||
expect(response.headers.get("cache-control")).toBe("no-store");
|
||||
});
|
||||
|
||||
it("still materializes a buffered body before the handler runs", async () => {
|
||||
// The pre-existing promise, unchanged: a buffered handler may call `request.json()` and know the
|
||||
// body is already there and already within its declared allowance.
|
||||
const handler = vi.fn<ModuleRouteHandler>(async (request) => {
|
||||
const text = await request.text();
|
||||
return Response.json({ length: text.length });
|
||||
});
|
||||
const { app } = buildModuleApp({
|
||||
registry: permissionRegistry([{ id: writePermission, baseline: "public" }]),
|
||||
routes: [
|
||||
moduleRoute({
|
||||
bodyMode: "buffered",
|
||||
handler,
|
||||
maxBodyBytes: 128,
|
||||
method: "POST",
|
||||
moduleId: "calendar",
|
||||
path: "/events",
|
||||
permission: writePermission,
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
const withinLimit = await app.request("/api/v1/modules/calendar/events", {
|
||||
body: chunkedBody(128, 16),
|
||||
duplex: "half",
|
||||
headers: { origin: trustedOrigin },
|
||||
method: "POST",
|
||||
} as RequestInit);
|
||||
expect(withinLimit.status).toBe(200);
|
||||
expect(await withinLimit.json()).toEqual({ length: 128 });
|
||||
|
||||
const overLimit = await app.request("/api/v1/modules/calendar/events", {
|
||||
body: chunkedBody(129, 16),
|
||||
duplex: "half",
|
||||
headers: { origin: trustedOrigin },
|
||||
method: "POST",
|
||||
} as RequestInit);
|
||||
expect(overLimit.status).toBe(413);
|
||||
// A buffered route's handler never sees an over-limit request at all.
|
||||
expect(handler).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user