fix: preserve writeHead headers when statusMessage is undefined - #288
Open
dyk1454683243-sudo wants to merge 1 commit into
Open
dyk1454683243-sudo wants to merge 1 commit into
dyk1454683243-sudo wants to merge 1 commit into
Conversation
on-headers treats a non-string 2nd writeHead argument as the headers object, so writeHead(code, undefined, headers) dropped the headers. Normalize that 3-arg form the same way Node does (obj ??= reason). Fixes expressjs#254 Co-authored-by: David <dyk1454683243-sudo@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #254
Problem
res.writeHead(statusCode, undefined, headers)(and the same call withnullas the status text) silently drops the headers whencompressionis mounted. Nativehttp.ServerResponseapplies those headers and uses the default reason phrase for the status code (Createdfor 201,OKfor 200). It does not stringifyundefinedinto the status line.This is the Vercel AI SDK path:
response.writeHead(status ?? 200, statusText, headers)whenstatusTextis omitted.Root cause:
on-headers@1.1.xpicks the headers argument withtypeof arguments[1] === 'string'. A non-string second argument is treated as the headers object, soarguments[2]is never read. jshttp/on-headers#49 is still open; #254 stays broken until that ships or compression compensates.Fix
Wrap
res.writeHeadoutsideon-headersand rewrite the 3-argument form the same way Node does (obj ??= reason):writeHead(code, undefined|null, headers)→writeHead(code, headers)writeHead(code, headers, undefined|null)→writeHead(code, headers)(keeps the fix: preserve headers when statusText is undefined in writeHead #282 regression from coming back)String status messages, 1- and 2-argument calls, and the empty-string reason phrase are unchanged.
Tests
undefined/nullstatus textundefined/null(the overload fix: preserve headers when statusText is undefined in writeHead #282 dropped)filter: false, andwrite()afterwriteHeadstatusMessagematches native Node (Created/OK/ custom; never"undefined")Notes
Earlier compression PRs for this issue (#273, #274, #282) were closed in favor of on-headers#49. This change is a small, local workaround so #254 can close without waiting on that release, and it does not regress the 3-arg headers-in-the-second-slot overload.