◐ Shell
reader mode source ↗
Skip to content

http2: validate non-link headers in writeEarlyHints#62017

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
mcollina:http-early-hints-validate-headers
May 15, 2026
Merged

http2: validate non-link headers in writeEarlyHints#62017
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
mcollina:http-early-hints-validate-headers

Conversation

@mcollina

@mcollina mcollina commented Feb 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

The HTTP/1.1 portion of the original PR landed separately in #61897. This PR now contains only the HTTP/2 compat layer counterpart.

  • Validate header names and values for non-link hints in Http2ServerResponse.prototype.writeEarlyHints() using assertValidHeader() and checkIsHttpToken()
  • Throws a clear error at the call site instead of forwarding invalid header names/values deeper into the HTTP/2 stack

Test plan

  • test/parallel/test-http2-compat-write-early-hints-invalid-header.js — verifies ERR_INVALID_HTTP_TOKEN for bad header names and ERR_HTTP2_INVALID_HEADER_VALUE for bad values
  • Existing early hints tests still pass

@nodejs-github-bot

nodejs-github-bot commented Feb 27, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/http2
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. labels Feb 27, 2026
Copilot AI reviewed Feb 27, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hide comment

Pull request overview

This PR hardens writeEarlyHints() by validating non-link headers (names + values) before emitting 103 Early Hints, aligning behavior with other header-writing code paths across the HTTP stack.

Changes:

  • Add validateHeaderName() / validateHeaderValue() checks for non-link early hints in the HTTP/1.1 server response path.
  • Add assertValidHeader() checks for non-link early hints in the HTTP/2 compat server response path.
  • Add/extend parallel tests covering invalid early-hints header name/value handling for HTTP/1.1 and HTTP/2.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
lib/_http_server.js Validates non-link early-hints header names/values before writing the 103 response.
lib/internal/http2/compat.js Validates non-link early-hints headers in the HTTP/2 compat layer before sending informational headers.
test/parallel/test-http-early-hints-invalid-argument.js Adds assertions for invalid early-hints header names and CRLF in values (HTTP/1.1).
test/parallel/test-http2-compat-write-early-hints-invalid-header.js Adds assertions for invalid early-hints header names and invalid values (HTTP/2 compat).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mcollina mcollina force-pushed the http-early-hints-validate-headers branch 3 times, most recently from bf5cf85 to 69e6ce4 Compare February 27, 2026 12:36
@codecov

codecov Bot commented Feb 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.05%. Comparing base (130398a) to head (ee38509).
⚠️ Report is 38 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/http2/compat.js 80.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #62017      +/-   ##
==========================================
+ Coverage   90.03%   90.05%   +0.01%     
==========================================
  Files         713      714       +1     
  Lines      224950   225246     +296     
  Branches    42530    42576      +46     
==========================================
+ Hits       202541   202840     +299     
+ Misses      14184    14180       -4     
- Partials     8225     8226       +1
Files with missing lines Coverage Δ
lib/internal/http2/compat.js 97.07% <80.00%> (+0.11%) ⬆️

... and 37 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.

@mcollina mcollina requested a review from RafaelGSS February 27, 2026 17:49
@lpinca

lpinca commented Feb 27, 2026

Copy link
Copy Markdown
Member

semver-major?

pimterry
pimterry requested changes Mar 2, 2026
View reviewed changes

@pimterry pimterry 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.

Hide comment

There's conflicts here due to the preceding PR (#61897) that covered some of this already. I imagine you're aware of that since you approved it 😆

The key difference is that this handles the HTTP/2 side - once the conflicts are fixed, can we update the commit message to make it clear this change just fixes the HTTP/2 compat API?

@mcollina

mcollina commented Mar 3, 2026

Copy link
Copy Markdown
Member Author

I review endless stream of PRs 😰

@mcollina mcollina force-pushed the http-early-hints-validate-headers branch from 69e6ce4 to 0ed213a Compare May 13, 2026 13:17
@mcollina mcollina changed the title http: validate non-link headers in writeEarlyHints May 13, 2026
@mcollina
http2: validate non-link headers in writeEarlyHints …
ee38509 
Validate header names and values for non-link hints passed to
writeEarlyHints() in the HTTP/2 compat layer using assertValidHeader()
and checkIsHttpToken(), consistent with the HTTP/1.1 validation added
in nodejs#61897.

Previously, hints were forwarded into the headers object without any
validation, allowing invalid characters in header names/values to
surface as opaque errors deeper in the HTTP/2 stack.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina mcollina force-pushed the http-early-hints-validate-headers branch from 0ed213a to ee38509 Compare May 13, 2026 13:20
@mcollina

mcollina commented May 13, 2026

Copy link
Copy Markdown
Member Author

cc @pimterry

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label May 13, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 13, 2026
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

nodejs-github-bot commented May 15, 2026 •
edited by jasnell
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/73457/ 💛

Hide details View details @nodejs-github-bot nodejs-github-bot merged commit 274d799 into nodejs:main May 15, 2026
73 of 74 checks passed
@nodejs-github-bot

nodejs-github-bot commented May 15, 2026

Copy link
Copy Markdown
Collaborator

Landed in 274d799

aduh95 pushed a commit that referenced this pull request May 19, 2026
@mcollina @aduh95
http2: validate non-link headers in writeEarlyHints …
f712e68 
Validate header names and values for non-link hints passed to
writeEarlyHints() in the HTTP/2 compat layer using assertValidHeader()
and checkIsHttpToken(), consistent with the HTTP/1.1 validation added
in #61897.

Previously, hints were forwarded into the headers object without any
validation, allowing invalid characters in header names/values to
surface as opaque errors deeper in the HTTP/2 stack.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #62017
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@github-actions github-actions Bot mentioned this pull request May 19, 2026
Merged
aduh95 pushed a commit that referenced this pull request May 23, 2026
@mcollina @aduh95
http2: validate non-link headers in writeEarlyHints …
77123f4 
Validate header names and values for non-link hints passed to
writeEarlyHints() in the HTTP/2 compat layer using assertValidHeader()
and checkIsHttpToken(), consistent with the HTTP/1.1 validation added
in #61897.

Previously, hints were forwarded into the headers object without any
validation, allowing invalid characters in header names/values to
surface as opaque errors deeper in the HTTP/2 stack.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #62017
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
araujogui pushed a commit to araujogui/node that referenced this pull request May 26, 2026
@mcollina @araujogui
http2: validate non-link headers in writeEarlyHints …
29a4537 
Validate header names and values for non-link hints passed to
writeEarlyHints() in the HTTP/2 compat layer using assertValidHeader()
and checkIsHttpToken(), consistent with the HTTP/1.1 validation added
in nodejs#61897.

Previously, hints were forwarded into the headers object without any
validation, allowing invalid characters in header names/values to
surface as opaque errors deeper in the HTTP/2 stack.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: nodejs#62017
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Jun 18, 2026
@mcollina @aduh95
http2: validate non-link headers in writeEarlyHints …
8a8f212 
Validate header names and values for non-link hints passed to
writeEarlyHints() in the HTTP/2 compat layer using assertValidHeader()
and checkIsHttpToken(), consistent with the HTTP/1.1 validation added
in #61897.

Previously, hints were forwarded into the headers object without any
validation, allowing invalid characters in header names/values to
surface as opaque errors deeper in the HTTP/2 stack.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #62017
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@github-actions github-actions Bot mentioned this pull request Jun 19, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@pimterry pimterry pimterry approved these changes

@RafaelGSS RafaelGSS RafaelGSS approved these changes

@Flarna Flarna Awaiting requested review from Flarna

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@mcollina @nodejs-github-bot @lpinca @jasnell @pimterry @RafaelGSS