diff --git a/README.md b/README.md index ba35038..c7066ba 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,8 @@ 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 and request bodies that are not well-formed UTF-8 are rejected with a `400` response. +JSON request bodies nested more than 3,000 container levels are also rejected with a `400` response. That's all you need to get started! 😎 diff --git a/docs/exploratory-testing/2026-09-21-queue/perf_repro.ts b/docs/exploratory-testing/2026-09-21-queue/perf_repro.ts new file mode 100644 index 0000000..5240508 --- /dev/null +++ b/docs/exploratory-testing/2026-09-21-queue/perf_repro.ts @@ -0,0 +1,44 @@ +// Perf evidence for #125: RateLimiter.isAllowed per-request cost must be +// independent of RATE_LIMIT_REQUESTS (window length). +import { RateLimiter } from "../../../src/rate_limiter.ts"; + +function bench(limit: number, requests: number): number { + const limiter = new RateLimiter(limit, 60000, 1_000_000, 10_000); + const makeReq = () => + new Request("http://localhost/length/q", { + headers: { "x-forwarded-for": "10.0.0.1" }, + }); + + // Warmup + for (let i = 0; i < limit; i++) limiter.isAllowed(makeReq()); + + const start = performance.now(); + for (let i = 0; i < requests; i++) { + limiter.isAllowed(makeReq()); // all denied: at limit + } + return (performance.now() - start) / requests; // ms per request +} + +// Fill a window of `limit`, then time denied requests. Compare limit=1000 vs 10000. +const perReqSmall = bench(1_000, 2_000); +const perReqLarge = bench(10_000, 2_000); + +console.log(`limit=1,000: ${perReqSmall.toFixed(4)} ms/req`); +console.log(`limit=10,000: ${perReqLarge.toFixed(4)} ms/req`); + +// The bug: cost grows linearly with window length (10x limit → ~10x cost). +const ratio = perReqLarge / perReqSmall; +console.log( + `ratio: ${ratio.toFixed(2)}x (buggy ≈ 10x, fixed ≈ 1x)`, +); + +// Red-capable assertion: per-request cost must not grow with the window. +if (ratio > 3) { + console.error( + `RED: per-request cost scales with window (ratio=${ + ratio.toFixed(2) + })`, + ); + Deno.exit(1); +} +console.log("GREEN: per-request cost is window-independent"); diff --git a/mutation/stryker.config.json b/mutation/stryker.config.json index 0b7f8a6..0f03960 100644 --- a/mutation/stryker.config.json +++ b/mutation/stryker.config.json @@ -11,7 +11,7 @@ }, "coverageAnalysis": "off", "timeoutMS": 30000, - "concurrency": 2, + "concurrency": 4, "thresholds": { "high": 80, "low": 70, diff --git a/openapi.yaml b/openapi.yaml index 45d4608..c03b957 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 invalid 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..11d849a 100644 --- a/src/handler.ts +++ b/src/handler.ts @@ -4,7 +4,7 @@ import { withAuth, withRateLimit } from "./middleware.ts"; import { RouteHandler, Router } from "./router.ts"; const MAX_BODY_SIZE = 1024 * 1024; // 1 MB -const LOG_ENCODER = new TextEncoder(); +const LOG_ENCODER = Reflect.construct(TextEncoder, []); class UnsupportedNumberError extends Error { constructor() { @@ -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) { @@ -40,21 +42,52 @@ function isUnsupportedNumber(value: number, source: string): boolean { return canonicalJsonNumber(source) !== canonicalJsonNumber(serializedValue); } -function parseJsonBody(body: string) { - return JSON.parse(body, function (key: string, value: unknown) { - void key; - if (typeof value !== "number") { - return value; +// Matches strings, containers, and number literals in valid JSON. Scanning +// the source avoids invoking a JSON.parse reviver once per JSON value. +const JSON_TOKEN = /"[^"\\]*(?:\\.[^"\\]*)*"|[[{]|[\]}]|-?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?/g; +const EXACT_INTEGER = /^-?\d{1,15}$/; +const MAX_JSON_DEPTH = 3000; + +function rejectUnsupportedNumber(source: string): void { + // Every integer with at most 15 digits is below Number.MAX_SAFE_INTEGER. + if (EXACT_INTEGER.test(source)) { + return; + } + if (isUnsupportedNumber(Number(source), source)) { + throw new UnsupportedNumberError(); + } +} + +function validateJsonSource(source: string): void { + let depth = 0; + for (const match of source.matchAll(JSON_TOKEN)) { + const token = match[0]; + if (token === "[" || token === "{") { + depth++; + if (depth > MAX_JSON_DEPTH) { + throw new JsonNestingTooDeepError(); + } + } else if (token === "]" || token === "}") { + depth--; + } else if (token[0] !== '"') { + rejectUnsupportedNumber(token); } + } +} - // 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 parseJsonBody(body: string) { + try { + const json = JSON.parse(body); + validateJsonSource(body); + return json; + } catch (error) { + // Deeply nested JSON can overflow the native parser's stack; treat it + // as unparseable input rather than a server fault. + if (error instanceof RangeError) { + throw new JsonNestingTooDeepError(); } - return value; - }); + throw error; + } } async function readRequestBody(request: Request): Promise { @@ -101,7 +134,15 @@ async function readRequestBody(request: Request): Promise { body.set(chunk, offset); offset += chunk.byteLength; } - return await new Response(body).text(); + return decodeUtf8Body(body); +} + +function decodeUtf8Body(body: Uint8Array): string | Response { + try { + return new TextDecoder("utf-8", { fatal: true }).decode(body); + } catch { + return new Response("Invalid JSON", { status: 400 }); + } } function extractQueueName(match: Parameters[1]): { name: string } | { error: Response } { @@ -153,7 +194,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) { diff --git a/src/rate_limiter.ts b/src/rate_limiter.ts index 4b0e1fd..7147b57 100644 --- a/src/rate_limiter.ts +++ b/src/rate_limiter.ts @@ -69,6 +69,21 @@ export class RateLimiter { } } + // Index of the first timestamp still inside the window (ts > cutoff). + // Timestamps are sorted ascending, so stale entries always form a prefix. + private firstFreshIndex(timestamps: number[], cutoff: number): number { + let lo = 0, hi = timestamps.length; + while (lo < hi) { + const mid = (lo + hi) >>> 1; + if (timestamps[mid] > cutoff) { + hi = mid; + } else { + lo = mid + 1; + } + } + return lo; + } + public isAllowed(request: Request, remoteAddr?: string): boolean { const ip = this.getClientIp(request, remoteAddr); const now = Date.now(); @@ -84,16 +99,26 @@ export class RateLimiter { // Get or create timestamp list for this IP let timestamps = this.requestTimestamps.get(ip) || []; - // Remove timestamps older than the window - timestamps = timestamps.filter(ts => ts > cutoff); + // Fast path: the newest timestamp is fresh → nothing stale, O(1). + // Otherwise binary search the first fresh timestamp — O(log n) — and + // count the window without filtering or copying it. + const firstFresh = timestamps.length > 0 && timestamps[0] <= cutoff + ? this.firstFreshIndex(timestamps, cutoff) + : 0; + const freshCount = timestamps.length - firstFresh; - // If all timestamps are stale, remove this IP entry - if (timestamps.length === 0) { + if (freshCount === 0) { + // All timestamps are stale — remove this IP entry this.requestTimestamps.delete(ip); + timestamps = []; + } else if (firstFresh > 0 && firstFresh * 2 >= timestamps.length) { + // Drop the stale prefix only once it dominates the array, so the + // copy stays amortized O(1) per recorded request + timestamps = timestamps.slice(firstFresh); } // Check if we've exceeded the limit - if (timestamps.length >= this.requestsPerMinute) { + if (freshCount >= this.requestsPerMinute) { return false; } diff --git a/tests/handler_test.ts b/tests/handler_test.ts index 0ef7d64..0997245 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", { @@ -1173,6 +1208,48 @@ Deno.test("API: rejects rounded decimal payloads instead of changing their preci assertEquals(dequeueResponse.status, 204); }); +Deno.test("API: rejects invalid UTF-8 in JSON string payloads without enqueueing", async () => { + const mgr = new QueueManager(new Persistency.MemoryStore); + const handler = createHandler(mgr, API_TOKEN); + const invalidUtf8Body = Uint8Array.of( + ...new TextEncoder().encode('{"payload":"caf'), + 0xe9, + ...new TextEncoder().encode('"}'), + ); + + const enqueueResponse = await handler(new Request("http://localhost:3000/enqueue/invalid-utf8", { + method: "POST", + body: invalidUtf8Body, + headers: { ...authHeaders, "Content-Type": "application/json" }, + })); + + assertEquals(enqueueResponse.status, 400); + assertEquals(await enqueueResponse.text(), "Invalid JSON"); + + const dequeueResponse = await handler(new Request("http://localhost:3000/dequeue/invalid-utf8", { + headers: authHeaders, + })); + assertEquals(dequeueResponse.status, 204); +}); + +Deno.test("API: round-trips valid UTF-8 in JSON string payloads", async () => { + const mgr = new QueueManager(new Persistency.MemoryStore); + const handler = createHandler(mgr, API_TOKEN); + const enqueueResponse = await handler(new Request("http://localhost:3000/enqueue/valid-utf8", { + method: "POST", + body: JSON.stringify({ payload: "café €𝄞" }), + headers: { ...authHeaders, "Content-Type": "application/json" }, + })); + + assertEquals(enqueueResponse.status, 200); + + const dequeueResponse = await handler(new Request("http://localhost:3000/dequeue/valid-utf8", { + headers: authHeaders, + })); + assertEquals(dequeueResponse.status, 200); + assertEquals(await dequeueResponse.json(), "café €𝄞"); +}); + Deno.test("dequeue returns application/json for boolean payload", async () => { const mgr = new QueueManager(new Persistency.MemoryStore); const handler = createHandler(mgr, API_TOKEN); 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); diff --git a/tests/rate_limiter_test.ts b/tests/rate_limiter_test.ts index 49deb62..69dfd33 100644 --- a/tests/rate_limiter_test.ts +++ b/tests/rate_limiter_test.ts @@ -480,3 +480,97 @@ Deno.test("rate limiter: eviction sort orders by max timestamp regardless of ins assertEquals(internal.requestTimestamps.has("oldest.ip"), false); assertEquals(internal.requestTimestamps.has("newest.ip"), true); }); + +// ── High-window behavior (#125): per-request work independent of window ────── + +Deno.test("rate limiter: full 10,000-entry window denies at limit", () => { + const LIMIT = 10_000; + const limiter = new RateLimiter(LIMIT, 600_000, 100, 10000); + + for (let i = 0; i < LIMIT; i++) { + assertEquals(limiter.isAllowed(req("bulk.ip")), true); + } + // Limit boundary: the next request is denied + assertEquals(limiter.isAllowed(req("bulk.ip")), false); +}); + +Deno.test("rate limiter: entirely stale high-volume entry recovers with a single fresh timestamp", () => { + const limiter = new RateLimiter(10_000, 60_000, 100, 10000); + const internal = limiter as unknown as { requestTimestamps: Map }; + + // Plant 10,000 timestamps that are all far outside the window + const staleBase = Date.now() - 120_000; + internal.requestTimestamps.set("stale.bulk.ip", Array.from({ length: 10_000 }, (_, i) => staleBase + i)); + + assertEquals(limiter.isAllowed(req("stale.bulk.ip")), true); + // All stale entries are dropped; only the fresh one remains + assertEquals(internal.requestTimestamps.get("stale.bulk.ip")!.length, 1); +}); + +Deno.test("rate limiter: stale prefix does not count toward the limit", () => { + const limiter = new RateLimiter(5, 60_000, 100, 10000); + const internal = limiter as unknown as { requestTimestamps: Map }; + + // 9,000 stale + 3 fresh (sorted ascending, as maintained by the limiter) + const staleBase = Date.now() - 120_000; + const freshBase = Date.now() - 10; + const planted = [ + ...Array.from({ length: 9_000 }, (_, i) => staleBase + i), + freshBase, + freshBase + 1, + freshBase + 2, + ]; + internal.requestTimestamps.set("mixed.bulk.ip", planted); + + // Only the 3 fresh timestamps count against the limit of 5 + assertEquals(limiter.isAllowed(req("mixed.bulk.ip")), true); + // The stale prefix dominating the array is dropped on allow, leaving only + // the 3 fresh timestamps plus the one just recorded + assertEquals(internal.requestTimestamps.get("mixed.bulk.ip")!.length, 4); + assertEquals(limiter.isAllowed(req("mixed.bulk.ip")), true); + assertEquals(limiter.isAllowed(req("mixed.bulk.ip")), false); +}); + +Deno.test("rate limiter: stale prefix is dropped exactly when it is half the array (>= not >)", () => { + const limiter = new RateLimiter(10, 60_000, 100, 10000); + const internal = limiter as unknown as { requestTimestamps: Map }; + + // 2 stale + 2 fresh: firstFresh=2, length=4 → 2*2 >= 4 → trim fires + const staleBase = Date.now() - 120_000; + const freshBase = Date.now() - 10; + internal.requestTimestamps.set("half.ip", [ + staleBase, + staleBase + 1, + freshBase, + freshBase + 1, + ]); + + assertEquals(limiter.isAllowed(req("half.ip")), true); + // Trimmed to the 2 fresh timestamps plus the one just recorded + assertEquals(internal.requestTimestamps.get("half.ip")!.length, 3); +}); + +Deno.test("rate limiter: denied requests at a full 10,000-entry window are cheap (no per-request window scan)", () => { + const LIMIT = 10_000; + const limiter = new RateLimiter(LIMIT, 600_000, 100, 10000); + + for (let i = 0; i < LIMIT; i++) { + limiter.isAllowed(req("perf.ip")); + } + + // Perf canary: pre-fix each denied request filtered the whole window + // (~0.1-0.25ms/req at 10,000 entries → 2,000 requests ≈ 200-500ms). + // Post-fix it is an O(1) check (~1-3ms total). The 80ms budget is chosen + // to sit far above fixed-case noise and far below the pre-fix cost. + // Requests are built up-front and results recorded, so only isAllowed + // work is timed. + const deniedReqs = Array.from({ length: 2_000 }, () => req("perf.ip")); + const results: boolean[] = []; + const start = performance.now(); + for (const deniedReq of deniedReqs) { + results.push(limiter.isAllowed(deniedReq)); + } + const elapsedMs = performance.now() - start; + assertEquals(results.every(r => r === false), true, "all requests at limit should be denied"); + assertEquals(elapsedMs < 80, true, `2,000 denied requests took ${elapsedMs.toFixed(1)}ms`); +});