diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index 61733d24c052..c2b78632482a 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -696,6 +696,7 @@ void ConnectionImpl::goAway() { auto status = sendPendingFrames(); // See comment in the `encodeHeadersBase()` method about this RELEASE_ASSERT. RELEASE_ASSERT(status.ok(), "sendPendingFrames() failure in non dispatching context"); + checkProtocolConstraintViolation(); } void ConnectionImpl::shutdownNotice() { diff --git a/source/common/http/http2/codec_impl_legacy.cc b/source/common/http/http2/codec_impl_legacy.cc index e94d2de720ec..3654b481fe54 100644 --- a/source/common/http/http2/codec_impl_legacy.cc +++ b/source/common/http/http2/codec_impl_legacy.cc @@ -665,6 +665,7 @@ void ConnectionImpl::goAway() { ASSERT(rc == 0); sendPendingFrames(); + checkProtocolConstraintViolation(); } void ConnectionImpl::shutdownNotice() { diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index b1be79bcf63e..e4640585533b 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -2299,6 +2299,46 @@ TEST_P(Http2CodecImplTest, EmptyDataFloodOverride) { EXPECT_TRUE(status.ok()); } +// Verify that codec detects flood of outbound frames caused by goAway() method +TEST_P(Http2CodecImplTest, GoAwayCausesOutboundFlood) { + initialize(); + + TestRequestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + EXPECT_CALL(request_decoder_, decodeHeaders_(_, false)); + request_encoder_->encodeHeaders(request_headers, false); + + int frame_count = 0; + Buffer::OwnedImpl buffer; + ON_CALL(server_connection_, write(_, _)) + .WillByDefault(Invoke([&buffer, &frame_count](Buffer::Instance& frame, bool) { + ++frame_count; + buffer.move(frame); + })); + + auto* violation_callback = + new NiceMock(&server_connection_.dispatcher_); + + TestResponseHeaderMapImpl response_headers{{":status", "200"}}; + response_encoder_->encodeHeaders(response_headers, false); + // Account for the single HEADERS frame above + for (uint32_t i = 0; i < CommonUtility::OptionsLimits::DEFAULT_MAX_OUTBOUND_FRAMES - 1; ++i) { + Buffer::OwnedImpl data("0"); + EXPECT_NO_THROW(response_encoder_->encodeData(data, false)); + } + + EXPECT_FALSE(violation_callback->enabled_); + + server_->goAway(); + + EXPECT_TRUE(violation_callback->enabled_); + EXPECT_CALL(server_connection_, close(Envoy::Network::ConnectionCloseType::NoFlush)); + violation_callback->invokeCallback(); + + EXPECT_EQ(frame_count, CommonUtility::OptionsLimits::DEFAULT_MAX_OUTBOUND_FRAMES + 1); + EXPECT_EQ(1, server_stats_store_.counter("http2.outbound_flood").value()); +} + // Verify that codec detects flood of outbound frames caused by shutdownNotice() method TEST_P(Http2CodecImplTest, ShudowNoticeCausesOutboundFlood) { initialize(); diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index 60e344475395..ac52fd13d5a3 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -2011,6 +2011,32 @@ TEST_P(Http2FloodMitigationTest, RST_STREAM) { test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value()); } +// Verify detection of frame flood when sending second GOAWAY frame on drain timeout +TEST_P(Http2FloodMitigationTest, GoAwayOverflowOnDrainTimeout) { + config_helper_.addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { + auto* drain_time_out = hcm.mutable_drain_timeout(); + std::chrono::milliseconds timeout(1000); + auto seconds = std::chrono::duration_cast(timeout); + drain_time_out->set_seconds(seconds.count()); + + auto* http_protocol_options = hcm.mutable_common_http_protocol_options(); + auto* idle_time_out = http_protocol_options->mutable_idle_timeout(); + idle_time_out->set_seconds(seconds.count()); + }); + // pre-fill two away from overflow + prefillOutboundDownstreamQueue(AllFrameFloodLimit - 2); + + // connection idle timeout will send first GOAWAY frame and start drain timer + // drain timeout will send second GOAWAY frame which should trigger flood protection + // Wait for connection to be flooded with outbound GOAWAY frame and disconnected. + tcp_client_->waitForDisconnect(); + + // Verify that the flood check was triggered + EXPECT_EQ(1, test_server_->counter("http2.outbound_flood")->value()); +} + // Verify detection of overflowing outbound frame queue with the GOAWAY frames sent after the // downstream idle connection timeout disconnects the connection. // The test verifies protocol constraint violation handling in the