From 20968198268ee14b934bd1984e4dedf56888dc9b Mon Sep 17 00:00:00 2001 From: Kyle June Date: Fri, 25 Sep 2026 00:22:57 -0400 Subject: [PATCH 1/2] fix: refuse request paths the URL parser would rewrite Hono routes on the path as the client sent it while React Router matches the resolved one, so GET /docs/../admin matched a /docs catch-all's middleware in Hono and ran the /admin loader in React Router, skipping /admin's guard. createServer now answers 400 before any route middleware whenever the raw path differs from new URL(url).pathname: dot segments, raw or percent-encoded, backslashes and fragments. Browsers resolve those before sending, so only a hand-built request is refused. Co-Authored-By: Claude Fable 5.1 --- docs/middleware.md | 9 +++ src/server.test.tsx | 154 ++++++++++++++++++++++++++++++++++++++++++++ src/server.tsx | 32 ++++++++- 3 files changed, 194 insertions(+), 1 deletion(-) diff --git a/docs/middleware.md b/docs/middleware.md index a6248b5..f279de2 100644 --- a/docs/middleware.md +++ b/docs/middleware.md @@ -119,6 +119,15 @@ app.use("/api/*", cors()); app.use("/admin/*", requireAdmin); ``` +Path-specific middleware matches the path as the client sent it. Before any of +it runs, the server refuses with `400` a request whose path the URL parser would +rewrite — a `..` or `.` segment, raw or percent-encoded (`%2e%2e`), a backslash, +or a fragment. Without that refusal, `GET /docs/../admin` would match `/docs/*` +middleware in Hono while React Router, which matches the resolved path, ran the +`/admin` loader, so `app.use("/admin/*", requireAdmin)` would never see it. +Browsers resolve such paths before sending them; only a hand-built request is +refused. + ### Common Patterns #### Authentication diff --git a/src/server.test.tsx b/src/server.test.tsx index 5f7b87d..b932321 100644 --- a/src/server.test.tsx +++ b/src/server.test.tsx @@ -2762,3 +2762,157 @@ describe("build id (deploy-skew handshake)", () => { } }); }); + +describe("a request path the URL parser would rewrite", () => { + async function overSocket( + server: { fetch(request: Request): Response | Promise }, + path: string, + headers: Record = {}, + ): Promise<{ status: number; body: string }> { + const listener = Deno.serve( + { port: 0, hostname: "127.0.0.1", onListen() {} }, + (request) => server.fetch(request), + ); + const connection = await Deno.connect({ + hostname: "127.0.0.1", + port: listener.addr.port, + }); + try { + const lines = [ + `GET ${path} HTTP/1.1`, + "host: localhost", + "connection: close", + ...Object.entries(headers).map(([name, value]) => `${name}: ${value}`), + ]; + await connection.write( + new TextEncoder().encode(`${lines.join("\r\n")}\r\n\r\n`), + ); + const chunks: number[] = []; + const buffer = new Uint8Array(65536); + for (let read; (read = await connection.read(buffer)) !== null;) { + chunks.push(...buffer.subarray(0, read)); + } + const raw = new TextDecoder().decode(new Uint8Array(chunks)); + return { + status: Number(raw.split(" ")[1]), + body: raw.slice(raw.indexOf("\r\n\r\n") + 4), + }; + } finally { + connection.close(); + await listener.shutdown(); + } + } + + function fixture(): { + server: ReturnType; + runs: { guard: number; admin: number; docs: number }; + } { + const runs = { guard: 0, admin: 0, docs: 0 }; + const client = new Client({ + path: "/", + main: { default: () => }, + children: [ + { + path: "docs", + main: { default: () => }, + catchall: () => + Promise.resolve({ + default: () =>
Public docs
, + }), + }, + { + path: "admin", + main: { default: () =>
Private admin
}, + }, + ], + }); + const server = createServer(import.meta.url, client, { + path: "/", + children: [ + { + path: "docs", + catchall: { + loader: () => { + runs.docs++; + return { splat: true }; + }, + }, + }, + { + path: "admin", + main: { + default: new Hono().use(() => { + runs.guard++; + throw new HttpError(403, "Guarded"); + }), + loader: () => { + runs.admin++; + return { secret: true }; + }, + }, + }, + ], + }); + return { server, runs }; + } + + for ( + const path of [ + "/docs/../admin", + "/docs/%2e%2e/admin", + "/docs/%2E%2E/admin", + "/docs/.%2e/admin", + "/docs/%2e./admin", + "/docs\\..\\admin", + "/docs/./admin", + "/docs/x/../../admin", + ] + ) { + it( + `refuses ${JSON.stringify(path)} before Hono or React Router routes it`, + async () => { + const { server, runs } = fixture(); + const response = await overSocket(server, path); + assertEquals( + response.status, + 400, + `Hono matched the raw path under /docs while React Router resolved it to /admin, so the guard on /admin never ran: ${response.status} ${response.body}`, + ); + assertEquals(runs, { guard: 0, admin: 0, docs: 0 }); + }, + ); + } + + it("refuses a data request the same way", async () => { + const { server, runs } = fixture(); + const response = await overSocket(server, "/docs/../admin", { + "x-juniper-route-id": "/admin/main", + }); + assertEquals(response.status, 400); + assertEquals(runs, { guard: 0, admin: 0, docs: 0 }); + }); + + it("still lets the route's own guard answer the resolved path", async () => { + const { server, runs } = fixture(); + const response = await overSocket(server, "/admin"); + assertEquals(response.status, 403); + assertEquals(runs, { guard: 1, admin: 0, docs: 0 }); + }); + + it("serves paths the parser leaves alone, including dots inside a segment and in the query", async () => { + const { server, runs } = fixture(); + for ( + const path of [ + "/docs/a%20b", + "/docs/v1..v2/notes.md", + "/docs/x?next=../admin", + "/docs/x?next=%2e%2e%2Fadmin", + ] + ) { + const response = await overSocket(server, path); + assertEquals(response.status, 200, `${path}: ${response.body}`); + assertStringIncludes(response.body, "Public docs"); + } + assertEquals(runs, { guard: 0, admin: 0, docs: 4 }); + }); +}); diff --git a/src/server.tsx b/src/server.tsx index 7221679..0098abf 100644 --- a/src/server.tsx +++ b/src/server.tsx @@ -45,13 +45,36 @@ function varyByRoute(headers: Headers): void { mergeVary(headers, ["accept", "x-juniper-route-id"]); } +function rawRequestPath(url: string): string { + const pathStart = url.indexOf("/", url.indexOf("://") + 3); + if (pathStart === -1) return "/"; + const queryStart = url.indexOf("?", pathStart); + return url.slice(pathStart, queryStart === -1 ? undefined : queryStart); +} + +/** + * Whether the URL parser would rewrite the request path before React Router + * matched it: a dot segment, raw (`..`) or percent-encoded (`%2e%2e`), a + * backslash, or a fragment. Hono routes on the path as sent, so such a request + * can match one route's middleware while running another route's loader. + */ +function isUnresolvedPath(request: Request): boolean { + return rawRequestPath(request.url) !== new URL(request.url).pathname; +} + /** * Creates the Hono application from generated client and server route trees. * * `Builder` normally writes this call into `main.ts`; customize behavior in route * modules instead of editing generated files. Each request gets a fresh router * context. Hono middleware runs before SSR or route-data handlers. Errors denied - * by middleware render without invoking loaders. Responses vary by `Accept` and + * by middleware render without invoking loaders. A request whose path the URL + * parser would rewrite — a `..` or `.` segment, raw or percent-encoded, a + * backslash, or a fragment — is refused with 400 before any route middleware + * runs, because Hono matches the path as sent while React Router matches the + * resolved one, and a request the two disagree on could pass one route's + * middleware and run another route's loader. Browsers resolve such paths before + * sending them, so only a hand-built request sees the refusal. Responses vary by `Accept` and * `X-Juniper-Route-Id` while retaining application cache variation. Route data * responses and redirects sent to data requests default to `Cache-Control: * private, no-cache`, plus `no-transform` when deferred; a policy route @@ -92,6 +115,13 @@ export function createServer< const projectRoot = path.dirname(path.fromFileUrl(moduleUrl)); const appWrapper = new Hono({ strict: true }); + appWrapper.use(async (c, next) => { + if (isUnresolvedPath(c.req.raw)) { + throw new HttpError(400, "Request path is not normalized"); + } + await next(); + }); + appWrapper.use(async (c, next) => { c.set("context", new RouterContextProvider()); const buildId = await getBuildId(projectRoot); From 2f54ef0d959cc11ff8c05048f2f5674c991e16a7 Mon Sep 17 00:00:00 2001 From: Kyle June Date: Fri, 25 Sep 2026 00:44:00 -0400 Subject: [PATCH 2/2] docs: state the full set the path check refuses The raw-versus-parsed comparison refuses more than dot segments: raw UTF-8 and other bytes the URL parser would percent-encode, and a Host carrying a slash, a question mark or a backslash. The createServer JSDoc and the middleware guide now say so, fail closed. Co-Authored-By: Claude Fable 5.1 --- docs/middleware.md | 9 ++++++--- src/server.tsx | 12 +++++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/docs/middleware.md b/docs/middleware.md index f279de2..110277f 100644 --- a/docs/middleware.md +++ b/docs/middleware.md @@ -124,9 +124,12 @@ it runs, the server refuses with `400` a request whose path the URL parser would rewrite — a `..` or `.` segment, raw or percent-encoded (`%2e%2e`), a backslash, or a fragment. Without that refusal, `GET /docs/../admin` would match `/docs/*` middleware in Hono while React Router, which matches the resolved path, ran the -`/admin` loader, so `app.use("/admin/*", requireAdmin)` would never see it. -Browsers resolve such paths before sending them; only a hand-built request is -refused. +`/admin` loader, so `app.use("/admin/*", requireAdmin)` would never see it. The +check compares the raw path with `URL.pathname`, so it also refuses, fail +closed, bytes the parser percent-encodes rather than resolves — raw UTF-8 such +as `/café`, `"`, `<`, `>`, `` ` ``, `{`, `}` — and a `Host` header carrying `/`, +`?` or `\`. Browsers resolve and encode such paths before sending them; only a +hand-built request is refused. ### Common Patterns diff --git a/src/server.tsx b/src/server.tsx index 0098abf..1708b79 100644 --- a/src/server.tsx +++ b/src/server.tsx @@ -56,7 +56,10 @@ function rawRequestPath(url: string): string { * Whether the URL parser would rewrite the request path before React Router * matched it: a dot segment, raw (`..`) or percent-encoded (`%2e%2e`), a * backslash, or a fragment. Hono routes on the path as sent, so such a request - * can match one route's middleware while running another route's loader. + * can match one route's middleware while running another route's loader. The + * raw string is compared with `URL.pathname`, so bytes the parser + * percent-encodes rather than resolves (raw UTF-8, `"`, `<`, `>`, `` ` ``, `{`, + * `}`) and a `Host` carrying `/`, `?` or `\` are flagged too, fail closed. */ function isUnresolvedPath(request: Request): boolean { return rawRequestPath(request.url) !== new URL(request.url).pathname; @@ -73,8 +76,11 @@ function isUnresolvedPath(request: Request): boolean { * backslash, or a fragment — is refused with 400 before any route middleware * runs, because Hono matches the path as sent while React Router matches the * resolved one, and a request the two disagree on could pass one route's - * middleware and run another route's loader. Browsers resolve such paths before - * sending them, so only a hand-built request sees the refusal. Responses vary by `Accept` and + * middleware and run another route's loader. The same comparison also refuses, + * fail closed, bytes the parser percent-encodes rather than resolves — raw + * UTF-8 such as `/café`, `"`, `<`, `>`, `` ` ``, `{`, `}` — and a `Host` + * header carrying `/`, `?` or `\`. Browsers resolve and encode such paths + * before sending them, so only a hand-built request sees the refusal. Responses vary by `Accept` and * `X-Juniper-Route-Id` while retaining application cache variation. Route data * responses and redirects sent to data requests default to `Cache-Control: * private, no-cache`, plus `no-transform` when deferred; a policy route