From ac4a36fbbb126f144b2bd3dd48f43a25f6258d27 Mon Sep 17 00:00:00 2001 From: Jonathan Baldie Date: Mon, 21 Sep 2026 13:02:16 +0100 Subject: [PATCH 1/2] fix: reject request bodies that are not well-formed UTF-8 (#116) readRequestBody decoded the body with Response.text(), a non-fatal UTF-8 decode that maps invalid byte sequences to U+FFFD. Inside a JSON string that still parses, so the server returned 200 and stored a corrupted payload. Decode with a fatal TextDecoder instead and answer 400 "Invalid JSON", so nothing is enqueued. Co-Authored-By: Claude Opus 5 --- README.md | 2 +- openapi.yaml | 2 +- src/handler.ts | 7 ++++++- tests/handler_test.ts | 30 ++++++++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index ba35038..4e233b1 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ To get the next payload from the `foo` queue, send a get request to `/dequeue/:q curl -X GET -H "Authorization: Bearer replace-with-a-secret-token" http://127.0.0.1:1991/dequeue/foo ``` -This returns the oldest added payload on queue `foo` as JSON and removes it, guaranteeing both the order and that each payload will only be read once. Strings, numbers, booleans, arrays, and objects all use `application/json` so a string `"0"` is distinct from the number `0`. Numeric values that JavaScript cannot represent without loss are rejected with a `400` response. +This returns the oldest added payload on queue `foo` as JSON and removes it, guaranteeing both the order and that each payload will only be read once. Strings, numbers, booleans, arrays, and objects all use `application/json` so a string `"0"` is distinct from the number `0`. Numeric values that JavaScript cannot represent without loss are rejected with a `400` response, as are request bodies that are not well-formed UTF-8. That's all you need to get started! 😎 diff --git a/openapi.yaml b/openapi.yaml index 45d4608..a23c9b4 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -84,7 +84,7 @@ paths: schema: type: string '400': - description: Bad Request - Invalid JSON, unsupported number, or Queue name too long + description: Bad Request - Invalid JSON (including a body that is not well-formed UTF-8), unsupported number, or Queue name too long '401': description: Unauthorized '413': diff --git a/src/handler.ts b/src/handler.ts index ab1d35c..99728c9 100644 --- a/src/handler.ts +++ b/src/handler.ts @@ -101,7 +101,12 @@ async function readRequestBody(request: Request): Promise { body.set(chunk, offset); offset += chunk.byteLength; } - return await new Response(body).text(); + try { + return new TextDecoder("utf-8", { fatal: true }).decode(body); + } catch { + // Not well-formed UTF-8 (RFC 8259 §8.1); rejecting beats storing U+FFFD. + return new Response("Invalid JSON", { status: 400 }); + } } function extractQueueName(match: Parameters[1]): { name: string } | { error: Response } { diff --git a/tests/handler_test.ts b/tests/handler_test.ts index 0ef7d64..d6b0452 100644 --- a/tests/handler_test.ts +++ b/tests/handler_test.ts @@ -602,6 +602,36 @@ Deno.test("response body: invalid JSON returns 'Invalid JSON'", async () => { assertEquals(await res.text(), "Invalid JSON"); }); +for (const [label, bytes] of [ + ["Latin-1 byte inside a string", [...new TextEncoder().encode('{"payload":"caf'), 0xe9, ...new TextEncoder().encode('"}')]], + ["lone continuation and truncated sequence inside a string", [...new TextEncoder().encode('{"payload":"a'), 0x80, 0x62, 0xe2, 0x82, ...new TextEncoder().encode('"}')]], +] as const) { + Deno.test(`enqueue rejects invalid UTF-8 (${label}) with 400 and enqueues nothing`, async () => { + const handler = makeHandler(); + const res = await handler(new Request("http://localhost/enqueue/q", { + method: "POST", + body: new Uint8Array(bytes), + headers: auth, + })); + assertEquals(res.status, 400); + assertEquals(await res.text(), "Invalid JSON"); + const dequeued = await handler(new Request("http://localhost/dequeue/q", { headers: auth })); + assertEquals(dequeued.status, 204); + }); +} + +Deno.test("enqueue round-trips valid multibyte UTF-8", async () => { + const handler = makeHandler(); + const res = await handler(new Request("http://localhost/enqueue/q", { + method: "POST", + body: new TextEncoder().encode('{"payload":"café €𝄞"}'), + headers: auth, + })); + assertEquals(res.status, 200); + const dequeued = await handler(new Request("http://localhost/dequeue/q", { headers: auth })); + assertEquals(await dequeued.json(), "café €𝄞"); +}); + 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 5999711dbb646c7cf1e3e3804096211969a3a115 Mon Sep 17 00:00:00 2001 From: Jonathan Baldie Date: Mon, 21 Sep 2026 13:05:21 +0100 Subject: [PATCH 2/2] refactor: move request body reading into src/body.ts Adding TextDecoder pushed readRequestBody over the cyclomatic complexity limit and the handler module to the coupling limit. Reading and strictly decoding the body now live in their own module, which is included in the production quality gate. writeLog's destination is now typed as the Deno stream it receives. Co-Authored-By: Claude Opus 5 --- scripts/messcript.mjs | 1 + src/body.ts | 57 +++++++++++++++++++++++++++++++++++++++++++ src/handler.ts | 56 ++---------------------------------------- 3 files changed, 60 insertions(+), 54 deletions(-) create mode 100644 src/body.ts diff --git a/scripts/messcript.mjs b/scripts/messcript.mjs index 12c01f6..5b44dae 100644 --- a/scripts/messcript.mjs +++ b/scripts/messcript.mjs @@ -19,6 +19,7 @@ const productionUnits = new Map([ ["queue-manager", ["src/manager.ts"]], ["persist-engine", ["src/persist.ts"]], ["http-handler", ["src/handler.ts"]], + ["request-body", ["src/body.ts"]], ["entrypoint", ["main.ts"]], ]); const requiredComplexityRules = [ diff --git a/src/body.ts b/src/body.ts new file mode 100644 index 0000000..5b92a6a --- /dev/null +++ b/src/body.ts @@ -0,0 +1,57 @@ +const MAX_BODY_SIZE = 1024 * 1024; // 1 MB + +export async function readRequestBody(request: Request): Promise { + const contentLength = request.headers.get("content-length"); + if (contentLength && parseInt(contentLength) > MAX_BODY_SIZE) { + return new Response("Payload too large", { status: 413 }); + } + + if (request.body === null) { + return ""; + } + + const reader = request.body.getReader(); + const chunks: Uint8Array[] = []; + let bodySize = 0; + try { + while (true) { + const { done, value } = await reader.read(); + if (done) { + break; + } + bodySize += value.byteLength; + if (bodySize > MAX_BODY_SIZE) { + await reader.cancel(); + return new Response("Payload too large", { status: 413 }); + } + chunks.push(value); + } + } catch { + try { + await reader.cancel(); + } catch (error) { + // The stream may already be closed or errored. + void error; + } + return new Response("Payload too large", { status: 413 }); + } finally { + reader.releaseLock(); + } + + const body = new Uint8Array(bodySize); + let offset = 0; + for (const chunk of chunks) { + body.set(chunk, offset); + offset += chunk.byteLength; + } + return decodeUtf8(body); +} + +function decodeUtf8(body: Uint8Array): string | Response { + try { + return new TextDecoder("utf-8", { fatal: true }).decode(body); + } catch { + // Not well-formed UTF-8 (RFC 8259 §8.1); rejecting beats storing U+FFFD. + return new Response("Invalid JSON", { status: 400 }); + } +} diff --git a/src/handler.ts b/src/handler.ts index 99728c9..a2191f0 100644 --- a/src/handler.ts +++ b/src/handler.ts @@ -2,8 +2,8 @@ import QueueManager, { QueueNameTooLongError } from "./manager.ts"; import { RateLimiter } from "./rate_limiter.ts"; import { withAuth, withRateLimit } from "./middleware.ts"; import { RouteHandler, Router } from "./router.ts"; +import { readRequestBody } from "./body.ts"; -const MAX_BODY_SIZE = 1024 * 1024; // 1 MB const LOG_ENCODER = new TextEncoder(); class UnsupportedNumberError extends Error { @@ -57,58 +57,6 @@ function parseJsonBody(body: string) { }); } -async function readRequestBody(request: Request): Promise { - const contentLength = request.headers.get("content-length"); - if (contentLength && parseInt(contentLength) > MAX_BODY_SIZE) { - return new Response("Payload too large", { status: 413 }); - } - - if (request.body === null) { - return ""; - } - - const reader = request.body.getReader(); - const chunks: Uint8Array[] = []; - let bodySize = 0; - try { - while (true) { - const { done, value } = await reader.read(); - if (done) { - break; - } - bodySize += value.byteLength; - if (bodySize > MAX_BODY_SIZE) { - await reader.cancel(); - return new Response("Payload too large", { status: 413 }); - } - chunks.push(value); - } - } catch { - try { - await reader.cancel(); - } catch (error) { - // The stream may already be closed or errored. - void error; - } - return new Response("Payload too large", { status: 413 }); - } finally { - reader.releaseLock(); - } - - const body = new Uint8Array(bodySize); - let offset = 0; - for (const chunk of chunks) { - body.set(chunk, offset); - offset += chunk.byteLength; - } - try { - return new TextDecoder("utf-8", { fatal: true }).decode(body); - } catch { - // Not well-formed UTF-8 (RFC 8259 §8.1); rejecting beats storing U+FFFD. - return new Response("Invalid JSON", { status: 400 }); - } -} - function extractQueueName(match: Parameters[1]): { name: string } | { error: Response } { const raw = match.pathname.groups.queue; if (raw === undefined) { @@ -251,7 +199,7 @@ function registerRoutes(router: Router, mgr: QueueManager): void { router.get("/length/:queue", lengthHandler(mgr)); } -function writeLog(destination: { writeSync(data: Uint8Array): number }, message: string): void { +function writeLog(destination: Pick, message: string): void { destination.writeSync(LOG_ENCODER.encode(`${message}\n`)); }