fix: aggregate array request header values like a real HTTP server#388
Open
LeSingh1 wants to merge 1 commit into
Open
fix: aggregate array request header values like a real HTTP server#388LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
When a header value is supplied as an array via inject(), the values were serialized with a bare '+ value' coercion, producing a single bare-comma string (e.g. 'first,second') and a single rawHeaders entry. This diverges from how a real Node.js HTTP server exposes multiple occurrences of a header on request.headers. Aggregate array values the same way http.IncomingMessage does: keep set-cookie as an array, join cookie values with '; ', discard duplicates of single-value headers (keeping the first), and join all other headers with ', '. Each supplied value is preserved as a separate rawHeaders entry. Closes fastify#264
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.
Problem
inject()accepts array-valued request headers (allowed by thehttp.OutgoingHttpHeadersinput type) to simulate the same header being sent multiple times. However, array values were serialized with a bare'' + valuecoercion, which does not match how a real Node.js HTTP server aggregates repeated header lines ontorequest.headers/request.rawHeaders.Given:
headers.baz"first,second""first, second"headers['set-cookie']"a=1,b=2"["a=1", "b=2"]headers.cookie"x=1,y=2""x=1; y=2"rawHeaders["baz","first,second", ...]["baz","first","baz","second", ...]The Node behavior in the right column was verified empirically against
http.createServerreceiving duplicate header lines over a raw socket. See issue #264.Fix
Aggregate array header values exactly as
http.IncomingMessagedoes (per themessage.headersdocs):set-cookieis kept as an array of its individual values.cookievalues are joined with'; '.content-type,host,authorization, …) are discarded, keeping the first.', '.rawHeadersentry.Non-array (string/number) header values are unchanged, so existing behavior is fully preserved for the common case. One existing test that asserted the old bare-comma output (
'1,two,3') was updated to the Node-faithful'1, two, 3'.Verification
', 'join +rawHeadersexpansion,set-cookiearray preservation,cookie'; 'join, single-value-header duplicate discarding, and theundefined-inside-array assertion.npm testpasses: lint, 172 unit tests at 100% coverage (c8--100), and 54 tstyche type assertions.Closes #264