Add an oghttp2 option to disable sending GOAWAYs as a client. This CL adds OgHttp2Session::Options::send_goaway_as_client, a bool that controls whether OgHttp2Session will enqueue/send GOAWAYs when OgHttp2Adapter::SubmitGoAway() is called. This CL default-disables the option, which has the functional change of ignoring a client attempt to send a GOAWAY and aims to resolve the issue described at https://github.com/envoyproxy/envoy/issues/37143. Behavior prior to this CL can be restored by setting this option to true. PiperOrigin-RevId: 697840360
diff --git a/quiche/http2/adapter/oghttp2_adapter.cc b/quiche/http2/adapter/oghttp2_adapter.cc index 708bed8..0813452 100644 --- a/quiche/http2/adapter/oghttp2_adapter.cc +++ b/quiche/http2/adapter/oghttp2_adapter.cc
@@ -15,7 +15,6 @@ namespace { -using spdy::SpdyGoAwayIR; using spdy::SpdyPingIR; using spdy::SpdyPriorityIR; using spdy::SpdyWindowUpdateIR; @@ -61,10 +60,9 @@ void OgHttp2Adapter::SubmitGoAway(Http2StreamId last_accepted_stream_id, Http2ErrorCode error_code, absl::string_view opaque_data) { - session_->EnqueueFrame(std::make_unique<SpdyGoAwayIR>( - last_accepted_stream_id, TranslateErrorCode(error_code), - std::string(opaque_data))); + session_->SubmitGoAway(last_accepted_stream_id, error_code, opaque_data); } + void OgHttp2Adapter::SubmitWindowUpdate(Http2StreamId stream_id, int window_increment) { session_->EnqueueFrame(
diff --git a/quiche/http2/adapter/oghttp2_adapter_test.cc b/quiche/http2/adapter/oghttp2_adapter_test.cc index e1e7029..5f1db1d 100644 --- a/quiche/http2/adapter/oghttp2_adapter_test.cc +++ b/quiche/http2/adapter/oghttp2_adapter_test.cc
@@ -2419,10 +2419,11 @@ EXPECT_THAT(serialized, EqualsFrames({SpdyFrameType::SETTINGS})); } -TEST(OgHttp2AdapterTest, ClientSubmitsGoAwayAfterRequest) { +TEST(OgHttp2AdapterTest, ClientSubmitsGoAwayAfterRequestOptionEnabled) { TestVisitor visitor; OgHttp2Adapter::Options options; options.perspective = Perspective::kClient; + options.send_goaway_as_client = true; auto adapter = OgHttp2Adapter::Create(visitor, options); testing::InSequence s; @@ -2494,6 +2495,80 @@ EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::GOAWAY})); } +TEST(OgHttp2AdapterTest, ClientSubmitsGoAwayAfterRequestOptionDisabled) { + TestVisitor visitor; + OgHttp2Adapter::Options options; + options.perspective = Perspective::kClient; + options.send_goaway_as_client = false; + auto adapter = OgHttp2Adapter::Create(visitor, options); + + testing::InSequence s; + + const std::vector<Header> headers = + ToHeaders({{":method", "GET"}, + {":scheme", "http"}, + {":authority", "example.com"}, + {":path", "/this/is/request/one"}}); + + const int32_t stream_id = + adapter->SubmitRequest(headers, nullptr, true, nullptr); + ASSERT_GT(stream_id, 0); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id, _, + END_STREAM_FLAG | END_HEADERS_FLAG)); + EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id, _, + END_STREAM_FLAG | END_HEADERS_FLAG, 0)); + + int result = adapter->Send(); + EXPECT_EQ(0, result); + absl::string_view data = visitor.data(); + EXPECT_THAT(data, testing::StartsWith(spdy::kHttp2ConnectionHeaderPrefix)); + data.remove_prefix(strlen(spdy::kHttp2ConnectionHeaderPrefix)); + EXPECT_THAT(data, + EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::HEADERS})); + visitor.Clear(); + + const std::string stream_frames = + TestFrameSequence() + .ServerPreface() + .Headers(stream_id, {{":status", "200"}}, /*fin=*/true) + .Serialize(); + + // Server preface (empty SETTINGS) + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + + EXPECT_CALL(visitor, OnFrameHeader(stream_id, _, HEADERS, + END_HEADERS_FLAG | END_STREAM_FLAG)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(stream_id)); + EXPECT_CALL(visitor, OnHeaderForStream(stream_id, ":status", "200")); + EXPECT_CALL(visitor, OnEndHeadersForStream(stream_id)); + EXPECT_CALL(visitor, OnEndStream(stream_id)); + EXPECT_CALL(visitor, + OnCloseStream(stream_id, Http2ErrorCode::HTTP2_NO_ERROR)); + + const int64_t stream_result = adapter->ProcessBytes(stream_frames); + EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result)); + + // Now that the stream has been processed, the highest stream ID should be + // updated. Attempting to send a GOAWAY with this stream ID should be a no-op + // because the option is disabled. + EXPECT_EQ(adapter->GetHighestReceivedStreamId(), stream_id); + adapter->SubmitGoAway(adapter->GetHighestReceivedStreamId(), + Http2ErrorCode::HTTP2_NO_ERROR, "opaque_data"); + EXPECT_TRUE(adapter->want_write()); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, ACK_FLAG)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, ACK_FLAG, 0)); + + result = adapter->Send(); + EXPECT_EQ(0, result); + EXPECT_THAT(visitor.data(), EqualsFrames({SpdyFrameType::SETTINGS})); +} + TEST(OgHttp2AdapterTest, ClientReceivesGoAway) { TestVisitor visitor; OgHttp2Adapter::Options options;
diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc index c124f2b..fe59df7 100644 --- a/quiche/http2/adapter/oghttp2_session.cc +++ b/quiche/http2/adapter/oghttp2_session.cc
@@ -20,6 +20,7 @@ #include "quiche/http2/adapter/noop_header_validator.h" #include "quiche/http2/adapter/oghttp2_util.h" #include "quiche/http2/core/spdy_protocol.h" +#include "quiche/common/platform/api/quiche_logging.h" #include "quiche/common/quiche_callbacks.h" namespace http2 { @@ -1077,6 +1078,22 @@ EnqueueFrame(std::move(frame)); } +void OgHttp2Session::SubmitGoAway(Http2StreamId last_accepted_stream_id, + Http2ErrorCode error_code, + absl::string_view opaque_data) { + if (!IsServerSession() && !options_.send_goaway_as_client) { + QUICHE_VLOG(2) + << "Dropping GOAWAY from client with last_accepted_stream_id " + << last_accepted_stream_id << ", error_code " + << Http2ErrorCodeToString(error_code) << ", opaque_data " + << opaque_data; + return; + } + EnqueueFrame(std::make_unique<spdy::SpdyGoAwayIR>( + last_accepted_stream_id, TranslateErrorCode(error_code), + std::string(opaque_data))); +} + void OgHttp2Session::OnError(SpdyFramerError error, std::string detailed_error) { QUICHE_VLOG(1) << "Error: "
diff --git a/quiche/http2/adapter/oghttp2_session.h b/quiche/http2/adapter/oghttp2_session.h index 8953d25..4a4221c 100644 --- a/quiche/http2/adapter/oghttp2_session.h +++ b/quiche/http2/adapter/oghttp2_session.h
@@ -89,6 +89,8 @@ // If true, crumbles `Cookie` header field values for potentially better // HPACK compression. bool crumble_cookies = false; + // If true, allows a GOAWAY to be sent even when acting as a client. + bool send_goaway_as_client = false; }; OgHttp2Session(Http2VisitorInterface& visitor, Options options); @@ -116,6 +118,8 @@ std::unique_ptr<MetadataSource> source); void SubmitMetadata(Http2StreamId stream_id); void SubmitSettings(absl::Span<const Http2Setting> settings); + void SubmitGoAway(Http2StreamId last_accepted_stream_id, + Http2ErrorCode error_code, absl::string_view opaque_data); bool IsServerSession() const { return options_.perspective == Perspective::kServer;