-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix(start): use setHeader in preview server for HTTP/2 compatibility … #6996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sotasan
wants to merge
1
commit into
TanStack:main
Choose a base branch
from
sotasan:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1
−1
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
For Node.js FetchHeaders, doesheaders.forEachpreserve multipleSet-Cookievalues distinctly, and what is the recommended way to forward them tohttp.ServerResponse/Http2ServerResponsewithout 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:
OutgoingMessage.setHeaders(Headers)doesn't handleSet-Cookieheaders properly. nodejs/node#51599🏁 Script executed:
Repository: TanStack/router
Length of output: 1123
🏁 Script executed:
Repository: TanStack/router
Length of output: 1084
🏁 Script executed:
Repository: TanStack/router
Length of output: 86
🏁 Script executed:
Repository: TanStack/router
Length of output: 41
🏁 Script executed:
Repository: TanStack/router
Length of output: 4516
🏁 Script executed:
Repository: TanStack/router
Length of output: 228
🏁 Script executed:
Repository: TanStack/router
Length of output: 41
🏁 Script executed:
Repository: TanStack/router
Length of output: 2551
🏁 Script executed:
Repository: TanStack/router
Length of output: 81
🏁 Script executed:
Repository: 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:
Use
getSetCookie()to preserve multipleSet-Cookieheaders.Line 57 iterates headers with
forEach(), which combines multipleSet-Cookievalues into a single comma-separated string. This loses distinct cookies beforewriteHead()locks them in. CallgetSetCookie()to retrieve them as an array and pass the array tores.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
📝 Committable suggestion
🤖 Prompt for AI Agents