[PR #1034] [MERGED] refactor(app-rsc): extract request normalization into server/app-rsc-request-normalization #1038

New issue

Closed

opened 2026-05-06 13:11:41 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:11:41 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/1034
Author: @NathanDrake2406
Created: 5/3/2026
Status: ✅ Merged
Merged: 5/3/2026
Merged by: @james-elicx

Base: main ← Head: refactor/app-rsc-request-normalization

📝 Commits (8)

01cf0eb refactor(app-rsc): introduce app-rsc-request-normalization module
9b027b5 refactor(app-rsc): wire normalizeRscRequest into the entry and callsites
b1a40e2 fix(app-rsc): remove orphaned url declaration from _handleRequest
b0e5fff fix(test): update cleanPathname assertion for normalizeRscRequest destructuring
940de65 refactor(app-rsc): extract normalizeMountedSlotsHeader into neutral module and add null-byte test
dcc1fda Merge remote-tracking branch 'upstream/main' into refactor/app-rsc-request-normalization
a257332 ci: retrigger CI
b197314 ci: retrigger CI 2

📊 Changes

7 files changed (+502 additions, -65 deletions)

View changed files

📝 packages/vinext/src/entries/app-rsc-entry.ts (+13 -39)
📝 packages/vinext/src/server/app-elements.ts (+1 -10)
➕ packages/vinext/src/server/app-mounted-slots-header.ts (+19 -0)
➕ packages/vinext/src/server/app-rsc-request-normalization.ts (+107 -0)
📝 packages/vinext/src/server/isr-cache.ts (+2 -14)
📝 tests/app-router.test.ts (+2 -2)
➕ tests/app-rsc-request-normalization.test.ts (+358 -0)

📄 Description

What this changes

Introduces server/app-rsc-request-normalization.ts and wires it into the App
Router RSC entry. The generated entry no longer owns the normalization logic
itself; it calls normalizeRscRequest and destructures the typed result.

Also extracts normalizeMountedSlotsHeader into a neutral module
(server/app-mounted-slots-header.ts) used by all three consumers: request
normalization (reads the incoming header), app-elements (outgoing header
construction), and isr-cache (RSC cache key generation). The generated RSC
entry no longer calls the cache-layer export directly; request normalization
now owns reading and canonicalising the incoming header.

Why

The normalization pipeline is the most security and compatibility-sensitive
part of the request lifecycle: protocol-relative URL open redirects,
%2F-encoded path segment boundaries, basePath bypass via /__vinext/,
RSC content-type detection, and null-byte header injection all live here.
Having this logic inlined in a template string means:

Zero unit test coverage for the security-sensitive paths
Impossible to audit the ordering invariants (guard must fire before
normalizePath collapses //) by reading a single file
normalizeMountedSlotsHeader duplicated across isr-cache.ts and
app-elements.ts (with the copy in app-elements.ts already diverging
from the canonical isr-cache.ts version)

Approach

normalizeRscRequest(request, basePath) encodes early exits as Response
returns (400/404) and success as NormalizedRscRequest. The discriminant is
instanceof Response, which callers already use for every other early-exit
check in the entry. No new control-flow patterns introduced.

Step ordering inside the function is documented with numbered comments that
explain the security constraint each ordering decision satisfies:

Step 2 (protocol-relative guard) must precede step 4 (normalizePath):
normalizePath collapses //evil.com to /evil.com, causing the guard
to miss it. Source: vercel/next.js — server/lib/router-utils/guard-protocol-relative-url.ts
Step 3 (strict percent-decode) must precede step 5 (basePath check):
a %2F-encoded slash in the basePath position would decode to / and
create a fake match. Source: vercel/next.js — server/lib/router-utils/decode-path-params.ts
Step 5 (/__vinext/ bypass): internal prerender endpoints must be
reachable regardless of the configured basePath. These are
/__vinext/prerender/* endpoints consumed by wrangler unstable_startWorker
during Cloudflare Workers builds.

normalizeMountedSlotsHeader moves to server/app-mounted-slots-header.ts and
is re-exported from isr-cache.ts for backward compatibility. Only the
app-elements.ts private copy is removed — the generated entry was already
importing it from isr-cache.ts.

Validation

44 behavior tests in tests/app-rsc-request-normalization.test.ts cover:

Protocol-relative variants: //, /\, /%5C, /%2F
Ordering guarantee: guard fires before normalizePath (tested explicitly)
Malformed percent sequences: %GG, truncated %, single-digit %A
basePath enforcement: missing prefix, exact match, non-segment-boundary prefix
/__vinext/ bypass under a configured basePath
%2F preservation (not decoded to path separator)
RSC detection: .rsc suffix and Accept: text/x-component
cleanPathname stripping
Interception context header: absent, empty, legitimate value, null-byte injection
Mounted slots: sort, dedup, absent, blank

395 tests pass across the 6 affected test files (new + isr-cache, app-elements,
request-pipeline, entry-templates, app-router).

Risks / follow-ups

The app-rsc-entry.ts generator now has one fewer resolved path variable
(normalizePathnameForRouteMatchStrict and guardProtocolRelativeUrl no
longer need separate resolveEntryPath entries). The normalizePathModulePath
and routingUtilsPath constants are retained because __normalizePath and
__normalizePathnameForRouteMatch are still used in the handler() outer
function for applyConfigHeadersToResponse.

No behavior change. The generated RSC entry produces the same request handling
semantics; only the source of the logic has moved.

_{🔄 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/1034 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 5/3/2026 **Status:** ✅ Merged **Merged:** 5/3/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `refactor/app-rsc-request-normalization` --- ### 📝 Commits (8) - [`01cf0eb`](https://github.com/cloudflare/vinext/commit/01cf0eb91a3b500bb27fdef7ffd3bcc7a68108a0) refactor(app-rsc): introduce app-rsc-request-normalization module - [`9b027b5`](https://github.com/cloudflare/vinext/commit/9b027b58e00e7b88e0ece621cbbe281a17b7862f) refactor(app-rsc): wire normalizeRscRequest into the entry and callsites - [`b1a40e2`](https://github.com/cloudflare/vinext/commit/b1a40e2264dfae60d8cb6b524e0b05bcf01247fd) fix(app-rsc): remove orphaned url declaration from _handleRequest - [`b0e5fff`](https://github.com/cloudflare/vinext/commit/b0e5fffcc338d3ebe05ce7ea3d1ec57908d45e21) fix(test): update cleanPathname assertion for normalizeRscRequest destructuring - [`940de65`](https://github.com/cloudflare/vinext/commit/940de65dde1d6b5e137af014da8fb25402648775) refactor(app-rsc): extract normalizeMountedSlotsHeader into neutral module and add null-byte test - [`dcc1fda`](https://github.com/cloudflare/vinext/commit/dcc1fda7076a6c437219d84a3e0e9c837f7936c5) Merge remote-tracking branch 'upstream/main' into refactor/app-rsc-request-normalization - [`a257332`](https://github.com/cloudflare/vinext/commit/a257332c4c26e0b9f7f4351fde28af843154bc75) ci: retrigger CI - [`b197314`](https://github.com/cloudflare/vinext/commit/b1973147b8481e4fc2b74ad141667979db9207c4) ci: retrigger CI 2 ### 📊 Changes **7 files changed** (+502 additions, -65 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/entries/app-rsc-entry.ts` (+13 -39) 📝 `packages/vinext/src/server/app-elements.ts` (+1 -10) ➕ `packages/vinext/src/server/app-mounted-slots-header.ts` (+19 -0) ➕ `packages/vinext/src/server/app-rsc-request-normalization.ts` (+107 -0) 📝 `packages/vinext/src/server/isr-cache.ts` (+2 -14) 📝 `tests/app-router.test.ts` (+2 -2) ➕ `tests/app-rsc-request-normalization.test.ts` (+358 -0) </details> ### 📄 Description ## What this changes Introduces `server/app-rsc-request-normalization.ts` and wires it into the App Router RSC entry. The generated entry no longer owns the normalization logic itself; it calls `normalizeRscRequest` and destructures the typed result. Also extracts `normalizeMountedSlotsHeader` into a neutral module (`server/app-mounted-slots-header.ts`) used by all three consumers: request normalization (reads the incoming header), app-elements (outgoing header construction), and isr-cache (RSC cache key generation). The generated RSC entry no longer calls the cache-layer export directly; request normalization now owns reading and canonicalising the incoming header. ## Why The normalization pipeline is the most security and compatibility-sensitive part of the request lifecycle: protocol-relative URL open redirects, `%2F`-encoded path segment boundaries, basePath bypass via `/__vinext/`, RSC content-type detection, and null-byte header injection all live here. Having this logic inlined in a template string means: - Zero unit test coverage for the security-sensitive paths - Impossible to audit the ordering invariants (guard must fire before `normalizePath` collapses `//`) by reading a single file - `normalizeMountedSlotsHeader` duplicated across `isr-cache.ts` and `app-elements.ts` (with the copy in `app-elements.ts` already diverging from the canonical `isr-cache.ts` version) ## Approach `normalizeRscRequest(request, basePath)` encodes early exits as `Response` returns (400/404) and success as `NormalizedRscRequest`. The discriminant is `instanceof Response`, which callers already use for every other early-exit check in the entry. No new control-flow patterns introduced. Step ordering inside the function is documented with numbered comments that explain the security constraint each ordering decision satisfies: - Step 2 (protocol-relative guard) must precede step 4 (`normalizePath`): `normalizePath` collapses `//evil.com` to `/evil.com`, causing the guard to miss it. Source: [vercel/next.js — `server/lib/router-utils/guard-protocol-relative-url.ts`](https://github.com/vercel/next.js/blob/canary/packages/next/src/server/lib/router-utils/guard-protocol-relative-url.ts) - Step 3 (strict percent-decode) must precede step 5 (basePath check): a `%2F`-encoded slash in the basePath position would decode to `/` and create a fake match. Source: [vercel/next.js — `server/lib/router-utils/decode-path-params.ts`](https://github.com/vercel/next.js/blob/canary/packages/next/src/server/lib/router-utils/decode-path-params.ts) - Step 5 (`/__vinext/` bypass): internal prerender endpoints must be reachable regardless of the configured basePath. These are `/__vinext/prerender/*` endpoints consumed by `wrangler unstable_startWorker` during Cloudflare Workers builds. `normalizeMountedSlotsHeader` moves to `server/app-mounted-slots-header.ts` and is re-exported from `isr-cache.ts` for backward compatibility. Only the `app-elements.ts` private copy is removed — the generated entry was already importing it from `isr-cache.ts`. ## Validation 44 behavior tests in `tests/app-rsc-request-normalization.test.ts` cover: - Protocol-relative variants: `//`, `/\`, `/%5C`, `/%2F` - Ordering guarantee: guard fires before `normalizePath` (tested explicitly) - Malformed percent sequences: `%GG`, truncated `%`, single-digit `%A` - basePath enforcement: missing prefix, exact match, non-segment-boundary prefix - `/__vinext/` bypass under a configured basePath - `%2F` preservation (not decoded to path separator) - RSC detection: `.rsc` suffix and `Accept: text/x-component` - `cleanPathname` stripping - Interception context header: absent, empty, legitimate value, null-byte injection - Mounted slots: sort, dedup, absent, blank 395 tests pass across the 6 affected test files (new + isr-cache, app-elements, request-pipeline, entry-templates, app-router). ## Risks / follow-ups The `app-rsc-entry.ts` generator now has one fewer resolved path variable (`normalizePathnameForRouteMatchStrict` and `guardProtocolRelativeUrl` no longer need separate `resolveEntryPath` entries). The `normalizePathModulePath` and `routingUtilsPath` constants are retained because `__normalizePath` and `__normalizePathnameForRouteMatch` are still used in the `handler()` outer function for `applyConfigHeadersToResponse`. No behavior change. The generated RSC entry produces the same request handling semantics; only the source of the logic has moved. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>