From 8cf219830a58affd311e8cddadd7f95836519268 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 9 Oct 2026 19:09:21 +0200 Subject: [PATCH] http2: don't suppress internal termination detection on a peer GOAWAY 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 --- src/node_http2.cc | 16 ++++++++++++---- src/node_http2.h | 6 ++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index 3b8d87a2215b..85d34e06f6fb 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1321,15 +1321,22 @@ int Http2Session::OnFrameSent(nghttp2_session* handle, // error like oversized frames, padding errors, or HPACK compression // failures), it calls nghttp2_session_terminate_session() directly which // queues a GOAWAY but does not invoke any application-level callback. - // Detect that case here: a GOAWAY was sent but we never initiated it - // (no Close(), no session.close(), no session.goaway()). + // Detect that case here: a GOAWAY was sent but it was not the one we + // initiated through session.goaway() / session.close(). Note that those + // methods can be reached without application code when a peer sends + // GOAWAY, so goaway_initiated_ and graceful_close_initiated_ can both be + // set even though the GOAWAY we are seeing here was not requested by us. + // A GOAWAY we submitted carries the error code we passed, so if nghttp2 + // sends a frame with a different code it was not requested by us and is + // the result of an internal termination. // // We set a flag here, and then throw the error at the end of // SendPendingData, to wait until the GOAWAY is written before the session // is torn down. if (frame->hd.type == NGHTTP2_GOAWAY && !session->is_closing() && - !session->is_destroyed() && !session->IsGracefulCloseInitiated() && - !session->goaway_initiated_) { + !session->is_destroyed() && + (!session->goaway_initiated_ || + frame->goaway.error_code != session->goaway_code())) { Debug(session, "nghttp2 session terminated internally"); session->internal_goaway_sent_ = true; } @@ -3084,6 +3091,7 @@ void Http2Session::Goaway(uint32_t code, return; goaway_initiated_ = true; + goaway_code_ = code; Http2Scope h2scope(this); // the last proc stream id is the most recently created Http2Stream. if (lastStreamID <= 0) diff --git a/src/node_http2.h b/src/node_http2.h index c042c46ab595..c228f179eed6 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -850,6 +850,9 @@ class Http2Session : public AsyncWrap, void SetGracefulCloseInitiated(bool value) { graceful_close_initiated_ = value; } + uint32_t goaway_code() const { + return goaway_code_; + } private: void EmitStatistics(); @@ -1031,6 +1034,9 @@ class Http2Session : public AsyncWrap, bool graceful_close_initiated_ = false; bool goaway_initiated_ = false; bool internal_goaway_sent_ = false; + // Error code passed to the last session.goaway()/session.close() call; + // used by OnFrameSent to tell an internal nghttp2 GOAWAY from ours. + uint32_t goaway_code_ = NGHTTP2_NO_ERROR; }; struct Http2SessionPerformanceEntryTraits {