diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index 9217b6d0c9332..9b9e61edb8392 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -37,7 +37,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 58] +// [#next-free-field: 59] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager"; @@ -447,6 +447,21 @@ message HttpConnectionManager { config.core.v3.HttpProtocolOptions common_http_protocol_options = 35 [(udpa.annotations.security).configure_for_untrusted_downstream = true]; + // If set to true, Envoy will not start a drain timer for downstream HTTP1 connections after + // :ref:`common_http_protocol_options.max_connection_duration + // ` passes. + // Instead, Envoy will wait for the next downstream request, add connection:close to the response + // headers, then close the connection after the stream ends. + // + // This behavior is compliant with `RFC 9112 section 9.6 `_ + // + // If set to false, ``max_connection_duration`` will cause Envoy to enter the normal drain + // sequence for HTTP1 with Envoy eventually closing the connection (once there are no active + // streams). + // + // Has no effect if ``max_connection_duration`` is unset. Defaults to false. + bool http1_safe_max_connection_duration = 58; + // Additional HTTP/1 settings that are passed to the HTTP/1 codec. // [#comment:TODO: The following fields are ignored when the // :ref:`header validation configuration ` diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 934894d405d79..e9f0c7bda101a 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -6,6 +6,15 @@ behavior_changes: change: | Removed support for (long deprecated) opentracing. See `issue 27401 `_ for details. +- area: http + change: | + Added HTTP1-safe option for :ref:`max_connection_duration + ` in + HttpConnectionManager. When enabled, ``max_connection_duration`` will only drain downstream + HTTP1 connections by adding the Connection:close response header; it will never cause the + HttpConnectionManager to close the connection itself. Defaults to off ("unsafe" -- check + \#34356) and is configurable via :ref:`http1_safe_max_connection_duration + `. - area: stats scoped_rds change: | Added new tag extraction so that scoped rds stats have their :ref:'scope_route_config_name diff --git a/docs/root/configuration/http/http_conn_man/stats.rst b/docs/root/configuration/http/http_conn_man/stats.rst index 2605414255a16..fbd4e3ba4a691 100644 --- a/docs/root/configuration/http/http_conn_man/stats.rst +++ b/docs/root/configuration/http/http_conn_man/stats.rst @@ -26,6 +26,7 @@ statistics: ``downstream_cx_ssl_active``, Gauge, Total active TLS connections ``downstream_cx_http1_active``, Gauge, Total active HTTP/1.1 connections ``downstream_cx_upgrades_active``, Gauge, Total active upgraded connections. These are also counted as active http1/http2 connections. + ``downstream_cx_http1_soft_drain``, Gauge, Total active HTTP/1.x connections waiting for another downstream request to safely close the connection. ``downstream_cx_http2_active``, Gauge, Total active HTTP/2 connections ``downstream_cx_http3_active``, Gauge, Total active HTTP/3 connections ``downstream_cx_protocol_error``, Counter, Total protocol errors diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index a02a45ddc988e..7a2b300b839f2 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -83,6 +83,7 @@ namespace Http { GAUGE(downstream_cx_ssl_active, Accumulate) \ GAUGE(downstream_cx_tx_bytes_buffered, Accumulate) \ GAUGE(downstream_cx_upgrades_active, Accumulate) \ + GAUGE(downstream_cx_http1_soft_drain, Accumulate) \ GAUGE(downstream_rq_active, Accumulate) \ HISTOGRAM(downstream_cx_length_ms, Milliseconds) \ HISTOGRAM(downstream_rq_time, Milliseconds) @@ -290,6 +291,12 @@ class ConnectionManagerConfig { */ virtual absl::optional maxConnectionDuration() const PURE; + /** + * @return whether maxConnectionDuration allows HTTP1 clients to choose when to close connection + * (rather than Envoy closing the connection itself when there are no active streams). + */ + virtual bool http1SafeMaxConnectionDuration() const PURE; + /** * @return maximum request headers size the connection manager will accept. */ diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index a9e4a70cc25dc..65a2b032ec4ea 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -213,6 +213,10 @@ ConnectionManagerImpl::~ConnectionManagerImpl() { } } + if (soft_drain_http1_) { + stats_.named_.downstream_cx_http1_soft_drain_.dec(); + } + conn_length_->complete(); user_agent_.completeConnectionLength(*conn_length_); } @@ -413,6 +417,12 @@ RequestDecoder& ConnectionManagerImpl::newStream(ResponseEncoder& response_encod stats_.named_.downstream_cx_max_requests_reached_.inc(); } + if (soft_drain_http1_) { + new_stream->filter_manager_.streamInfo().setShouldDrainConnectionUponCompletion(true); + // Prevent erroneous debug log of closing due to incoming connection close header. + drain_state_ = DrainState::Closing; + } + new_stream->state_.is_internally_created_ = is_internally_created; new_stream->response_encoder_ = &response_encoder; new_stream->response_encoder_->getStream().addCallbacks(*new_stream); @@ -724,7 +734,15 @@ void ConnectionManagerImpl::onConnectionDurationTimeout() { StreamInfo::CoreResponseFlag::DurationTimeout, StreamInfo::ResponseCodeDetails::get().DurationTimeout); } else if (drain_state_ == DrainState::NotDraining) { - startDrainSequence(); + if (config_->http1SafeMaxConnectionDuration() && codec_->protocol() < Protocol::Http2) { + ENVOY_CONN_LOG(debug, + "HTTP1-safe max connection duration is configured -- skipping drain sequence.", + read_callbacks_->connection()); + stats_.named_.downstream_cx_http1_soft_drain_.inc(); + soft_drain_http1_ = true; + } else { + startDrainSequence(); + } } } diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index a325dff0dfb21..34c96fab02f1c 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -606,6 +606,8 @@ class ConnectionManagerImpl : Logger::Loggable, // A connection duration timer. Armed during handling new connection if enabled in config. Event::TimerPtr connection_duration_timer_; Event::TimerPtr drain_timer_; + // When set to true, add Connection:close response header to nudge downstream client to reconnect. + bool soft_drain_http1_{false}; Random::RandomGenerator& random_generator_; Runtime::Loader& runtime_; const LocalInfo::LocalInfo& local_info_; diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index e538fbe89e721..e54182218b1c7 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -375,6 +375,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( idle_timeout_(PROTOBUF_GET_OPTIONAL_MS(config.common_http_protocol_options(), idle_timeout)), max_connection_duration_( PROTOBUF_GET_OPTIONAL_MS(config.common_http_protocol_options(), max_connection_duration)), + http1_safe_max_connection_duration_(config.http1_safe_max_connection_duration()), max_stream_duration_( PROTOBUF_GET_OPTIONAL_MS(config.common_http_protocol_options(), max_stream_duration)), stream_idle_timeout_( diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index 34dbfafcc2f9e..c7a25b431d1d2 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -180,6 +180,9 @@ class HttpConnectionManagerConfig : Logger::Loggable, absl::optional maxConnectionDuration() const override { return max_connection_duration_; } + bool http1SafeMaxConnectionDuration() const override { + return http1_safe_max_connection_duration_; + } std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } std::chrono::milliseconds requestTimeout() const override { return request_timeout_; } std::chrono::milliseconds requestHeadersTimeout() const override { @@ -324,6 +327,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, const uint32_t max_request_headers_count_; absl::optional idle_timeout_; absl::optional max_connection_duration_; + const bool http1_safe_max_connection_duration_; absl::optional max_stream_duration_; std::chrono::milliseconds stream_idle_timeout_; std::chrono::milliseconds request_timeout_; diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index 92db9a75a6ad4..80a91efc3ea0b 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -148,6 +148,7 @@ class AdminImpl : public Admin, absl::optional maxConnectionDuration() const override { return max_connection_duration_; } + bool http1SafeMaxConnectionDuration() const override { return false; } uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } uint32_t maxRequestHeadersCount() const override { return max_request_headers_count_; } std::chrono::milliseconds streamIdleTimeout() const override { return {}; } diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index 065b4d53f3772..59a5012ae8575 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -153,6 +153,9 @@ class FuzzConfig : public ConnectionManagerConfig { absl::optional maxConnectionDuration() const override { return max_connection_duration_; } + bool http1SafeMaxConnectionDuration() const override { + return http1_safe_max_connection_duration_; + } absl::optional maxStreamDuration() const override { return max_stream_duration_; } @@ -276,6 +279,7 @@ class FuzzConfig : public ConnectionManagerConfig { uint32_t max_request_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; absl::optional idle_timeout_; absl::optional max_connection_duration_; + bool http1_safe_max_connection_duration_{false}; absl::optional max_stream_duration_; std::chrono::milliseconds stream_idle_timeout_{}; std::chrono::milliseconds request_timeout_{}; diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index e9094f6bcdf96..d98b1008e2fd6 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -5,6 +5,8 @@ #include "test/test_common/logging.h" #include "test/test_common/test_runtime.h" +#include "conn_manager_impl_test_base.h" + using testing::_; using testing::An; using testing::AnyNumber; @@ -22,7 +24,7 @@ namespace Envoy { namespace Http { TEST_F(HttpConnectionManagerImplTest, HeaderOnlyRequestAndResponse) { - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); // Store the basic request encoder during filter chain setup. std::shared_ptr filter(new NiceMock()); @@ -92,7 +94,7 @@ TEST_F(HttpConnectionManagerImplTest, HeaderOnlyRequestAndResponse) { // Similar to HeaderOnlyRequestAndResponse but uses newStreamHandle and has // lifetime checks. TEST_F(HttpConnectionManagerImplTest, HandleLifetime) { - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); Http::RequestDecoderHandlePtr decoder_handle; // Store the basic request encoder during filter chain setup. @@ -169,7 +171,7 @@ TEST_F(HttpConnectionManagerImplTest, HandleLifetime) { } TEST_F(HttpConnectionManagerImplTest, HeaderOnlyRequestAndResponseWithEarlyHeaderMutation) { - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); auto mock_early_header_mutation_1 = std::make_unique>(); auto mock_early_header_mutation_2 = std::make_unique>(); @@ -260,7 +262,7 @@ TEST_F(HttpConnectionManagerImplTest, HeaderOnlyRequestAndResponseWithEarlyHeade TEST_F(HttpConnectionManagerImplTest, 1xxResponse) { proxy_100_continue_ = true; - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); // Store the basic request encoder during filter chain setup. std::shared_ptr filter(new NiceMock()); @@ -318,7 +320,7 @@ TEST_F(HttpConnectionManagerImplTest, 1xxResponse) { TEST_F(HttpConnectionManagerImplTest, 1xxResponseWithEncoderFiltersProxyingDisabled) { proxy_100_continue_ = false; - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -345,7 +347,7 @@ TEST_F(HttpConnectionManagerImplTest, 1xxResponseWithEncoderFiltersProxyingDisab TEST_F(HttpConnectionManagerImplTest, 1xxResponseWithEncoderFilters) { proxy_100_continue_ = true; - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -371,7 +373,7 @@ TEST_F(HttpConnectionManagerImplTest, 1xxResponseWithEncoderFilters) { TEST_F(HttpConnectionManagerImplTest, PauseResume1xx) { proxy_100_continue_ = true; - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -405,7 +407,7 @@ TEST_F(HttpConnectionManagerImplTest, PauseResume1xx) { // Regression test for https://github.com/envoyproxy/envoy/issues/10923. TEST_F(HttpConnectionManagerImplTest, 1xxResponseWithDecoderPause) { proxy_100_continue_ = true; - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); std::shared_ptr filter(new NiceMock()); @@ -472,7 +474,7 @@ TEST_F(HttpConnectionManagerImplTest, 1xxResponseWithDecoderPause) { // When create new stream, the stream info will be populated from the connection. TEST_F(HttpConnectionManagerImplTest, PopulateStreamInfo) { - setup(true, "", false); + setup(SetupOpts().setSsl(true).setTracing(false)); // Set up the codec. Buffer::OwnedImpl fake_input("input"); @@ -496,7 +498,7 @@ TEST_F(HttpConnectionManagerImplTest, PopulateStreamInfo) { // By default, Envoy will set the server header to the server name, here "custom-value" TEST_F(HttpConnectionManagerImplTest, ServerHeaderOverwritten) { - setup(false, "custom-value", false); + setup(SetupOpts().setServerName("custom-value").setTracing(false)); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -510,7 +512,7 @@ TEST_F(HttpConnectionManagerImplTest, ServerHeaderOverwritten) { // When configured APPEND_IF_ABSENT if the server header is present it will be retained. TEST_F(HttpConnectionManagerImplTest, ServerHeaderAppendPresent) { server_transformation_ = HttpConnectionManagerProto::APPEND_IF_ABSENT; - setup(false, "custom-value", false); + setup(SetupOpts().setServerName("custom-value").setTracing(false)); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -524,7 +526,7 @@ TEST_F(HttpConnectionManagerImplTest, ServerHeaderAppendPresent) { // When configured APPEND_IF_ABSENT if the server header is absent the server name will be set. TEST_F(HttpConnectionManagerImplTest, ServerHeaderAppendAbsent) { server_transformation_ = HttpConnectionManagerProto::APPEND_IF_ABSENT; - setup(false, "custom-value", false); + setup(SetupOpts().setServerName("custom-value").setTracing(false)); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -538,7 +540,7 @@ TEST_F(HttpConnectionManagerImplTest, ServerHeaderAppendAbsent) { // When configured PASS_THROUGH, the server name will pass through. TEST_F(HttpConnectionManagerImplTest, ServerHeaderPassthroughPresent) { server_transformation_ = HttpConnectionManagerProto::PASS_THROUGH; - setup(false, "custom-value", false); + setup(SetupOpts().setServerName("custom-value").setTracing(false)); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -552,7 +554,7 @@ TEST_F(HttpConnectionManagerImplTest, ServerHeaderPassthroughPresent) { // When configured PASS_THROUGH, the server header will not be added if absent. TEST_F(HttpConnectionManagerImplTest, ServerHeaderPassthroughAbsent) { server_transformation_ = HttpConnectionManagerProto::PASS_THROUGH; - setup(false, "custom-value", false); + setup(SetupOpts().setServerName("custom-value").setTracing(false)); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -565,7 +567,7 @@ TEST_F(HttpConnectionManagerImplTest, ServerHeaderPassthroughAbsent) { TEST_F(HttpConnectionManagerImplTest, InvalidPathWithDualFilter) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance& data) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -604,7 +606,7 @@ TEST_F(HttpConnectionManagerImplTest, InvalidPathWithDualFilter) { // Invalid paths are rejected with 400. TEST_F(HttpConnectionManagerImplTest, PathFailedtoSanitize) { - setup(false, ""); + setup(); // Enable path sanitizer normalize_path_ = true; @@ -652,7 +654,7 @@ TEST_F(HttpConnectionManagerImplTest, PathFailedtoSanitize) { // Filters observe normalized paths, not the original path, when path // normalization is configured. TEST_F(HttpConnectionManagerImplTest, FilterShouldUseSantizedPath) { - setup(false, ""); + setup(); // Enable path sanitizer normalize_path_ = true; const std::string original_path = "/x/%2E%2e/z"; @@ -699,7 +701,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterShouldUseSantizedPath) { // The router observes normalized paths, not the original path, when path // normalization is configured. TEST_F(HttpConnectionManagerImplTest, RouteShouldUseSantizedPath) { - setup(false, ""); + setup(); // Enable path sanitizer normalize_path_ = true; const std::string original_path = "/x/%2E%2e/z"; @@ -795,7 +797,7 @@ TEST_F(HttpConnectionManagerImplTest, EscapedSlashesRedirectedAfterOtherNormaliz } TEST_F(HttpConnectionManagerImplTest, AllNormalizationsWithEscapedSlashesForwarded) { - setup(false, ""); + setup(); // Enable path sanitizer normalize_path_ = true; merge_slashes_ = true; @@ -844,7 +846,7 @@ TEST_F(HttpConnectionManagerImplTest, AllNormalizationsWithEscapedSlashesForward } TEST_F(HttpConnectionManagerImplTest, RouteOverride) { - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -1045,7 +1047,7 @@ TEST_F(HttpConnectionManagerImplTest, RouteOverride) { // original route returned by route config (upstream cluster default), when the setRoute filter // callback is applied. TEST_F(HttpConnectionManagerImplTest, FilterSetRouteToDelegatingRouteWithClusterOverride) { - setup(false, ""); + setup(); setupFilterChain(2, 0); // Cluster mocks: default and foo @@ -1131,7 +1133,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterSetRouteToDelegatingRouteWithCluster // Test that all methods supported by DelegatingRouteEntry delegate correctly TEST_F(HttpConnectionManagerImplTest, DelegatingRouteEntryAllCalls) { - setup(false, ""); + setup(); setupFilterChain(2, 0); // Cluster mock: foo @@ -1326,7 +1328,7 @@ TEST_F(HttpConnectionManagerImplTest, DelegatingRouteEntryAllCalls) { // Filters observe host header w/o port's part when port's removal is configured TEST_F(HttpConnectionManagerImplTest, FilterShouldUseNormalizedHost) { - setup(false, ""); + setup(); // Enable port removal strip_port_type_ = Http::StripPortType::MatchingHost; const std::string original_host = "host:443"; @@ -1371,7 +1373,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterShouldUseNormalizedHost) { // The router observes host header w/o port, not the original host, when // remove_port is configured TEST_F(HttpConnectionManagerImplTest, RouteShouldUseNormalizedHost) { - setup(false, ""); + setup(); // Enable port removal strip_port_type_ = Http::StripPortType::MatchingHost; const std::string original_host = "host:443"; @@ -1411,7 +1413,7 @@ TEST_F(HttpConnectionManagerImplTest, RouteShouldUseNormalizedHost) { // Observe that we strip the trailing dot. TEST_F(HttpConnectionManagerImplTest, StripTrailingHostDot) { - setup(false, ""); + setup(); // Enable removal of host's trailing dot. strip_trailing_host_dot_ = true; const std::string original_host = "host."; @@ -1431,7 +1433,7 @@ TEST_F(HttpConnectionManagerImplTest, StripTrailingHostDot) { } TEST_F(HttpConnectionManagerImplTest, HostWithoutTrailingDot) { - setup(false, ""); + setup(); // Enable removal of host's trailing dot. strip_trailing_host_dot_ = true; const std::string original_host = "host"; @@ -1451,7 +1453,7 @@ TEST_F(HttpConnectionManagerImplTest, HostWithoutTrailingDot) { } TEST_F(HttpConnectionManagerImplTest, DateHeaderNotPresent) { - setup(false, ""); + setup(); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); const auto* modified_headers = sendResponseHeaders( @@ -1462,7 +1464,7 @@ TEST_F(HttpConnectionManagerImplTest, DateHeaderNotPresent) { } TEST_F(HttpConnectionManagerImplTest, DateHeaderPresent) { - setup(false, ""); + setup(); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); const std::string expected_date{"Tue, 15 Nov 1994 08:12:31 GMT"}; @@ -1476,7 +1478,7 @@ TEST_F(HttpConnectionManagerImplTest, DateHeaderPresent) { } TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlow) { - setup(false, ""); + setup(); auto* span = new NiceMock(); EXPECT_CALL(*tracer_, startSpan_(_, _, _, _)) @@ -1636,7 +1638,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlow) { } TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowIngressDecorator) { - setup(false, ""); + setup(); auto* span = new NiceMock(); EXPECT_CALL(*tracer_, startSpan_(_, _, _, _)) @@ -1704,7 +1706,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowIngressDecorat } TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowIngressDecoratorPropagateFalse) { - setup(false, ""); + setup(); auto* span = new NiceMock(); EXPECT_CALL(*tracer_, startSpan_(_, _, _, _)) @@ -1773,7 +1775,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowIngressDecorat } TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowIngressDecoratorOverrideOp) { - setup(false, ""); + setup(); auto* span = new NiceMock(); EXPECT_CALL(*tracer_, startSpan_(_, _, _, _)) @@ -1843,7 +1845,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowIngressDecorat } TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecorator) { - setup(false, ""); + setup(); envoy::type::v3::FractionalPercent percent1; percent1.set_numerator(100); envoy::type::v3::FractionalPercent percent2; @@ -1927,7 +1929,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecorato } TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecoratorPropagateFalse) { - setup(false, ""); + setup(); envoy::type::v3::FractionalPercent percent1; percent1.set_numerator(100); envoy::type::v3::FractionalPercent percent2; @@ -2013,7 +2015,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecorato } TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecoratorOverrideOp) { - setup(false, ""); + setup(); envoy::type::v3::FractionalPercent percent1; percent1.set_numerator(100); envoy::type::v3::FractionalPercent percent2; @@ -2091,7 +2093,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecorato TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecoratorOverrideOpNoActiveSpan) { - setup(false, ""); + setup(); envoy::type::v3::FractionalPercent percent1; percent1.set_numerator(100); envoy::type::v3::FractionalPercent percent2; @@ -2148,7 +2150,7 @@ TEST_F(HttpConnectionManagerImplTest, } TEST_F(HttpConnectionManagerImplTest, NoHCMTracingConfigAndActiveSpanWouldBeNullSpan) { - setup(false, ""); + setup(); // Null HCM tracing config. tracing_config_ = nullptr; @@ -2206,7 +2208,7 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLog) { // stream_info.downstreamRemoteAddress will infer the address from request // headers instead of the physical connection use_remote_address_ = false; - setup(false, ""); + setup(); std::shared_ptr filter(new NiceMock()); std::shared_ptr handler(new NiceMock()); @@ -2264,7 +2266,7 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLog) { } TEST_F(HttpConnectionManagerImplTest, TestFilterCanEnrichAccessLogs) { - setup(false, ""); + setup(); std::shared_ptr filter(new NiceMock()); std::shared_ptr handler(new NiceMock()); @@ -2316,7 +2318,7 @@ TEST_F(HttpConnectionManagerImplTest, TestFilterCanEnrichAccessLogs) { } TEST_F(HttpConnectionManagerImplTest, TestRemoteDownstreamDisconnectAccessLog) { - setup(false, ""); + setup(); std::shared_ptr filter(new NiceMock()); std::shared_ptr handler(new NiceMock()); @@ -2360,7 +2362,7 @@ TEST_F(HttpConnectionManagerImplTest, TestRemoteDownstreamDisconnectAccessLog) { } TEST_F(HttpConnectionManagerImplTest, TestLocalDownstreamDisconnectAccessLog) { - setup(false, ""); + setup(); std::shared_ptr filter(new NiceMock()); std::shared_ptr handler(new NiceMock()); @@ -2402,7 +2404,7 @@ TEST_F(HttpConnectionManagerImplTest, TestLocalDownstreamDisconnectAccessLog) { } TEST_F(HttpConnectionManagerImplTest, TestAccessLogWithTrailers) { - setup(false, ""); + setup(); std::shared_ptr filter(new NiceMock()); std::shared_ptr handler(new NiceMock()); @@ -2455,7 +2457,7 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLogWithTrailers) { } TEST_F(HttpConnectionManagerImplTest, TestAccessLogWithInvalidRequest) { - setup(false, ""); + setup(); std::shared_ptr filter(new NiceMock()); std::shared_ptr handler(new NiceMock()); @@ -2501,7 +2503,7 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLogWithInvalidRequest) { } TEST_F(HttpConnectionManagerImplTest, TestAccessLogOnNewRequest) { - setup(false, ""); + setup(); std::shared_ptr filter(new NiceMock()); std::shared_ptr handler(new NiceMock()); @@ -2563,7 +2565,7 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLogOnNewRequest) { } TEST_F(HttpConnectionManagerImplTest, TestAccessLogOnTunnelEstablished) { - setup(false, ""); + setup(); std::shared_ptr filter(new NiceMock()); std::shared_ptr handler(new NiceMock()); @@ -2627,7 +2629,8 @@ TEST_F(HttpConnectionManagerImplTest, TestPeriodicAccessLogging) { request_timeout_ = std::chrono::milliseconds(0); request_headers_timeout_ = std::chrono::milliseconds(0); max_stream_duration_ = std::nullopt; - setup(false, "server_name"); + + setup(SetupOpts().setServerName("server-opts")); std::shared_ptr filter(new NiceMock()); std::shared_ptr handler(new NiceMock()); @@ -2708,7 +2711,7 @@ class StreamErrorOnInvalidHttpMessageTest : public HttpConnectionManagerImplTest public: void sendInvalidRequestAndVerifyConnectionState(bool stream_error_on_invalid_http_message, bool send_complete_request = true) { - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)) .WillRepeatedly(Invoke([&](Buffer::Instance& data) -> Http::Status { @@ -2786,7 +2789,7 @@ TEST_F(StreamErrorOnInvalidHttpMessageTest, ConnectionOpenIfCodecStreamErrorIsTr } TEST_F(HttpConnectionManagerImplTest, TestAccessLogSsl) { - setup(true, ""); + setup(SetupOpts().setSsl(true)); std::shared_ptr filter(new NiceMock()); std::shared_ptr handler(new NiceMock()); @@ -2840,7 +2843,7 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLogSsl) { } TEST_F(HttpConnectionManagerImplTest, DoNotStartSpanIfTracingIsNotEnabled) { - setup(false, ""); + setup(); // Disable tracing. tracing_config_.reset(); @@ -2883,7 +2886,7 @@ TEST_F(HttpConnectionManagerImplTest, DoNotStartSpanIfTracingIsNotEnabled) { } TEST_F(HttpConnectionManagerImplTest, NoPath) { - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance& data) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -2907,7 +2910,7 @@ TEST_F(HttpConnectionManagerImplTest, NoPath) { // per-route level. The connection manager config is responsible for managing // the default configuration aspects. TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutNotConfigured) { - setup(false, ""); + setup(); EXPECT_CALL(filter_callbacks_.connection_.dispatcher_, createTimer_(_)).Times(0); EXPECT_CALL(*codec_, dispatch(_)) @@ -2933,7 +2936,7 @@ TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutNotConfigured) { // headers, if it fires we don't faceplant. TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutGlobal) { stream_idle_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillRepeatedly(Invoke([&](Buffer::Instance&) -> Http::Status { Event::MockTimer* idle_timer = setUpTimer(); @@ -2965,7 +2968,7 @@ TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutGlobal) { TEST_F(HttpConnectionManagerImplTest, AccessEncoderRouteBeforeHeadersArriveOnIdleTimeout) { stream_idle_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); std::shared_ptr filter(new NiceMock()); @@ -3017,7 +3020,7 @@ TEST_F(HttpConnectionManagerImplTest, AccessEncoderRouteBeforeHeadersArriveOnIdl TEST_F(HttpConnectionManagerImplTest, TestStreamIdleAccessLog) { stream_idle_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillRepeatedly(Invoke([&](Buffer::Instance&) -> Http::Status { Event::MockTimer* idle_timer = setUpTimer(); @@ -3072,7 +3075,7 @@ TEST_F(HttpConnectionManagerImplTest, TestStreamIdleAccessLog) { // Test timeout variants. TEST_F(HttpConnectionManagerImplTest, DurationTimeout) { stream_idle_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); setupFilterChain(1, 0); RequestHeaderMap* latched_headers = nullptr; @@ -3272,7 +3275,7 @@ TEST_F(HttpConnectionManagerImplTest, DurationTimeout) { // Per-route timeouts override the global stream idle timeout. TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutRouteOverride) { stream_idle_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); ON_CALL(route_config_provider_.route_config_->route_->route_entry_, idleTimeout()) .WillByDefault(Return(std::chrono::milliseconds(30))); @@ -3302,7 +3305,7 @@ TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutRouteOverride) { // Per-route zero timeout overrides the global stream idle timeout. TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutRouteZeroOverride) { stream_idle_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); ON_CALL(route_config_provider_.route_config_->route_->route_entry_, idleTimeout()) .WillByDefault(Return(std::chrono::milliseconds(0))); @@ -3331,7 +3334,7 @@ TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutRouteZeroOverride) { // Validate the per-stream idle timeout after having sent downstream headers. TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutAfterDownstreamHeaders) { - setup(false, ""); + setup(); ON_CALL(route_config_provider_.route_config_->route_->route_entry_, idleTimeout()) .WillByDefault(Return(std::chrono::milliseconds(10))); @@ -3372,7 +3375,7 @@ TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutAfterDownstreamHeaders // Validate the per-stream idle timer is properly disabled when the stream terminates normally. TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutNormalTermination) { - setup(false, ""); + setup(); ON_CALL(route_config_provider_.route_config_->route_->route_entry_, idleTimeout()) .WillByDefault(Return(std::chrono::milliseconds(10))); @@ -3402,7 +3405,7 @@ TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutNormalTermination) { // Validate the per-stream idle timeout after having sent downstream // headers+body. TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutAfterDownstreamHeadersAndBody) { - setup(false, ""); + setup(); ON_CALL(route_config_provider_.route_config_->route_->route_entry_, idleTimeout()) .WillByDefault(Return(std::chrono::milliseconds(10))); @@ -3446,7 +3449,7 @@ TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutAfterDownstreamHeaders // Validate the per-stream idle timeout after upstream headers have been sent. TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutAfterUpstreamHeaders) { - setup(false, ""); + setup(); ON_CALL(route_config_provider_.route_config_->route_->route_entry_, idleTimeout()) .WillByDefault(Return(std::chrono::milliseconds(10))); @@ -3497,7 +3500,7 @@ TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutAfterUpstreamHeaders) // Validate the per-stream idle timeout after a sequence of header/data events. TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutAfterBidiData) { - setup(false, ""); + setup(); ON_CALL(route_config_provider_.route_config_->route_->route_entry_, idleTimeout()) .WillByDefault(Return(std::chrono::milliseconds(10))); proxy_100_continue_ = true; @@ -3571,7 +3574,7 @@ TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutAfterBidiData) { } TEST_F(HttpConnectionManagerImplTest, RequestTimeoutDisabledByDefault) { - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { EXPECT_CALL(filter_callbacks_.connection_.dispatcher_, createTimer_).Times(0); @@ -3588,7 +3591,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestTimeoutDisabledByDefault) { TEST_F(HttpConnectionManagerImplTest, RequestTimeoutDisabledIfSetToZero) { request_timeout_ = std::chrono::milliseconds(0); - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { EXPECT_CALL(filter_callbacks_.connection_.dispatcher_, createTimer_).Times(0); @@ -3603,7 +3606,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestTimeoutDisabledIfSetToZero) { TEST_F(HttpConnectionManagerImplTest, RequestTimeoutValidlyConfigured) { request_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { Event::MockTimer* request_timer = setUpTimer(); @@ -3623,7 +3626,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestTimeoutValidlyConfigured) { TEST_F(HttpConnectionManagerImplTest, RequestTimeoutCallbackDisarmsAndReturns408) { request_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); std::string response_body; EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { @@ -3653,7 +3656,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestTimeoutCallbackDisarmsAndReturns408 TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsNotDisarmedOnIncompleteRequestWithHeader) { request_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { Event::MockTimer* request_timer = setUpTimer(); @@ -3680,7 +3683,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsNotDisarmedOnIncompleteReq TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsDisarmedOnCompleteRequestWithHeader) { request_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { Event::MockTimer* request_timer = setUpTimer(); @@ -3706,7 +3709,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsDisarmedOnCompleteRequestW TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsDisarmedOnCompleteRequestWithData) { request_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance& data) -> Http::Status { Event::MockTimer* request_timer = setUpTimer(); @@ -3733,7 +3736,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsDisarmedOnCompleteRequestW TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsDisarmedOnCompleteRequestWithTrailers) { request_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance& data) -> Http::Status { Event::MockTimer* request_timer = setUpTimer(); @@ -3762,7 +3765,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsDisarmedOnCompleteRequestW TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsDisarmedOnEncodeHeaders) { request_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); std::shared_ptr filter(new NiceMock()); EXPECT_CALL(filter_factory_, createFilterChain(_)) .WillRepeatedly(Invoke([&](FilterChainManager& manager) -> bool { @@ -3799,7 +3802,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsDisarmedOnEncodeHeaders) { TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsDisarmedOnConnectionTermination) { request_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); Event::MockTimer* request_timer = setUpTimer(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { @@ -3825,7 +3828,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsDisarmedOnConnectionTermin TEST_F(HttpConnectionManagerImplTest, RequestHeaderTimeoutDisarmedAfterHeaders) { request_headers_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); Event::MockTimer* request_header_timer; EXPECT_CALL(*codec_, dispatch(_)) @@ -3863,7 +3866,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestHeaderTimeoutDisarmedAfterHeaders) TEST_F(HttpConnectionManagerImplTest, RequestHeaderTimeoutCallbackDisarmsAndReturns408) { request_headers_timeout_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); Event::MockTimer* request_header_timer; EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { @@ -3888,7 +3891,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestHeaderTimeoutCallbackDisarmsAndRetu TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationDisabledIfSetToZero) { max_stream_duration_ = std::chrono::milliseconds(0); - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { EXPECT_CALL(filter_callbacks_.connection_.dispatcher_, createTimer_).Times(0); @@ -3903,7 +3906,7 @@ TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationDisabledIfSetToZero) { TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationValidlyConfigured) { max_stream_duration_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { Event::MockTimer* duration_timer = setUpTimer(); @@ -3921,7 +3924,7 @@ TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationValidlyConfigured) { TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationCallbackResetStream) { max_stream_duration_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); Event::MockTimer* duration_timer = setUpTimer(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { @@ -3944,7 +3947,7 @@ TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationCallbackResetStream) { TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationFiredReturn408IfRequestWasNotComplete) { max_stream_duration_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); Event::MockTimer* duration_timer = setUpTimer(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance& data) -> Http::Status { @@ -3974,7 +3977,7 @@ TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationFiredReturn408IfRequestWa TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationFiredReturn504IfRequestWasFullyRead) { max_stream_duration_ = std::chrono::milliseconds(10); - setup(false, ""); + setup(); Event::MockTimer* duration_timer = setUpTimer(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance& data) -> Http::Status { @@ -4003,7 +4006,7 @@ TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationFiredReturn504IfRequestWa TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationCallbackNotCalledIfResetStreamValidly) { max_stream_duration_ = std::chrono::milliseconds(5000); - setup(false, ""); + setup(); Event::MockTimer* duration_timer = setUpTimer(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { @@ -4023,7 +4026,7 @@ TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationCallbackNotCalledIfResetS } TEST_F(HttpConnectionManagerImplTest, RejectWebSocketOnNonWebSocketRoute) { - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance& data) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); RequestHeaderMapPtr headers{new TestRequestHeaderMapImpl{{":authority", "host"}, @@ -4054,7 +4057,7 @@ TEST_F(HttpConnectionManagerImplTest, RejectWebSocketOnNonWebSocketRoute) { // Make sure for upgrades, we do not append Connection: Close when draining. TEST_F(HttpConnectionManagerImplTest, FooUpgradeDrainClose) { - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); // Store the basic request encoder during filter chain setup. auto* filter = new MockStreamFilter(); @@ -4120,7 +4123,7 @@ TEST_F(HttpConnectionManagerImplTest, FooUpgradeDrainClose) { // Make sure CONNECT requests hit the upgrade filter path. TEST_F(HttpConnectionManagerImplTest, ConnectAsUpgrade) { - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); EXPECT_CALL(filter_factory_, createUpgradeFilterChain("CONNECT", _, _, _)) .WillRepeatedly(Return(true)); @@ -4144,7 +4147,7 @@ TEST_F(HttpConnectionManagerImplTest, ConnectAsUpgrade) { } TEST_F(HttpConnectionManagerImplTest, ConnectWithEmptyPath) { - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); EXPECT_CALL(filter_factory_, createUpgradeFilterChain("CONNECT", _, _, _)) .WillRepeatedly(Return(true)); @@ -4170,7 +4173,7 @@ TEST_F(HttpConnectionManagerImplTest, ConnectWithEmptyPath) { // Regression test for https://github.com/envoyproxy/envoy/issues/10138 TEST_F(HttpConnectionManagerImplTest, DrainCloseRaceWithClose) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -4215,7 +4218,7 @@ TEST_F(HttpConnectionManagerImplTest, DrainCloseRaceWithClose) { TEST_F(HttpConnectionManagerImplTest, FilterThatWaitsForBodyCanBeCalledAfterFilterThatAddsBodyEvenIfItIsNotLast) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -4263,7 +4266,7 @@ TEST_F(HttpConnectionManagerImplTest, } TEST_F(HttpConnectionManagerImplTest, DrainClose) { - setup(true, ""); + setup(SetupOpts().setSsl(true)); MockStreamDecoderFilter* filter = new NiceMock(); EXPECT_CALL(filter_factory_, createFilterChain(_)) @@ -4319,7 +4322,7 @@ TEST_F(HttpConnectionManagerImplTest, DrainClose) { class ProxyStatusTest : public HttpConnectionManagerImplTest { public: void initialize() { - setup(/*ssl=*/false, servername_, /*tracing=*/false); + setup(SetupOpts().setServerName(servername_).setTracing(false)); setUpEncoderAndDecoder(/*request_with_data_and_trailers=*/false, /*decode_headers_stop_all=*/false); sendRequestHeadersAndData(); diff --git a/test/common/http/conn_manager_impl_test_2.cc b/test/common/http/conn_manager_impl_test_2.cc index e829a42e42ab8..fead362c33280 100644 --- a/test/common/http/conn_manager_impl_test_2.cc +++ b/test/common/http/conn_manager_impl_test_2.cc @@ -20,7 +20,7 @@ namespace Envoy { namespace Http { TEST_F(HttpConnectionManagerImplTest, ResponseBeforeRequestComplete) { - setup(false, "envoy-server-test"); + setup(); setupFilterChain(1, 0); EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, false)) @@ -44,7 +44,7 @@ TEST_F(HttpConnectionManagerImplTest, ResponseBeforeRequestComplete) { TEST_F(HttpConnectionManagerImplTest, ResponseBeforeRequestComplete10) { EXPECT_CALL(*codec_, protocol()).WillRepeatedly(Return(Protocol::Http10)); - setup(false, "envoy-server-test"); + setup(); setupFilterChain(1, 0); EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, false)) @@ -69,7 +69,7 @@ TEST_F(HttpConnectionManagerImplTest, ResponseBeforeRequestComplete10) { TEST_F(HttpConnectionManagerImplTest, ResponseBeforeRequestComplete10NoOptimize) { EXPECT_CALL(runtime_.snapshot_, getBoolean(_, _)).WillRepeatedly(Return(false)); EXPECT_CALL(*codec_, protocol()).WillRepeatedly(Return(Protocol::Http10)); - setup(false, "envoy-server-test"); + setup(); setupFilterChain(1, 0); EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, false)) @@ -94,7 +94,7 @@ TEST_F(HttpConnectionManagerImplTest, ResponseBeforeRequestComplete10NoOptimize) } TEST_F(HttpConnectionManagerImplTest, DisconnectOnProxyConnectionDisconnect) { - setup(false, "envoy-server-test"); + setup(); setupFilterChain(1, 0); @@ -120,7 +120,7 @@ TEST_F(HttpConnectionManagerImplTest, DisconnectOnProxyConnectionDisconnect) { } TEST_F(HttpConnectionManagerImplTest, ResponseStartBeforeRequestComplete) { - setup(false, ""); + setup(SetupOpts().setServerName("")); // This is like ResponseBeforeRequestComplete, but it tests the case where we start the reply // before the request completes, but don't finish the reply until after the request completes. @@ -181,7 +181,7 @@ TEST_F(HttpConnectionManagerImplTest, ResponseStartBeforeRequestComplete) { TEST_F(HttpConnectionManagerImplTest, DownstreamDisconnect) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance& data) -> Http::Status { conn_manager_->newStream(response_encoder_); @@ -201,7 +201,7 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamDisconnect) { TEST_F(HttpConnectionManagerImplTest, DownstreamProtocolError) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { conn_manager_->newStream(response_encoder_); @@ -225,7 +225,7 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamProtocolError) { TEST_F(HttpConnectionManagerImplTest, TestDownstreamProtocolErrorAccessLog) { std::shared_ptr handler(new NiceMock()); access_logs_ = {handler}; - setup(false, ""); + setup(); EXPECT_CALL(*handler, log(_, _)) .WillOnce(Invoke( @@ -246,7 +246,7 @@ TEST_F(HttpConnectionManagerImplTest, TestDownstreamProtocolErrorAccessLog) { } TEST_F(HttpConnectionManagerImplTest, TestDownstreamProtocolErrorAfterHeadersAccessLog) { - setup(false, ""); + setup(); std::shared_ptr filter(new NiceMock()); std::shared_ptr handler(new NiceMock()); @@ -289,7 +289,7 @@ TEST_F(HttpConnectionManagerImplTest, FrameFloodError) { std::shared_ptr log_handler = std::make_shared>(); access_logs_ = {log_handler}; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { conn_manager_->newStream(response_encoder_); @@ -323,7 +323,7 @@ TEST_F(HttpConnectionManagerImplTest, EnvoyOverloadError) { std::shared_ptr log_handler = std::make_shared>(); access_logs_ = {log_handler}; - setup(false, ""); + setup(); ASSERT_EQ(0U, stats_.named_.downstream_rq_overload_close_.value()); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { @@ -360,7 +360,7 @@ TEST_F(HttpConnectionManagerImplTest, IdleTimeoutNoCodec) { idle_timeout_ = (std::chrono::milliseconds(10)); Event::MockTimer* idle_timer = setUpTimer(); EXPECT_CALL(*idle_timer, enableTimer(_, _)); - setup(false, ""); + setup(); EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::FlushWrite, _)); EXPECT_CALL(*idle_timer, disableTimer()); @@ -373,7 +373,7 @@ TEST_F(HttpConnectionManagerImplTest, IdleTimeout) { idle_timeout_ = (std::chrono::milliseconds(10)); Event::MockTimer* idle_timer = setUpTimer(); EXPECT_CALL(*idle_timer, enableTimer(_, _)); - setup(false, ""); + setup(); MockStreamDecoderFilter* filter = new NiceMock(); EXPECT_CALL(filter_factory_, createFilterChain(_)) @@ -418,7 +418,7 @@ TEST_F(HttpConnectionManagerImplTest, ConnectionDurationResponseFlag) { max_connection_duration_ = (std::chrono::milliseconds(10)); Event::MockTimer* connection_duration_timer = setUpTimer(); EXPECT_CALL(*connection_duration_timer, enableTimer(_, _)); - setup(false, ""); + setup(); EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::FlushWrite, _)); filter_callbacks_.connection_.streamInfo().setResponseFlag( @@ -440,7 +440,7 @@ TEST_F(HttpConnectionManagerImplTest, ConnectionDurationNoCodec) { max_connection_duration_ = (std::chrono::milliseconds(10)); Event::MockTimer* connection_duration_timer = setUpTimer(); EXPECT_CALL(*connection_duration_timer, enableTimer(_, _)); - setup(false, ""); + setup(); EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::FlushWrite, _)); EXPECT_CALL(*connection_duration_timer, disableTimer()); @@ -454,7 +454,7 @@ TEST_F(HttpConnectionManagerImplTest, ConnectionDurationNoCodec) { TEST_F(HttpConnectionManagerImplTest, MaxRequests) { max_requests_per_connection_ = 1; codec_->protocol_ = Protocol::Http2; - setup(false, ""); + setup(); Event::MockTimer* drain_timer = setUpTimer(); EXPECT_CALL(*drain_timer, enableTimer(_, _)); @@ -483,7 +483,7 @@ TEST_F(HttpConnectionManagerImplTest, ConnectionDuration) { max_connection_duration_ = (std::chrono::milliseconds(10)); Event::MockTimer* connection_duration_timer = setUpTimer(); EXPECT_CALL(*connection_duration_timer, enableTimer(_, _)); - setup(false, ""); + setup(); MockStreamDecoderFilter* filter = new NiceMock(); EXPECT_CALL(filter_factory_, createFilterChain(_)) @@ -520,8 +520,51 @@ TEST_F(HttpConnectionManagerImplTest, ConnectionDuration) { EXPECT_EQ(1U, stats_.named_.downstream_cx_max_duration_reached_.value()); } +TEST_F(HttpConnectionManagerImplTest, ConnectionDurationSafeHttp1) { + EXPECT_CALL(*codec_, protocol()).WillRepeatedly(Return(Protocol::Http10)); + max_connection_duration_ = std::chrono::milliseconds(10); + Event::MockTimer* connection_duration_timer = setUpTimer(); + EXPECT_CALL(*connection_duration_timer, enableTimer(_, _)); + setup(SetupOpts().setHttp1SafeMaxConnectionDuration(true)); + + MockStreamDecoderFilter* filter = new NiceMock(); + EXPECT_CALL(filter_factory_, createFilterChain(_)) + .WillOnce(Invoke([&](FilterChainManager& manager) -> bool { + auto factory = createDecoderFilterFactoryCb(StreamDecoderFilterSharedPtr{filter}); + manager.applyFilterFactoryCb({}, factory); + return true; + })); + + EXPECT_CALL(*filter, decodeHeaders(_, false)) + .WillOnce(Return(FilterHeadersStatus::StopIteration)); + EXPECT_CALL(*filter, decodeData(_, true)) + .WillOnce(Return(FilterDataStatus::StopIterationNoBuffer)); + startRequest(true, "hello"); + + EXPECT_CALL(*connection_duration_timer, disableTimer()); + connection_duration_timer->invokeCallback(); + EXPECT_EQ(1U, stats_.named_.downstream_cx_http1_soft_drain_.value()); + EXPECT_EQ(1U, stats_.named_.downstream_cx_max_duration_reached_.value()); + + // Connection manager now waits to send another response, adds the Connection:close header to it, + // then closes the connection. + EXPECT_CALL(response_encoder_, encodeHeaders(_, _)) + .WillOnce(Invoke([&](const ResponseHeaderMap& headers, bool) { + // Check that the connection:close header is present. + ASSERT_NE(headers.Connection(), nullptr); + EXPECT_EQ(headers.getConnectionValue(), Headers::get().ConnectionValues.Close); + response_encoder_.stream_.codec_callbacks_->onCodecEncodeComplete(); + })); + // Expect stream & connection to close after response is sent. + expectOnDestroy(); + + ResponseHeaderMapPtr response_headers{new TestResponseHeaderMapImpl{{":status", "200"}}}; + filter->callbacks_->streamInfo().setResponseCodeDetails(""); + filter->callbacks_->encodeHeaders(std::move(response_headers), true, "details"); +} + TEST_F(HttpConnectionManagerImplTest, IntermediateBufferingEarlyResponse) { - setup(false, ""); + setup(); setupFilterChain(2, 0); @@ -554,7 +597,7 @@ TEST_F(HttpConnectionManagerImplTest, IntermediateBufferingEarlyResponse) { } TEST_F(HttpConnectionManagerImplTest, DoubleBuffering) { - setup(false, ""); + setup(); setupFilterChain(3, 0); EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, false)) @@ -589,7 +632,7 @@ TEST_F(HttpConnectionManagerImplTest, DoubleBuffering) { } TEST_F(HttpConnectionManagerImplTest, ZeroByteDataFiltering) { - setup(false, ""); + setup(); setupFilterChain(2, 0); EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, false)) @@ -621,7 +664,7 @@ TEST_F(HttpConnectionManagerImplTest, ZeroByteDataFiltering) { TEST_F(HttpConnectionManagerImplTest, FilterAddTrailersInTrailersCallback) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -708,7 +751,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterAddTrailersInTrailersCallback) { } TEST_F(HttpConnectionManagerImplTest, FilterAddTrailersInDataCallbackNoTrailers) { - setup(false, ""); + setup(); setupFilterChain(2, 2); std::string trailers_data("trailers"); @@ -788,7 +831,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterAddTrailersInDataCallbackNoTrailers) TEST_F(HttpConnectionManagerImplTest, FilterAddBodyInTrailersCallback) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -871,7 +914,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterAddBodyInTrailersCallback) { // Don't send data frames, only headers and trailers. TEST_F(HttpConnectionManagerImplTest, FilterAddBodyInTrailersCallback_NoDataFrames) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -932,7 +975,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterAddBodyInTrailersCallback_NoDataFram // Don't send data frames, only headers and trailers. TEST_F(HttpConnectionManagerImplTest, FilterAddBodyInTrailersCallback_ContinueAfterCallback) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -999,7 +1042,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterAddBodyInTrailersCallback_ContinueAf // Add*Data during the *Data callbacks. TEST_F(HttpConnectionManagerImplTest, FilterAddBodyDuringDecodeData) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -1068,7 +1111,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterAddBodyDuringDecodeData) { } TEST_F(HttpConnectionManagerImplTest, FilterAddBodyInline) { - setup(false, ""); + setup(); setupFilterChain(2, 2); EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, true)) @@ -1110,7 +1153,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterAddBodyInline) { } TEST_F(HttpConnectionManagerImplTest, BlockRouteCacheTest) { - setup(false, ""); + setup(); MockStreamDecoderFilter* filter = new NiceMock(); EXPECT_CALL(filter_factory_, createFilterChain(_)) @@ -1183,7 +1226,7 @@ TEST_F(HttpConnectionManagerImplTest, BlockRouteCacheTest) { } TEST_F(HttpConnectionManagerImplTest, Filter) { - setup(false, ""); + setup(); setupFilterChain(3, 2); const std::string fake_cluster1_name = "fake_cluster1"; @@ -1244,7 +1287,7 @@ TEST_F(HttpConnectionManagerImplTest, Filter) { // the line. Also tests that setRoute(nullptr) is equivalent to attempting route resolution and // failing to find a route. TEST_F(HttpConnectionManagerImplTest, FilterSetRouteToNullPtr) { - setup(false, ""); + setup(); setupFilterChain(2, 1); const std::string fake_cluster1_name = "fake_cluster1"; @@ -1288,7 +1331,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterSetRouteToNullPtr) { } TEST_F(HttpConnectionManagerImplTest, UpstreamWatermarkCallbacks) { - setup(false, ""); + setup(); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -1327,7 +1370,7 @@ TEST_F(HttpConnectionManagerImplTest, UpstreamWatermarkCallbacks) { } TEST_F(HttpConnectionManagerImplTest, UnderlyingConnectionWatermarksPassedOnWithLazyCreation) { - setup(false, ""); + setup(); // Make sure codec_ is created. EXPECT_CALL(*codec_, dispatch(_)); @@ -1389,7 +1432,7 @@ TEST_F(HttpConnectionManagerImplTest, UnderlyingConnectionWatermarksPassedOnWith } TEST_F(HttpConnectionManagerImplTest, UnderlyingConnectionWatermarksUnwoundWithLazyCreation) { - setup(false, ""); + setup(); // Make sure codec_ is created. EXPECT_CALL(*codec_, dispatch(_)); @@ -1454,7 +1497,7 @@ TEST_F(HttpConnectionManagerImplTest, UnderlyingConnectionWatermarksUnwoundWithL TEST_F(HttpConnectionManagerImplTest, AlterFilterWatermarkLimits) { initial_buffer_limit_ = 100; - setup(false, ""); + setup(); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -1487,7 +1530,7 @@ TEST_F(HttpConnectionManagerImplTest, HitFilterWatermarkLimits) { initial_buffer_limit_ = 1; streaming_filter_ = true; - setup(false, ""); + setup(); setUpEncoderAndDecoder(false, false); // The filter is a streaming filter. Sending 4 bytes should hit the @@ -1547,7 +1590,7 @@ TEST_F(HttpConnectionManagerImplTest, HitFilterWatermarkLimits) { TEST_F(HttpConnectionManagerImplTest, HitRequestBufferLimits) { initial_buffer_limit_ = 10; streaming_filter_ = false; - setup(false, ""); + setup(); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -1572,7 +1615,7 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamConnectionTermination) { std::shared_ptr handler(new NiceMock()); access_logs_ = {handler}; - setup(false, ""); + setup(); EXPECT_CALL(*handler, log(_, _)) .WillOnce(Invoke( [](const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo& stream_info) { @@ -1598,7 +1641,7 @@ TEST_F(HttpConnectionManagerImplTest, HitRequestBufferLimitsIntermediateFilter) { InSequence s; initial_buffer_limit_ = 10; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -1642,7 +1685,7 @@ TEST_F(HttpConnectionManagerImplTest, HitRequestBufferLimitsIntermediateFilter) TEST_F(HttpConnectionManagerImplTest, HitResponseBufferLimitsBeforeHeaders) { initial_buffer_limit_ = 10; - setup(false, ""); + setup(); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -1683,7 +1726,7 @@ TEST_F(HttpConnectionManagerImplTest, HitResponseBufferLimitsBeforeHeaders) { TEST_F(HttpConnectionManagerImplTest, HitResponseBufferLimitsAfterHeaders) { initial_buffer_limit_ = 10; - setup(false, ""); + setup(); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -1720,7 +1763,7 @@ TEST_F(HttpConnectionManagerImplTest, HitResponseBufferLimitsAfterHeaders) { TEST_F(HttpConnectionManagerImplTest, FilterHeadReply) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance& data) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -1763,7 +1806,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterHeadReply) { // up resetting the stream in the doEndStream() path (e.g., via filter reset due to timeout, etc.), // we emit a reset to the codec. TEST_F(HttpConnectionManagerImplTest, ResetWithStoppedFilter) { - setup(false, ""); + setup(); setupFilterChain(1, 1); EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, true)) @@ -1802,7 +1845,7 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamRemoteResetConnectError) { std::shared_ptr handler(new NiceMock()); access_logs_ = {handler}; - setup(false, ""); + setup(); codec_->protocol_ = Protocol::Http2; EXPECT_CALL(*handler, log(_, _)) .WillOnce(Invoke( @@ -1831,7 +1874,7 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamRemoteReset) { std::shared_ptr handler(new NiceMock()); access_logs_ = {handler}; - setup(false, ""); + setup(); codec_->protocol_ = Protocol::Http2; EXPECT_CALL(*handler, log(_, _)) .WillOnce(Invoke( @@ -1860,7 +1903,7 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamRemoteResetRefused) { std::shared_ptr handler(new NiceMock()); access_logs_ = {handler}; - setup(false, ""); + setup(); codec_->protocol_ = Protocol::Http2; EXPECT_CALL(*handler, log(_, _)) .WillOnce(Invoke( @@ -1885,7 +1928,7 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamRemoteResetRefused) { // Filter stops headers iteration without ending the stream, then injects a body later. TEST_F(HttpConnectionManagerImplTest, FilterStopIterationInjectBody) { - setup(false, ""); + setup(); setupFilterChain(2, 2); // Decode filter 0 changes end_stream to false. @@ -1931,7 +1974,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterStopIterationInjectBody) { // Filter continues headers iteration without ending the stream, then injects a body later. TEST_F(HttpConnectionManagerImplTest, FilterContinueDontEndStreamInjectBody) { - setup(false, ""); + setup(); setupFilterChain(2, 2); // Decode filter 0 changes end_stream to false. @@ -1976,7 +2019,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterContinueDontEndStreamInjectBody) { } TEST_F(HttpConnectionManagerImplTest, FilterAddBodyContinuation) { - setup(false, ""); + setup(); setupFilterChain(2, 2); EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, true)) @@ -2052,7 +2095,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterAddBodyContinuation) { // filter1->encodeData(, true) is NOT called. // TEST_F(HttpConnectionManagerImplTest, AddDataWithAllContinue) { - setup(false, ""); + setup(); setupFilterChain(3, 3); EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, true)) @@ -2145,7 +2188,7 @@ TEST_F(HttpConnectionManagerImplTest, AddDataWithAllContinue) { // filter1->encodeData(, true) is NOT called. // TEST_F(HttpConnectionManagerImplTest, AddDataWithStopAndContinue) { - setup(false, ""); + setup(); setupFilterChain(3, 3); @@ -2212,7 +2255,7 @@ TEST_F(HttpConnectionManagerImplTest, AddDataWithStopAndContinue) { // Use filter direct decode/encodeData() calls without trailers. TEST_F(HttpConnectionManagerImplTest, FilterDirectDecodeEncodeDataNoTrailers) { - setup(false, ""); + setup(); EXPECT_CALL(*route_config_provider_.route_config_, route(_, _, _, _)); setupFilterChain(2, 2); @@ -2281,7 +2324,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterDirectDecodeEncodeDataNoTrailers) { // Use filter direct decode/encodeData() calls with trailers. TEST_F(HttpConnectionManagerImplTest, FilterDirectDecodeEncodeDataTrailers) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -2377,7 +2420,7 @@ TEST_F(HttpConnectionManagerImplTest, FilterDirectDecodeEncodeDataTrailers) { TEST_F(HttpConnectionManagerImplTest, MultipleFilters) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -2492,7 +2535,7 @@ TEST_F(HttpConnectionManagerImplTest, NoNewStreamWhenOverloaded) { getState(Server::OverloadActionNames::get().StopAcceptingRequests)) .WillByDefault(ReturnRef(stop_accepting_requests)); - setup(false, ""); + setup(); EXPECT_CALL(random_, random()) .WillRepeatedly(Return(static_cast(Random::RandomGenerator::max()) * 0.5)); @@ -2518,7 +2561,7 @@ TEST_F(HttpConnectionManagerImplTest, DisableHttp1KeepAliveWhenOverloaded) { .WillByDefault(ReturnRef(disable_http_keep_alive)); codec_->protocol_ = Protocol::Http11; - setup(false, ""); + setup(); EXPECT_CALL(random_, random()) .WillRepeatedly(Return(static_cast(Random::RandomGenerator::max()) * 0.5)); @@ -2567,7 +2610,7 @@ TEST_F(HttpConnectionManagerImplTest, DisableHttp2KeepAliveWhenOverloaded) { .WillByDefault(ReturnRef(disable_http_keep_alive)); codec_->protocol_ = Protocol::Http2; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, shutdownNotice); std::shared_ptr filter(new NiceMock()); @@ -2612,7 +2655,7 @@ TEST_F(HttpConnectionManagerImplTest, CodecCreationLoadShedPointCanCloseConnecti getLoadShedPoint(Server::LoadShedPointName::get().HcmDecodeHeaders)) .WillOnce(Return(nullptr)); - setup(false, ""); + setup(); EXPECT_CALL(close_connection_creating_codec_point, shouldShedLoad()).WillOnce(Return(true)); EXPECT_CALL(filter_callbacks_.connection_, close(_, _)); @@ -2638,7 +2681,7 @@ TEST_F(HttpConnectionManagerImplTest, CodecCreationLoadShedPointBypasscheck) { getLoadShedPoint(Server::LoadShedPointName::get().HttpDownstreamFilterCheck)) .WillOnce(Return(nullptr)); - setup(false, ""); + setup(); EXPECT_CALL(close_connection_creating_codec_point, shouldShedLoad()).WillOnce(Return(false)); @@ -2668,7 +2711,7 @@ TEST_F(HttpConnectionManagerImplTest, DecodeHeaderLoadShedPointCanRejectNewStrea getLoadShedPoint(Server::LoadShedPointName::get().HttpDownstreamFilterCheck)) .WillRepeatedly(Return(nullptr)); - setup(false, ""); + setup(); setupFilterChain(1, 0); EXPECT_CALL(accept_new_stream_point, shouldShedLoad()).WillOnce(Return(true)); @@ -2702,7 +2745,7 @@ TEST_F(HttpConnectionManagerImplTest, DecodeHeaderLoadShedPointCanRejectNewStrea } TEST_F(HttpConnectionManagerImplTest, TestStopAllIterationAndBufferOnDecodingPathFirstFilter) { - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); setUpEncoderAndDecoder(true, true); // Kick off the incoming data. @@ -2726,7 +2769,7 @@ TEST_F(HttpConnectionManagerImplTest, TestStopAllIterationAndBufferOnDecodingPat } TEST_F(HttpConnectionManagerImplTest, TestStopAllIterationAndBufferOnDecodingPathSecondFilter) { - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); setUpEncoderAndDecoder(true, false); // Verify headers go through both filters, and data and trailers go through the first filter only. @@ -2751,7 +2794,7 @@ TEST_F(HttpConnectionManagerImplTest, TestStopAllIterationAndBufferOnDecodingPat } TEST_F(HttpConnectionManagerImplTest, TestStopAllIterationAndBufferOnEncodingPath) { - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -2793,7 +2836,7 @@ TEST_F(HttpConnectionManagerImplTest, TestStopAllIterationAndBufferOnEncodingPat } TEST_F(HttpConnectionManagerImplTest, DisableKeepAliveWhenDraining) { - setup(false, ""); + setup(); EXPECT_CALL(drain_close_, drainClose()).WillOnce(Return(true)); @@ -2832,7 +2875,7 @@ TEST_F(HttpConnectionManagerImplTest, DisableKeepAliveWhenDraining) { } TEST_F(HttpConnectionManagerImplTest, TestSessionTrace) { - setup(false, ""); + setup(); // Set up the codec. EXPECT_CALL(*codec_, dispatch(_)) @@ -2899,7 +2942,7 @@ TEST_F(HttpConnectionManagerImplTest, TestSessionTrace) { // SRDS no scope found. TEST_F(HttpConnectionManagerImplTest, TestSrdsRouteNotFound) { - setup(false, "", true, true); + setup(SetupOpts().setUseSrds(true)); setupFilterChain(1, 0); // Recreate the chain for second stream. EXPECT_CALL(*static_cast(scopeKeyBuilder().ptr()), @@ -2935,7 +2978,7 @@ TEST_F(HttpConnectionManagerImplTest, TestSrdsRouteNotFound) { // SRDS updating scopes affects routing. TEST_F(HttpConnectionManagerImplTest, TestSrdsUpdate) { - setup(false, "", true, true); + setup(SetupOpts().setUseSrds(true)); EXPECT_CALL(*static_cast(scopeKeyBuilder().ptr()), computeScopeKey(_)) @@ -2990,7 +3033,7 @@ TEST_F(HttpConnectionManagerImplTest, TestSrdsUpdate) { // SRDS Scope header update cause cross-scope reroute. TEST_F(HttpConnectionManagerImplTest, TestSrdsCrossScopeReroute) { - setup(false, "", true, true); + setup(SetupOpts().setUseSrds(true)); std::shared_ptr route_config1 = std::make_shared>(); @@ -3063,7 +3106,7 @@ TEST_F(HttpConnectionManagerImplTest, TestSrdsCrossScopeReroute) { // SRDS scoped RouteConfiguration found and route found. TEST_F(HttpConnectionManagerImplTest, TestSrdsRouteFound) { - setup(false, "", true, true); + setup(SetupOpts().setUseSrds(true)); setupFilterChain(1, 0); const std::string fake_cluster1_name = "fake_cluster1"; @@ -3109,7 +3152,7 @@ TEST_F(HttpConnectionManagerImplTest, TestSrdsRouteFound) { } TEST_F(HttpConnectionManagerImplTest, NewConnection) { - setup(false, "", true, true); + setup(SetupOpts().setUseSrds(true)); filter_callbacks_.connection_.stream_info_.protocol_ = absl::nullopt; EXPECT_CALL(filter_callbacks_.connection_.stream_info_, protocol()); @@ -3127,7 +3170,7 @@ TEST_F(HttpConnectionManagerImplTest, NewConnection) { } TEST_F(HttpConnectionManagerImplTest, HeaderOnlyRequestAndResponseUsingHttp3) { - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); filter_callbacks_.connection_.stream_info_.protocol_ = Envoy::Http::Protocol::Http3; codec_->protocol_ = Http::Protocol::Http3; @@ -3195,7 +3238,7 @@ TEST_F(HttpConnectionManagerImplTest, ConnectionFilterState) { "connection_provided_data", std::make_shared(555), StreamInfo::FilterState::StateType::ReadOnly); - setup(false, "envoy-custom-server", false); + setup(SetupOpts().setTracing(false)); setupFilterChain(1, 0, /* num_requests = */ 3); EXPECT_CALL(*codec_, dispatch(_)) @@ -3299,7 +3342,7 @@ class HttpConnectionManagerImplDeathTest : public HttpConnectionManagerImplTest // HCM config can only have either RouteConfigProvider or ScopedRoutesConfigProvider. TEST_F(HttpConnectionManagerImplDeathTest, InvalidConnectionManagerConfig) { - setup(false, ""); + setup(); Buffer::OwnedImpl fake_input("1234"); EXPECT_CALL(*codec_, dispatch(_)).WillRepeatedly(Invoke([&](Buffer::Instance&) -> Http::Status { @@ -3337,7 +3380,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestRejectedViaIPDetection) { use_remote_address_ = false; - setup(false, ""); + setup(); // 403 direct response when IP detection fails. EXPECT_CALL(response_encoder_, encodeHeaders(_, false)) @@ -3354,7 +3397,7 @@ TEST_F(HttpConnectionManagerImplTest, RequestRejectedViaIPDetection) { } TEST_F(HttpConnectionManagerImplTest, DisconnectDuringEncodeHeader) { - setup(false, "envoy-server-test"); + setup(); setupFilterChain(1, 0); EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, true)) @@ -3376,7 +3419,7 @@ TEST_F(HttpConnectionManagerImplTest, DisconnectDuringEncodeHeader) { } TEST_F(HttpConnectionManagerImplTest, DisconnectDuringEncodeBody) { - setup(false, "envoy-server-test"); + setup(); setupFilterChain(1, 0); EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, true)) @@ -3405,7 +3448,7 @@ TEST_F(HttpConnectionManagerImplTest, DisconnectDuringEncodeBody) { // Verify that trailers added during a data encoding continuation are not double continued. TEST_F(HttpConnectionManagerImplTest, AddTrailersDuringdDecodingContinue) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -3449,7 +3492,7 @@ TEST_F(HttpConnectionManagerImplTest, AddTrailersDuringdDecodingContinue) { // Verify that trailers added during a data decoding continuation are not double continued. TEST_F(HttpConnectionManagerImplTest, AddTrailersDuringEncodingContinue) { InSequence s; - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); @@ -3489,7 +3532,7 @@ TEST_F(HttpConnectionManagerImplTest, AddTrailersDuringEncodingContinue) { } TEST_F(HttpConnectionManagerImplTest, DisconnectDuringEncodeTrailer) { - setup(false, "envoy-server-test"); + setup(); setupFilterChain(1, 0); EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, true)) @@ -3520,7 +3563,7 @@ TEST_F(HttpConnectionManagerImplTest, DisconnectDuringEncodeTrailer) { TEST_F(HttpConnectionManagerImplTest, DirectLocalReplyCausesDisconnect) { initial_buffer_limit_ = 10; - setup(false, ""); + setup(); setUpEncoderAndDecoder(false, false); sendRequestHeadersAndData(); @@ -3563,7 +3606,7 @@ TEST_F(HttpConnectionManagerImplTest, DirectLocalReplyCausesDisconnect) { // Header validator rejects header map for HTTP/1.x protocols TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRejectHttp1) { - setup(false, ""); + setup(); expectUhvHeaderCheck(HeaderValidator::ValidationResult( HeaderValidator::ValidationResult::Action::Reject, "bad_header_map"), ServerHeaderValidator::RequestHeadersTransformationResult::success()); @@ -3609,7 +3652,7 @@ TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRejectHttp1) { // Header validator rejects header map for HTTP/2 protocols TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRejectHttp2) { codec_->protocol_ = Protocol::Http2; - setup(false, ""); + setup(); expectUhvHeaderCheck(HeaderValidator::ValidationResult( HeaderValidator::ValidationResult::Action::Reject, "bad_header_map"), ServerHeaderValidator::RequestHeadersTransformationResult::success()); @@ -3638,7 +3681,7 @@ TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRejectHttp2) { // Header validator rejects gRPC request TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRejectGrpcRequest) { codec_->protocol_ = Protocol::Http2; - setup(false, ""); + setup(); expectUhvHeaderCheck(HeaderValidator::ValidationResult( HeaderValidator::ValidationResult::Action::Reject, "bad_header_map"), ServerHeaderValidator::RequestHeadersTransformationResult::success()); @@ -3668,7 +3711,7 @@ TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRejectGrpcRequest) { // Header validator redirects TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRedirect) { - setup(false, ""); + setup(); expectUhvHeaderCheck( HeaderValidator::ValidationResult::success(), ServerHeaderValidator::RequestHeadersTransformationResult( @@ -3698,7 +3741,7 @@ TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRedirect) { // Header validator redirects gRPC request TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRedirectGrpcRequest) { codec_->protocol_ = Protocol::Http2; - setup(false, ""); + setup(); expectUhvHeaderCheck( HeaderValidator::ValidationResult::success(), ServerHeaderValidator::RequestHeadersTransformationResult( @@ -3731,7 +3774,7 @@ TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRedirectGrpcRequest) { // Header validator rejects trailer map before response has started TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRejectTrailersBeforeResponseHttp1) { codec_->protocol_ = Protocol::Http11; - setup(false, ""); + setup(); expectUhvTrailerCheck(HeaderValidator::ValidationResult( HeaderValidator::ValidationResult::Action::Reject, "bad_trailer_map"), HeaderValidator::TransformationResult::success()); @@ -3760,7 +3803,7 @@ TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRejectTrailersBeforeRespons TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRejectTrailersBeforeResponseHttp2) { codec_->protocol_ = Protocol::Http2; - setup(false, ""); + setup(); expectUhvTrailerCheck(HeaderValidator::ValidationResult( HeaderValidator::ValidationResult::Action::Reject, "bad_trailer_map"), HeaderValidator::TransformationResult::success(), false); @@ -3785,7 +3828,7 @@ TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRejectTrailersBeforeRespons TEST_F(HttpConnectionManagerImplTest, HeaderValidatorFailTrailersTransformationBeforeResponse) { codec_->protocol_ = Protocol::Http11; - setup(false, ""); + setup(); expectUhvTrailerCheck(HeaderValidator::ValidationResult( HeaderValidator::ValidationResult::Action::Reject, "bad_trailer_map"), HeaderValidator::TransformationResult::success()); @@ -3815,7 +3858,7 @@ TEST_F(HttpConnectionManagerImplTest, HeaderValidatorFailTrailersTransformationB // Header validator rejects trailer map after response has started TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRejectTrailersAfterResponse) { codec_->protocol_ = Protocol::Http2; - setup(false, ""); + setup(); setupFilterChain(1, 0, 1); expectUhvTrailerCheck(HeaderValidator::ValidationResult( HeaderValidator::ValidationResult::Action::Reject, "bad_trailer_map"), @@ -3855,7 +3898,7 @@ TEST_F(HttpConnectionManagerImplTest, HeaderValidatorRejectTrailersAfterResponse // Request completes normally if header validator accepts it TEST_F(HttpConnectionManagerImplTest, HeaderValidatorAccept) { - setup(false, ""); + setup(); expectUhvHeaderCheck(HeaderValidator::ValidationResult::success(), ServerHeaderValidator::RequestHeadersTransformationResult::success()); @@ -3908,7 +3951,7 @@ TEST_F(HttpConnectionManagerImplTest, HeaderValidatorAccept) { TEST_F(HttpConnectionManagerImplTest, NoProxyProtocolAdded) { add_proxy_protocol_connection_state_ = false; - setup(false, "server_name"); + setup(); Buffer::OwnedImpl fake_input("input"); conn_manager_->createCodec(fake_input); @@ -3927,7 +3970,7 @@ TEST_F(HttpConnectionManagerImplTest, LimitWorkPerIOCycle) { EXPECT_CALL(runtime_.snapshot_, getInteger(_, _)).WillRepeatedly(ReturnArg<1>()); // Process 1 request per I/O cycle auto* deferred_request_callback = enableStreamsPerIoLimit(1); - setup(false, ""); + setup(); // Store the basic request encoder during filter chain setup. std::vector> decoder_filters; @@ -4069,7 +4112,7 @@ TEST_F(HttpConnectionManagerImplTest, StreamDeferralPreservesOrder) { EXPECT_CALL(runtime_.snapshot_, getInteger(_, _)).WillRepeatedly(ReturnArg<1>()); // Process 1 request per I/O cycle auto* deferred_request_callback = enableStreamsPerIoLimit(1); - setup(false, ""); + setup(); std::vector> encoder_filters; int expected_request_id = 0; @@ -4179,7 +4222,7 @@ TEST_F(HttpConnectionManagerImplTest, StreamDeferralPreservesOrder) { } TEST_F(HttpConnectionManagerImplTest, DownstreamTimingsRecordWhenRequestHeaderProcessingIsDone) { - setup(/*ssl=*/true, /*server_name=*/"", /*tracing=*/false); + setup(SetupOpts().setSsl(true).setTracing(false)); // Set up the codec. Buffer::OwnedImpl fake_input("input"); @@ -4211,7 +4254,7 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamTimingsRecordWhenRequestHeaderPr } TEST_F(HttpConnectionManagerImplTest, PassMatchUpstreamSchemeHintToStreamInfo) { - setup(/*ssl=*/false, /*server_name=*/"", /*tracing=*/false); + setup(SetupOpts().setTracing(false)); scheme_match_upstream_ = true; // Store the basic request encoder during filter chain setup. diff --git a/test/common/http/conn_manager_impl_test_base.cc b/test/common/http/conn_manager_impl_test_base.cc index 4294d29929552..32cca79b165f9 100644 --- a/test/common/http/conn_manager_impl_test_base.cc +++ b/test/common/http/conn_manager_impl_test_base.cc @@ -53,6 +53,9 @@ class ConnectionManagerConfigProxyObject : public ConnectionManagerConfig { absl::optional maxConnectionDuration() const override { return parent_.maxConnectionDuration(); } + bool http1SafeMaxConnectionDuration() const override { + return parent_.http1SafeMaxConnectionDuration(); + } uint32_t maxRequestHeadersKb() const override { return parent_.maxRequestHeadersKb(); } uint32_t maxRequestHeadersCount() const override { return parent_.maxRequestHeadersCount(); } std::chrono::milliseconds streamIdleTimeout() const override { @@ -183,14 +186,14 @@ HttpConnectionManagerImplMixin::requestHeaderCustomTag(const std::string& header return std::make_shared(header, headerTag); } -void HttpConnectionManagerImplMixin::setup(bool ssl, const std::string& server_name, bool tracing, - bool use_srds) { - use_srds_ = use_srds; - if (ssl) { +void HttpConnectionManagerImplMixin::setup(const SetupOpts& opts) { + use_srds_ = opts.use_srds_; + http1_safe_max_connection_duration_ = opts.http1_safe_max_connection_duration_; + if (opts.ssl_) { ssl_connection_ = std::make_shared(); } - server_name_ = server_name; + server_name_ = opts.server_name_; ON_CALL(filter_callbacks_.connection_, ssl()).WillByDefault(Return(ssl_connection_)); ON_CALL(Const(filter_callbacks_.connection_), ssl()).WillByDefault(Return(ssl_connection_)); ON_CALL(filter_callbacks_.connection_.dispatcher_, createScaledTypedTimer_) @@ -214,7 +217,7 @@ void HttpConnectionManagerImplMixin::setup(bool ssl, const std::string& server_n conn_manager_->initializeReadFilterCallbacks(filter_callbacks_); - if (tracing) { + if (opts.tracing_) { envoy::type::v3::FractionalPercent percent1; percent1.set_numerator(100); envoy::type::v3::FractionalPercent percent2; @@ -421,7 +424,7 @@ void HttpConnectionManagerImplMixin::doRemoteClose(bool deferred) { void HttpConnectionManagerImplMixin::testPathNormalization( const RequestHeaderMap& request_headers, const ResponseHeaderMap& expected_response) { - setup(false, ""); + setup(); EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance& data) -> Http::Status { decoder_ = &conn_manager_->newStream(response_encoder_); diff --git a/test/common/http/conn_manager_impl_test_base.h b/test/common/http/conn_manager_impl_test_base.h index 5e24d36a926f5..4be397ffb5789 100644 --- a/test/common/http/conn_manager_impl_test_base.h +++ b/test/common/http/conn_manager_impl_test_base.h @@ -29,6 +29,38 @@ using testing::NiceMock; namespace Envoy { namespace Http { +struct SetupOpts { + SetupOpts& setSsl(bool ssl) { + ssl_ = ssl; + return *this; + } + + SetupOpts& setServerName(absl::string_view server_name) { + server_name_ = server_name; + return *this; + } + + SetupOpts& setTracing(bool tracing) { + tracing_ = tracing; + return *this; + } + + SetupOpts& setUseSrds(bool use_srds) { + use_srds_ = use_srds; + return *this; + } + + SetupOpts& setHttp1SafeMaxConnectionDuration(bool http1_safe_max_connection_duration) { + http1_safe_max_connection_duration_ = http1_safe_max_connection_duration; + return *this; + } + + bool ssl_{false}; + std::string server_name_{"envoy-server-test"}; + bool tracing_{true}; + bool use_srds_{false}; + bool http1_safe_max_connection_duration_{false}; +}; // Base class for HttpConnectionManagerImpl related tests. This base class is used by tests under // common/http as well as test/extensions/filters/http/ext_proc/, to reuse the many mocks/default // impls of ConnectionManagerConfig that we need to provide to HttpConnectionManagerImpl. @@ -37,7 +69,7 @@ class HttpConnectionManagerImplMixin : public ConnectionManagerConfig { HttpConnectionManagerImplMixin(); ~HttpConnectionManagerImplMixin() override; Tracing::CustomTagConstSharedPtr requestHeaderCustomTag(const std::string& header); - void setup(bool ssl, const std::string& server_name, bool tracing = true, bool use_srds = false); + void setup(const SetupOpts& opts = {}); void setupFilterChain(int num_decoder_filters, int num_encoder_filters, int num_requests = 1); void setUpBufferLimits(); @@ -86,6 +118,9 @@ class HttpConnectionManagerImplMixin : public ConnectionManagerConfig { absl::optional maxConnectionDuration() const override { return max_connection_duration_; } + bool http1SafeMaxConnectionDuration() const override { + return http1_safe_max_connection_duration_; + } std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } std::chrono::milliseconds requestTimeout() const override { return request_timeout_; } std::chrono::milliseconds requestHeadersTimeout() const override { @@ -248,6 +283,7 @@ class HttpConnectionManagerImplMixin : public ConnectionManagerConfig { uint64_t max_requests_per_connection_{}; absl::optional idle_timeout_; absl::optional max_connection_duration_; + bool http1_safe_max_connection_duration_{false}; std::chrono::milliseconds stream_idle_timeout_{}; std::chrono::milliseconds request_timeout_{}; std::chrono::milliseconds request_headers_timeout_{}; diff --git a/test/extensions/filters/http/ext_proc/filter_test.cc b/test/extensions/filters/http/ext_proc/filter_test.cc index 15c3023c90c74..6fd45bff4495c 100644 --- a/test/extensions/filters/http/ext_proc/filter_test.cc +++ b/test/extensions/filters/http/ext_proc/filter_test.cc @@ -4261,7 +4261,7 @@ TEST_F(HttpFilter2Test, LastDecodeDataCallExceedsStreamBufferLimitWouldJustRaise envoy_grpc: cluster_name: "ext_proc_server" )EOF"); - HttpConnectionManagerImplMixin::setup(false, "fake-server"); + HttpConnectionManagerImplMixin::setup(Envoy::Http::SetupOpts().setServerName("fake-server")); HttpConnectionManagerImplMixin::initial_buffer_limit_ = 10; HttpConnectionManagerImplMixin::setUpBufferLimits(); @@ -4352,7 +4352,7 @@ TEST_F(HttpFilter2Test, LastEncodeDataCallExceedsStreamBufferLimitWouldJustRaise response_trailer_mode: "SKIP" )EOF"); - HttpConnectionManagerImplMixin::setup(false, "fake-server"); + HttpConnectionManagerImplMixin::setup(Envoy::Http::SetupOpts().setServerName("fake-server")); HttpConnectionManagerImplMixin::initial_buffer_limit_ = 10; HttpConnectionManagerImplMixin::setUpBufferLimits(); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 261ddd3c430ef..f45f177a5b2e5 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -2858,6 +2858,69 @@ TEST_P(ProtocolIntegrationTest, TestDownstreamResetIdleTimeout) { EXPECT_THAT(waitForAccessLog(access_log_name_), Not(HasSubstr("DPE"))); } +// Test that with http1_safe_max_connection_duration set to true, drain_timeout is not used for +// http1 connections after max_connection_duration is reached. Instead, envoy waits for the next +// request, adds connection:close to the response headers, then closes the connection after the +// stream ends. +TEST_P(ProtocolIntegrationTest, Http1SafeConnDurationTimeout) { + config_helper_.setDownstreamMaxConnectionDuration(std::chrono::milliseconds(500)); + config_helper_.addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { + hcm.mutable_drain_timeout()->set_nanos(1'000'000 /*=1ms*/); + hcm.set_http1_safe_max_connection_duration(true); + }); + initialize(); + + codec_client_ = makeRawHttpConnection(makeClientConnection(lookupPort("http")), absl::nullopt); + + auto response = codec_client_->makeRequestWithBody(default_request_headers_, 1024); + waitForNextUpstreamRequest(); + + upstream_request_->encodeHeaders(default_response_headers_, true); + ASSERT_TRUE(response->waitForEndStream()); + + EXPECT_TRUE(upstream_request_->complete()); + EXPECT_TRUE(response->complete()); + test_server_->waitForCounterGe("cluster.cluster_0.upstream_cx_total", 1); + test_server_->waitForCounterGe("cluster.cluster_0.upstream_rq_200", 1); + + if (downstream_protocol_ != Http::CodecType::HTTP1) { + ASSERT_TRUE(codec_client_->waitForDisconnect(std::chrono::milliseconds(10000))); + test_server_->waitForCounterGe("http.config_test.downstream_cx_max_duration_reached", 1); + EXPECT_EQ(test_server_->gauge("http.config_test.downstream_cx_http1_soft_drain")->value(), 0); + // The rest of the test is only for http1. + return; + } + + // Wait until after the max connection duration + test_server_->waitForCounterGe("http.config_test.downstream_cx_max_duration_reached", 1); + test_server_->waitForGaugeGe("http.config_test.downstream_cx_http1_soft_drain", 1); + + // Envoy now waits for one more request/response over this connection before sending the + // connection:close header and closing the connection. No matter how long the request/response + // takes, envoy will not close the connection until it's able to send the connection:close header + // downstream in a response. + // + // Sleeping for longer than the drain phase duration just to show it is no longer relevant. + absl::SleepFor(absl::Seconds(1)); + + auto soft_drain_response = codec_client_->makeRequestWithBody(default_request_headers_, 1024); + waitForNextUpstreamRequest(); + + upstream_request_->encodeHeaders(default_response_headers_, true); + ASSERT_TRUE(soft_drain_response->waitForEndStream()); + // Envoy will close the connection after the response has been sent. + ASSERT_TRUE(codec_client_->waitForDisconnect(std::chrono::milliseconds(10000))); + + EXPECT_TRUE(upstream_request_->complete()); + EXPECT_TRUE(soft_drain_response->complete()); + + // The client must have been notified that the connection will be closed. + EXPECT_EQ(soft_drain_response->headers().getConnectionValue(), + Http::Headers::get().ConnectionValues.Close); +} + // Test connection is closed after single request processed. TEST_P(ProtocolIntegrationTest, ConnDurationTimeoutBasic) { config_helper_.setDownstreamMaxConnectionDuration(std::chrono::milliseconds(500)); diff --git a/test/mocks/http/mocks.h b/test/mocks/http/mocks.h index a09d7f283a01b..351db788e5779 100644 --- a/test/mocks/http/mocks.h +++ b/test/mocks/http/mocks.h @@ -638,6 +638,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_METHOD(absl::optional, idleTimeout, (), (const)); MOCK_METHOD(bool, isRoutable, (), (const)); MOCK_METHOD(absl::optional, maxConnectionDuration, (), (const)); + MOCK_METHOD(bool, http1SafeMaxConnectionDuration, (), (const)); MOCK_METHOD(absl::optional, maxStreamDuration, (), (const)); MOCK_METHOD(std::chrono::milliseconds, streamIdleTimeout, (), (const)); MOCK_METHOD(std::chrono::milliseconds, requestTimeout, (), (const));