Skip to content

http: reject responses exceeding header limit - #65010

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:semver-major-reject-response-header-overflow
Open

http: reject responses exceeding header limit#65010
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:semver-major-reject-response-header-overflow

Conversation

@mcollina

@mcollina mcollina commented Aug 4, 2026

Copy link
Copy Markdown
Member

Previously, ClientRequest silently truncated responses that exceeded request.maxHeadersCount while llhttp continued using omitted headers.

Reject these responses with HPE_HEADER_OVERFLOW so parser state and exposed headers cannot diverge. This is a semver-major behavior change.

Previously, ClientRequest silently truncated responses that exceeded
request.maxHeadersCount while llhttp continued using omitted headers.

Reject these responses with HPE_HEADER_OVERFLOW so parser state and
exposed headers cannot diverge. This is a semver-major behavior change.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina mcollina added the semver-major PRs that contain breaking changes and should be released in the next major version. label Aug 4, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. needs-ci PRs that need a full CI run. labels Aug 4, 2026
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.27%. Comparing base (f43086d) to head (df8e874).
⚠️ Report is 46 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65010      +/-   ##
==========================================
- Coverage   90.27%   90.27%   -0.01%     
==========================================
  Files         762      762              
  Lines      247515   247644     +129     
  Branches    46683    46697      +14     
==========================================
+ Hits       223447   223552     +105     
- Misses      15485    15528      +43     
+ Partials     8583     8564      -19     
Files with missing lines Coverage Δ
src/node_http_parser.cc 84.94% <ø> (-0.04%) ⬇️

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

Comment thread src/node_http_parser.cc
if (parser_.type != HTTP_REQUEST) {
return 0;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@lpinca FYI since you mentioned those lines in the 26.6.0 proposal

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

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants