diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index b958b448dfe1..d7f3eb748d8b 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -20,6 +20,7 @@ Removed Config or Runtime *Normally occurs at the end of the* :ref:`deprecation period ` * ext_authz: removed auto ignore case in HTTP-based `ext_authz` header matching and the runtime guard `envoy.reloadable_features.ext_authz_http_service_enable_case_sensitive_string_matcher`. To ignore case, set the :ref:`ignore_case ` field to true. +* http: removed `envoy.reloadable_features.http1_flood_protection` and legacy code path for turning flood protection off. New Features ------------ diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index 382478f27f20..42b7f4dfce6a 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -264,9 +264,6 @@ void StreamEncoderImpl::endEncode() { } void ServerConnectionImpl::maybeAddSentinelBufferFragment(Buffer::WatermarkBuffer& output_buffer) { - if (!flood_protection_) { - return; - } // It's messy and complicated to try to tag the final write of an HTTP response for response // tracking for flood protection. Instead, write an empty buffer fragment after the response, // to allow for tracking. @@ -281,9 +278,6 @@ void ServerConnectionImpl::maybeAddSentinelBufferFragment(Buffer::WatermarkBuffe Status ServerConnectionImpl::doFloodProtectionChecks() const { ASSERT(dispatching_); - if (!flood_protection_) { - return okStatus(); - } // Before processing another request, make sure that we are below the response flood protection // threshold. if (outbound_responses_ >= max_outbound_responses_) { @@ -859,8 +853,6 @@ ServerConnectionImpl::ServerConnectionImpl( // maintainer team as it will otherwise be removed entirely soon. max_outbound_responses_( Runtime::getInteger("envoy.do_not_use_going_away_max_http2_outbound_responses", 2)), - flood_protection_( - Runtime::runtimeFeatureEnabled("envoy.reloadable_features.http1_flood_protection")), headers_with_underscores_action_(headers_with_underscores_action) {} uint32_t ServerConnectionImpl::getHeadersSize() { diff --git a/source/common/http/http1/codec_impl.h b/source/common/http/http1/codec_impl.h index 0b66ea129171..2ae00fb03400 100644 --- a/source/common/http/http1/codec_impl.h +++ b/source/common/http/http1/codec_impl.h @@ -545,7 +545,6 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl { // of Envoy wish to enable pipelining (which is dangerous and ill supported) // we could make this configurable. uint32_t max_outbound_responses_{}; - bool flood_protection_{}; // TODO(mattklein123): This should be a member of ActiveRequest but this change needs dedicated // thought as some of the reset and no header code paths make this difficult. Headers are // populated on message begin. Trailers are populated on the first parsed trailer field (if diff --git a/source/common/http/http1/codec_impl_legacy.cc b/source/common/http/http1/codec_impl_legacy.cc index 01b267bb3c3e..332f1241eb7d 100644 --- a/source/common/http/http1/codec_impl_legacy.cc +++ b/source/common/http/http1/codec_impl_legacy.cc @@ -265,9 +265,6 @@ void StreamEncoderImpl::endEncode() { } void ServerConnectionImpl::maybeAddSentinelBufferFragment(Buffer::WatermarkBuffer& output_buffer) { - if (!flood_protection_) { - return; - } // It's messy and complicated to try to tag the final write of an HTTP response for response // tracking for flood protection. Instead, write an empty buffer fragment after the response, // to allow for tracking. @@ -281,9 +278,6 @@ void ServerConnectionImpl::maybeAddSentinelBufferFragment(Buffer::WatermarkBuffe } void ServerConnectionImpl::doFloodProtectionChecks() const { - if (!flood_protection_) { - return; - } // Before processing another request, make sure that we are below the response flood protection // threshold. if (outbound_responses_ >= max_outbound_responses_) { @@ -800,8 +794,6 @@ ServerConnectionImpl::ServerConnectionImpl( // maintainer team as it will otherwise be removed entirely soon. max_outbound_responses_( Runtime::getInteger("envoy.do_not_use_going_away_max_http2_outbound_responses", 2)), - flood_protection_( - Runtime::runtimeFeatureEnabled("envoy.reloadable_features.http1_flood_protection")), headers_with_underscores_action_(headers_with_underscores_action) {} uint32_t ServerConnectionImpl::getHeadersSize() { diff --git a/source/common/http/http1/codec_impl_legacy.h b/source/common/http/http1/codec_impl_legacy.h index 01c8a51aea25..8a1b68b0fad4 100644 --- a/source/common/http/http1/codec_impl_legacy.h +++ b/source/common/http/http1/codec_impl_legacy.h @@ -519,7 +519,6 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl { // of Envoy wish to enable pipelining (which is dangerous and ill supported) // we could make this configurable. uint32_t max_outbound_responses_{}; - bool flood_protection_{}; // TODO(mattklein123): This should be a member of ActiveRequest but this change needs dedicated // thought as some of the reset and no header code paths make this difficult. Headers are // populated on message begin. Trailers are populated on the first parsed trailer field (if diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 6616bf19c8b1..28fc36b5eb31 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -53,7 +53,6 @@ uint64_t getInteger(absl::string_view feature, uint64_t default_value) { // problem of the bugs being found after the old code path has been removed. constexpr const char* runtime_features[] = { // Enabled - "envoy.reloadable_features.http1_flood_protection", "envoy.reloadable_features.test_feature_true", // Begin alphabetically sorted section. "envoy.deprecated_features.allow_deprecated_extension_names", diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 0a78208255ef..200ba2423191 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -1073,42 +1073,6 @@ TEST_P(Http1ServerConnectionImplTest, FloodProtection) { } } -TEST_P(Http1ServerConnectionImplTest, FloodProtectionOff) { - TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.http1_flood_protection", "false"}}); - initialize(); - - NiceMock decoder; - Buffer::OwnedImpl local_buffer; - // With flood protection off, many responses can be queued up. - for (int i = 0; i < 4; ++i) { - Http::ResponseEncoder* response_encoder = nullptr; - EXPECT_CALL(callbacks_, newStream(_, _)) - .WillOnce(Invoke([&](Http::ResponseEncoder& encoder, bool) -> Http::RequestDecoder& { - response_encoder = &encoder; - return decoder; - })); - - Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\n\r\n"); - auto status = codec_->dispatch(buffer); - EXPECT_TRUE(status.ok()); - EXPECT_EQ(0U, buffer.length()); - - // In most tests the write output is serialized to a buffer here it is - // ignored to build up queued "end connection" sentinels. - EXPECT_CALL(connection_, write(_, _)) - - .WillOnce(Invoke([&](Buffer::Instance& data, bool) -> void { - // Move the response out of data while preserving the buffer fragment sentinels. - local_buffer.move(data); - })); - - TestResponseHeaderMapImpl headers{{":status", "200"}}; - response_encoder->encodeHeaders(headers, true); - } -} - TEST_P(Http1ServerConnectionImplTest, HostHeaderTranslation) { initialize();