Skip to content

Conversation

@RajeshKumar11
Copy link

This PR aligns Node.js HTTP header value validation with the Fetch spec, which only forbids:

  • 0x00 (NUL)
  • 0x0a (LF)
  • 0x0d (CR)
  • Characters > 0xff (non-byte sequences)

Changes

The headerCharRegex in lib/_http_common.js is updated from:

const headerCharRegex = /[^\t\x20-\x7e\x80-\xff]/;

to:

const headerCharRegex = /[\x00\x0a\x0d]|[^\x00-\xff]/;

Security Consideration

@Renegade334 raised a valid concern about CVE-2016-2216 (response splitting). This change is safe because:

  1. Response splitting requires CRLF injection - The attack relies on injecting \r\n (0x0d 0x0a) to create fake headers. This fix still rejects both CR and LF.

  2. Other CTL characters cannot cause response splitting - Characters like 0x01-0x08, 0x0b-0x0c, 0x0e-0x1f, and 0x7f have no special meaning in HTTP protocol parsing.

  3. Browser alignment - Browsers already allow these characters per Fetch spec (verified via WPT test fetch/api/headers/header-values.any.js).

Tests

Updated test/parallel/test-http-invalidheaderfield2.js to reflect the new valid/invalid character sets.

Fixes: #61582
Refs: https://fetch.spec.whatwg.org/#header-value

Per the Fetch spec, header values should only reject NUL (0x00),
LF (0x0a), CR (0x0d), and characters above 0xff.

Previously, Node.js rejected all CTL characters which was more
restrictive than the spec and prevented valid use cases.

This change relaxes the validation to match browser behavior
while still protecting against response splitting attacks.

Fixes: nodejs#61582
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jan 31, 2026
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina
Copy link
Member

cc @nodejs/tsc this require a few more sets of eyes

@codecov
Copy link

codecov bot commented Jan 31, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.75%. Comparing base (784ca7b) to head (bf3c5a1).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61597      +/-   ##
==========================================
- Coverage   89.77%   89.75%   -0.02%     
==========================================
  Files         673      673              
  Lines      203840   203949     +109     
  Branches    39180    39194      +14     
==========================================
+ Hits       182998   183064      +66     
- Misses      13156    13194      +38     
- Partials     7686     7691       +5     
Files with missing lines Coverage Δ
lib/_http_common.js 100.00% <100.00%> (ø)

... and 35 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

Even though this seems like a fix, I believe this could be considered a semver-major PR.

@RafaelGSS RafaelGSS added the needs-citgm PRs that need a CITGM CI run. label Jan 31, 2026
@Renegade334
Copy link
Member

Even though this seems like a fix, I believe this could be considered a semver-major PR.

For node:http, this is not a fix but an elective behavioural change:

Field values containing other CTL characters are also invalid; however, recipients MAY retain such characters for the sake of robustness when they appear within a safe context (e.g., an application-specific quoted string that will not be processed by any downstream HTTP parser).1

So, not required, but compliant optional.

Footnotes

  1. https://www.rfc-editor.org/rfc/rfc9110.html#name-field-values

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

Labels

http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. needs-citgm PRs that need a CITGM CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HTTP module does not allow sending all valid header values

5 participants