From 2eeba97173c8984677e95bc22800948c432a0b9f Mon Sep 17 00:00:00 2001 From: Nic <162764842+Nic-Polumeyv@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:22:30 -0400 Subject: [PATCH] http: percent-decode path segments for literal route matching Bun.serve routes matched literal segments against the raw request bytes while :param segments captured raw and percent-decoded afterwards, so '/robots.tx%74' missed the literal '/robots.txt' route (and fell through to fetch) while '/user/a%20b' matched '/user/:id' with id 'a b'. Decode per segment, matching what params already do: the path is still split on raw '/' first, then each segment is percent-decoded for literal comparison, so %2F never creates a new separator. Route pattern segments are normalized the same way at registration, so a route key spelled '/caf%C3%A9' matches both request spellings. A segment with no '%' costs one memchr; a segment with any malformed escape falls back to raw-byte comparison instead of erroring. Param capture still pushes the raw bytes so the params consumer's decode is not applied twice, and pattern segments whose decoded form would look like a param or wildcard keep their raw spelling (the raw request spelling still matches them). This lives in HttpRouter so every consumer - static Response routes, function routes, HTML bundle routes, negative routes, and the HTTP/3 router - matches identically. Requests that previously fell through to a :param, wildcard, or the fetch handler now hit the literal route when an encoded spelling decodes to it; that precedence shift is the fix. Fixes #37603 --- packages/bun-uws/src/HttpRouter.h | 109 +++++++++++++++++---- test/js/bun/http/bun-serve-routes.test.ts | 112 ++++++++++++++++++++++ 2 files changed, 202 insertions(+), 19 deletions(-) diff --git a/packages/bun-uws/src/HttpRouter.h b/packages/bun-uws/src/HttpRouter.h index 8a48cd6b8ea7..cf0e4ba244c2 100644 --- a/packages/bun-uws/src/HttpRouter.h +++ b/packages/bun-uws/src/HttpRouter.h @@ -50,8 +50,79 @@ struct HttpRouter { /* Current URL cache */ std::string_view currentUrl = {}; std::string_view urlSegmentVector[MAX_URL_SEGMENTS] = {}; + /* Percent-decoded view of each segment, used for static (literal) segment + * comparison only. Param captures keep the raw bytes so downstream decoding + * (ServerRouteList) is not applied twice. Falls back to the raw view when + * the segment has no '%' or contains a malformed escape. */ + std::string_view urlMatchSegmentVector[MAX_URL_SEGMENTS] = {}; + std::string urlDecodedStorage[MAX_URL_SEGMENTS] = {}; int urlSegmentTop = -1; + static int hexValue(char c) { + if (c >= '0' && c <= '9') return c - '0'; + if (c >= 'a' && c <= 'f') return c - 'a' + 10; + if (c >= 'A' && c <= 'F') return c - 'A' + 10; + return -1; + } + + /* Percent-decode one path segment. Returns false - leaving out untouched - + * when the segment has no escapes (the common case, one memchr) or when any + * escape is malformed, in which case callers compare the raw bytes. */ + static bool decodePercentSegment(std::string_view segment, std::string &out) { + size_t first = segment.find('%'); + if (first == std::string_view::npos) { + return false; + } + out.clear(); + out.reserve(segment.length()); + out.append(segment.substr(0, first)); + for (size_t i = first; i < segment.length(); ) { + if (segment[i] == '%') { + if (i + 2 >= segment.length()) { + return false; + } + int hi = hexValue(segment[i + 1]); + int lo = hexValue(segment[i + 2]); + if (hi < 0 || lo < 0) { + return false; + } + out.push_back((char) ((hi << 4) | lo)); + i += 3; + } else { + out.push_back(segment[i]); + i++; + } + } + return true; + } + + /* Decode a literal pattern segment the same way request segments are + * decoded, so '/robots.tx%74' and '/robots.txt' register the same node. + * Segments whose decoded form would be mistaken for a param or wildcard + * node (':', '*') keep their raw spelling. */ + static void normalizePatternSegment(std::string &segment) { + if (segment.starts_with(':') || segment.starts_with('*')) { + return; + } + std::string decoded; + if (decodePercentSegment(segment, decoded) && decoded[0] != ':' && decoded[0] != '*') { + segment = std::move(decoded); + } + } + + /* Collapse a pattern segment to the form add() stores in the tree: param + * segments become only ":", literal segments are percent-decoded. Both + * add() and findHandler() navigate with this one rule, so remove() finds + * exactly what add() stored (findHandler's param arm matches on + * starts_with(':'), so the collapse is harmless there). */ + static void normalizeRegistrationSegment(std::string &segment) { + if (segment.length() > 1 && segment[0] == ':') { + segment.resize(1); + } else { + normalizePatternSegment(segment); + } + } + /* The matching tree */ struct Node { std::string name = {}; @@ -142,21 +213,20 @@ struct HttpRouter { auto segmentLength = currentUrl.find('/'); if (segmentLength == std::string::npos) { segmentLength = currentUrl.length(); + } - /* Push to url segment vector */ - urlSegmentVector[urlSegment] = currentUrl.substr(0, segmentLength); - urlSegmentTop++; - - /* Update currentUrl */ - currentUrl = currentUrl.substr(segmentLength); + /* Push to url segment vector */ + std::string_view segment = currentUrl.substr(0, segmentLength); + urlSegmentVector[urlSegment] = segment; + if (decodePercentSegment(segment, urlDecodedStorage[urlSegment])) { + urlMatchSegmentVector[urlSegment] = urlDecodedStorage[urlSegment]; } else { - /* Push to url segment vector */ - urlSegmentVector[urlSegment] = currentUrl.substr(0, segmentLength); - urlSegmentTop++; - - /* Update currentUrl */ - currentUrl = currentUrl.substr(segmentLength); + urlMatchSegmentVector[urlSegment] = segment; } + urlSegmentTop++; + + /* Update currentUrl */ + currentUrl = currentUrl.substr(segmentLength); } /* In any case we return it */ return {urlSegmentVector[urlSegment], false}; @@ -188,14 +258,17 @@ struct HttpRouter { } } } else if (p->name.starts_with(':') && !segment.empty()) { - /* Parameter match */ + /* Parameter match (raw bytes; decoded later by the params consumer) */ routeParameters.push(segment); if (executeHandlers(p.get(), urlSegment + 1, userData)) { return true; } routeParameters.pop(); - } else if (p->name == segment) { - /* Static match */ + } else if (p->name == urlMatchSegmentVector[urlSegment] || + (urlMatchSegmentVector[urlSegment].data() != segment.data() && p->name == segment)) { + /* Static match, against the percent-decoded segment. The raw + * spelling also matches so pattern segments that keep their raw + * form (see normalizePatternSegment) stay reachable. */ if (executeHandlers(p.get(), urlSegment + 1, userData)) { return true; } @@ -213,6 +286,7 @@ struct HttpRouter { for (int i = 0; !getUrlSegment(i).second; i++) { /* Go to next segment or quit */ std::string segment(getUrlSegment(i).first); + normalizeRegistrationSegment(segment); Node *next = nullptr; for (const std::unique_ptr &child : n->children) { if (((segment.starts_with(':') && child->name.starts_with(':')) || child->name == segment) && child->isHighPriority == (priority == HIGH_PRIORITY)) { @@ -288,10 +362,7 @@ struct HttpRouter { setUrl(pattern); for (int i = 0; !getUrlSegment(i).second; i++) { std::string strippedSegment(getUrlSegment(i).first); - if (strippedSegment.length() > 1 && strippedSegment[0] == ':') { - /* Parameter routes must be named only : */ - strippedSegment.resize(1); - } + normalizeRegistrationSegment(strippedSegment); node = getNode(node, strippedSegment, priority == HIGH_PRIORITY); } /* Insert handler in order sorted by priority (most significant 1 byte) */ diff --git a/test/js/bun/http/bun-serve-routes.test.ts b/test/js/bun/http/bun-serve-routes.test.ts index 13eebe267fc1..b6a834942cc6 100644 --- a/test/js/bun/http/bun-serve-routes.test.ts +++ b/test/js/bun/http/bun-serve-routes.test.ts @@ -327,6 +327,118 @@ describe("static responses", () => { }); }); +// https://github.com/oven-sh/bun/issues/37603 +describe("percent-encoded literal routes", () => { + let server: Server; + + beforeAll(() => { + server = Bun.serve({ + port: 0, + fetch: () => new Response("fallback", { status: 404 }), + routes: { + "/robots.txt": new Response("static-hit"), + "/fn/route": () => new Response("fn-hit"), + "/caf%C3%A9": () => new Response("encoded-key-hit"), + "/lit%zz": () => new Response("malformed-key-hit"), + "/api/users": () => new Response("literal"), + "/api/:rest": (req: BunRequest<"/api/:rest">) => new Response(`param ${req.params.rest}`), + "/a/b": () => new Response("two-segments"), + }, + }); + server.unref(); + }); + + afterAll(() => { + server.stop(true); + }); + + it("matches a literal static route with an encoded unreserved character", async () => { + const res = await fetch(`${server.url}robots.tx%74`); + expect(res.status).toBe(200); + expect(await res.text()).toBe("static-hit"); + }); + + it("matches a literal function route with encoded characters in any segment", async () => { + const res = await fetch(`${server.url}fn/rout%65`); + expect(res.status).toBe(200); + expect(await res.text()).toBe("fn-hit"); + const res2 = await fetch(`${server.url}f%6E/route`); + expect(res2.status).toBe(200); + expect(await res2.text()).toBe("fn-hit"); + }); + + it("matches a fully percent-encoded spelling", async () => { + const res = await fetch(`${server.url}%72%6f%62%6F%74%73%2etx%74`); + expect(res.status).toBe(200); + expect(await res.text()).toBe("static-hit"); + }); + + it("treats hex digits case-insensitively", async () => { + const res = await fetch(`${server.url}caf%C3%A9`); + expect(res.status).toBe(200); + const res2 = await fetch(`${server.url}caf%c3%a9`); + expect(res2.status).toBe(200); + }); + + it("matches an encoded route key against both request spellings", async () => { + // key registered as "/caf%C3%A9" (keys must be ASCII, so the encoded + // spelling is the only way to register this path) + expect(await (await fetch(`${server.url}caf%C3%A9`)).text()).toBe("encoded-key-hit"); + expect(await (await fetch(`${server.url}café`)).text()).toBe("encoded-key-hit"); + }); + + it("matches raw high-bit bytes against a decoded route key", async () => { + // fetch() percent-encodes non-ASCII itself, so send the raw bytes. + const request = Buffer.concat([ + Buffer.from("GET /caf"), + Buffer.from([0xc3, 0xa9]), + Buffer.from(" HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n"), + ]); + const { promise, resolve, reject } = Promise.withResolvers(); + const socket = net.connect(server.port, "127.0.0.1"); + const chunks: Buffer[] = []; + socket.on("error", reject); + socket.on("data", chunk => chunks.push(chunk)); + socket.on("end", () => resolve(Buffer.concat(chunks).toString("utf8"))); + socket.on("connect", () => socket.write(request)); + const response = await promise; + expect(response).toContain("HTTP/1.1 200"); + expect(response).toContain("encoded-key-hit"); + }); + + it("falls back to raw bytes for malformed percent sequences", async () => { + // "%zz" is not a valid escape; the segment matches its raw spelling. + const res = await fetch(`${server.url}lit%zz`); + expect(res.status).toBe(200); + expect(await res.text()).toBe("malformed-key-hit"); + // A truncated escape doesn't match a route it doesn't spell. + const res2 = await fetch(`${server.url}robots.tx%7`); + expect(res2.status).toBe(404); + }); + + it("never lets %2F create a path separator in a literal match", async () => { + // "a%2Fb" is ONE segment whose decoded value contains "/"; it must not + // match the two-segment literal route "/a/b". + const res = await fetch(`${server.url}a%2Fb`); + expect(res.status).toBe(404); + expect(await fetch(`${server.url}a/b`).then(r => r.text())).toBe("two-segments"); + }); + + it("prefers a literal route over a param route for encoded spellings", async () => { + const res = await fetch(`${server.url}api/user%73`); + expect(res.status).toBe(200); + expect(await res.text()).toBe("literal"); + const res2 = await fetch(`${server.url}api/other`); + expect(await res2.text()).toBe("param other"); + }); + + it("handles HEAD on an encoded literal static route", async () => { + const res = await fetch(`${server.url}robots.tx%74`, { method: "HEAD" }); + expect(res.status).toBe(200); + expect(await res.text()).toBe(""); + }); +}); + describe("route precedence", () => { let server: Server;