[PR #888] [MERGED] fix: centralize protocol-relative URL guard to handle percent-encoded delimiters #920

Closed
opened 2026-05-06 13:10:50 +02:00 by BreizhHardware · 0 comments

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/888
Author: @southpolesteve
Created: 4/24/2026
Status: Merged
Merged: 4/24/2026
Merged by: @southpolesteve

Base: mainHead: fix/normalize-encoded-delimiters-in-protocol-guard


📝 Commits (1)

  • 4c8d73d fix: centralize protocol-relative URL guard to handle percent-encoded delimiters

📊 Changes

9 files changed (+244 additions, -43 deletions)

View changed files

📝 packages/vinext/src/deploy.ts (+21 -4)
📝 packages/vinext/src/index.ts (+7 -5)
📝 packages/vinext/src/server/app-router-entry.ts (+7 -7)
📝 packages/vinext/src/server/app-ssr-entry.ts (+4 -1)
📝 packages/vinext/src/server/prod-server.ts (+28 -19)
📝 packages/vinext/src/server/request-pipeline.ts (+58 -3)
📝 tests/deploy.test.ts (+8 -3)
📝 tests/request-pipeline.test.ts (+108 -0)
📝 tests/shims.test.ts (+3 -1)

📄 Description

Summary

Six places in the request pipeline inlined a variation of:

pathname.replaceAll("\\", "/").startsWith("//")

to reject protocol-relative paths before they reach the trailing-slash redirect emitter (which would otherwise echo them back into a Location header). The check handles literal delimiters but not their percent-encoded forms — /%5Cevil.com/ survives the guard, then normalizePathnameForRouteMatchStrict re-encodes the decoded backslash back to %5C (preserving it as part of a single path segment), so the 308 trailing-slash redirect ends up with Location: /%5Cevil.com. Browsers percent-decode Location headers and WHATWG URL treats \ as /, so the browser resolves that to a protocol-relative host.

Changes

Factor the leading-segment shape check into a new isOpenRedirectShaped helper in server/request-pipeline.ts that recognises literal (//, /\) and percent-encoded (%5C, %2F) variants, and swap each call site over to it. Also add a defense-in-depth check in normalizeTrailingSlash so a future caller that skips the upstream guard still can't emit a bad Location.

Updated call sites:

  • server/request-pipeline.tsguardProtocolRelativeUrl + normalizeTrailingSlash
  • server/prod-server.ts — App Router and Pages Router Node handlers
  • server/app-router-entry.ts — default Cloudflare Worker entry
  • server/app-ssr-entry.ts — SSR environment entry
  • index.ts — Pages Router Vite dev middleware
  • deploy.ts — generated Pages Router Cloudflare Worker (inlined copy kept in sync)

Tests

Regression tests in tests/request-pipeline.test.ts cover the encoded variants plus the defense-in-depth path. Updates the pinned string assertion in tests/deploy.test.ts that was checking for the old inlined pattern.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/888 **Author:** [@southpolesteve](https://github.com/southpolesteve) **Created:** 4/24/2026 **Status:** ✅ Merged **Merged:** 4/24/2026 **Merged by:** [@southpolesteve](https://github.com/southpolesteve) **Base:** `main` ← **Head:** `fix/normalize-encoded-delimiters-in-protocol-guard` --- ### 📝 Commits (1) - [`4c8d73d`](https://github.com/cloudflare/vinext/commit/4c8d73ddef6199a6d9b0ba08923e51602e4afc5e) fix: centralize protocol-relative URL guard to handle percent-encoded delimiters ### 📊 Changes **9 files changed** (+244 additions, -43 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/deploy.ts` (+21 -4) 📝 `packages/vinext/src/index.ts` (+7 -5) 📝 `packages/vinext/src/server/app-router-entry.ts` (+7 -7) 📝 `packages/vinext/src/server/app-ssr-entry.ts` (+4 -1) 📝 `packages/vinext/src/server/prod-server.ts` (+28 -19) 📝 `packages/vinext/src/server/request-pipeline.ts` (+58 -3) 📝 `tests/deploy.test.ts` (+8 -3) 📝 `tests/request-pipeline.test.ts` (+108 -0) 📝 `tests/shims.test.ts` (+3 -1) </details> ### 📄 Description ## Summary Six places in the request pipeline inlined a variation of: ```ts pathname.replaceAll("\\", "/").startsWith("//") ``` to reject protocol-relative paths before they reach the trailing-slash redirect emitter (which would otherwise echo them back into a `Location` header). The check handles literal delimiters but not their percent-encoded forms — `/%5Cevil.com/` survives the guard, then `normalizePathnameForRouteMatchStrict` re-encodes the decoded backslash back to `%5C` (preserving it as part of a single path segment), so the 308 trailing-slash redirect ends up with `Location: /%5Cevil.com`. Browsers percent-decode `Location` headers and WHATWG URL treats `\` as `/`, so the browser resolves that to a protocol-relative host. ## Changes Factor the leading-segment shape check into a new `isOpenRedirectShaped` helper in `server/request-pipeline.ts` that recognises literal (`//`, `/\`) and percent-encoded (`%5C`, `%2F`) variants, and swap each call site over to it. Also add a defense-in-depth check in `normalizeTrailingSlash` so a future caller that skips the upstream guard still can't emit a bad `Location`. Updated call sites: - `server/request-pipeline.ts` — `guardProtocolRelativeUrl` + `normalizeTrailingSlash` - `server/prod-server.ts` — App Router and Pages Router Node handlers - `server/app-router-entry.ts` — default Cloudflare Worker entry - `server/app-ssr-entry.ts` — SSR environment entry - `index.ts` — Pages Router Vite dev middleware - `deploy.ts` — generated Pages Router Cloudflare Worker (inlined copy kept in sync) ## Tests Regression tests in `tests/request-pipeline.test.ts` cover the encoded variants plus the defense-in-depth path. Updates the pinned string assertion in `tests/deploy.test.ts` that was checking for the old inlined pattern. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
BreizhHardware 2026-05-06 13:10:50 +02:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/vinext#920
No description provided.