Skip to content

Conversation

@mcollina
Copy link
Member

The HTTP/2 spec (RFC 7540) defines SETTINGS_INITIAL_WINDOW_SIZE maximum as 2^31-1. Values above this must be treated as a FLOW_CONTROL_ERROR. Previously, Node.js allowed values up to 2^32-1 which caused nghttp2_submit_settings() to return NGHTTP2_ERR_INVALID_ARGUMENT, triggering an uncatchable assertion failure and crashing the process.

This change adds proper validation to reject values >= 2^31 with a catchable RangeError before they reach nghttp2.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net

@mcollina mcollina requested a review from jasnell January 16, 2026 16:48
@nodejs-github-bot nodejs-github-bot added http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. labels Jan 16, 2026
@mcollina mcollina requested review from RafaelGSS and ronag January 16, 2026 16:48
The HTTP/2 spec (RFC 7540) defines SETTINGS_INITIAL_WINDOW_SIZE
maximum as 2^31-1. Values above this must be treated as a
FLOW_CONTROL_ERROR. Previously, Node.js allowed values up to
2^32-1 which caused nghttp2_submit_settings() to return
NGHTTP2_ERR_INVALID_ARGUMENT, triggering an uncatchable
assertion failure and crashing the process.

This change adds proper validation to reject values >= 2^31
with a catchable RangeError before they reach nghttp2.
@mcollina mcollina force-pushed the fix-http2-initial-window-size-validation branch from 9a664ef to 7fd969e Compare January 16, 2026 17:44
@codecov
Copy link

codecov bot commented Jan 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.54%. Comparing base (3da4dce) to head (7fd969e).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #61402   +/-   ##
=======================================
  Coverage   88.54%   88.54%           
=======================================
  Files         704      704           
  Lines      208806   208807    +1     
  Branches    40316    40323    +7     
=======================================
+ Hits       184878   184887    +9     
+ Misses      15914    15910    -4     
+ Partials     8014     8010    -4     
Files with missing lines Coverage Δ
lib/internal/http2/core.js 95.22% <100.00%> (+<0.01%) ⬆️

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

@Mohataseem89
Copy link
Contributor

great catch aligning this with RFC 7540
Validating SETTINGS_INITIAL_WINDOW_SIZE at 2^31-1 prevents nghttp2 from hitting an unrecoverable assertion and crashing the process, which is a big stability win

the new RangeError makes the failure mode predictable and user-catchable, and the added test coverage looks solid.

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 17, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 17, 2026
@nodejs-github-bot
Copy link
Collaborator

@pimterry pimterry added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 18, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 18, 2026
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@mcollina mcollina added lts-watch-v20.x PRs that may need to be released in v20.x lts-watch-v22.x PRs that may need to be released in v22.x lts-watch-v24.x PRs that may need to be released in v24.x commit-queue Add this label to land a pull request using GitHub Actions. labels Jan 18, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 19, 2026
@nodejs-github-bot nodejs-github-bot merged commit 57a71cd into nodejs:main Jan 19, 2026
82 of 83 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 57a71cd

aduh95 pushed a commit that referenced this pull request Jan 20, 2026
The HTTP/2 spec (RFC 7540) defines SETTINGS_INITIAL_WINDOW_SIZE
maximum as 2^31-1. Values above this must be treated as a
FLOW_CONTROL_ERROR. Previously, Node.js allowed values up to
2^32-1 which caused nghttp2_submit_settings() to return
NGHTTP2_ERR_INVALID_ARGUMENT, triggering an uncatchable
assertion failure and crashing the process.

This change adds proper validation to reject values >= 2^31
with a catchable RangeError before they reach nghttp2.

PR-URL: #61402
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

http2 Issues or PRs related to the http2 subsystem. lts-watch-v20.x PRs that may need to be released in v20.x lts-watch-v22.x PRs that may need to be released in v22.x lts-watch-v24.x PRs that may need to be released in v24.x needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants