fix: handle undefined statusText in writeHead arguments#49
Open
mahmoodhamdi wants to merge 1 commit intojshttp:masterfrom
Open
fix: handle undefined statusText in writeHead arguments#49mahmoodhamdi wants to merge 1 commit intojshttp:masterfrom
mahmoodhamdi wants to merge 1 commit intojshttp:masterfrom
Conversation
When writeHead is called with three arguments where the second argument (statusText) is undefined or null, the headers object at the third argument position was being ignored. This is because the argument parsing only checked if the second argument was a string to determine the header index, without considering that three arguments always means the third is headers. This caused headers set via writeHead(status, undefined, headers) to be silently dropped, which affected downstream packages like compression middleware (expressjs/compression#254). The fix checks if there are more than two arguments, and if so, always reads headers from the third position, matching Node.js native writeHead behavior.
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.
Summary
Fixes argument parsing in
setWriteHeadHeadersto correctly handlewriteHead(statusCode, undefined, headers)— where the statusText is explicitly passed asundefinedornull.Problem
When
res.writeHead(200, undefined, { 'X-Custom': 'value' })is called throughon-headers, the headers object at the third argument position is silently dropped.Root cause: The argument parsing logic determines the header index based only on whether
arguments[1]is a string:When
arguments[1]isundefined, it's not a string, soheaderIndex = 1. Thenarguments[1](which isundefined) is read as the headers, and the actual headers atarguments[2]are never accessed.Without
on-headers: Native Node.jswriteHead(200, undefined, headers)works correctly.With
on-headers: Headers are silently dropped.This was reported as expressjs/compression#254, where the compression middleware (which uses
on-headers) caused headers to be lost when the Vercel AI SDK calledwriteHead(status, undefined, headers).Fix
When there are more than 2 arguments, the headers are always at index 2 regardless of whether the second argument is a string, matching Node.js native
writeHeadbehavior:Testing
Added 2 new tests:
writeHead(status, undefined, obj)— headers are set correctlywriteHead(status, null, obj)— headers are set correctlyAll 26 tests pass. Lint clean.