Skip to content

fix: aggregate array request header values like a real HTTP server#388

Open
LeSingh1 wants to merge 1 commit into
fastify:mainfrom
LeSingh1:fix/multi-value-request-headers
Open

fix: aggregate array request header values like a real HTTP server#388
LeSingh1 wants to merge 1 commit into
fastify:mainfrom
LeSingh1:fix/multi-value-request-headers

Conversation

@LeSingh1

Copy link
Copy Markdown

Problem

inject() accepts array-valued request headers (allowed by the http.OutgoingHttpHeaders input type) to simulate the same header being sent multiple times. However, array values were serialized with a bare '' + value coercion, which does not match how a real Node.js HTTP server aggregates repeated header lines onto request.headers / request.rawHeaders.

Given:

inject(dispatch, {
  method: 'GET',
  url: '/',
  headers: { baz: ['first', 'second'], 'set-cookie': ['a=1', 'b=2'], cookie: ['x=1', 'y=2'] }
})
light-my-request (before) Real Node.js HTTP server
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.createServer receiving duplicate header lines over a raw socket. See issue #264.

Fix

Aggregate array header values exactly as http.IncomingMessage does (per the message.headers docs):

  • set-cookie is kept as an array of its individual values.
  • cookie values are joined with '; '.
  • Duplicates of single-value headers (content-type, host, authorization, …) are discarded, keeping the first.
  • All other headers are joined with ', '.
  • Every supplied value is preserved as a separate rawHeaders entry.

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

  • Added regression tests covering the ', ' join + rawHeaders expansion, set-cookie array preservation, cookie '; ' join, single-value-header duplicate discarding, and the undefined-inside-array assertion.
  • npm test passes: lint, 172 unit tests at 100% coverage (c8 --100), and 54 tstyche type assertions.

Closes #264

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

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multi-value request headers are serialized to comma-separated string

2 participants