fix: implement URL normalization for Bun and Deno to prevent protocol-relative paths

[Varixo]

No description provided.

[changeset-bot]

🦋 Changeset detected

Latest commit: f96081f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
@builder.io/qwik-city	Patch
eslint-plugin-qwik	Patch
@builder.io/qwik	Patch
create-qwik	Patch
@builder.io/qwik-react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[thejackshelton]

PR Review: fix: implement URL normalization for Bun and Deno to prevent protocol-relative paths

Summary

This PR addresses a real security vulnerability — Bun and Deno middleware don't normalize URLs before constructing them with the configured origin, allowing protocol-relative URL attacks (e.g., //attacker.com being interpreted as https://attacker.com). The fix extracts the existing Node normalization logic into a shared normalizeRequestUrl() function and improves it with a two-step regex to handle the ///attacker.com edge case.

Critical Issue: Wrong Package Path

The PR modifies packages/qwik-city/ files, which don't exist on main (v2). The qwik-city package on main only contains a node_modules/ compatibility shim. The actual middleware code lives in packages/qwik-router/. This PR will not apply cleanly to the target branch.

The same vulnerability does exist in the v2 code at:

• packages/qwik-router/src/middleware/bun/index.ts:25 — new URL(...) without normalization
• packages/qwik-router/src/middleware/deno/index.ts:38 — same pattern

The fix needs to be rebased onto the current main branch structure with:

• packages/qwik-city/ → packages/qwik-router/
• @builder.io/qwik imports → @qwik.dev/core imports
• Changeset: '@builder.io/qwik-city': patch → '@qwik.dev/router': patch

Security Analysis: The Fix Itself

The two-step regex approach is an improvement over the original single-pass regex:

Original (single regex):

url.replace(/\/\/|\\\\/g, '/')

This fails on ///attacker.com → replaces first // → //attacker.com still has a leading // (the global flag doesn't restart after replacement).

New (two-step):

url.replace(/^[\\/]{2,}/, '/').replace(/\/\/|\\\\/g, '/')

First strips all leading slashes/backslashes to one, then handles internal doubles. ///attacker.com → /attacker.com → safe.

The approach correctly handles: //, \\\\, ///, \/, /\\, and arbitrary combinations of leading slashes/backslashes.

Minor Observations

1. Regex allocation: Both LEADING_DOUBLE_SLASH_REG and DOUBLE_SLASH_REG are re-created on every call (declared inside the function). This is intentional for the /g regex (avoids lastIndex gotcha), but LEADING_DOUBLE_SLASH_REG doesn't use /g and could be a module-level constant. Very minor — not a blocker.

2. Test coverage is good: Tests cover //, \\\\, internal //, and /// with query/hash. Consider also testing \/ (mixed slash-backslash) if being thorough.

3. Other adapters are fine: Cloudflare, Netlify Edge, Vercel Edge, etc. don't have origin override functionality — they use new URL(request.url) directly, so they're not affected.

Verdict

The security fix is correct and the approach is sound. However, the PR needs to be rebased onto the current v2 codebase (packages/qwik-router/ with @qwik.dev/* imports) before it can be merged into main.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@builder.io/qwik@8463

npm i https://pkg.pr.new/@builder.io/qwik-city@8463

npm i https://pkg.pr.new/eslint-plugin-qwik@8463

npm i https://pkg.pr.new/create-qwik@8463

commit: b71ca14

[github-actions]

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name	Status	Preview	Last Commit
qwik-docs	✅ Ready (View Log)	Visit Preview	b71ca14