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 a4c115c68da0e..c2254c4c117af 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: 41] +// [#next-free-field: 42] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager"; @@ -360,6 +360,14 @@ message HttpConnectionManager { google.protobuf.Duration request_timeout = 28 [(udpa.annotations.security).configure_for_untrusted_downstream = true]; + // The amount of time that Envoy will wait for the request headers to be received. The timer is + // activated when the first byte of the headers is received, and is disarmed when the last byte of + // the headers has been received. If not specified or set to 0, this timeout is disabled. + google.protobuf.Duration request_headers_timeout = 41 [ + (validate.rules).duration = {gte {}}, + (udpa.annotations.security).configure_for_untrusted_downstream = true + ]; + // The time that Envoy will wait between sending an HTTP/2 “shutdown // notification” (GOAWAY frame with max stream ID) and a final GOAWAY frame. // This is used so that Envoy provides a grace period for new streams that diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index ceb7f4a65a1fa..a44d35f86ae24 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -36,7 +36,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 41] +// [#next-free-field: 42] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"; @@ -359,6 +359,14 @@ message HttpConnectionManager { google.protobuf.Duration request_timeout = 28 [(udpa.annotations.security).configure_for_untrusted_downstream = true]; + // The amount of time that Envoy will wait for the request headers to be received. The timer is + // activated when the first byte of the headers is received, and is disarmed when the last byte of + // the headers has been received. If not specified or set to 0, this timeout is disabled. + google.protobuf.Duration request_headers_timeout = 41 [ + (validate.rules).duration = {gte {}}, + (udpa.annotations.security).configure_for_untrusted_downstream = true + ]; + // The time that Envoy will wait between sending an HTTP/2 “shutdown // notification” (GOAWAY frame with max stream ID) and a final GOAWAY frame. // This is used so that Envoy provides a grace period for new streams that diff --git a/docs/protodoc_manifest.yaml b/docs/protodoc_manifest.yaml index 2e2afff3264da..ecdf19115a349 100644 --- a/docs/protodoc_manifest.yaml +++ b/docs/protodoc_manifest.yaml @@ -42,6 +42,9 @@ fields: envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_idle_timeout: edge_config: example: 300s # 5 mins + envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.request_headers_timeout: + edge_config: + example: 10s # 10 seconds envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.request_timeout: edge_config: note: > diff --git a/docs/root/faq/configuration/timeouts.rst b/docs/root/faq/configuration/timeouts.rst index bdf42877c33bd..2b44ce1353465 100644 --- a/docs/root/faq/configuration/timeouts.rst +++ b/docs/root/faq/configuration/timeouts.rst @@ -50,6 +50,12 @@ context request/stream is interchangeable. This timeout is not enforced by default as it is not compatible with streaming requests (requests that never end). See the stream idle timeout that follows. However, if using the :ref:`buffer filter `, it is recommended to configure this timeout. +* The HTTP connection manager :ref:`request_headers_timeout + ` + determines the amount of time the client has to send *only the headers* on the request stream + before the stream is cancelled. This can be used to prevent clients from consuming too much + memory by creating large numbers of mostly-idle streams waiting for headers. The request header + timeout is disabled by default. * The HTTP connection manager :ref:`stream_idle_timeout ` is the amount of time that the connection manager will allow a stream to exist with no upstream @@ -120,4 +126,4 @@ Transport Socket * The :ref:`transport_socket_connect_timeout ` specifies the amount of time Envoy will wait for a downstream client to complete transport-level negotiations. When configured on a filter chain with a TLS or ALTS transport socket, this limits - the amount of time allowed to finish the encrypted handshake after establishing a TCP connection. \ No newline at end of file + the amount of time allowed to finish the encrypted handshake after establishing a TCP connection. diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index a33ae97a46b0a..f0a67bfc838ae 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -39,6 +39,7 @@ New Features * grpc: implemented header value syntax support when defining :ref:`initial metadata ` for gRPC-based `ext_authz` :ref:`HTTP ` and :ref:`network ` filters, and :ref:`ratelimit ` filters. * hds: added support for delta updates in the :ref:`HealthCheckSpecifier `, making only the Endpoints and Health Checkers that changed be reconstructed on receiving a new message, rather than the entire HDS. * health_check: added option to use :ref:`no_traffic_healthy_interval ` which allows a different no traffic interval when the host is healthy. +* http: added HCM :ref:`timeout config field ` to control how long a downstream has to finish sending headers before the stream is cancelled. * http: added frame flood and abuse checks to the upstream HTTP/2 codec. This check is off by default and can be enabled by setting the `envoy.reloadable_features.upstream_http2_flood_checks` runtime key to true. * listener: added an optional :ref:`default filter chain `. If this field is supplied, and none of the :ref:`filter_chains ` matches, this default filter chain is used to serve the connection. * lua: added `downstreamDirectRemoteAddress()` and `downstreamLocalAddress()` APIs to :ref:`streamInfo() `. diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index d26ce2ffee96a..250c91077fa13 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/generated_api_shadow/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: 41] +// [#next-free-field: 42] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager"; @@ -362,6 +362,14 @@ message HttpConnectionManager { google.protobuf.Duration request_timeout = 28 [(udpa.annotations.security).configure_for_untrusted_downstream = true]; + // The amount of time that Envoy will wait for the request headers to be received. The timer is + // activated when the first byte of the headers is received, and is disarmed when the last byte of + // the headers has been received. If not specified or set to 0, this timeout is disabled. + google.protobuf.Duration request_headers_timeout = 41 [ + (validate.rules).duration = {gte {}}, + (udpa.annotations.security).configure_for_untrusted_downstream = true + ]; + // The time that Envoy will wait between sending an HTTP/2 “shutdown // notification” (GOAWAY frame with max stream ID) and a final GOAWAY frame. // This is used so that Envoy provides a grace period for new streams that diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index ceb7f4a65a1fa..a44d35f86ae24 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -36,7 +36,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 41] +// [#next-free-field: 42] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"; @@ -359,6 +359,14 @@ message HttpConnectionManager { google.protobuf.Duration request_timeout = 28 [(udpa.annotations.security).configure_for_untrusted_downstream = true]; + // The amount of time that Envoy will wait for the request headers to be received. The timer is + // activated when the first byte of the headers is received, and is disarmed when the last byte of + // the headers has been received. If not specified or set to 0, this timeout is disabled. + google.protobuf.Duration request_headers_timeout = 41 [ + (validate.rules).duration = {gte {}}, + (udpa.annotations.security).configure_for_untrusted_downstream = true + ]; + // The time that Envoy will wait between sending an HTTP/2 “shutdown // notification” (GOAWAY frame with max stream ID) and a final GOAWAY frame. // This is used so that Envoy provides a grace period for new streams that diff --git a/include/envoy/stream_info/stream_info.h b/include/envoy/stream_info/stream_info.h index 858ff0f0952f3..f6f4e6694a4c6 100644 --- a/include/envoy/stream_info/stream_info.h +++ b/include/envoy/stream_info/stream_info.h @@ -115,6 +115,8 @@ struct ResponseCodeDetailValues { const std::string MaxDurationTimeout = "max_duration_timeout"; // The per-stream total request timeout was exceeded. const std::string RequestOverallTimeout = "request_overall_timeout"; + // The per-stream request header timeout was exceeded. + const std::string RequestHeaderTimeout = "request_header_timeout"; // The request was rejected due to the Overload Manager reaching configured resource limits. const std::string Overload = "overload"; // The HTTP/1.0 or HTTP/0.9 request was rejected due to HTTP/1.0 support not being configured. @@ -168,6 +170,8 @@ struct ResponseCodeDetailValues { const std::string DownstreamLocalDisconnect = "downstream_local_disconnect"; // The max connection duration was exceeded. const std::string DurationTimeout = "duration_timeout"; + // The max request downstream header duration was exceeded. + const std::string DownstreamHeaderTimeout = "downstream_header_timeout"; // The response was generated by the admin filter. const std::string AdminFilterResponse = "admin_filter_response"; // The original stream was replaced with an internal redirect. diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index b67afc95a64c7..12a7885273e1c 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -58,6 +58,7 @@ namespace Http { COUNTER(downstream_rq_response_before_rq_complete) \ COUNTER(downstream_rq_rx_reset) \ COUNTER(downstream_rq_timeout) \ + COUNTER(downstream_rq_header_timeout) \ COUNTER(downstream_rq_too_large) \ COUNTER(downstream_rq_total) \ COUNTER(downstream_rq_tx_reset) \ @@ -289,6 +290,12 @@ class ConnectionManagerConfig { */ virtual std::chrono::milliseconds requestTimeout() const PURE; + /** + * @return request header timeout for incoming connection manager connections. Zero indicates a + * disabled request header timeout. + */ + virtual std::chrono::milliseconds requestHeadersTimeout() const PURE; + /** * @return delayed close timeout for downstream HTTP connections. Zero indicates a disabled * timeout. See http_connection_manager.proto for a detailed description of this timeout. diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index ae40a06b0daa1..4589054b1c11d 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -227,6 +227,10 @@ void ConnectionManagerImpl::doDeferredStreamDestroy(ActiveStream& stream) { stream.stream_idle_timer_ = nullptr; } stream.filter_manager_.disarmRequestTimeout(); + if (stream.request_header_timer_ != nullptr) { + stream.request_header_timer_->disableTimer(); + stream.request_header_timer_ = nullptr; + } stream.completeRequest(); stream.filter_manager_.onStreamComplete(); @@ -639,10 +643,19 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect } if (connection_manager_.config_.requestTimeout().count()) { - std::chrono::milliseconds request_timeout_ms_ = connection_manager_.config_.requestTimeout(); + std::chrono::milliseconds request_timeout = connection_manager_.config_.requestTimeout(); request_timer_ = connection_manager.read_callbacks_->connection().dispatcher().createTimer( [this]() -> void { onRequestTimeout(); }); - request_timer_->enableTimer(request_timeout_ms_, this); + request_timer_->enableTimer(request_timeout, this); + } + + if (connection_manager_.config_.requestHeadersTimeout().count()) { + std::chrono::milliseconds request_headers_timeout = + connection_manager_.config_.requestHeadersTimeout(); + request_header_timer_ = + connection_manager.read_callbacks_->connection().dispatcher().createTimer( + [this]() -> void { onRequestHeaderTimeout(); }); + request_header_timer_->enableTimer(request_headers_timeout, this); } const auto max_stream_duration = connection_manager_.config_.maxStreamDuration(); @@ -736,6 +749,14 @@ void ConnectionManagerImpl::ActiveStream::onRequestTimeout() { StreamInfo::ResponseCodeDetails::get().RequestOverallTimeout); } +void ConnectionManagerImpl::ActiveStream::onRequestHeaderTimeout() { + connection_manager_.stats_.named_.downstream_rq_header_timeout_.inc(); + sendLocalReply(request_headers_ != nullptr && + Grpc::Common::isGrpcRequestHeaders(*request_headers_), + Http::Code::RequestTimeout, "request header timeout", nullptr, absl::nullopt, + StreamInfo::ResponseCodeDetails::get().RequestHeaderTimeout); +} + void ConnectionManagerImpl::ActiveStream::onStreamMaxDurationReached() { ENVOY_STREAM_LOG(debug, "Stream max duration time reached", *this); connection_manager_.stats_.named_.downstream_rq_max_duration_reached_.inc(); @@ -819,6 +840,10 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he connection_manager_.read_callbacks_->connection().dispatcher()); request_headers_ = std::move(headers); filter_manager_.requestHeadersInitialized(); + if (request_header_timer_ != nullptr) { + request_header_timer_->disableTimer(); + request_header_timer_.reset(); + } Upstream::HostDescriptionConstSharedPtr upstream_host = connection_manager_.read_callbacks_->upstreamHost(); diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index 09c66089b7fea..ebc4e59a35632 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -314,6 +314,8 @@ class ConnectionManagerImpl : Logger::Loggable, void onIdleTimeout(); // Per-stream request timeout callback. void onRequestTimeout(); + // Per-stream request header timeout callback. + void onRequestHeaderTimeout(); // Per-stream alive duration reached. void onStreamMaxDurationReached(); bool hasCachedRoute() { return cached_route_.has_value() && cached_route_.value(); } @@ -354,11 +356,18 @@ class ConnectionManagerImpl : Logger::Loggable, Tracing::SpanPtr active_span_; ResponseEncoder* response_encoder_{}; Stats::TimespanPtr request_response_timespan_; - // Per-stream idle timeout. + // Per-stream idle timeout. This timer gets reset whenever activity occurs on the stream, and, + // when triggered, will close the stream. Event::TimerPtr stream_idle_timer_; - // Per-stream request timeout. + // Per-stream request timeout. This timer is enabled when the stream is created and disabled + // when the stream ends. If triggered, it will close the stream. Event::TimerPtr request_timer_; - // Per-stream alive duration. + // Per-stream request header timeout. This timer is enabled when the stream is created and + // disabled when the downstream finishes sending headers. If triggered, it will close the + // stream. + Event::TimerPtr request_header_timer_; + // Per-stream alive duration. This timer is enabled once when the stream is created and, if + // triggered, will close the stream. Event::TimerPtr max_stream_duration_timer_; std::chrono::milliseconds idle_timeout_ms_{}; State state_; diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 93fff20c3ca99..2222f052e590b 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -223,6 +223,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( stream_idle_timeout_( PROTOBUF_GET_MS_OR_DEFAULT(config, stream_idle_timeout, StreamIdleTimeoutMs)), request_timeout_(PROTOBUF_GET_MS_OR_DEFAULT(config, request_timeout, RequestTimeoutMs)), + request_headers_timeout_( + PROTOBUF_GET_MS_OR_DEFAULT(config, request_headers_timeout, RequestHeaderTimeoutMs)), drain_timeout_(PROTOBUF_GET_MS_OR_DEFAULT(config, drain_timeout, 5000)), generate_request_id_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, generate_request_id, true)), preserve_external_request_id_(config.preserve_external_request_id()), diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index 47cc707bdb897..180e67cec2946 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -125,6 +125,9 @@ class HttpConnectionManagerConfig : Logger::Loggable, } 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 { + return request_headers_timeout_; + } absl::optional maxStreamDuration() const override { return max_stream_duration_; } @@ -233,6 +236,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, absl::optional max_stream_duration_; std::chrono::milliseconds stream_idle_timeout_; std::chrono::milliseconds request_timeout_; + std::chrono::milliseconds request_headers_timeout_; Router::RouteConfigProviderSharedPtr route_config_provider_; Config::ConfigProviderPtr scoped_routes_config_provider_; std::chrono::milliseconds drain_timeout_; @@ -255,6 +259,8 @@ class HttpConnectionManagerConfig : Logger::Loggable, static const uint64_t StreamIdleTimeoutMs = 5 * 60 * 1000; // request timeout is disabled by default static const uint64_t RequestTimeoutMs = 0; + // request header timeout is disabled by default + static const uint64_t RequestHeaderTimeoutMs = 0; }; /** diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index e16f827da776b..feddd8855c4bc 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -134,6 +134,7 @@ class AdminImpl : public Admin, uint32_t maxRequestHeadersCount() const override { return max_request_headers_count_; } std::chrono::milliseconds streamIdleTimeout() const override { return {}; } std::chrono::milliseconds requestTimeout() const override { return {}; } + std::chrono::milliseconds requestHeadersTimeout() const override { return {}; } std::chrono::milliseconds delayedCloseTimeout() const override { return {}; } absl::optional maxStreamDuration() const override { return max_stream_duration_; diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index 2e97cfbd42f18..fdf1bbe7ef2a0 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -154,6 +154,9 @@ class FuzzConfig : public ConnectionManagerConfig { } 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 { + return request_headers_timeout_; + } std::chrono::milliseconds delayedCloseTimeout() const override { return delayed_close_timeout_; } Router::RouteConfigProvider* routeConfigProvider() override { if (use_srds_) { @@ -232,6 +235,7 @@ class FuzzConfig : public ConnectionManagerConfig { absl::optional max_stream_duration_; std::chrono::milliseconds stream_idle_timeout_{}; std::chrono::milliseconds request_timeout_{}; + std::chrono::milliseconds request_headers_timeout_{}; std::chrono::milliseconds delayed_close_timeout_{}; bool use_remote_address_{true}; Http::ForwardClientCertType forward_client_cert_; diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index a022f18c2cd80..95cb95f2acf70 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -11,6 +11,7 @@ using testing::HasSubstr; using testing::InSequence; using testing::Invoke; using testing::InvokeWithoutArgs; +using testing::Mock; using testing::Return; using testing::ReturnRef; @@ -2835,6 +2836,68 @@ TEST_F(HttpConnectionManagerImplTest, RequestTimeoutIsDisarmedOnConnectionTermin filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose); } +TEST_F(HttpConnectionManagerImplTest, RequestHeaderTimeoutDisarmedAfterHeaders) { + request_headers_timeout_ = std::chrono::milliseconds(10); + setup(false, ""); + + Event::MockTimer* request_header_timer; + EXPECT_CALL(*codec_, dispatch(_)) + .WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { + request_header_timer = setUpTimer(); + EXPECT_CALL(*request_header_timer, enableTimer(request_headers_timeout_, _)); + + decoder_ = &conn_manager_->newStream(response_encoder_); + return Http::okStatus(); + })) + .WillOnce(Return(Http::okStatus())) + .WillOnce([&](Buffer::Instance&) { + RequestHeaderMapPtr headers{new TestRequestHeaderMapImpl{ + {":authority", "localhost:8080"}, {":path", "/"}, {":method", "GET"}}}; + + EXPECT_CALL(*request_header_timer, disableTimer).Times(1); + decoder_->decodeHeaders(std::move(headers), false); + return Http::okStatus(); + }); + + Buffer::OwnedImpl first_line("GET /HTTP/1.1\r\n"); + Buffer::OwnedImpl second_line("Host: localhost:8080\r\n"); + Buffer::OwnedImpl empty_line("\r\n"); + conn_manager_->onData(first_line, false); + EXPECT_TRUE(request_header_timer->enabled_); + conn_manager_->onData(second_line, false); + EXPECT_TRUE(request_header_timer->enabled_); + conn_manager_->onData(empty_line, false); + Mock::VerifyAndClearExpectations(codec_); + Mock::VerifyAndClearExpectations(request_header_timer); + + expectOnDestroy(); + filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose); +} + +TEST_F(HttpConnectionManagerImplTest, RequestHeaderTimeoutCallbackDisarmsAndReturns408) { + request_headers_timeout_ = std::chrono::milliseconds(10); + setup(false, ""); + + Event::MockTimer* request_header_timer; + EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status { + request_header_timer = setUpTimer(); + EXPECT_CALL(*request_header_timer, enableTimer(request_headers_timeout_, _)).Times(1); + + conn_manager_->newStream(response_encoder_); + EXPECT_CALL(filter_callbacks_.connection_.dispatcher_, setTrackedObject(_)).Times(2); + return Http::okStatus(); + })); + + Buffer::OwnedImpl fake_input("GET /resource HTTP/1.1\r\n\r\n"); + conn_manager_->onData(fake_input, false); // kick off request + + // The client took too long to send headers. + EXPECT_CALL(*request_header_timer, disableTimer).Times(1); + request_header_timer->invokeCallback(); + + EXPECT_EQ(1U, stats_.named_.downstream_rq_header_timeout_.value()); +} + TEST_F(HttpConnectionManagerImplTest, MaxStreamDurationDisabledIfSetToZero) { max_stream_duration_ = std::chrono::milliseconds(0); setup(false, ""); diff --git a/test/common/http/conn_manager_impl_test_base.h b/test/common/http/conn_manager_impl_test_base.h index 067ca0a1f3699..35cd115e38ee6 100644 --- a/test/common/http/conn_manager_impl_test_base.h +++ b/test/common/http/conn_manager_impl_test_base.h @@ -83,6 +83,9 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan } 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 { + return request_headers_timeout_; + } std::chrono::milliseconds delayedCloseTimeout() const override { return delayed_close_timeout_; } absl::optional maxStreamDuration() const override { return max_stream_duration_; @@ -170,6 +173,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan absl::optional max_connection_duration_; std::chrono::milliseconds stream_idle_timeout_{}; std::chrono::milliseconds request_timeout_{}; + std::chrono::milliseconds request_headers_timeout_{}; std::chrono::milliseconds delayed_close_timeout_{}; absl::optional max_stream_duration_{}; NiceMock random_; diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 4b43487108447..10f755873a2b0 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -109,6 +109,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_METHOD(absl::optional, maxStreamDuration, (), (const)); MOCK_METHOD(std::chrono::milliseconds, streamIdleTimeout, (), (const)); MOCK_METHOD(std::chrono::milliseconds, requestTimeout, (), (const)); + MOCK_METHOD(std::chrono::milliseconds, requestHeadersTimeout, (), (const)); MOCK_METHOD(std::chrono::milliseconds, delayedCloseTimeout, (), (const)); MOCK_METHOD(Router::RouteConfigProvider*, routeConfigProvider, ()); MOCK_METHOD(Config::ConfigProvider*, scopedRouteConfigProvider, ()); diff --git a/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc b/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc index f9e76ee3024b1..6e378d31788f5 100644 --- a/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc +++ b/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc @@ -192,12 +192,12 @@ class RawWriteSslIntegrationTest : public SslIntegrationTest { initialize(); // write_request_cb will write each of the items in request_chunks as a separate SSL_write. - auto write_request_cb = [&request_chunks](Network::ClientConnection& client) { + auto write_request_cb = [&request_chunks](Buffer::Instance& buffer) { if (!request_chunks.empty()) { - Buffer::OwnedImpl buffer(request_chunks.front()); - client.write(buffer, false); + buffer.add(request_chunks.front()); request_chunks.pop_front(); } + return false; }; auto client_transport_socket_factory_ptr = diff --git a/test/integration/http_timeout_integration_test.cc b/test/integration/http_timeout_integration_test.cc index 77ea916b467a0..c2df233dd2ca6 100644 --- a/test/integration/http_timeout_integration_test.cc +++ b/test/integration/http_timeout_integration_test.cc @@ -4,6 +4,8 @@ namespace Envoy { +using testing::HasSubstr; + INSTANTIATE_TEST_SUITE_P(IpVersions, HttpTimeoutIntegrationTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); @@ -499,4 +501,46 @@ void HttpTimeoutIntegrationTest::testRouterRequestAndResponseWithHedgedPerTryTim EXPECT_EQ("200", response->headers().getStatusValue()); } +// Starts a request with a header timeout specified, sleeps for longer than the +// timeout, and ensures that a timeout is received. +TEST_P(HttpTimeoutIntegrationTest, RequestHeaderTimeout) { + if (downstreamProtocol() != Http::CodecClient::Type::HTTP1) { + // This test requires that the downstream be using HTTP1. + return; + } + + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { + auto* request_headers_timeout = hcm.mutable_request_headers_timeout(); + request_headers_timeout->set_seconds(1); + request_headers_timeout->set_nanos(0); + }); + initialize(); + + const std::string input_request = ("GET / HTTP/1.1\r\n" + // Omit trailing \r\n that would indicate the end of headers. + "Host: localhost\r\n"); + std::string response; + + auto connection_driver = createConnectionDriver( + lookupPort("http"), input_request, + [&response](Network::ClientConnection&, const Buffer::Instance& data) -> void { + response.append(data.toString()); + }); + + while (!connection_driver->allBytesSent()) { + connection_driver->run(Event::Dispatcher::RunType::NonBlock); + } + test_server_->waitForGaugeGe("http.config_test.downstream_rq_active", 1); + ASSERT_FALSE(connection_driver->closed()); + + timeSystem().advanceTimeWait(std::chrono::milliseconds(1001)); + connection_driver->run(); + + // The upstream should send a 40x response and send a local reply. + EXPECT_TRUE(connection_driver->closed()); + EXPECT_THAT(response, AllOf(HasSubstr("408"), HasSubstr("header"))); +} + } // namespace Envoy diff --git a/test/integration/utility.cc b/test/integration/utility.cc index ddbd0816f53e6..1943ccbd6523f 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -33,11 +33,12 @@ namespace { RawConnectionDriver::DoWriteCallback writeBufferCallback(Buffer::Instance& data) { auto shared_data = std::make_shared(); shared_data->move(data); - return [shared_data](Network::ClientConnection& client) { + return [shared_data](Buffer::Instance& dest) { if (shared_data->length() > 0) { - client.write(*shared_data, false); + dest.add(*shared_data); shared_data->drain(shared_data->length()); } + return false; }; } @@ -143,11 +144,15 @@ RawConnectionDriver::RawConnectionDriver(uint32_t port, DoWriteCallback write_re Network::Address::IpVersion version, Event::Dispatcher& dispatcher, Network::TransportSocketPtr transport_socket) - : dispatcher_(dispatcher) { + : dispatcher_(dispatcher), remaining_bytes_to_send_(0) { api_ = Api::createApiForTest(stats_store_); Event::GlobalTimeSystem time_system; - callbacks_ = std::make_unique( - [this, write_request_callback]() { write_request_callback(*client_); }); + callbacks_ = std::make_unique([this, write_request_callback]() { + Buffer::OwnedImpl buffer; + const bool close_after = write_request_callback(buffer); + remaining_bytes_to_send_ += buffer.length(); + client_->write(buffer, close_after); + }); if (transport_socket == nullptr) { transport_socket = Network::Test::createRawBufferSocket(); @@ -164,6 +169,7 @@ RawConnectionDriver::RawConnectionDriver(uint32_t port, DoWriteCallback write_re client_->addConnectionCallbacks(*callbacks_); client_->addReadFilter( Network::ReadFilterSharedPtr{new ForwardingFilter(*this, response_data_callback)}); + client_->addBytesSentCallback([&](uint64_t bytes) { remaining_bytes_to_send_ -= bytes; }); client_->connect(); } @@ -182,6 +188,8 @@ void RawConnectionDriver::run(Event::Dispatcher::RunType run_type) { dispatcher_ void RawConnectionDriver::close() { client_->close(Network::ConnectionCloseType::FlushWrite); } +bool RawConnectionDriver::allBytesSent() const { return remaining_bytes_to_send_ == 0; } + WaitForPayloadReader::WaitForPayloadReader(Event::Dispatcher& dispatcher) : dispatcher_(dispatcher) {} diff --git a/test/integration/utility.h b/test/integration/utility.h index 497fe872472b4..cad8b153363d6 100644 --- a/test/integration/utility.h +++ b/test/integration/utility.h @@ -63,7 +63,10 @@ using BufferingStreamDecoderPtr = std::unique_ptr; */ class RawConnectionDriver { public: - using DoWriteCallback = std::function; + // Callback that is executed to write data to connection. The provided buffer + // should be populated with the data to write. If the callback returns true, + // the connection will be closed after writing. + using DoWriteCallback = std::function; using ReadCallback = std::function; RawConnectionDriver(uint32_t port, DoWriteCallback write_request_callback, @@ -86,6 +89,7 @@ class RawConnectionDriver { void waitForConnection(); bool closed() { return callbacks_->closed(); } + bool allBytesSent() const; private: struct ForwardingFilter : public Network::ReadFilterBaseImpl { @@ -137,6 +141,7 @@ class RawConnectionDriver { Event::Dispatcher& dispatcher_; std::unique_ptr callbacks_; Network::ClientConnectionPtr client_; + uint64_t remaining_bytes_to_send_; }; /**