diff --git a/CONTEXT.md b/CONTEXT.md index 5da6cae..e3c6728 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -3,6 +3,7 @@ ## Glossary - **Queue**: A named, ordered sequence of items (FIFO data structure). +- **Queue Name**: The identifier of a Queue. A valid Queue Name is non-empty and at most 128 Unicode code points long; over HTTP it arrives percent-encoded and is decoded exactly once. Rules live in `src/queue_name.ts`. - **Payload**: The arbitrary data object placed onto a Queue. - **Queue Manager**: The central coordinator that tracks the lifecycles of all Queues and persists their state. - **Persist Engine**: The storage mechanism for Queue state. Currently modeled as an append-only log. diff --git a/openapi.yaml b/openapi.yaml index c03b957..cea4fc8 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -84,7 +84,7 @@ paths: schema: type: string '400': - description: Bad Request - Invalid JSON (including invalid UTF-8), unsupported number, or Queue name too long + description: Bad Request - Invalid JSON (including invalid UTF-8), unsupported number, Invalid queue name (malformed percent-encoding), or Queue name too long '401': description: Unauthorized '413': @@ -115,7 +115,7 @@ paths: '204': description: No Content - Queue is empty '400': - description: Bad Request - Queue name too long + description: Bad Request - Invalid queue name (malformed percent-encoding) or Queue name too long '401': description: Unauthorized '429': @@ -142,7 +142,7 @@ paths: '204': description: No Content - Queue is empty '400': - description: Bad Request - Queue name too long + description: Bad Request - Invalid queue name (malformed percent-encoding) or Queue name too long '401': description: Unauthorized '429': @@ -167,7 +167,7 @@ paths: schema: type: integer '400': - description: Bad Request - Queue name too long + description: Bad Request - Invalid queue name (malformed percent-encoding) or Queue name too long '401': description: Unauthorized '429': diff --git a/scripts/messcript.mjs b/scripts/messcript.mjs index 9796ba4..9f29e34 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"]], ["payload", ["src/payload.ts"]], + ["queue-name", ["src/queue_name.ts"]], ["http-handler", ["src/handler.ts"]], ["entrypoint", ["main.ts"]], ]); diff --git a/src/handler.ts b/src/handler.ts index b593e7d..47e1733 100644 --- a/src/handler.ts +++ b/src/handler.ts @@ -1,37 +1,41 @@ -import QueueManager, { QueueNameTooLongError } from "./manager.ts"; +import QueueManager from "./manager.ts"; import { RateLimiter } from "./rate_limiter.ts"; import { withAuth, withRateLimit } from "./middleware.ts"; import { Router } from "./router.ts"; import * as Payload from "./payload.ts"; +import * as QueueName from "./queue_name.ts"; type JsonPayload = Payload.Payload; type RouteHandler = Parameters[1]; -type RouteMatch = Parameters[1]; +type QueueRouteHandler = (queueName: string, request: Request) => Response | Promise; const LOG_ENCODER = Reflect.construct(TextEncoder, []); -function extractQueueName(match: RouteMatch): { name: string } | { error: Response } { - const raw = match.pathname.groups.queue; - if (raw === undefined) { - return { error: new Response("Invalid queue name", { status: 400 }) }; +function queueNameErrorResponse(error: unknown): Response { + if (error instanceof QueueName.InvalidQueueNameError) { + return new Response("Invalid queue name", { status: 400 }); } - try { - return { name: decodeURIComponent(raw) }; - } catch (error) { - if (error instanceof URIError) { - return { error: new Response("Invalid queue name", { status: 400 }) }; - } - throw error; + if (error instanceof QueueName.QueueNameTooLongError) { + return new Response("Queue name too long", { status: 400 }); } + throw error; } -function enqueueHandler(mgr: QueueManager): RouteHandler { +function queueRoute( + handle: QueueRouteHandler, + parseName: (raw: string | undefined) => string = QueueName.parseQueueName, +): RouteHandler { return async (request, match) => { - const queueResult = extractQueueName(match); - if ("error" in queueResult) { - return queueResult.error; + try { + return await handle(parseName(match.pathname.groups.queue), request); + } catch (error) { + return queueNameErrorResponse(error); } - const queueName = queueResult.name; + }; +} + +function enqueueHandler(mgr: QueueManager): QueueRouteHandler { + return async (queueName, request) => { try { const contentLength = request.headers.get("content-length"); if (contentLength && parseInt(contentLength) > Payload.DEFAULT_MAX_PAYLOAD_SIZE) { @@ -65,13 +69,6 @@ function enqueueErrorResponse(error: unknown): Response { if (error instanceof Payload.InvalidPayloadError) { return new Response(error.message, { status: 400 }); } - return queueNameErrorResponse(error); -} - -function queueNameErrorResponse(error: unknown): Response { - if (error instanceof QueueNameTooLongError) { - return new Response("Queue name too long", { status: 400 }); - } throw error; } @@ -84,53 +81,19 @@ function itemResponse(item: JsonPayload | undefined): Response { }); } -function dequeueHandler(mgr: QueueManager): RouteHandler { - return (request, match) => { - void request; - const queueResult = extractQueueName(match); - if ("error" in queueResult) { - return queueResult.error; - } - try { - const item = request.method === "HEAD" - ? mgr.peek(queueResult.name) - : mgr.dequeue(queueResult.name); - return itemResponse(item); - } catch (error) { - return queueNameErrorResponse(error); - } +function dequeueHandler(mgr: QueueManager): QueueRouteHandler { + return (queueName, request) => { + const item = request.method === "HEAD" ? mgr.peek(queueName) : mgr.dequeue(queueName); + return itemResponse(item); }; } -function peekHandler(mgr: QueueManager): RouteHandler { - return (request, match) => { - void request; - const queueResult = extractQueueName(match); - if ("error" in queueResult) { - return queueResult.error; - } - try { - return itemResponse(mgr.peek(queueResult.name)); - } catch (error) { - return queueNameErrorResponse(error); - } - }; +function peekHandler(mgr: QueueManager): QueueRouteHandler { + return (queueName) => itemResponse(mgr.peek(queueName)); } -function lengthHandler(mgr: QueueManager): RouteHandler { - return (request, match) => { - void request; - const queueResult = extractQueueName(match); - if ("error" in queueResult) { - return queueResult.error; - } - try { - const length = mgr.length(queueResult.name); - return new Response(`${length}`); - } catch (error) { - return queueNameErrorResponse(error); - } - }; +function lengthHandler(mgr: QueueManager): QueueRouteHandler { + return (queueName) => new Response(`${mgr.length(queueName)}`); } function registerRoutes(router: Router, mgr: QueueManager): void { @@ -146,10 +109,12 @@ function registerRoutes(router: Router, mgr: QueueManager): void { headers: { "Content-Type": "application/json" }, }); }); - router.post("/enqueue/:queue", enqueueHandler(mgr)); - router.get("/dequeue/:queue", dequeueHandler(mgr)); - router.get("/peek/:queue", peekHandler(mgr)); - router.get("/length/:queue", lengthHandler(mgr)); + // Enqueue only decodes here; QueueManager applies the length rule after the + // payload is validated, so payload errors keep precedence over it. + router.post("/enqueue/:queue", queueRoute(enqueueHandler(mgr), QueueName.decodeQueueName)); + router.get("/dequeue/:queue", queueRoute(dequeueHandler(mgr))); + router.get("/peek/:queue", queueRoute(peekHandler(mgr))); + router.get("/length/:queue", queueRoute(lengthHandler(mgr))); } function writeLog(destination: { writeSync(data: Uint8Array): number }, message: string): void { diff --git a/src/manager.ts b/src/manager.ts index 8bce202..849538d 100644 --- a/src/manager.ts +++ b/src/manager.ts @@ -1,12 +1,6 @@ import { QueueEvent, QueueStore } from "./persist.ts" -export const MAX_QUEUE_NAME_LENGTH = 128; - -export class QueueNameTooLongError extends Error { - constructor() { - super("Queue name too long"); - this.name = "QueueNameTooLongError"; - } -} +import { validateQueueName } from "./queue_name.ts"; +export { MAX_QUEUE_NAME_LENGTH, QueueNameTooLongError } from "./queue_name.ts"; /** * FIFO queue with O(1) amortized enqueue and dequeue. @@ -77,18 +71,12 @@ export default class Manager { return this; } - private validateName(name: string): void { - if (Array.from(name).length > MAX_QUEUE_NAME_LENGTH) { - throw new QueueNameTooLongError(); - } - } - public canCreateQueue(): boolean { return this.queues.size < this.queueCountLimit; } public canEnqueue(name: string): boolean { - this.validateName(name); + validateQueueName(name); const queue = this.find(name); if (!queue) { return this.canCreateQueue() && 0 < this.queueDepthLimit; @@ -101,7 +89,7 @@ export default class Manager { } public enqueue(name: string, payload: T): Manager { - this.validateName(name); + validateQueueName(name); const existing = this.find(name); if (!existing && !this.canCreateQueue()) { throw new Error("Queue count limit reached"); @@ -123,7 +111,7 @@ export default class Manager { } public dequeue(name: string): T | undefined { - this.validateName(name); + validateQueueName(name); const queue = this.find(name); if (!queue) { return undefined; @@ -144,7 +132,7 @@ export default class Manager { } public peek(name: string): T | undefined { - this.validateName(name); + validateQueueName(name); const queue = this.find(name); if (queue === undefined) { @@ -155,7 +143,7 @@ export default class Manager { } public length(name: string): number { - this.validateName(name); + validateQueueName(name); const queue = this.find(name); return queue ? queue.length : 0; } diff --git a/src/queue_name.ts b/src/queue_name.ts new file mode 100644 index 0000000..9ef9b42 --- /dev/null +++ b/src/queue_name.ts @@ -0,0 +1,55 @@ +export const MAX_QUEUE_NAME_LENGTH = 128; + +export class InvalidQueueNameError extends Error { + constructor(message: string = "Invalid queue name") { + super(message); + this.name = "InvalidQueueNameError"; + } +} + +export class QueueNameTooLongError extends Error { + constructor(message: string = "Queue name too long") { + super(message); + this.name = "QueueNameTooLongError"; + } +} + +/** + * Checks an already-decoded Queue name against the domain rules: it must be + * non-empty and at most MAX_QUEUE_NAME_LENGTH Unicode code points long. + */ +export function validateQueueName(name: string): string { + if (name === "") { + throw new InvalidQueueNameError(); + } + if (Array.from(name).length > MAX_QUEUE_NAME_LENGTH) { + throw new QueueNameTooLongError(); + } + return name; +} + +/** + * Percent-decodes a raw Queue name without applying the length rule. + * Callers that decode with this must validate the name before use. + */ +export function decodeQueueName(raw: string | undefined): string { + if (raw === undefined) { + throw new InvalidQueueNameError(); + } + try { + return decodeURIComponent(raw); + } catch (error) { + if (error instanceof URIError) { + throw new InvalidQueueNameError(); + } + throw error; + } +} + +/** + * Parses a raw, percent-encoded Queue name (e.g. a URL path segment) into a + * validated Queue name. + */ +export function parseQueueName(raw: string | undefined): string { + return validateQueueName(decodeQueueName(raw)); +} diff --git a/tests/handler_test.ts b/tests/handler_test.ts index a6063f8..14a21e5 100644 --- a/tests/handler_test.ts +++ b/tests/handler_test.ts @@ -2238,24 +2238,35 @@ Deno.test("malformed percent-encoded queue names return 400 and do not create qu body: JSON.stringify({ payload: "item" }), })); assertEquals(enqRes.status, 400, `enqueue with ${bad} should return 400`); + assertEquals(await enqRes.text(), "Invalid queue name"); // dequeue const deqRes = await handler(new Request(`http://localhost/dequeue/${bad}`, { headers: authHeaders, })); assertEquals(deqRes.status, 400, `dequeue with ${bad} should return 400`); + assertEquals(await deqRes.text(), "Invalid queue name"); // peek const peekRes = await handler(new Request(`http://localhost/peek/${bad}`, { headers: authHeaders, })); assertEquals(peekRes.status, 400, `peek with ${bad} should return 400`); + assertEquals(await peekRes.text(), "Invalid queue name"); // length const lenRes = await handler(new Request(`http://localhost/length/${bad}`, { headers: authHeaders, })); assertEquals(lenRes.status, 400, `length with ${bad} should return 400`); + assertEquals(await lenRes.text(), "Invalid queue name"); + + // HEAD dequeue + const headRes = await handler(new Request(`http://localhost/dequeue/${bad}`, { + method: "HEAD", + headers: authHeaders, + })); + assertEquals(headRes.status, 400, `HEAD dequeue with ${bad} should return 400`); } // Ensure no queues were created @@ -2324,3 +2335,35 @@ Deno.test("queue name length counts Unicode code points", async () => { assertEquals(rejected.status, 400); assertEquals(await rejected.text(), "Queue name too long"); }); + +Deno.test("enqueue: payload errors take precedence over an over-length queue name (#148)", async () => { + const handler = makeHandler(); + const longName = "a".repeat(129); + + const tooLarge = await handler(new Request(`http://localhost/enqueue/${longName}`, { + method: "POST", + headers: { ...authHeaders, "Content-Type": "application/json", "Content-Length": "99999999" }, + body: "{}", + })); + assertEquals(tooLarge.status, 413); + assertEquals(await tooLarge.text(), "Payload too large"); + + const badJson = await handler(new Request(`http://localhost/enqueue/${longName}`, { + method: "POST", + headers: { ...authHeaders, "Content-Type": "application/json" }, + body: "{", + })); + assertEquals(badJson.status, 400); + assertEquals(await badJson.text(), "Invalid JSON"); +}); + +Deno.test("enqueue: malformed queue name encoding takes precedence over payload errors (#148)", async () => { + const handler = makeHandler(); + const res = await handler(new Request("http://localhost/enqueue/%E0%A4%A", { + method: "POST", + headers: { ...authHeaders, "Content-Type": "application/json", "Content-Length": "99999999" }, + body: "{", + })); + assertEquals(res.status, 400); + assertEquals(await res.text(), "Invalid queue name"); +}); diff --git a/tests/manager_test.ts b/tests/manager_test.ts index 979c391..18dfe5a 100644 --- a/tests/manager_test.ts +++ b/tests/manager_test.ts @@ -1,5 +1,6 @@ import { assertEquals, assertNotEquals, assertThrows, assertRejects } from "jsr:@std/assert@1.0"; -import QueueManager, { QueueNameTooLongError } from "../src/manager.ts"; +import QueueManager, { MAX_QUEUE_NAME_LENGTH, QueueNameTooLongError } from "../src/manager.ts"; +import * as QueueName from "../src/queue_name.ts"; import { createHandler } from "../src/handler.ts"; import * as Persistency from "../src/persist.ts"; import { RateLimiter } from "../src/rate_limiter.ts"; @@ -595,6 +596,28 @@ Deno.test("manager canEnqueue validates queue name", () => { assertThrows(() => mgr.canEnqueue("x".repeat(129)), QueueNameTooLongError); }); +Deno.test("manager re-exports QueueNameTooLongError and MAX_QUEUE_NAME_LENGTH", () => { + assertEquals(QueueNameTooLongError, QueueName.QueueNameTooLongError); + assertEquals(MAX_QUEUE_NAME_LENGTH, QueueName.MAX_QUEUE_NAME_LENGTH); +}); + +Deno.test("manager applies QueueName rules to every operation without decoding", () => { + const mgr = new QueueManager(new Persistency.MemoryStore()); + for (const operation of [ + (name: string) => mgr.canEnqueue(name), + (name: string) => mgr.enqueue(name, "item"), + (name: string) => mgr.dequeue(name), + (name: string) => mgr.peek(name), + (name: string) => mgr.length(name), + ]) { + assertThrows(() => operation(""), QueueName.InvalidQueueNameError); + assertThrows(() => operation("x".repeat(129)), QueueName.QueueNameTooLongError); + } + mgr.enqueue("%41", "raw"); + assertEquals(mgr.listQueues(), ["%41"]); + assertEquals(mgr.length("A"), 0); +}); + Deno.test("manager enqueue throws when queue depth limit reached", () => { const mgr = new QueueManager(new Persistency.MemoryStore(), 2, 1000); mgr.enqueue("q", "a"); diff --git a/tests/queue_name_test.ts b/tests/queue_name_test.ts new file mode 100644 index 0000000..fde708d --- /dev/null +++ b/tests/queue_name_test.ts @@ -0,0 +1,105 @@ +import { assertEquals, assertThrows } from "jsr:@std/assert@1.0"; +import { + decodeQueueName, + InvalidQueueNameError, + MAX_QUEUE_NAME_LENGTH, + parseQueueName, + QueueNameTooLongError, + validateQueueName, +} from "../src/queue_name.ts"; + +Deno.test("queue name: MAX_QUEUE_NAME_LENGTH is 128", () => { + assertEquals(MAX_QUEUE_NAME_LENGTH, 128); +}); + +Deno.test("queue name: InvalidQueueNameError has default message and name", () => { + const error = new InvalidQueueNameError(); + assertEquals(error.message, "Invalid queue name"); + assertEquals(error.name, "InvalidQueueNameError"); +}); + +Deno.test("queue name: QueueNameTooLongError has default message and name", () => { + const error = new QueueNameTooLongError(); + assertEquals(error.message, "Queue name too long"); + assertEquals(error.name, "QueueNameTooLongError"); +}); + +Deno.test("queue name: parse returns a plain name unchanged", () => { + assertEquals(parseQueueName("orders"), "orders"); +}); + +Deno.test("queue name: parse percent-decodes the raw name", () => { + assertEquals(parseQueueName("my%20queue"), "my queue"); + assertEquals(parseQueueName("%F0%9F%98%80"), "😀"); +}); + +Deno.test("queue name: parse decodes exactly once", () => { + assertEquals(parseQueueName("%2541"), "%41"); +}); + +Deno.test("queue name: parse rejects a missing name", () => { + assertThrows(() => parseQueueName(undefined), InvalidQueueNameError, "Invalid queue name"); +}); + +Deno.test("queue name: parse rejects an empty name", () => { + assertThrows(() => parseQueueName(""), InvalidQueueNameError, "Invalid queue name"); +}); + +Deno.test("queue name: parse rejects malformed percent-encoding", () => { + assertThrows(() => parseQueueName("%"), InvalidQueueNameError, "Invalid queue name"); + assertThrows(() => parseQueueName("%E0%A4%A"), InvalidQueueNameError, "Invalid queue name"); + assertThrows(() => parseQueueName("%FF"), InvalidQueueNameError, "Invalid queue name"); +}); + +Deno.test("queue name: parse measures length after decoding", () => { + assertEquals(parseQueueName("%61".repeat(128)), "a".repeat(128)); + assertThrows(() => parseQueueName("%61".repeat(129)), QueueNameTooLongError, "Queue name too long"); +}); + +Deno.test("queue name: parse accepts 128 astral code points and rejects 129", () => { + const name128 = "😀".repeat(128); + assertEquals(parseQueueName(encodeURIComponent(name128)), name128); + assertThrows(() => parseQueueName(encodeURIComponent("😀".repeat(129))), QueueNameTooLongError); +}); + +Deno.test("queue name: decode percent-decodes without applying the length rule", () => { + assertEquals(decodeQueueName("a%20b"), "a b"); + assertEquals(decodeQueueName("%61".repeat(129)), "a".repeat(129)); +}); + +Deno.test("queue name: decode rejects missing and malformed names", () => { + assertThrows(() => decodeQueueName(undefined), InvalidQueueNameError, "Invalid queue name"); + assertThrows(() => decodeQueueName("%E0%A4%A"), InvalidQueueNameError, "Invalid queue name"); +}); + +Deno.test("queue name: validate accepts exactly MAX_QUEUE_NAME_LENGTH code points", () => { + const name = "x".repeat(MAX_QUEUE_NAME_LENGTH); + assertEquals(validateQueueName(name), name); +}); + +Deno.test("queue name: validate rejects MAX_QUEUE_NAME_LENGTH + 1 code points", () => { + assertThrows( + () => validateQueueName("x".repeat(MAX_QUEUE_NAME_LENGTH + 1)), + QueueNameTooLongError, + "Queue name too long", + ); +}); + +Deno.test("queue name: validate counts Unicode code points, not UTF-16 units", () => { + const name128 = "😀".repeat(128); + assertEquals(validateQueueName(name128), name128); + assertThrows(() => validateQueueName("😀".repeat(129)), QueueNameTooLongError); +}); + +Deno.test("queue name: validate rejects an empty name", () => { + assertThrows(() => validateQueueName(""), InvalidQueueNameError, "Invalid queue name"); +}); + +Deno.test("queue name: validate does not percent-decode", () => { + assertEquals(validateQueueName("%41"), "%41"); + assertEquals(validateQueueName("%"), "%"); +}); + +Deno.test("queue name: validate accepts a single-character name", () => { + assertEquals(validateQueueName("a"), "a"); +});