[robobun]

Repro

const r = new Bun.FileSystemRouter({ style: "nextjs", dir });
// pages/top.tsx, pages/op.tsx, pages/sub/[id].tsx, pages/index.tsx exist

r.match("Xtop")?.name      // "/top"   (one junk byte where '/' belongs)
r.match(" top")?.name      // "/top"
r.match("%58top")?.name    // "/top"   (%58 decodes to 'X')
r.match("top")?.name       // "/op"    (first byte 't' stripped)
r.match("Xsub/x")?.params  // { id: "x" }
r.match("?anything")?.name // "/"      (empty path falls through to index)

Anything that feeds user-supplied strings to match() gets a confident wrong match instead of the null that would surface the bad input, and .pathname echoes a string that does not start with /.

Cause

URLPath::parse computes the route-lookup key as decoded_pathname[1.min(path_end)..path_end], stripping byte 0 without checking that it is /. The route table is keyed without the leading slash, so whatever follows the first byte is looked up directly. FileSystemRouter::match never validated the leading byte of a path-string argument before handing it to the parser.

Fix

In FileSystemRouter::match, after the full-URL branch has extracted a pathname, return null when the path does not begin with /. The full-URL branch always yields a /-prefixed pathname, and the empty-input case is already normalised to /, so only inputs that were never valid URL paths change behaviour.

The existing degenerate-input test (leading ?, %PUBLIC_URL%) is updated to expect null; its original purpose (no panic on those inputs) is preserved.

Verification

test/js/bun/util/filesystem_router.test.ts gains match() returns null when the path string does not start with '/', which exercises static routes, dynamic routes, percent-encoded junk, and the leading-? case alongside /-prefixed controls. Fails on main (Xtop matches /top); passes with this change. All 28 tests in the file pass.

---

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/util/filesystem_router.test.ts

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 91a518f8-382d-42be-b47d-c73d5f8bb8fc

📥 Commits

Reviewing files that changed from the base of the PR and between 43ee038 and 59672b6.

📒 Files selected for processing (2)

• src/runtime/api/filesystem_router.rs
• test/js/bun/util/filesystem_router.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 5:29 AM PT - Jul 12th, 2026}

❌ @robobun, your commit 59672b6 has 1 failures in Build #72174 (All Failures):

• test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js - SIGABRT on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 34028

That installs a local version of the PR into your bun-34028 executable, so you can run:

bun-34028 --bun

[claude]

LGTM — a 4-line guard that rejects non-/-prefixed path strings before they reach URLPath::parse's unconditional byte-0 strip.

Checked that the guard is placed after both the empty-input normalisation (→ /) and the full-URL branch (bun_url::URL::parse guarantees a /-prefixed pathname via its empty→/ fallback at src/url/lib.rs:715), so Request/Response and http(s):// string inputs are unaffected. The early return drops the owned path slice normally — no leak. The updated degenerate-input test expectations ("?", "%PUBLIC_URL%" → null instead of matching /) are a deliberate correction of accidental behaviour, and "%2Ftop" → null is right since the check runs pre-decode and %2F is not a structural separator.

Extended reasoning...

Overview

Adds a single guard in FileSystemRouter::match (src/runtime/api/filesystem_router.rs) returning JSValue::NULL when the resolved path string does not begin with /. Updates one existing test's expectations and adds a new test in test/js/bun/util/filesystem_router.test.ts covering static routes, dynamic routes, percent-encoded junk bytes, bare names, and leading-? inputs, plus /-prefixed controls.

Security risks

None. This tightens input validation on a routing API — previously, a junk leading byte on user-supplied input produced a confident wrong match; now it produces null. No new parsing, allocation, or unsafe code is introduced.

Level of scrutiny

Low-to-moderate. Bun.FileSystemRouter is a public API, but the change is a 4-line early-return guard with no memory-management implications: path is an owned ZigStringSlice at that point and drops cleanly on early return. I verified the guard's placement is correct relative to the two upstream normalisations — the empty/"/" case is rewritten to b"/" before the guard, and bun_url::URL::parse always yields a /-prefixed pathname (it defaults empty to b"/" and the leading-// collapse loop preserves at least one /), so Request/Response inputs and full-URL strings still match as before.

Other factors

The existing degenerate-input test had its expectations changed from matching the index route to null. That test's original invariant (no panic) is preserved by the subprocess structure; the new expectations reflect that "?" / "%PUBLIC_URL%" were never valid path inputs and only matched / by accident of the byte-0 strip. The new test is comprehensive: it includes positive controls (/top, /sub/x, "" → index) alongside the negative cases, so it can't pass vacuously. The "%2Ftop" case correctly returns null because the guard runs before percent-decoding and an encoded %2F is data, not a path separator. All 28 tests in the file are reported passing.

[claude]

No bugs found. This tightens user-facing match() semantics — inputs that previously fell through to the index route ("?foo=bar", "%PUBLIC_URL%") now return null — so worth a maintainer sign-off on the API decision even though the old behavior was clearly accidental.

Checked that the guard is placed after both normalisation branches: empty input is rewritten to "/" first, and bun_url::URL::parse always yields a /-prefixed pathname (it defaults empty to b"/" at src/url/lib.rs:715), so full-URL and Request/Response inputs are unaffected. The early return drops the owned path slice via RAII, matching the other early-return paths in this function.

Extended reasoning...

Overview

Adds a single guard in FileSystemRouter::match (src/runtime/api/filesystem_router.rs) that returns JSValue::NULL when the resolved path does not begin with /, preventing URLPath::parse from stripping an arbitrary first byte and producing false-positive route matches. Adds one new test and updates one existing degenerate-input test to expect null instead of an index-route match.

Security risks

None introduced. If anything this closes a mild input-confusion vector where malformed path strings could match unintended routes. No auth, crypto, or filesystem-write paths touched.

Level of scrutiny

Medium. The runtime change is 3 lines plus a comment, but it alters observable behavior of a public Bun API (Bun.FileSystemRouter#match). I verified the placement is correct: it sits after the empty-string→"/" normalisation and after the http/https/file URL-parse branch, and bun_url::URL::parse guarantees pathname starts with / (src/url/lib.rs:715–724), so full URLs and Request/Response arguments cannot regress. The only inputs whose result changes are strings that never began with / — which previously produced nonsensical matches ("Xtop"→/top, "top"→/op).

Other factors

• The updated existing test changes expectations for "?", "?foo=bar", and "%PUBLIC_URL%" from index-route matches to null. This is an intentional semantic change the PR calls out, but it's the kind of user-visible tightening a maintainer typically wants to acknowledge explicitly.
• No memory-safety concern on the new early return: path is an owned ZigStringSlice and drops on scope exit like the adjacent URLPath::parse error branch.
• Test coverage is thorough (static/dynamic routes, percent-encoded junk, control cases proving /-prefixed and empty inputs still resolve).

Given the API-semantics aspect, deferring rather than approving.

[robobun]

The only red lane across both CI runs (#72169, #72174) is test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js on debian 13 x64-asan, a JSC exception-scope assertion (!scope.exception() || !hasSlot in JSCJSValuePropertyInlines.h:51) during worker termination. That path does not intersect this diff, which only adds an early return Ok(JSValue::NULL) in FileSystemRouter::match. test/js/bun/util/filesystem_router.test.ts (all 28 tests) passes on every lane.

Ready for review.