fix(router)!: reject empty segments inside :name+ / :name* like URLPattern

[pi0x]

Every segment a :name+, :name* or **:name takes now needs a value, as URLPattern's :name+ ([^/]+(?:/[^/]+)*). This removes the "Empty segments in :name+" row from the README differences table.

Warning

Breaking: paths with an empty segment inside the value no longer match these routes. They fall through to less specific routes (a **, a *), or match nothing.

Before / after

Route	Path	Before	After (= URLPattern)
/foo/:bar+	/foo//a	{ bar: "/a" }	no match
/foo/:bar+	/foo/a//b	{ bar: "a//b" }	no match
/foo/:bar+	/foo///	{ bar: "/" }	no match
/foo/:bar*	/foo/a//	{ bar: "a/" }	no match
/foo/:bar+/baz	/foo/a//b/baz	{ bar: "a//b" }	no match
/foo/**:bar	/foo//a	{ bar: "/a" }	no match
/foo/:bar+	/foo/a/b/	{ bar: "a/b" }	{ bar: "a/b" } (unchanged: one trailing slash is still ignored)
/foo/**	/foo//a	{ 0: "/a" }	{ 0: "/a" } (unchanged: **, * and constraints still take empty segments)

A user-written **:name follows the same rule. It is the same tree entry as :name+, and its docs already said it "needs a value".

Changes

• Interpreter: emptyParam rejects a **:name value with an empty segment, not just "". It reuses the existing slow path: only paths that contain an empty segment reach it (segments.includes("")), so other paths pay nothing new.
• Compiler: each **:name matcher is guarded by a small loop helper (VALUES) in a data slot, like RANK. Only lookups that reach a **:name matcher run it. In a microbenchmark it was faster than Array#lastIndexOf("") or a p.includes("//") flag computed for every lookup. On lookups resolved by a multi-segment **:name it costs about 5 to 10% (other lookups are unchanged).
• routeToRegExp: a **:name / :name+ / :name* is now [^/]+(?:/[^/]+)* (lazy …*? before optional segments). It can't end in /, so these routes get a plain /?$ ending:
  • the 0.11 closed (?://|…) ending and its "group unset where the router reports /" trade-off are gone, along with isRequiredSegmentGap in the capture sweep;
  • 13 sweep patterns (/a/**:r/:y?, /:x+/{b}?, …) no longer need a look-behind, so they now work in RE2 engines.
• regExpToRoute reads the new form, and the 0.11 forms still read as :x+ / :x*.
• compareRoutes / overlap: a some tail is now NON_EMPTY in every segment, not only when it is a single segment. New verdicts:
  • /a/** ⊋ /a/**:x
  • /a/:x/** ⊋ /a/:y+
  • /**/:y/p ⊋ /**:n/p
  • /a/**:rest vs /a/b/** is now partial (it was superset, which is wrong under the new rule)

New ordering carve-out (B)

The new containment conflicts with the traversal order. /p/:x/** (and /p/:x/*, /:a/**/p, /**/:y/p) is now a strict superset of /p/:x+ (and /**:n/p). The tree still lists a node's wildcard before its param child, and a suffix trie's shallower entry first. As a result, findAllRoutes lists the narrower :x+ route first and findRoute picks the broader one.

This PR does not change the router's priority: on /p/a/b, /p/:x/** still wins over /p/:x+, as before. The case is documented as carve-out B (README "Result ordering", .agents/matching.md) and pinned in find-all.test.ts. The ordering sweeps in find-all.test.ts and suffix.test.ts accept exactly this class. A real fix would be the same global re-sort that carve-out A3 needs.

Tests

• New regression tests (all failed before the fix):
  • find.test.ts: interpreter, findAllRoutes, JIT, AOT, matchAll, and routeToRegExp against the router, per route.
  • EMPTY_SEGMENT_CASES in wpt.test.ts, checked against Node 24's URLPattern. EMPTY_SEGMENT_DIFFS is gone because no diff is left.
  • router.test.ts, find-all.test.ts, and the regexp fixtures.
• The 0.11 regex forms are pinned in regexp-to-route.test.ts.
• pnpm test (lint, types, vitest) passes: 3285 tests.
• The bundle budget grows by 10 B raw / 6 B gzip. The comment in bundle.test.ts explains why.

🤖 Generated with AI assistant

Summary by CodeRabbit

• Behavior Changes
  • Named catch-all and repeated-parameter routes (**:name, :name+, and :name*) now reject values containing empty path segments, such as those created by doubled slashes. Bare wildcards retain their distinct matching behavior.
  • Route ordering and selection now account for a documented exception involving named catch-all routes.
  • Regex conversion supports additional catch-all patterns and updated capture behavior.
• Documentation
  • Clarified route matching, ordering, overlap comparisons, and regular-expression behavior.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (5)

.agents/regexp.md — configured
.agents/testing.md — configured
.agents/compiler.md — configured
.agents/matching.md — configured
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 55ce1fd7-86ec-49a7-8891-f96904cb5c4e

📥 Commits

Reviewing files that changed from the base of the PR and between 3e7aff9 and a373908.

📒 Files selected for processing (26)

• .agents/compiler.md
• .agents/matching.md
• .agents/overlap.md
• .agents/regexp.md
• .agents/testing.md
• AGENTS.md
• README.md
• src/_overlap.ts
• src/_regexp-scan.ts
• src/_subsume.ts
• src/_trailing-slash.ts
• src/compiler.ts
• src/operations/_utils.ts
• src/operations/find.ts
• src/regexp-to-route.ts
• src/regexp.ts
• test/_regexp-cases.ts
• test/bench/bundle.test.ts
• test/find-all.test.ts
• test/find.test.ts
• test/overlap.test.ts
• test/regexp-to-route.test.ts
• test/regexp.test.ts
• test/router.test.ts
• test/suffix.test.ts
• test/wpt.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

Named catch-all and repeated-parameter captures now reject empty path segments. The change updates matching guards, regular-expression generation and reversal, route overlap comparisons, ordering checks, tests, and documentation.

Changes

Named Catch-All Segment Semantics

Layer / File(s)	Summary
Reject empty segments in named captures src/compiler.ts, src/operations/*, test/find.test.ts, test/router.test.ts, test/wpt.test.ts, .agents/*, AGENTS.md, README.md	Generated matching guards and parameter checks reject empty segments in required named catch-all and repeated-parameter values. Tests and guidance cover these cases.
Generate and reverse route regular expressions src/regexp.ts, src/_regexp-scan.ts, src/_trailing-slash.ts, src/regexp-to-route.ts, test/_regexp-cases.ts, test/regexp*.test.ts, test/bench/bundle.test.ts, .agents/regexp.md, .agents/testing.md, README.md	Regular expressions encode required named segments as non-empty slash-delimited values. Trailing-slash handling and reversal support the updated forms, with fixtures and capture sweeps revised.
Update route comparison and ordering rules src/_overlap.ts, src/_subsume.ts, test/overlap.test.ts, test/find-all.test.ts, test/suffix.test.ts, .agents/matching.md, .agents/overlap.md, AGENTS.md, README.md	Shape comparison applies non-empty constraints to each required tail segment. Ordering tests and documentation cover catch-all carve-outs where a narrower route can precede a containing route.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a3739

The stricter named-capture behavior and ordering exceptions are documented and covered by regression tests. No actionable merge-blocking issue is established; merge after normal checks pass.

Security Architecture Review

Security architecture risk: 🔵 Low · up to a3739

Paths containing empty segments can stop matching a named route while still matching an existing wildcard fallback. No access-control bypass was established, but applications using these patterns for access rules should validate fallback behavior before upgrading.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The security-relevant exposure is changed route eligibility in consuming applications: a request with empty captured segments can lose a named match while retaining an existing bare-wildcard match. Whether this crosses a protection boundary depends on the consumer's route data and dispatch policy.

Trust Boundaries and Controls

• observed — Non-empty validation constrains named captures rather than rejecting every path containing empty segments. Bare wildcards remain eligible, so the new validation is a matching rule rather than a global request-rejection control.

Resilience and Maintainability Implications

• inferred — Because helper maps and slot arrays are owned by each compilation call, failure while generating the new guard does not publish partial helper state into the shared router through the inspected compilation APIs.

Hardening Proposals

• proposed — For applications using named repeated patterns to select authorization rules or middleware, validate double-slash requests against wildcard fallbacks during upgrade. Where those requests are invalid, reject them consistently before policy selection rather than relying on the named route's rejection.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 19 files. (7 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: rejecting empty segments inside repeated named parameters to match URLPattern behavior. It omits the related **:name update, but it still acc…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 19 files. (7 skipped: 7 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks each path with care,
No empty segment slips in there.
Regex leaves a clearer track,
And route comparisons answer back.
The burrow’s tests confirm the way,
Then hops along its leafy way.}

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!