fix: apply middleware request-header overrides to App Route request objects

[southpolesteve]

Summary

App Route handlers now receive a request object rebuilt from middleware request-header overrides, so request.headers and headers() agree after middleware mutations.

Details

Middleware request-header overrides were already applied to the next/headers ALS context, so headers() reflected the middleware-modified request state. But App Route handlers still received a tracked NextRequest built from the original request object, so request.headers exposed the pre-middleware Authorization/Cookie headers.

This change:

• preserves a clone of the middleware response headers before internal x-middleware-* headers are stripped for response safety
• threads that preserved header set into the typed App Route execution module
• rebuilds the underlying Request with buildRequestHeadersFromMiddlewareResponse() before constructing the tracked NextRequest

Tests

Adds an App Route fixture and integration test proving that after middleware deletes credential headers and injects x-from-middleware, both:

• request.headers
• headers()

see the same middleware-modified values.

[southpolesteve]

/bigbonk review this PR

[ask-bonk]

Clean, well-scoped fix. The approach — clone _mwCtx.headers before applyMiddlewareRequestHeaders / processMiddlewareHeaders mutate it, then thread the preserved set into the App Route handler to rebuild the request — is correct and minimally invasive.

Verified:

• buildRequestHeadersFromMiddlewareResponse is idempotent over the preserved header set (it reads only x-middleware-request-* / x-middleware-override-headers from the snapshot), so the clone() path in wrapRequest is safe — calling it on already-modified headers produces identical results.
• executeAppRouteHandler is only called from the generated RSC entry, which serves both dev and prod App Router. No dev/prod parity gap.
• The dev server (index.ts) already had middlewareRequestHeaders for Pages Router; for App Router dev, middleware context is forwarded via x-vinext-mw-ctx to the RSC entry, hitting the same new code path.
• Test passes: vp test run tests/app-router.test.ts -t "middleware request header overrides also apply to App Route" — green.

Two non-blocking observations:

1. rebuildRequestWithHeaders copies all RequestInit properties (cache, credentials, integrity, keepalive, mode, redirect, referrer, referrerPolicy). On Cloudflare Workers, some of these are unsupported on the Request constructor and may throw instead of being silently ignored (unlike Node). Worth a quick Workers smoke test to confirm, or defensively omit the properties that Workers doesn't support. Not a blocker — the existing test fixture covers the Node/dev path, and Workers may simply ignore them.

2. Minor inefficiency on clone(): wrapRequest(target.clone()) (line 204) re-runs buildRequestHeadersFromMiddlewareResponse on the already-rebuilt request. The result is correct (idempotent), but the clone already carries the right headers, so the rebuild is redundant work. Negligible cost, just noting it.

Test fixture and integration test are well-structured — covering both access paths (request.headers and headers()) with header deletion + injection is exactly the right shape.

[ask-bonk]

Nit: On Cloudflare Workers, Request constructor doesn't support all standard RequestInit properties — mode, credentials, integrity, and keepalive can throw in workerd. Since this code runs in the RSC entry (which also runs on Workers in prod), consider either:

• Wrapping the constructor in a try/catch that falls back to a minimal init
• Only copying the properties that Workers supports (method, headers, redirect, signal, body, duplex)

Not a blocker since the existing deploy pipeline + smoke tests should catch this if it's an issue, but worth keeping in mind.

[ask-bonk]

Nit: When clone() is called, wrapRequest(target.clone()) re-runs buildRequestHeadersFromMiddlewareResponse on the cloned request that already has the middleware-modified headers. It's idempotent (the function reads x-middleware-request-* from the preserved snapshot, not from baseHeaders), so this is correct — just redundant work. Not worth optimizing unless clone is hot-pathed.

[ask-bonk]

Review posted. Here's a summary:

PR #809 — Approved

The fix correctly addresses the inconsistency where App Route handlers received pre-middleware request.headers while headers() already reflected middleware modifications.

What it does:

• Preserves a clone of _mwCtx.headers before the x-middleware-* stripping functions mutate it
• Threads the preserved headers into executeAppRouteHandler → createTrackedAppRouteRequest
• Rebuilds the underlying Request with middleware-applied headers before constructing NextRequest

Verified:

• Test passes (vp test run tests/app-router.test.ts -t "middleware request header overrides...")
• No dev/prod parity gap — executeAppRouteHandler is only called from the generated RSC entry (both environments)
• Clone path is safe — buildRequestHeadersFromMiddlewareResponse is idempotent over the preserved header snapshot

Two non-blocking inline comments posted:

1. rebuildRequestWithHeaders copies RequestInit properties (mode, credentials, integrity, keepalive) that Cloudflare Workers may not support — worth a smoke test
2. Minor redundant work on clone() path (correct but rebuilds headers unnecessarily)

github run

[southpolesteve]

/bigbonk Rebased onto latest main and resolved the conflict cleanly. Focused integration coverage still passes locally for all three middleware override cases: App page, App route, and mixed Pages fallback. Please re-review.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/vinext@809

commit: 60c4910

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original

[ask-bonk]

Clean, well-targeted fix. The data flow is correct:

1. _mwCtx.requestHeaders = new Headers(_mwCtx.headers) captures the middleware response headers before applyMiddlewareRequestHeaders / processMiddlewareHeaders strip the x-middleware-* entries
2. The preserved snapshot is threaded through executeAppRouteHandler → runAppRouteHandler → createTrackedAppRouteRequest
3. buildRequestHeadersFromMiddlewareResponse reads x-middleware-override-headers and x-middleware-request-* from the snapshot to reconstruct the intended request headers, then rebuildRequestWithHeaders creates a new Request with those headers before wrapping in NextRequest

This correctly closes the gap where headers() (via ALS) already reflected middleware mutations but request.headers in App Route handlers did not.

Verified: Test passes — vp test run tests/app-router.test.ts -t "middleware request header overrides also apply to App Route" is green. The fixture covers both access paths (request.headers.get(...) and (await headers()).get(...)) with header deletion + injection, which is the right shape.

Agreeing with the two non-blocking observations from the prior review:

1. rebuildRequestWithHeaders copies mode/credentials/integrity/keepalive — these may throw on workerd's Request constructor. The deploy previews and smoke tests should catch this, but a defensive subset (method, headers, redirect, signal, body, duplex) would be safer. Worth a follow-up if smoke tests surface issues.

2. clone() redundancy — wrapRequest(target.clone()) at line 204 re-runs buildRequestHeadersFromMiddlewareResponse on the already-rebuilt request. Idempotent and correct, just unnecessary work. Not worth blocking on.

No other concerns. Test fixture and middleware matcher update are well-structured.

[ask-bonk]

Reviewed and approved PR #809. The fix correctly addresses the inconsistency where App Route handlers' request.headers didn't reflect middleware mutations while headers() already did. The approach — snapshot middleware response headers before stripping, thread them into the route handler, rebuild the Request — is minimal and correct. Test passes and covers both access paths.

github run