Skip to content

Conversation

pimterry
Copy link
Member

@pimterry pimterry commented May 6, 2025

This relates to...

When passing a flat header array and using HTTP/2, duplicate keys were discarded and lost unexpectedly.

In addition, the value-combining formatting (effectively join(',')) for explicitly array-valued headers in HTTP/2 didn't quite match the formatting for both Undici HTTP/1 or for Node HTTP/2's value-combining behaviour for headers that differ only by case (both of which do join(', ') - i.e. with whitespace) which was a bit weird.

Rationale

const client = new undici.Client('https://testserver.host', { allowH2: true })

client.request({
  method: 'GET',
  path: '/anything',
  headers: ['a', 'b', 'a', 'c']
}).then(async (response) => {
  console.log(await response.body.text())
})

The response shows what the server receives: headers including a: c but without a: b (all but the last header value is lost).

The header array-to-object logic ignored duplicate keys like this.

Regarding the formatting:

  • In Undici HTTP/1, { 'a': ['b', 'c'] } is sent as a: b, c (whitespace)
  • In Node built-in HTTP/2, { 'a': 'b', 'A': 'c' } is sent as a: b, c (whitespace)
  • In Undici HTTP/2, until now, { 'a': ['b', 'c'] } was sent as a: b,c (no whitespace)

(I'm not claiming this formatting is particularly important, but I would've had to reproduce the inconsistency in the tests, and it's nicer to tidy it up & make these headers a bit more readable en route).

Once nodejs/node#57917 is released (e.g. Node v24) we will be able to skip this entirely, and pass raw headers as-is directly to Node's HTTP/2 APIs, so these headers won't be combined at all - but that will have to wait until later, and will only apply for Node versions including that change anyway.

Changes

N/A

Features

N/A

Bug Fixes

  • Duplicate keys in HTTP/2 flat header arrays are now combined correctly, not discarded
  • Multi-valued HTTP/2 header values which are combined now include whitespace, for consistency with HTTP/1 and Node itself.

Breaking Changes and Deprecations

N/A

Status

pimterry added 2 commits May 6, 2025 14:36
Undici's header handling combined array-valued header values without a
space. Node's built-in HTTP/2 header handling (used only for keys that
differ only in case) combined them with a space (", "). This makes those
match so everything has a space, which is a little more readable &
consistent.
@pimterry
Copy link
Member Author

pimterry commented May 6, 2025

Failing tests (inspection & sqlite-cache-store) seem unrelated to me.

@metcoder95 metcoder95 merged commit bd24250 into nodejs:main May 9, 2025
28 of 31 checks passed
This was referenced May 10, 2025
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.

3 participants