Skip to content

http2: don't suppress internal termination detection on a peer GOAWAY - #11

Open
mcollina wants to merge 1 commit into
mainfrom
http2-fix-goaway-detector
Open

mcollina wants to merge 1 commit into
mainfrom
http2-fix-goaway-detector

Conversation

@mcollina

@mcollina mcollina commented Oct 9, 2026

Copy link
Copy Markdown
Owner

When a peer sends GOAWAY(NO_ERROR), 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 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(). 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.

Defense-in-depth hardening; no test is included because the triggering
scenario is not reproducibly exercised by the default build.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant