[robobun]

Problem

• Bun.FileSystemRouter({ style: "nextjs" }) matches a route whose next segment is a [param] against a URL that has nothing in that position. With pages/docs/[page].js, match("/docs") (and "/docs/") returns /docs/[page] with params: { page: "" }; Next.js does not match pages/docs/[page].js for /docs at all.
• Same defect, other shapes: docs/[page]/[[...rest]] also matches /docs with page: "", and blog/[slug]/[id] matches /blog//42 with slug: "".
• Because Routes::match_dynamic returns the first candidate that matches, the bogus match also shadows routes that should own the URL: with docs/[page].js and docs/[[...rest]].js both present, /docs is served by docs/[page] with an empty param instead of the optional catch-all.
• Cause: the Value::Dynamic arm of Pattern::match_ (src/router/lib.rs, around line 1190) never checks that the URL segment it is about to consume is non-empty. When the URL is already exhausted, the pattern.is_end() branch pushes path_ (empty) as the value and returns true, and the optional catch-all lookahead branch does the same; when the URL has an empty segment (//), the index_of_char(b'/') == Some(0) branch pushes an empty slice. Robustness pass across install, css, ffi, crypto, spawn, shell, and node compat #36165 fixed the case where more pattern follows the dynamic segment; these paths were left as is. The CatchAll arm already requires !path_.is_empty().

Fix

• At the top of the Value::Dynamic arm, reject the candidate when path_ is empty or starts with /, i.e. when there is no non-empty URL segment for the parameter to take. The rejection clears params, like the arm's other failure paths, so a param recorded by an earlier segment of the losing route (org in [org]/settings/[id] vs /acme/settings) does not leak into the route that is tried next.
• Correct because a [param] segment stands for exactly one non-empty path segment (Next.js compiles it to ([^/]+?)), so a URL with no segment there cannot be a match for the route; returning false lets match_dynamic continue to the next candidate, which is how /docs reaches docs/[[...rest]]. Static segments are unaffected (they already compare against the exact segment), and catch-all segments keep their existing checks.
• Verified with test/js/bun/util/filesystem_router.test.ts: two new tests, one table of trees (docs/[page], docs/[page]/[[...rest]], [org]/settings/[id], blog/[slug]/[id], blog/[slug]/[[...rest]]) asserting the exact { name, params } or null for the short, trailing-slash, //, and control URLs, and one asserting that the short URL falls through to docs/[[...rest]] (params: {}) and to [org]/[...rest] (params: { org, rest }, which also fails if the new rejection did not clear org). Both fail on the released bun 1.4.0 with the page: "" result shown above and pass with this change; the whole file (35 tests) passes.
• src/router/lib.rs gains pattern_rejects_empty_dynamic_segment (the five patterns above at the Pattern::match_ level, also asserting params ends up empty) and a positive docs/[page]/[[...rest]] vs docs/intro row in pattern_match. Run with bun run rust:miri -p bun_router --lib -- pattern (the crate's test binary does not link outside Miri because bun_core::strings calls into the C++ Highway helpers): all three pass, and the new one fails with the guard disabled.

Related open PRs, each a different defect in the same file: #39291 (dynamic route ordering, which found this one), #39290 (root-level [[...slug]] vs /), #36680 (param leak on other failure paths and single-character trailing segments; it removes the per-path params.clear() calls in favour of one truncate in a wrapper, so whichever lands second drops or keeps one clear() line).

Background

• FileSystemRouter.match() looks the path up in a hash map of fully static routes first, then walks the sorted list of dynamic routes and returns the first one for which Pattern::match_ returns true, so for dynamic routes list order is precedence and a false positive from one route hides every route after it.
• Pattern::match_ walks the route name one pattern at a time (Pattern::init yields the next static segment, [param], [...param] or [[...param]]) while consuming the URL (path_, already lowercased and without its leading slash, with trailing slashes and /index suffixes stripped by the caller) segment by segment. pattern.is_end() reports that the pattern just consumed was the last one in the route name.
• Every candidate route in one match() call shares a single params scratch list; a route that pushed a param and then failed has to clear it, otherwise the winning route reports that name twice (the symptom from Bun.FileSystemRouter router match unknown behaviour #12206).

Released bun 1.4.0 vs this branch on the repro trees

routes: /docs/[page]  /opt/[page]/[[...rest]]  /blog/[slug]/[id]  /both/[page]  /both/[[...rest]]

                 bun 1.4.0                                   this branch
"/docs"      =>  /docs/[page] {"page":""}                    null
"/docs/"     =>  /docs/[page] {"page":""}                    null
"/docs/x"    =>  /docs/[page] {"page":"x"}                   /docs/[page] {"page":"x"}
"/opt"       =>  /opt/[page]/[[...rest]] {"page":""}         null
"/opt/x"     =>  /opt/[page]/[[...rest]] {"page":"x"}        /opt/[page]/[[...rest]] {"page":"x"}
"/blog//b"   =>  /blog/[slug]/[id] {"slug":"","id":"b"}      null
"/blog/a/b"  =>  /blog/[slug]/[id] {"slug":"a","id":"b"}     /blog/[slug]/[id] {"slug":"a","id":"b"}
"/both"      =>  /both/[page] {"page":""}                    /both/[[...rest]] {}
"/both/x"    =>  /both/[page] {"page":"x"}                   /both/[page] {"page":"x"}
"/both/x/y"  =>  /both/[[...rest]] {"rest":"x/y"}            /both/[[...rest]] {"rest":"x/y"}

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 1 minute

Limit details: You’ve used all 5 included reviews currently available under your plan.

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: 372f67e0-16e9-4846-be89-ccd97b7cb7a9

📥 Commits

Reviewing files that changed from the base of the PR and between 8c5296a and 1cefe5c.

📒 Files selected for processing (2)

• src/router/lib.rs
• test/js/bun/util/filesystem_router.test.ts

---

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

[robobun]

Status: closed as a duplicate. The Dynamic arm guard this PR adds is already part of #39290 (opened shortly before this one), together with tests for the same posts/[id] vs /posts, [id]/[[...rest]] and [org]/settings/[id] shapes, so the fix for the reported bug is owned there.

The one case this branch covers beyond #39290 is an empty segment in the dynamic position (blog/[slug]/[id] matching /blog//42 with slug: "", which needs || path_[0] == b'/' in the same guard); that is noted on #39290 with the rows to copy, and this branch stays up as the reference.

Reproduced on the released bun 1.4.0 (main 8c5296ac45): with pages/docs/[page].js, new Bun.FileSystemRouter({ dir, style: "nextjs" }).match("/docs") returns /docs/[page] with params: { page: "" }.

Fix: #39290.

[claude]

LGTM — a 4-line guard at the top of the Value::Dynamic arm that rejects an empty URL segment, mirroring the !path_.is_empty() check the CatchAll arm already has.

What was reviewed:

• Traced the three previously-accepting paths in the Dynamic arm (empty path_ at is_end, empty at optional-catch-all lookahead, index_of_char == Some(0) on //) — all now short-circuit before pushing an empty param.
• Checked path_ cannot legitimately start with / at this point: the Static arm's [segment.len()+1..] and the entry trim_left always advance past the separator, so the only way to hit path_[0] == b'/' is a // in the input.
• Verified the fall-through tests' route ordering holds under sort_by_name (Dynamic < CatchAll < OptionalCatchAll), and that params.clear() on the new rejection is exercised by the [org]/[...rest] case.

Extended reasoning...

Overview

Adds an early rejection at the top of the Value::Dynamic match arm in Pattern::match_ (src/router/lib.rs): when path_ is empty or starts with /, there is no URL segment for a [param] to bind, so clear params and return false. Adds one positive row to the existing pattern_match unit test, a new pattern_rejects_empty_dynamic_segment unit test asserting no-match and empty-params for five route/URL shapes, and two integration tests in filesystem_router.test.ts (a table asserting exact {name, params} or null per URL, and a fall-through test proving the rejected route yields to the next candidate without leaking params).

Security risks

None. This is pure route-pattern matching over already-normalized byte slices; no allocation, no unsafe, no external I/O. The change only makes matching stricter (previously-matched inputs now return null), which cannot introduce new exposure.

Level of scrutiny

Medium. The production change is 4 lines and mechanically parallels the existing CatchAll arm's non-empty check. It is a user-visible behavior change (match("/docs") on docs/[page].tsx now returns null instead of {page: ""}), but that is the fix — Next.js compiles [param] to ([^/]+?), so the old result was a compat bug under style: "nextjs". I traced each of the three sub-paths inside the Dynamic arm that previously produced an empty param and confirmed the new guard preempts all of them, and confirmed that a leading / on path_ can only arise from a // in the input (segment-advance logic elsewhere always steps past exactly one separator).

Other factors

Test coverage is thorough: positive controls alongside every negative, exact-object toEqual assertions (not toMatchObject), the Rust unit test asserts params.is_empty() after rejection so the params.clear() line is load-bearing, and the fall-through test's [org]/[...rest] case would surface a duplicated org if clearing were dropped. The PR description documents fail-before/pass-after on both the Rust and TS suites and flags the merge-order interaction with #36680 (which centralizes the per-path params.clear() calls). No prior human reviews; the only timeline entry is a CodeRabbit rate-limit notice.

[robobun]

^{Updated 9:51 PM PT - Aug 15th, 2026}

✅ @robobun, your commit 1cefe5cf83b89beedca85e91e3a6b269db48bd5b passed in Build #99107! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 39294

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

bun-39294 --bun

[robobun]

Closing: the guard here is already carried by #39290, which also owns the root-level optional catch-all fix that depends on it. The extra empty-segment (//) case is noted on #39290.