Repository navigation
Conversation
When a peer sends GOAWAY(NO_ERROR), lib/internal/http2/core.js calls session.close(), which sets both graceful_close_initiated_ and goaway_initiated_. Those flags can be set without any application code and, once set, are never reset, so they permanently disarmed the detector that tears down the session when nghttp2 internally terminates it after a protocol error (e.g. an oversized frame). The detector now compares the GOAWAY error code against the code this session requested through session.goaway()/session.close(). A peer-triggered graceful close submits GOAWAY(NO_ERROR); if nghttp2 later emits a frame carrying a different error code it was not requested by us, which indicates an internal termination. In that case the session is torn down and the resulting error is surfaced, as it would be without the prior peer GOAWAY. Assisted-by: pi Signed-off-by: Matteo Collina <hello@matteocollina.com>
Collaborator
|
Review requested:
|
nodejs-github-bot
added
c++
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #66641 +/- ##
==========================================
+ Coverage 90.28% 90.44% +0.15%
==========================================
Files 790 791 +1
Lines 271642 276613 +4971
Branches 51846 53128 +1282
==========================================
+ Hits 245260 250177 +4917
+ Misses 16889 16853 -36
- Partials 9493 9583 +90
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a peer sends GOAWAY(NO_ERROR), core.js calls session.close(), which
sets both
graceful_close_initiated_andgoaway_initiated_. Those flagscan be set without any application code, and once set are never reset, so
they could permanently disarm the detector that tears down the session when
nghttp2 internally terminates it after a protocol error (for example an
oversized frame).
This makes the detector compare the GOAWAY error code against the code the
session requested through
session.goaway()/session.close(). Apeer-triggered graceful close submits GOAWAY(NO_ERROR); if nghttp2 later
emits a frame carrying a different error code it was not requested by us,
which indicates an internal termination. In that case the session is torn
down and the resulting error is surfaced, as it would be without the prior
peer GOAWAY.
Defense-in-depth hardening; no test is included because the triggering
scenario is not reproducibly exercised by the default build.
AI generated, reviewed by me.