diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 1ffd6ac11714f..1aa2fa98347ac 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -17,6 +17,7 @@ Minor Behavior Changes * dns: now respecting the returned DNS TTL for resolved hosts, rather than always relying on the hard-coded :ref:`dns_refresh_rate. ` This behavior can be temporarily reverted by setting the runtime guard ``envoy.reloadable_features.use_dns_ttl`` to false. * http: envoy will now proxy 102 and 103 headers from upstream, though as with 100s only the first 1xx response headers will be sent. This behavioral change by can temporarily reverted by setting runtime guard ``envoy.reloadable_features.proxy_102_103`` to false. * http: usage of the experimental matching API is no longer guarded behind a feature flag, as the corresponding protobuf fields have been marked as WIP. +* http: when envoy run out of ``max_requests_per_connection``, it will send an HTTP/2 "shutdown nofitication" (GOAWAY frame with max stream ID) and go to a default grace period of 5000 milliseconds (5 seconds) if ``drain_timeout`` is not specified. During this grace period, envoy will continue to accept new streams. After the grace period, a final GOAWAY is sent and envoy will start refusing new streams. However before bugfix, during the grace period, every time a new stream is received, old envoy will always send a "shutdown notification" and restart drain again which actually causes the grace period to be extended and is no longer equal to ``drain_timeout``. * json: switching from rapidjson to nlohmann/json. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.remove_legacy_json`` to false. * listener: destroy per network filter chain stats when a network filter chain is removed during the listener in place update. * quic: add back the support for IETF draft 29 which is guarded via ``envoy.reloadable_features.FLAGS_quic_reloadable_flag_quic_disable_version_draft_29``. It is off by default so Envoy only supports RFCv1 without flipping this runtime guard explicitly. Draft 29 is not recommended for use. diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index d9c654a596339..46d9550cf4d82 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -289,7 +289,7 @@ RequestDecoder& ConnectionManagerImpl::newStream(ResponseEncoder& response_encod new_stream->state_.saw_connection_close_ = true; // Prevent erroneous debug log of closing due to incoming connection close header. drain_state_ = DrainState::Closing; - } else { + } else if (drain_state_ == DrainState::NotDraining) { startDrainSequence(); } ENVOY_CONN_LOG(debug, "max requests per connection reached", read_callbacks_->connection()); diff --git a/test/common/http/conn_manager_impl_test_2.cc b/test/common/http/conn_manager_impl_test_2.cc index 44bd0db57ddec..a81b792a89828 100644 --- a/test/common/http/conn_manager_impl_test_2.cc +++ b/test/common/http/conn_manager_impl_test_2.cc @@ -349,6 +349,35 @@ TEST_F(HttpConnectionManagerImplTest, ConnectionDurationNoCodec) { EXPECT_EQ(1U, stats_.named_.downstream_cx_max_duration_reached_.value()); } +// Regression test for https://github.com/envoyproxy/envoy/issues/19045 +TEST_F(HttpConnectionManagerImplTest, MaxRequests) { + max_requests_per_connection_ = 1; + codec_->protocol_ = Protocol::Http2; + setup(false, ""); + + Event::MockTimer* drain_timer = setUpTimer(); + EXPECT_CALL(*drain_timer, enableTimer(_, _)); + + EXPECT_CALL(*codec_, dispatch(_)).WillRepeatedly(Invoke([&](Buffer::Instance&) -> Http::Status { + conn_manager_->newStream(response_encoder_); + return Http::okStatus(); + })); + + EXPECT_CALL(*codec_, goAway()); + EXPECT_CALL(*codec_, shutdownNotice()); + EXPECT_CALL(*drain_timer, disableTimer()); + + // Kick off two requests. + Buffer::OwnedImpl fake_input("hello"); + conn_manager_->onData(fake_input, false); + conn_manager_->onData(fake_input, false); + drain_timer->invokeCallback(); + + EXPECT_EQ(2U, stats_.named_.downstream_cx_max_requests_reached_.value()); + + conn_manager_->onEvent(Network::ConnectionEvent::RemoteClose); +} + TEST_F(HttpConnectionManagerImplTest, ConnectionDuration) { max_connection_duration_ = (std::chrono::milliseconds(10)); Event::MockTimer* connection_duration_timer = setUpTimer(); diff --git a/test/common/http/conn_manager_impl_test_base.h b/test/common/http/conn_manager_impl_test_base.h index 6225b61d854d2..f3a2c0eda8b2f 100644 --- a/test/common/http/conn_manager_impl_test_base.h +++ b/test/common/http/conn_manager_impl_test_base.h @@ -153,7 +153,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan originalIpDetectionExtensions() const override { return ip_detection_extensions_; } - uint64_t maxRequestsPerConnection() const override { return 0; } + uint64_t maxRequestsPerConnection() const override { return max_requests_per_connection_; } Envoy::Event::SimulatedTimeSystem test_time_; NiceMock route_config_provider_; @@ -184,6 +184,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan absl::optional user_agent_; uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB}; uint32_t max_request_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; + uint64_t max_requests_per_connection_{}; absl::optional idle_timeout_; absl::optional max_connection_duration_; std::chrono::milliseconds stream_idle_timeout_{};