-
-
Notifications
You must be signed in to change notification settings - Fork 34.3k
http2: add strictSingleValueFields option to allow relaxing header validation #59917
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
http2: add strictSingleValueFields option to allow relaxing header validation #59917
Conversation
|
Review requested:
|
Previously it was impossible to send multiple values for any header or trailer defined officially as supporting only a single value. This is a good default, but in practice many of these headers are used in weird & wonderful ways where this can be problematic. This new option allows for relaxing this restriction to support those cases where required. This option defaults to true so validation will still be applied as before, rejecting multiple single-value fields, unless explicitly disabled.
878c3bd to
ed3edd4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #59917 +/- ##
==========================================
- Coverage 88.28% 88.26% -0.02%
==========================================
Files 702 703 +1
Lines 206995 207394 +399
Branches 39833 39884 +51
==========================================
+ Hits 182740 183058 +318
- Misses 16265 16307 +42
- Partials 7990 8029 +39
🚀 New features to boost your workflow:
|
|
Would love to get a review from @nodejs/http2 here please. |
|
One tidy real-world example of this issue another user just reported in one of my libraries: curl -I https://tech.unblockedgames.world/
HTTP/2 200
date: Fri, 31 Oct 2025 10:09:56 GMT
content-type: text/html; charset=UTF-8
server: cloudflare
vary: Accept-Encoding
x-frame-options: SAMEORIGIN
x-frame-options: SAMEORIGIN
x-content-type-options: nosniff
x-content-type-options: nosniff
x-xss-protection: 1; mode=block
x-xss-protection: 1; mode=block
x-permitted-cross-domain-policies: master-only
x-permitted-cross-domain-policies: master-only
referrer-policy: same-origin
referrer-policy: same-origin
nel: {"report_to":"cf-nel","success_fraction":0.0,"max_age":604800}
cf-cache-status: DYNAMIC
report-to: {"group":"cf-nel","max_age":604800,"endpoints":[{"url":"https://a.nel.cloudflare.com/report/v4?s=IZ%2FpFHZUxgwOsymwKUZA7pAqP3qzhb%2BOlDMZfuLxhe9PtpacWHsAdMWVKgNiyWD%2BzP5EztuRLIWn7uCdRyMEv%2FHIcowiAH4LYKowyeLzM%2B9m%2FaZhNq3D"}]}
cf-ray: 99725c16d9ebcf94-MAD
alt-svc: h3=":443"; ma=86400The various security options here are duplicated, but officially should only appear once. It's impossible to accurately proxy this traffic with HTTP/2 in Node.js without the option added by this PR - the duplicated security headers are sent successfully by the real server, and accepted happily by browsers & curl as clients, but Node blocks them. I'd love to get this fixed! It seems we don't have many reviews for HTTP/2. Maybe somebody on @nodejs/http would be open to taking a look at this? |
Also HTTP/2 status message is also missing. |
Fixes #59651
Previously it was impossible to send multiple values for any header or trailer defined officially as supporting only a single value.
This is a reasonable default, but in practice many of these headers are used in the wild in weird & wonderful ways, where this limitation can be problematic. This is also inconsistent with Node's equivalent HTTP/1 behaviour, which has no such restriction.
This PR add a new option which allows for relaxing this restriction, to support the edge cases where this is helpful, but defaulting to true to preserve the existing behaviour. This is stored on the session, and added for both clients & servers via
connect()andcreate[Secure]Server().Aside: as mentioned in #59651, I do think it might be interesting to consider unifying the insecure/relaxed validation options and linking them to
--insecure-http-parserin future, to better match them to the corresponding HTTP/1 behaviour for people in scenarios where they want to more easily handle weird HTTP and accept the implications, but doing so forstrictFieldWhitespaceValidationwould be a breaking change anyway and there was no immediate enthusiasm for the suggestion there, so we can leave this for another day.