fix(start): use setHeader in preview server for HTTP/2 compatibility …#6996
fix(start): use setHeader in preview server for HTTP/2 compatibility …#6996sotasan wants to merge 1 commit intoTanStack:mainfrom
Conversation
📝 WalkthroughWalkthroughA method call that batch-copies response headers to a Node response object was replaced with explicit iteration over header entries, setting each header individually via separate calls before writeHead operations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/start-plugin-core/src/preview-server-plugin/plugin.ts`:
- Line 57: The current header-copying loop using webRes.headers.forEach(...) can
merge multiple Set-Cookie values into one string; instead, use
webRes.getSetCookie() to retrieve the Set-Cookie array and call
res.setHeader('Set-Cookie', cookieArray) so Node emits multiple Set-Cookie
headers; keep the existing loop for other headers but ensure you set the
Set-Cookie header from webRes.getSetCookie() before writeHead()/end() (refer to
webRes.getSetCookie(), res.setHeader('Set-Cookie', ...) and the existing
webRes.headers.forEach usage).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8aa42569-0f1d-480e-be9f-7ecc9e0d2229
📒 Files selected for processing (1)
packages/start-plugin-core/src/preview-server-plugin/plugin.ts
| // Vite preview's compression middleware doesn't support flattened array headers that srvx sets | ||
| // Call writeHead() before srvx to avoid corruption | ||
| res.setHeaders(webRes.headers) | ||
| webRes.headers.forEach((value, key) => res.setHeader(key, value)); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Node.js Fetch Headers, does headers.forEachpreserve multipleSet-Cookievalues distinctly, and what is the recommended way to forward them tohttp.ServerResponse/Http2ServerResponse without losing cookies?
💡 Result:
No, Node.js Fetch Headers.forEach does not preserve multiple Set-Cookie values distinctly. The Fetch Headers object follows the WHATWG spec where multiple values for the same header (including Set-Cookie) are combined into a single comma-separated string. Thus, headers.forEach((value, name) => ...) will iterate once over 'set-cookie' with value as the concatenated string (e.g., "cookie1=val1, cookie2=val2"), not once per cookie. This makes parsing unreliable due to commas in cookie attributes like Expires. The recommended way to access distinct Set-Cookie values and forward them to http.ServerResponse or Http2ServerResponse without loss: 1. Use response.headers.getSetCookie: Returns an array of individual Set-Cookie strings (Node.js 18.10.0+, undici-based Fetch). const cookies = response.headers.getSetCookie; 2. Set on ServerResponse: res.setHeader('Set-Cookie', cookies); // Array accepted, sends multiple distinct headers. Example proxy code: const { fetch } = require('undici'); // or global fetch in Node 18+ async function proxy(req, res) { const upstreamRes = await fetch(upstreamUrl); // Copy other headers... for (const [name, value] of upstreamRes.headers) { if (name.toLowerCase !== 'set-cookie') { res.setHeader(name, value); } } // Handle Set-Cookie distinctly const setCookies = upstreamRes.headers.getSetCookie; if (setCookies.length > 0) { res.setHeader('Set-Cookie', setCookies); } res.statusCode = upstreamRes.status; // Pipe body... } This ensures multiple Set-Cookie headers are forwarded as separate lines, per RFC 6265 recommendation against comma-folding Set-Cookie. Fallback for older node-fetch (non-standard): response.headers.raw['set-cookie'] returns array of strings. Node.js http.ServerResponse.setHeader('Set-Cookie', array) correctly emits multiple headers.
Citations:
- 1: fetch response.headers.get("set-cookie") not usable to extract array of cookies nodejs/node#47755
- 2: https://stackoverflow.com/questions/63204093/how-to-get-set-multiple-set-cookie-headers-using-fetch-api
- 3: Set-Cookie header gets concatenated in fetch if an interceptor is in place nodejs/undici#4389
- 4: I do not receive set-cookie in response.headers.getSetCookie() nodejs/undici#3448
- 5: Provide a way to access raw headers nodejs/undici#1437
- 6:
OutgoingMessage.setHeaders(Headers)doesn't handleSet-Cookieheaders properly. nodejs/node#51599 - 7: https://www.npmjs.com/package/node-fetch
- 8: https://github.com/node-fetch/node-fetch/blob/main/src/headers.js
- 9: https://stackoverflow.com/questions/59977044/node-js-how-to-set-multiple-cookies
🏁 Script executed:
# Check the context around line 57 and determine the type of webRes
sed -n '40,75p' packages/start-plugin-core/src/preview-server-plugin/plugin.tsRepository: TanStack/router
Length of output: 1123
🏁 Script executed:
# Check Node.js version requirement in package.json or similar
cat packages/start-plugin-core/package.json | grep -A 5 -B 5 "engines\|node"Repository: TanStack/router
Length of output: 1084
🏁 Script executed:
# Search for getSetCookie usage elsewhere in the codebase
rg "getSetCookie" --type ts --type tsx -A 2 -B 2Repository: TanStack/router
Length of output: 86
🏁 Script executed:
# Check if Response or webRes is typed somewhere
ast-grep --pattern 'let $_ = $$$' packages/start-plugin-core/src/preview-server-plugin/plugin.tsRepository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check what sendNodeResponse does with headers/body
rg "sendNodeResponse" --type ts -A 10 -B 2 | head -50Repository: TanStack/router
Length of output: 4516
🏁 Script executed:
# Look for the sendNodeResponse implementation
fd "sendNodeResponse\|send-node-response" --type ts --type tsxRepository: TanStack/router
Length of output: 228
🏁 Script executed:
# Check if there's a common pattern for forwarding headers in the codebase
rg "headers\.forEach\|getSetCookie" --type ts -C 3Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Get full view of the function containing line 57 to understand complete flow
sed -n '1,100p' packages/start-plugin-core/src/preview-server-plugin/plugin.ts | tail -60Repository: TanStack/router
Length of output: 2551
🏁 Script executed:
# Check srvx dependency version and see if there's any documentation
cat packages/start-plugin-core/package.json | grep -i srvxRepository: TanStack/router
Length of output: 81
🏁 Script executed:
# Search for any other patterns of header forwarding or Set-Cookie handling in the codebase
rg "Set-Cookie|set-cookie" --type-add 'ts:*.{ts,tsx}' --type ts -i -C 2Repository: TanStack/router
Length of output: 8518
🌐 Web query:
What is the srvx library for Node.js and how does sendNodeResponse handle headers from Fetch Response objects, particularly multi-value headers like Set-Cookie?
💡 Result:
srvx is a universal server library for Node.js, Deno, and Bun that implements web standard APIs (Request/Response/fetch handlers) with a full-featured CLI for development (watcher, logger, static serving). It provides seamless runtime integration and Node.js compatibility with near-native performance using lightweight proxies to wrap node:IncomingMessage as Request and convert Response to node:ServerResponse. sendNodeResponse is an internal utility in srvx's Node.js adapter (src/adapters/_node/send.ts) that converts a web-standard Fetch Response object to a Node.js ServerResponse. It sets the status, statusText, and headers on the Node.js response using res.writeHead. For headers, it processes the Response.headers using .flat multiple times to handle multi-value headers correctly, passing them as a raw array of [name, value] pairs (which Node.js accepts natively for efficiency). Specifically for multi-value headers like Set-Cookie, it properly splits and preserves all values (using cookie-es internally), ensuring all cookies are sent as separate header lines in the HTTP response. Past issues with header formatting (e.g., invalid output) were resolved and unrelated to srvx core (due to middleware like morgan), confirming robust handling in current versions (v0.11+ as of 2026).
Citations:
- 1: https://srvx.h3.dev/guide/node
- 2: sendNodeResponse writes invalid http headers h3js/srvx#61
- 3: @tus/server: use srvx for req/res conversion tus/tus-node-server#749
Use getSetCookie() to preserve multiple Set-Cookie headers.
Line 57 iterates headers with forEach(), which combines multiple Set-Cookie values into a single comma-separated string. This loses distinct cookies before writeHead() locks them in. Call getSetCookie() to retrieve them as an array and pass the array to res.setHeader('Set-Cookie', ...), which Node.js correctly emits as multiple headers. This matters for session/auth cookies and aligns with existing multi-cookie preservation logic in the codebase.
Proposed fix
- webRes.headers.forEach((value, key) => res.setHeader(key, value));
+ webRes.headers.forEach((value, key) => {
+ if (key.toLowerCase() !== 'set-cookie') {
+ res.setHeader(key, value)
+ }
+ })
+
+ const setCookies = webRes.headers.getSetCookie()
+ if (setCookies.length > 0) {
+ res.setHeader('set-cookie', setCookies)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| webRes.headers.forEach((value, key) => res.setHeader(key, value)); | |
| webRes.headers.forEach((value, key) => { | |
| if (key.toLowerCase() !== 'set-cookie') { | |
| res.setHeader(key, value) | |
| } | |
| }) | |
| const setCookies = webRes.headers.getSetCookie() | |
| if (setCookies.length > 0) { | |
| res.setHeader('set-cookie', setCookies) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/start-plugin-core/src/preview-server-plugin/plugin.ts` at line 57,
The current header-copying loop using webRes.headers.forEach(...) can merge
multiple Set-Cookie values into one string; instead, use webRes.getSetCookie()
to retrieve the Set-Cookie array and call res.setHeader('Set-Cookie',
cookieArray) so Node emits multiple Set-Cookie headers; keep the existing loop
for other headers but ensure you set the Set-Cookie header from
webRes.getSetCookie() before writeHead()/end() (refer to webRes.getSetCookie(),
res.setHeader('Set-Cookie', ...) and the existing webRes.headers.forEach usage).
1 participant
Fixes #6294
Http2ServerResponse (used by Vite when HTTPS is enabled) doesn't have setHeaders, which is only available on http.ServerResponse. This causes the preview server to crash when using HTTPS (e.g. via vite-plugin-mkcert).
Summary by CodeRabbit