fix(shims): copy request headers into mutable Headers in headersContextFromRequest#128
Conversation
…xtFromRequest In Cloudflare Workers, Request.headers is immutable per the Fetch API spec. headersContextFromRequest was storing the original request.headers reference directly in HeadersContext, so when applyMiddlewareRequestHeaders later called ctx.headers.set() (to propagate middleware-modified headers like x-request-url into the server component context), Workers threw: TypeError: Can't modify immutable headers. Fix by wrapping with new Headers(request.headers), which creates an independent mutable copy while preserving all header values.
Co-authored-by: ask-bonk[bot] <ask-bonk[bot]@users.noreply.github.com>
|
Can confirm this bug in production. We hit the exact same Environment:
Impact: Every route returned HTTP 500. The worker tail showed Our workaround: We cloned the request with mutable headers in our worker entry point before passing to vinext's app-router handler: // worker/index.ts
const requestWithMutableHeaders = new Request(request, {
headers: new Headers(request.headers),
cf: (request as Request & { cf?: unknown }).cf,
} as RequestInit);
return handler.fetch(requestWithMutableHeaders);This is essentially the same fix as this PR but applied at the app level instead of inside vinext. Would be great to get this merged upstream so we can remove our workaround. Thanks @harrisrobin for filing this! |
|
/bigbonk review this. Did we already fix this on main? We might have |
commit: |
There was a problem hiding this comment.
Clean, correct, and well-scoped fix. The change is minimal and targets exactly the right place.
Analysis:
-
headersContextFromRequestis called inapp-dev-server.ts:1178where theRequestcomes directly from the Cloudflare Workers runtime (immutable headers). Later,applyMiddlewareRequestHeaders(line 1354) calls.set()onctx.headers, which blows up on Workers. -
I checked the other places in the codebase that pass
request.headersinto context-like objects (app-dev-server.ts:1006andconfig-matchers.ts:253). Both are read-only contexts used forhas/missingcondition matching — they only call.get(), never.set()— so they don't need the same treatment. -
The Pages Router prod-server (
prod-server.ts:792) also calls.set()on request headers, but it constructs its ownnew Request(...)from Node.jsIncomingMessageheaders, so those are already mutable. No issue there. -
The test correctly verifies three properties: reference inequality, mutability, and isolation from the original request headers.
LGTM.
|
Approved. The fix is correct and well-scoped. Here's the summary of my review:
|
Summary
In Cloudflare Workers,
Request.headersis immutable per the Fetch API spec.headersContextFromRequestwas storing the originalrequest.headersreference directly inHeadersContext, so whenapplyMiddlewareRequestHeaderslater calledctx.headers.set()(to propagate middleware-modified headers into server component context), Workers threw:This is triggered on every request when middleware returns
NextResponse.next({ request: { headers } })— a common pattern for passing data likex-request-urlfrom middleware to server components.Fix
Wrap with
new Headers(request.headers), which creates an independent mutable copy while preserving all header values.Test plan
Request.headersreferenceoxlint)tsgo --noEmit)/bonk