Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
ba55e0a
Add request headers timeout to HCM
akonradi Sep 30, 2020
256924b
Merge remote-tracking branch 'upstream/master' into http-request-head…
akonradi Sep 30, 2020
97de0f2
Update documentation, fix formatting
akonradi Oct 12, 2020
a1e78ee
Merge remote-tracking branch 'upstream/master' into http-request-head…
akonradi Oct 12, 2020
773ded2
Use unique_ptr::reset() instead of disabling
akonradi Oct 12, 2020
f780c24
Address review feedback
akonradi Oct 12, 2020
0735254
Revert "Use unique_ptr::reset() instead of disabling"
akonradi Oct 13, 2020
068dbde
Merge remote-tracking branch 'upstream/master' into http-request-head…
akonradi Oct 13, 2020
bd8b3c8
Address review feedback
akonradi Oct 19, 2020
06e150f
Add validation annotation
akonradi Oct 19, 2020
99ec46b
Revert to sendLocalReply
akonradi Oct 23, 2020
df9c705
Add integration test
akonradi Oct 23, 2020
7924ff3
Merge remote-tracking branch 'upstream/master' into http-request-head…
akonradi Oct 23, 2020
9e65d85
Fix protodoc_manifest.yaml
akonradi Oct 23, 2020
a28a251
Fix formatting
akonradi Oct 23, 2020
991e437
Merge remote-tracking branch 'upstream/master' into http-request-head…
akonradi Oct 27, 2020
39dc1be
Don't reset request header timer on timeout
akonradi Oct 27, 2020
ccd2599
Address feedback
akonradi Oct 28, 2020
f478d1f
Re-add reset() after headers complete
akonradi Oct 28, 2020
97e07fd
Use RawConnectionSocket utility for test
akonradi Oct 30, 2020
e1411ff
Merge remote-tracking branch 'upstream/master' into http-request-head…
akonradi Oct 30, 2020
576f43b
Check message, fix stream info
akonradi Nov 4, 2020
b0d5025
Add header timeout to timeouts.rst
akonradi Nov 6, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// HTTP connection manager :ref:`configuration overview <config_http_conn_man>`.
// [#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";
Expand Down Expand Up @@ -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
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions docs/protodoc_manifest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: >
Expand Down
8 changes: 7 additions & 1 deletion docs/root/faq/configuration/timeouts.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <config_http_filters_buffer>`, it is recommended to configure this timeout.
* The HTTP connection manager :ref:`request_headers_timeout
<envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.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
<envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_idle_timeout>`
is the amount of time that the connection manager will allow a stream to exist with no upstream
Expand Down Expand Up @@ -120,4 +126,4 @@ Transport Socket
* The :ref:`transport_socket_connect_timeout <envoy_v3_api_field_config.listener.v3.FilterChain.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.
the amount of time allowed to finish the encrypted handshake after establishing a TCP connection.
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ New Features
* grpc: implemented header value syntax support when defining :ref:`initial metadata <envoy_v3_api_field_config.core.v3.GrpcService.initial_metadata>` for gRPC-based `ext_authz` :ref:`HTTP <envoy_v3_api_field_extensions.filters.http.ext_authz.v3.ExtAuthz.grpc_service>` and :ref:`network <envoy_v3_api_field_extensions.filters.network.ext_authz.v3.ExtAuthz.grpc_service>` filters, and :ref:`ratelimit <envoy_v3_api_field_config.ratelimit.v3.RateLimitServiceConfig.grpc_service>` filters.
* hds: added support for delta updates in the :ref:`HealthCheckSpecifier <envoy_v3_api_msg_service.health.v3.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 <envoy_v3_api_field_config.core.v3.HealthCheck.no_traffic_healthy_interval>` which allows a different no traffic interval when the host is healthy.
* http: added HCM :ref:`timeout config field <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.request_headers_timeout>` 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 <envoy_v3_api_field_config.listener.v3.Listener.default_filter_chain>`. If this field is supplied, and none of the :ref:`filter_chains <envoy_v3_api_field_config.listener.v3.Listener.filter_chains>` matches, this default filter chain is used to serve the connection.
* lua: added `downstreamDirectRemoteAddress()` and `downstreamLocalAddress()` APIs to :ref:`streamInfo() <config_http_filters_lua_stream_info_wrapper>`.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions include/envoy/stream_info/stream_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions source/common/http/conn_manager_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down Expand Up @@ -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.
Expand Down
29 changes: 27 additions & 2 deletions source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
15 changes: 12 additions & 3 deletions source/common/http/conn_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
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(); }
Expand Down Expand Up @@ -354,11 +356,18 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional, think it's worth mentioning which of these try to send a reply?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to punt on this since whether or not a reply is sent for some of these is dependent on a runtime override.

Event::TimerPtr max_stream_duration_timer_;
std::chrono::milliseconds idle_timeout_ms_{};
State state_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
}
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<std::chrono::milliseconds> maxStreamDuration() const override {
return max_stream_duration_;
}
Expand Down Expand Up @@ -233,6 +236,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
absl::optional<std::chrono::milliseconds> 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_;
Expand All @@ -255,6 +259,8 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
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;
};

/**
Expand Down
1 change: 1 addition & 0 deletions source/server/admin/admin.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::chrono::milliseconds> maxStreamDuration() const override {
return max_stream_duration_;
Expand Down
4 changes: 4 additions & 0 deletions test/common/http/conn_manager_impl_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_) {
Expand Down Expand Up @@ -232,6 +235,7 @@ class FuzzConfig : public ConnectionManagerConfig {
absl::optional<std::chrono::milliseconds> 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_;
Expand Down
Loading