From e8fba7323fa9cc7cb64431ffb8a24110798fddf6 Mon Sep 17 00:00:00 2001 From: Jonathan Baldie Date: Mon, 21 Sep 2026 09:30:56 +0100 Subject: [PATCH 1/3] fix: return 400 for JSON nested beyond the parser's stack depth (#123) Root cause: V8 applies a JSON.parse reviver recursively, one stack frame per nesting level. The enqueue reviver (unsupported-number check) made bodies nested ~3,100+ levels deep throw RangeError: Maximum call stack size exceeded. enqueueErrorResponse only maps SyntaxError, so the RangeError escaped as an uncaught 500. Plain JSON.parse and JSON.stringify both handle 100,000+ levels, so the parse step was the only point of failure. parseJsonBody now converts a RangeError raised by JSON.parse into a SyntaxError. The request gets the existing 400 "Invalid JSON" response and the Queue is not changed. The catch covers only the parse call, so RangeErrors from anywhere else still surface as 500s. Co-Authored-By: Claude Opus 5 --- src/handler.ts | 33 ++++++++++++++++++++++----------- tests/handler_test.ts | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/src/handler.ts b/src/handler.ts index ab1d35c..3e802c7 100644 --- a/src/handler.ts +++ b/src/handler.ts @@ -41,20 +41,31 @@ function isUnsupportedNumber(value: number, source: string): boolean { } function parseJsonBody(body: string) { - return JSON.parse(body, function (key: string, value: unknown) { - void key; - if (typeof value !== "number") { - return value; + try { + return JSON.parse(body, rejectUnsupportedNumbers); + } catch (error) { + // V8 applies the reviver recursively, so deeply nested JSON overflows + // the stack; treat it as unparseable input rather than a server fault. + if (error instanceof RangeError) { + throw new SyntaxError("JSON nested too deeply"); } + throw error; + } +} - // V8 supplies context.source at runtime; Deno's JSON.parse type still - // only declares the legacy two-argument reviver signature. - const context = arguments[2] as { source: string }; - if (isUnsupportedNumber(value, context.source)) { - throw new UnsupportedNumberError(); - } +function rejectUnsupportedNumbers(key: string, value: unknown) { + void key; + if (typeof value !== "number") { return value; - }); + } + + // V8 supplies context.source at runtime; Deno's JSON.parse type still + // only declares the legacy two-argument reviver signature. + const context = arguments[2] as { source: string }; + if (isUnsupportedNumber(value, context.source)) { + throw new UnsupportedNumberError(); + } + return value; } async function readRequestBody(request: Request): Promise { diff --git a/tests/handler_test.ts b/tests/handler_test.ts index 0ef7d64..ad49aa6 100644 --- a/tests/handler_test.ts +++ b/tests/handler_test.ts @@ -602,6 +602,41 @@ Deno.test("response body: invalid JSON returns 'Invalid JSON'", async () => { assertEquals(await res.text(), "Invalid JSON"); }); +Deno.test("enqueue of JSON nested beyond the parser's stack depth returns 400 and leaves the queue unchanged", async () => { + const handler = makeHandler(); + const depth = 100_000; + const res = await handler(new Request("http://localhost/enqueue/q", { + method: "POST", + body: `{"payload":${"[".repeat(depth)}${"]".repeat(depth)}}`, + headers: auth, + })); + assertEquals(res.status, 400); + assertEquals(await res.text(), "Invalid JSON"); + + const dequeueRes = await handler(new Request("http://localhost/dequeue/q", { + headers: auth, + })); + assertEquals(dequeueRes.status, 204); +}); + +Deno.test("enqueue of nested JSON within the parser's depth still succeeds", async () => { + const handler = makeHandler(); + const depth = 100; + const payload = `${"[".repeat(depth)}${"]".repeat(depth)}`; + const res = await handler(new Request("http://localhost/enqueue/q", { + method: "POST", + body: `{"payload":${payload}}`, + headers: auth, + })); + assertEquals(res.status, 200); + + const dequeueRes = await handler(new Request("http://localhost/dequeue/q", { + headers: auth, + })); + assertEquals(dequeueRes.status, 200); + assertEquals(await dequeueRes.text(), payload); +}); + Deno.test("enqueue of a JSON primitive returns 400 instead of throwing", async () => { const handler = makeHandler(); const res = await handler(new Request("http://localhost/enqueue/q", { From c21d12ec0811eea120a69a3936b6c19f2925ef3a Mon Sep 17 00:00:00 2001 From: Jonathan Baldie Date: Mon, 21 Sep 2026 09:34:57 +0100 Subject: [PATCH 2/3] refactor: map parser depth overflow via a local error type Throwing `new SyntaxError` added a module dependency and pushed handler.ts to the CouplingBetweenObjects limit (13) in the production quality gate. A local JsonNestingTooDeepError, mapped to the same 400 "Invalid JSON" response, follows the existing UnsupportedNumberError pattern and keeps coupling at 12. Co-Authored-By: Claude Opus 5 --- src/handler.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/handler.ts b/src/handler.ts index 3e802c7..ee6a9e4 100644 --- a/src/handler.ts +++ b/src/handler.ts @@ -12,6 +12,8 @@ class UnsupportedNumberError extends Error { } } +class JsonNestingTooDeepError extends Error {} + function canonicalJsonNumber(source: string): string { const match = /^(-?)(\d+)(?:\.(\d+))?(?:[eE]([+-]?\d+))?$/.exec(source); if (match === null) { @@ -47,7 +49,7 @@ function parseJsonBody(body: string) { // V8 applies the reviver recursively, so deeply nested JSON overflows // the stack; treat it as unparseable input rather than a server fault. if (error instanceof RangeError) { - throw new SyntaxError("JSON nested too deeply"); + throw new JsonNestingTooDeepError(); } throw error; } @@ -164,7 +166,7 @@ function enqueueHandler(mgr: QueueManager): RouteHandler { } function enqueueErrorResponse(error: unknown): Response { - if (error instanceof SyntaxError) { + if (error instanceof SyntaxError || error instanceof JsonNestingTooDeepError) { return new Response("Invalid JSON", { status: 400 }); } if (error instanceof UnsupportedNumberError) { From ad2b8fc7f6954b431dccfadda503d3947ecd63b0 Mon Sep 17 00:00:00 2001 From: Jonathan Baldie Date: Mon, 21 Sep 2026 11:02:32 +0100 Subject: [PATCH 3/3] test: restore persist.ts mutation coverage below the 80% gate The atomic-snapshot rewrite in #122 left persist.ts at 78-79% on Stryker, failing the per-file threshold for any PR that touches tests. Kill the surviving mutants through the public FileStore interface: full truncation of stale temp content, rethrowing unusable-temp-path errors, temp cleanup on failed rename, and multi-byte reassembly across 4096-byte read boundaries. --- tests/persist_test.ts | 79 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tests/persist_test.ts b/tests/persist_test.ts index 9ee34c7..cf279bb 100644 --- a/tests/persist_test.ts +++ b/tests/persist_test.ts @@ -404,6 +404,85 @@ Deno.test("FileStore.loadState does not read persist.dat.tmp", () => { Deno.removeSync(tmpDir, { recursive: true }); }); +// ── FileStore.replace() failure and truncation semantics ───────────────────── + +Deno.test("FileStore.replace fully truncates a stale temp file longer than the snapshot", () => { + const tmpDir = Deno.makeTempDirSync(); + // Stale temp content much longer than the new snapshot: without a real + // truncate, the overwritten prefix leaves stale line fragments behind. + const staleLine = '{"queue":"q","payload":"' + "A".repeat(300) + '","enqueue":true,"dequeue":false}\n'; + Deno.writeTextFileSync(tmpDir + "/persist.dat.tmp", staleLine + staleLine); + const persist = new Persistency.FileStore(); + persist.dir(tmpDir + "/"); + persist.replace([{ queue: "q", payload: "fresh", enqueue: true, dequeue: false }]); + persist.close(); + assertEquals(persist.loadState().map((event) => event.payload), ["fresh"]); + Deno.removeSync(tmpDir, { recursive: true }); +}); + +Deno.test("FileStore.replace rethrows errors when the temp file path is unusable", () => { + const tmpDir = Deno.makeTempDirSync(); + // A directory at persist.dat.tmp makes openSync fail after the store + // directory was created successfully. + Deno.mkdirSync(tmpDir + "/persist.dat.tmp"); + const persist = new Persistency.FileStore(); + persist.dir(tmpDir + "/"); + assertThrows( + () => persist.replace([{ queue: "q", payload: "x", enqueue: true, dequeue: false }]), + Deno.errors.IsADirectory, + ); + persist.close(); + Deno.removeSync(tmpDir, { recursive: true }); +}); + +Deno.test("FileStore.replace removes the temp file when the rename fails", () => { + const tmpDir = Deno.makeTempDirSync(); + // A directory at persist.dat makes renameSync fail after the temp file + // has been written and closed. + Deno.mkdirSync(tmpDir + "/persist.dat"); + const persist = new Persistency.FileStore(); + persist.dir(tmpDir + "/"); + assertThrows( + () => persist.replace([{ queue: "q", payload: "x", enqueue: true, dequeue: false }]), + Deno.errors.IsADirectory, + ); + let tempExists = false; + try { + Deno.statSync(tmpDir + "/persist.dat.tmp"); + tempExists = true; + } catch { /* expected */ } + assertEquals(tempExists, false); + persist.close(); + Deno.removeSync(tmpDir, { recursive: true }); +}); + +Deno.test("FileStore.loadState reassembles multi-byte characters split across 4096-byte chunks", () => { + const tmpDir = Deno.makeTempDirSync(); + // The stream decoder must hold partial multi-byte sequences between reads. + // The 2-byte "é" sits 10 bytes into the second line (after '{"queue":"'), + // so pad the first line to 4085 bytes: the é then starts at byte 4095 and + // straddles the 4096-byte read boundary. + const enc = new TextEncoder(); + const encodedLength = (payload: string, pad: number) => + enc.encode(JSON.stringify({ queue: "q", payload: payload.padEnd(pad, "A"), enqueue: true, dequeue: false }) + "\n").length; + let pad = 0; + while (encodedLength("x", pad) < 4085) pad++; + const first = JSON.stringify({ queue: "q", payload: "x".padEnd(pad, "A"), enqueue: true, dequeue: false }) + "\n"; + assertEquals(enc.encode(first).length, 4085); + // The é's first byte is the last byte of the first 4096-byte chunk. + const second = JSON.stringify({ queue: "éq", payload: "b", enqueue: true, dequeue: false }) + "\n"; + Deno.writeFileSync(tmpDir + "/persist.dat", enc.encode(first + second)); + + const persist = new Persistency.FileStore(); + persist.dir(tmpDir + "/"); + const events = persist.loadState(); + assertEquals(events.length, 2); + assertEquals(events[0].payload, "x".padEnd(pad, "A")); + assertEquals(events[1].queue, "éq"); + persist.close(); + Deno.removeSync(tmpDir, { recursive: true }); +}); + Deno.test("persist MemoryStore.replace() replaces existing events", () => { const p = new Persistency.MemoryStore(); p.saveEvent("q", "old", true);