fix(base): collapse leading-slash run in all base-stripping sites by pi0x · Pull Request #1471 · h3js/h3

pi0x · 2026-07-14T12:40:55Z

Both the mount middleware (src/h3.ts) and requestWithBaseURL (src/utils/request.ts) stripped the base with a bare .slice(base.length), so a request like /api//evil.com stripped to a protocol-relative //evil.com. A downstream handler that redirects to that pathname yields a protocol-relative open redirect (withBase already guarded against this via withoutBase).

This routes both sites through a shared internal stripBase helper that enforces a segment boundary and collapses the leading-slash run to a single slash. requestWithBaseURL now also skips stripping when base doesn't match on a boundary, aligning it with the mount middleware.

Regression tests added for the mount fetch path, the mounted-H3 middleware path, and requestWithBaseURL. The H3 gzip bundle limit is nudged (6.6kb -> 6.65kb) to fit the fix.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved base-path URL rewriting for mounted applications to safely normalize paths and avoid malformed/protocol-relative path outcomes.
- Collapsed repeated leading slashes after base stripping to ensure consistent request handling.
- Preserved pathnames unchanged when they don’t match the configured base.
Tests
- Added mount and request edge-case coverage for slash normalization and base-stripping behavior.
- Slightly increased the H3 gzipped bundle-size benchmark allowance.

Both the `mount` middleware and `requestWithBaseURL` stripped the base via a bare `.slice(base.length)`, so a request like `/api//evil.com` stripped to a protocol-relative `//evil.com`. A downstream handler redirecting to that pathname produces a protocol-relative open redirect. Route both sites through a shared `stripBase` helper that enforces a segment boundary and collapses the leading-slash run to a single slash, matching the existing `withoutBase` behavior. Co-Authored-By: Claude Fable 5 <[email protected]>

coderabbitai · 2026-07-14T12:41:14Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 96eeefc9-9361-4543-8483-f5d33c10b7dc

📥 Commits

Reviewing files that changed from the base of the PR and between aab331d and 5440028.

📒 Files selected for processing (1)

src/utils/internal/path.ts

🚧 Files skipped from review as they are similar to previous changes (1)

src/utils/internal/path.ts

📝 Walkthrough

Walkthrough

Changes

Base-path rewriting now uses shared boundary-aware slash normalization in mounted H3 applications and requestWithBaseURL. Tests cover protocol-relative-looking paths, unmatched bases, and the updated bundle-size threshold.

Base Path Normalization

Layer / File(s)	Summary
Safe base stripping `src/utils/internal/path.ts`	Adds `stripBase` to remove matching base segments, collapse leading slashes after removal, and support trailing-slash bases through `withoutBase`.
Mount and request integration `src/h3.ts`, `src/utils/request.ts`, `test/mount.test.ts`, `test/unit/request.test.ts`, `test/bench/bundle.test.ts`	Applies `stripBase` to mounted paths and proxied request URLs, with edge-case tests and an updated bundle-size limit.

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

Possibly related PRs

h3js/h3#1319: Related to H3.mount pathname handling.
h3js/h3#1342: Introduced the requestWithBaseURL utility used by this change.

Suggested reviewers: pi0

Poem

I’m a rabbit guarding every route,
Slashes trimmed when paths shoot out.
/api//evil.com becomes safe and neat,
Mounted paths hop on steady feet.
Base lines clear, the URLs sing—
A tidy trail for everything!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main fix: collapsing leading slashes in base-stripping paths across the affected call sites.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/base-strip-slash-collapse

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

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

pi0x

APPROVE — correct, minimal, well-tested fix for the protocol-relative open-redirect vector. Verified locally on the PR head.

What I checked

Regression tests genuinely fail without the fix: reverting only src/ leaves all 6 new assertions red (mount fetch, mounted-H3 child middleware, requestWithBaseURL collapse + no-match), green with it. Both stripping sites covered.
Full suite green: pnpm vitest run test/ → 1463 passed / 39 skipped, pnpm lint clean (oxlint + oxfmt + typecheck).
Edge cases (verified against a reference impl): /api+/api→/, /api/→/, /apifoo→unchanged (segment boundary holds), /api//evil.com→/evil.com, /api///x→/x, and crucially /api/a//b→/a//b (only the leading slash run collapses — deep multi-slash paths are preserved). Root/empty base are benign.
Query/URL consistency: mount only rewrites event.url.pathname (search untouched); requestWithBaseURL re-serializes via url.href, so query survives. Good.

Bundle-size bump is justified

Measured H3 minimal bundle: pre-fix 17334 / gzip 6594 → PR 17385 / gzip 6627. New gzip exceeds the old 6600 guard, so the bump to 6650 is warranted; raw limit stays 17400 (now 15 bytes of headroom — tight but passing). Nit: the PR description says pre-fix gzip was "exactly 6600" — it actually measured 6594, but the bump is still correct.

On `stripBase` vs the existing `withoutBase` (same file)

The two overlap heavily and I initially expected reuse. But I measured it: routing both sites through withoutBase is larger (17527 / gzip 6657) because it drags withoutTrailingSlash into the core bundle. So the compact purpose-built helper is the size-conscious choice here — the duplication is defensible. Two minor notes for the author's awareness (non-blocking):

stripBase does not normalize a trailing-slash base (stripBase('/api/foo', '/api/') returns /api/foo unstripped, where withoutBase yields /foo). This is unreachable via the mount middleware (its outer guard skips trailing-slash bases) and matches the pre-existing quirk in the fetch-handler path, so no regression — but worth a comment if stripBase is ever reused elsewhere.
In the mount middleware, stripBase's internal boundary if re-checks what the outer startsWith/segment guard already proved, so it's always-true there — negligible, and keeps the helper self-contained. The per-request replace(/^\/+/, "") is a cached literal regex on matched routes only; no meaningful hot-path cost.

Nice defense-in-depth aligning all base-stripping sites with the withoutBase hardening. LGTM.

🤖 Generated with Claude Code

stripBase and withoutBase each re-implemented the same security-critical segment-boundary strip + leading-slash collapse. Make stripBase the single implementation and have withoutBase delegate to it (after normalizing a trailing-slash base via withoutTrailingSlash). Core call sites keep calling the lean stripBase directly so the bundle-size budget is unaffected. Co-Authored-By: Claude Opus 4.8 <[email protected]>

pi0x requested a review from pi0 as a code owner July 14, 2026 12:40

pi0x commented Jul 14, 2026

View reviewed changes

pi0 approved these changes Jul 15, 2026

View reviewed changes

pi0 merged commit 19b3b46 into main Jul 15, 2026
9 checks passed

pi0 deleted the fix/base-strip-slash-collapse branch July 15, 2026 10:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(base): collapse leading-slash run in all base-stripping sites#1471

fix(base): collapse leading-slash run in all base-stripping sites#1471
pi0 merged 2 commits into
mainfrom
fix/base-strip-slash-collapse

pi0x commented Jul 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

pi0x left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pi0x commented Jul 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

pi0x left a comment

Choose a reason for hiding this comment

What I checked

Bundle-size bump is justified

On stripBase vs the existing withoutBase (same file)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pi0x commented Jul 14, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading

On `stripBase` vs the existing `withoutBase` (same file)