From 41b04787101349a2d8193fb154f1bac68b736481 Mon Sep 17 00:00:00 2001 From: Yuchen Dai Date: Thu, 26 Sep 2019 14:17:20 -0700 Subject: [PATCH] CVE-2019-15226-for-istio-envoy-release-1-1 Tracking header size and add limit for the number of headers Signed-off-by: Yuchen Dai --- api/envoy/api/v2/cds.proto | 4 +- api/envoy/api/v2/core/protocol.proto | 14 +- .../v2/http_connection_manager.proto | 15 +- include/envoy/http/codec.h | 7 + include/envoy/http/header_map.h | 34 +++- include/envoy/upstream/upstream.h | 6 + source/common/http/codec_client.cc | 5 +- source/common/http/conn_manager_config.h | 5 + source/common/http/conn_manager_impl.cc | 12 ++ source/common/http/conn_manager_utility.cc | 10 +- source/common/http/conn_manager_utility.h | 2 +- source/common/http/header_map_impl.cc | 63 +++++- source/common/http/header_map_impl.h | 24 ++- source/common/http/http1/codec_impl.cc | 37 ++-- source/common/http/http1/codec_impl.h | 11 +- source/common/http/http2/codec_impl.cc | 28 ++- source/common/http/http2/codec_impl.h | 15 +- source/common/router/router.cc | 1 + source/common/upstream/BUILD | 1 + source/common/upstream/upstream_impl.cc | 6 + source/common/upstream/upstream_impl.h | 2 + .../file/file_access_log_impl.cc | 4 + .../http_grpc/grpc_access_log_impl.cc | 8 +- .../filters/http/rbac/rbac_filter.cc | 8 + .../network/http_connection_manager/BUILD | 1 + .../network/http_connection_manager/config.cc | 24 ++- .../network/http_connection_manager/config.h | 2 + source/server/http/admin.cc | 2 +- source/server/http/admin.h | 2 + test/common/http/codec_impl_fuzz_test.cc | 21 +- .../http/conn_manager_impl_fuzz_test.cc | 2 + test/common/http/conn_manager_impl_test.cc | 28 +-- test/common/http/conn_manager_utility_test.cc | 1 + .../common/http/header_map_impl_speed_test.cc | 2 +- test/common/http/header_map_impl_test.cc | 51 +++++ test/common/http/http1/codec_impl_test.cc | 181 +++++++++++++----- test/common/http/http2/codec_impl_test.cc | 94 +++++++-- test/common/http/http2/codec_impl_test_util.h | 12 +- .../http_connection_manager/config_test.cc | 65 +++++++ test/integration/autonomous_upstream.cc | 2 +- test/integration/fake_upstream.cc | 18 +- test/integration/fake_upstream.h | 6 +- test/integration/http2_integration_test.cc | 33 ++++ .../http2_upstream_integration_test.cc | 62 ++++++ .../http2_upstream_integration_test.h | 2 + test/integration/http_integration.cc | 108 +++++++++-- test/integration/http_integration.h | 25 ++- test/integration/protocol_integration_test.cc | 124 +++++++++++- test/mocks/upstream/cluster_info.cc | 2 + test/mocks/upstream/cluster_info.h | 2 + 50 files changed, 986 insertions(+), 208 deletions(-) diff --git a/api/envoy/api/v2/cds.proto b/api/envoy/api/v2/cds.proto index 6fb858efd642..6116eecb9f4c 100644 --- a/api/envoy/api/v2/cds.proto +++ b/api/envoy/api/v2/cds.proto @@ -232,8 +232,8 @@ message Cluster { reserved 12; - // Additional options when handling HTTP requests. These options will be applicable to both - // HTTP1 and HTTP2 requests. + // Additional options when handling HTTP requests upstream. These options will be applicable to + // both HTTP1 and HTTP2 requests. core.HttpProtocolOptions common_http_protocol_options = 29; // Additional options when handling HTTP1 requests. diff --git a/api/envoy/api/v2/core/protocol.proto b/api/envoy/api/v2/core/protocol.proto index 88a82077a428..ed751dc67dc1 100644 --- a/api/envoy/api/v2/core/protocol.proto +++ b/api/envoy/api/v2/core/protocol.proto @@ -23,11 +23,19 @@ message TcpProtocolOptions { } message HttpProtocolOptions { - // The idle timeout for upstream connection pool connections. The idle timeout is defined as the + // The idle timeout for connections. The idle timeout is defined as the // period in which there are no active requests. If not set, there is no idle timeout. When the - // idle timeout is reached the connection will be closed. Note that request based timeouts mean - // that HTTP/2 PINGs will not keep the connection alive. + // idle timeout is reached the connection will be closed. If the connection is an HTTP/2 + // downstream connection a drain sequence will occur prior to closing the connection, see + // :ref:`drain_timeout + // `. + // Note that request based timeouts mean that HTTP/2 PINGs will not keep the connection alive. google.protobuf.Duration idle_timeout = 1 [(gogoproto.stdduration) = true]; + + // The maximum number of headers. If unconfigured, the default + // maximum number of request headers allowed is 100. Requests that exceed this limit will receive + // a 431 response for HTTP/1.x and cause a stream reset for HTTP/2. + google.protobuf.UInt32Value max_headers_count = 2 [(validate.rules).uint32 = {gte: 1}]; } message Http1ProtocolOptions { diff --git a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto index 18a479d3d7f9..e12dac55b901 100644 --- a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto +++ b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto @@ -24,7 +24,7 @@ import "gogoproto/gogo.proto"; // [#protodoc-title: HTTP connection manager] // HTTP connection manager :ref:`configuration overview `. -// [#comment:next free field: 31] +// [#comment:next free field: 36] message HttpConnectionManager { enum CodecType { option (gogoproto.goproto_enum_prefix) = false; @@ -127,6 +127,10 @@ message HttpConnectionManager { // `. Tracing tracing = 7; + // Additional settings for HTTP requests handled by the connection manager. These will be + // applicable to both HTTP1 and HTTP2 requests. + envoy.api.v2.core.HttpProtocolOptions common_http_protocol_options = 35; + // Additional HTTP/1 settings that are passed to the HTTP/1 codec. envoy.api.v2.core.Http1ProtocolOptions http_protocol_options = 8; @@ -149,10 +153,11 @@ message HttpConnectionManager { // idle timeout is defined as the period in which there are no active // requests. If not set, there is no idle timeout. When the idle timeout is // reached the connection will be closed. If the connection is an HTTP/2 - // connection a drain sequence will occur prior to closing the connection. See - // :ref:`drain_timeout - // `. - google.protobuf.Duration idle_timeout = 11 [(gogoproto.stdduration) = true]; + // connection a drain sequence will occur prior to closing the connection. + // This field is deprecated. Use :ref:`idle_timeout + // ` + // instead. + google.protobuf.Duration idle_timeout = 11 [(gogoproto.stdduration) = true, deprecated = true]; // The stream idle timeout for connections managed by the connection manager. // If not specified, this defaults to 5 minutes. The default value was selected diff --git a/include/envoy/http/codec.h b/include/envoy/http/codec.h index 2323f55da276..583b86809f74 100644 --- a/include/envoy/http/codec.h +++ b/include/envoy/http/codec.h @@ -15,6 +15,13 @@ namespace Http { // Legacy default value of 60K is safely under both codec default limits. static const uint32_t DEFAULT_MAX_REQUEST_HEADERS_KB = 60; +// Default maximum number of headers. +static const uint32_t DEFAULT_MAX_HEADERS_COUNT = 100; + +const char MaxRequestHeadersCountOverrideKey[] = + "envoy.reloadable_features.max_request_headers_count"; +const char MaxResponseHeadersCountOverrideKey[] = + "envoy.reloadable_features.max_response_headers_count"; class Stream; diff --git a/include/envoy/http/header_map.h b/include/envoy/http/header_map.h index 8a094c1d793c..17b253f549bb 100644 --- a/include/envoy/http/header_map.h +++ b/include/envoy/http/header_map.h @@ -445,9 +445,41 @@ class HeaderMap { virtual void setReferenceKey(const LowerCaseString& key, const std::string& value) PURE; /** + * HeaderMap contains an internal byte size count, updated as entries are added, removed, or + * modified through the HeaderMap interface. However, HeaderEntries can be accessed and modified + * by reference so that the HeaderMap can no longer accurately update the internal byte size + * count. + * + * Calling byteSize before a HeaderEntry is accessed will return the internal byte size count. The + * value is cleared when a HeaderEntry is accessed, and the value is updated and set again when + * refreshByteSize is called. + * + * To guarantee an accurate byte size count, call refreshByteSize. + * + * @return uint64_t the approximate size of the header map in bytes if valid. + */ + virtual absl::optional byteSize() const PURE; + + /** + * This returns the sum of the byte sizes of the keys and values in the HeaderMap. This also + * updates and sets the byte size count. + * + * To guarantee an accurate byte size count, use this. If it is known HeaderEntries have not been + * manipulated since a call to refreshByteSize, it is safe to use byteSize. + * + * @return uint64_t the approximate size of the header map in bytes. + */ + virtual uint64_t refreshByteSize() PURE; + + /** + * This returns the sum of the byte sizes of the keys and values in the HeaderMap. + * + * This iterates over the HeaderMap to calculate size and should only be called directly when the + * user wants an explicit recalculation of the byte size. + * * @return uint64_t the approximate size of the header map in bytes. */ - virtual uint64_t byteSize() const PURE; + virtual uint64_t byteSizeInternal() const PURE; /** * Get a header by key. diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index 3fdb15cdeee1..5582c960f06a 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -694,6 +694,12 @@ class ClusterInfo { */ virtual uint64_t maxRequestsPerConnection() const PURE; + /** + * @return uint32_t the maximum number of response headers. The default value is 100. Results in a + * reset if the number of headers exceeds this value. + */ + virtual uint32_t maxResponseHeadersCount() const PURE; + /** * @return the human readable name of the cluster. */ diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc index 3f8defb71ee6..bd356b654c4e 100644 --- a/source/common/http/codec_client.cc +++ b/source/common/http/codec_client.cc @@ -140,13 +140,14 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne : CodecClient(type, std::move(connection), host, dispatcher) { switch (type) { case Type::HTTP1: { - codec_ = std::make_unique(*connection_, *this); + codec_ = std::make_unique( + *connection_, *this, host->cluster().maxResponseHeadersCount()); break; } case Type::HTTP2: { codec_ = std::make_unique( *connection_, *this, host->cluster().statsScope(), host->cluster().http2Settings(), - Http::DEFAULT_MAX_REQUEST_HEADERS_KB); + Http::DEFAULT_MAX_REQUEST_HEADERS_KB, host->cluster().maxResponseHeadersCount()); break; } } diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index 2fb4ee9e9c57..b8327465ac79 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -228,6 +228,11 @@ class ConnectionManagerConfig { */ virtual uint32_t maxRequestHeadersKb() const PURE; + /** + * @return maximum number of request headers the codecs will accept. + */ + virtual uint32_t maxRequestHeadersCount() const PURE; + /** * @return per-stream idle timeout for incoming connection manager connections. Zero indicates a * disabled idle timeout. diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 659dd7e21882..33becdb68515 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -469,6 +469,18 @@ ConnectionManagerImpl::ActiveStream::~ActiveStream() { } connection_manager_.stats_.named_.downstream_rq_active_.dec(); + // Refresh byte sizes of the HeaderMaps before logging. + // TODO(asraa): Remove this when entries in HeaderMap can no longer be modified by reference and + // HeaderMap holds an accurate internal byte size count. + if (request_headers_ != nullptr) { + request_headers_->refreshByteSize(); + } + if (response_headers_ != nullptr) { + response_headers_->refreshByteSize(); + } + if (response_trailers_ != nullptr) { + response_trailers_->refreshByteSize(); + } for (const AccessLog::InstanceSharedPtr& access_log : connection_manager_.config_.accessLogs()) { access_log->log(request_headers_.get(), response_headers_.get(), response_trailers_.get(), stream_info_); diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index 164b8712c295..d741a0bf08bb 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -40,13 +40,15 @@ std::string ConnectionManagerUtility::determineNextProtocol(Network::Connection& ServerConnectionPtr ConnectionManagerUtility::autoCreateCodec( Network::Connection& connection, const Buffer::Instance& data, ServerConnectionCallbacks& callbacks, Stats::Scope& scope, const Http1Settings& http1_settings, - const Http2Settings& http2_settings, const uint32_t max_request_headers_kb) { + const Http2Settings& http2_settings, const uint32_t max_request_headers_kb, + const uint32_t max_request_headers_count) { if (determineNextProtocol(connection, data) == Http2::ALPN_STRING) { return std::make_unique(connection, callbacks, scope, - http2_settings, max_request_headers_kb); + http2_settings, max_request_headers_kb, + max_request_headers_count); } else { - return std::make_unique(connection, callbacks, http1_settings, - max_request_headers_kb); + return std::make_unique( + connection, callbacks, http1_settings, max_request_headers_kb, max_request_headers_count); } } diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h index 126982df7753..5b41a6d90218 100644 --- a/source/common/http/conn_manager_utility.h +++ b/source/common/http/conn_manager_utility.h @@ -37,7 +37,7 @@ class ConnectionManagerUtility { autoCreateCodec(Network::Connection& connection, const Buffer::Instance& data, ServerConnectionCallbacks& callbacks, Stats::Scope& scope, const Http1Settings& http1_settings, const Http2Settings& http2_settings, - const uint32_t max_request_headers_kb); + uint32_t max_request_headers_kb, uint32_t max_request_headers_count); /** * Mutates request headers in various ways. This functionality is broken out because of its diff --git a/source/common/http/header_map_impl.cc b/source/common/http/header_map_impl.cc index 66500d425383..5d2824d95066 100644 --- a/source/common/http/header_map_impl.cc +++ b/source/common/http/header_map_impl.cc @@ -295,14 +295,17 @@ struct HeaderMapImpl::StaticLookupTable : public TrieLookupTable { } }; -void HeaderMapImpl::appendToHeader(HeaderString& header, absl::string_view data) { +uint64_t HeaderMapImpl::appendToHeader(HeaderString& header, absl::string_view data) { if (data.empty()) { - return; + return 0; } + uint64_t byte_size = 0; if (!header.empty()) { header.append(",", 1); + byte_size += 1; } header.append(data.data(), data.size()); + return data.size() + byte_size; } HeaderMapImpl::HeaderMapImpl() { memset(&inline_headers_, 0, sizeof(inline_headers_)); } @@ -319,6 +322,20 @@ HeaderMapImpl::HeaderMapImpl( } } +void HeaderMapImpl::addSize(uint64_t size) { + // Adds size to cached_byte_size_ if it exists. + if (cached_byte_size_.has_value()) { + cached_byte_size_.value() += size; + } +} + +void HeaderMapImpl::subtractSize(uint64_t size) { + if (cached_byte_size_.has_value()) { + ASSERT(cached_byte_size_ >= size); + cached_byte_size_.value() -= size; + } +} + void HeaderMapImpl::copyFrom(const HeaderMap& header_map) { header_map.iterate( [](const HeaderEntry& header, void* context) -> HeaderMap::Iterate { @@ -359,10 +376,13 @@ void HeaderMapImpl::insertByKey(HeaderString&& key, HeaderString&& value) { if (*ref_lookup_response.entry_ == nullptr) { maybeCreateInline(ref_lookup_response.entry_, *ref_lookup_response.key_, std::move(value)); } else { - appendToHeader((*ref_lookup_response.entry_)->value(), value.c_str()); + const uint64_t added_size = + appendToHeader((*ref_lookup_response.entry_)->value(), value.getStringView()); + addSize(added_size); value.clear(); } } else { + addSize(key.size() + value.size()); std::list::iterator i = headers_.insert(std::move(key), std::move(value)); i->entry_ = i; } @@ -373,7 +393,8 @@ void HeaderMapImpl::addViaMove(HeaderString&& key, HeaderString&& value) { // the existing value. auto* entry = getExistingInline(key.c_str()); if (entry != nullptr) { - appendToHeader(entry->value(), value.c_str()); + const uint64_t added_size = appendToHeader(entry->value(), value.getStringView()); + addSize(added_size); key.clear(); value.clear(); } else { @@ -408,7 +429,8 @@ void HeaderMapImpl::addCopy(const LowerCaseString& key, uint64_t value) { if (entry != nullptr) { char buf[32]; StringUtil::itoa(buf, sizeof(buf), value); - appendToHeader(entry->value(), buf); + const uint64_t added_size = appendToHeader(entry->value(), buf); + addSize(added_size); return; } HeaderString new_key; @@ -423,7 +445,8 @@ void HeaderMapImpl::addCopy(const LowerCaseString& key, uint64_t value) { void HeaderMapImpl::addCopy(const LowerCaseString& key, const std::string& value) { auto* entry = getExistingInline(key.get().c_str()); if (entry != nullptr) { - appendToHeader(entry->value(), value); + const uint64_t added_size = appendToHeader(entry->value(), value); + addSize(added_size); return; } HeaderString new_key; @@ -451,13 +474,24 @@ void HeaderMapImpl::setReferenceKey(const LowerCaseString& key, const std::strin ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) } -uint64_t HeaderMapImpl::byteSize() const { +absl::optional HeaderMapImpl::byteSize() const { return cached_byte_size_; } + +uint64_t HeaderMapImpl::refreshByteSize() { + if (!cached_byte_size_.has_value()) { + // In this case, the cached byte size is not valid, and the byte size is computed via an + // iteration over the HeaderMap. The cached byte size is updated. + cached_byte_size_ = byteSizeInternal(); + } + return cached_byte_size_.value(); +} + +uint64_t HeaderMapImpl::byteSizeInternal() const { + // Computes the total byte size by summing the byte size of the keys and values. uint64_t byte_size = 0; for (const HeaderEntryImpl& header : headers_) { byte_size += header.key().size(); byte_size += header.value().size(); } - return byte_size; } @@ -474,6 +508,7 @@ const HeaderEntry* HeaderMapImpl::get(const LowerCaseString& key) const { HeaderEntry* HeaderMapImpl::get(const LowerCaseString& key) { for (HeaderEntryImpl& header : headers_) { if (header.key() == key.get().c_str()) { + cached_byte_size_.reset(); return &header; } } @@ -528,6 +563,7 @@ void HeaderMapImpl::remove(const LowerCaseString& key) { } else { for (auto i = headers_.begin(); i != headers_.end();) { if (i->key() == key.get().c_str()) { + subtractSize(i->key().size() + i->value().size()); i = headers_.erase(i); } else { ++i; @@ -537,7 +573,7 @@ void HeaderMapImpl::remove(const LowerCaseString& key) { } void HeaderMapImpl::removePrefix(const LowerCaseString& prefix) { - headers_.remove_if([&](const HeaderEntryImpl& entry) { + headers_.remove_if([&prefix, this](const HeaderEntryImpl& entry) { bool to_remove = absl::StartsWith(entry.key().getStringView(), prefix.get()); if (to_remove) { // If this header should be removed, make sure any references in the @@ -546,8 +582,13 @@ void HeaderMapImpl::removePrefix(const LowerCaseString& prefix) { if (cb) { StaticLookupResponse ref_lookup_response = cb(*this); if (ref_lookup_response.entry_) { + const uint32_t key_value_size = (*ref_lookup_response.entry_)->key().size() + + (*ref_lookup_response.entry_)->value().size(); + subtractSize(key_value_size); *ref_lookup_response.entry_ = nullptr; } + } else { + subtractSize(entry.key().size() + entry.value().size()); } } return to_remove; @@ -556,6 +597,7 @@ void HeaderMapImpl::removePrefix(const LowerCaseString& prefix) { HeaderMapImpl::HeaderEntryImpl& HeaderMapImpl::maybeCreateInline(HeaderEntryImpl** entry, const LowerCaseString& key) { + cached_byte_size_.reset(); if (*entry) { return **entry; } @@ -574,6 +616,7 @@ HeaderMapImpl::HeaderEntryImpl& HeaderMapImpl::maybeCreateInline(HeaderEntryImpl return **entry; } + addSize(key.get().size() + value.size()); std::list::iterator i = headers_.insert(key, std::move(value)); i->entry_ = i; *entry = &(*i); @@ -595,6 +638,8 @@ void HeaderMapImpl::removeInline(HeaderEntryImpl** ptr_to_entry) { } HeaderEntryImpl* entry = *ptr_to_entry; + const uint64_t size_to_subtract = entry->entry_->key().size() + entry->entry_->value().size(); + subtractSize(size_to_subtract); *ptr_to_entry = nullptr; headers_.erase(entry->entry_); } diff --git a/source/common/http/header_map_impl.h b/source/common/http/header_map_impl.h index 3bdbd1cd206e..2b04ada70179 100644 --- a/source/common/http/header_map_impl.h +++ b/source/common/http/header_map_impl.h @@ -16,12 +16,21 @@ namespace Http { /** * These are definitions of all of the inline header access functions described inside header_map.h + * + * When a non-const reference or pointer to a HeaderEntry is returned, the internal byte size count + * will be cleared, since HeaderMap will no longer be able to accurately update the size of that + * HeaderEntry. + * TODO(asraa): Remove functions with a non-const HeaderEntry return value. */ #define DEFINE_INLINE_HEADER_FUNCS(name) \ public: \ const HeaderEntry* name() const override { return inline_headers_.name##_; } \ - HeaderEntry* name() override { return inline_headers_.name##_; } \ + HeaderEntry* name() override { \ + cached_byte_size_.reset(); \ + return inline_headers_.name##_; \ + } \ HeaderEntry& insert##name() override { \ + cached_byte_size_.reset(); \ return maybeCreateInline(&inline_headers_.name##_, Headers::get().name); \ } \ void remove##name() override { removeInline(&inline_headers_.name##_); } @@ -43,7 +52,7 @@ class HeaderMapImpl : public HeaderMap, NonCopyable { * @param header the header to append to. * @param data to append to the header. */ - static void appendToHeader(HeaderString& header, absl::string_view data); + static uint64_t appendToHeader(HeaderString& header, absl::string_view data); HeaderMapImpl(); explicit HeaderMapImpl( @@ -71,7 +80,9 @@ class HeaderMapImpl : public HeaderMap, NonCopyable { void addCopy(const LowerCaseString& key, const std::string& value) override; void setReference(const LowerCaseString& key, const std::string& value) override; void setReferenceKey(const LowerCaseString& key, const std::string& value) override; - uint64_t byteSize() const override; + absl::optional byteSize() const override; + uint64_t refreshByteSize() override; + uint64_t byteSizeInternal() const override; const HeaderEntry* get(const LowerCaseString& key) const override; HeaderEntry* get(const LowerCaseString& key) override; void iterate(ConstIterateCb cb, void* context) const override; @@ -193,9 +204,16 @@ class HeaderMapImpl : public HeaderMap, NonCopyable { void removeInline(HeaderEntryImpl** entry); + void addSize(uint64_t size); + void subtractSize(uint64_t size); + AllInlineHeaders inline_headers_; HeaderList headers_; + // When present, this holds the internal byte size of the HeaderMap. The value is removed once an + // inline header entry is accessed and updated when refreshByteSize() is called. + absl::optional cached_byte_size_ = 0; + ALL_INLINE_HEADERS(DEFINE_INLINE_HEADER_FUNCS) }; diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index c9279542eecb..5906bc13c19d 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -4,8 +4,6 @@ #include #include -#include "absl/strings/match.h" - #include "envoy/buffer/buffer.h" #include "envoy/http/header_map.h" #include "envoy/network/connection.h" @@ -18,6 +16,8 @@ #include "common/http/headers.h" #include "common/http/utility.h" +#include "absl/strings/match.h" + namespace Envoy { namespace Http { namespace Http1 { @@ -319,10 +319,10 @@ const ToLowerTable& ConnectionImpl::toLowerTable() { } ConnectionImpl::ConnectionImpl(Network::Connection& connection, http_parser_type type, - uint32_t max_headers_kb) + uint32_t max_headers_kb, const uint32_t max_headers_count) : connection_(connection), output_buffer_([&]() -> void { this->onBelowLowWatermark(); }, [&]() -> void { this->onAboveHighWatermark(); }), - max_headers_kb_(max_headers_kb) { + max_headers_kb_(max_headers_kb), max_headers_count_(max_headers_count) { output_buffer_.setWatermarks(connection.bufferLimit()); http_parser_init(&parser_, type); parser_.data = this; @@ -336,6 +336,12 @@ void ConnectionImpl::completeLastHeader() { current_header_map_->addViaMove(std::move(current_header_field_), std::move(current_header_value_)); } + // Check if the number of headers exceeds the limit. + if (current_header_map_->size() > max_headers_count_) { + error_code_ = Http::Code::RequestHeaderFieldsTooLarge; + sendProtocolError(); + throw CodecProtocolException("headers size exceeds limit"); + } header_parsing_state_ = HeaderParsingState::Field; ASSERT(current_header_field_.empty()); @@ -431,8 +437,10 @@ void ConnectionImpl::onHeaderValue(const char* data, size_t length) { header_parsing_state_ = HeaderParsingState::Value; current_header_value_.append(data, length); - const uint32_t total = - current_header_field_.size() + current_header_value_.size() + current_header_map_->byteSize(); + // Verify that the cached value in byte size exists. + ASSERT(current_header_map_->byteSize().has_value()); + const uint32_t total = current_header_field_.size() + current_header_value_.size() + + current_header_map_->byteSize().value(); if (total > (max_headers_kb_ * 1024)) { error_code_ = Http::Code::RequestHeaderFieldsTooLarge; sendProtocolError(); @@ -443,6 +451,10 @@ void ConnectionImpl::onHeaderValue(const char* data, size_t length) { int ConnectionImpl::onHeadersCompleteBase() { ENVOY_CONN_LOG(trace, "headers complete", connection_); completeLastHeader(); + // Validate that the completed HeaderMap's cached byte size exists and is correct. + // This assert iterates over the HeaderMap. + ASSERT(current_header_map_->byteSize().has_value() && + current_header_map_->byteSize() == current_header_map_->byteSizeInternal()); if (!(parser_.http_major == 1 && parser_.http_minor == 1)) { // This is not necessarily true, but it's good enough since higher layers only care if this is // HTTP/1.1 or not. @@ -512,9 +524,10 @@ void ConnectionImpl::onResetStreamBase(StreamResetReason reason) { ServerConnectionImpl::ServerConnectionImpl(Network::Connection& connection, ServerConnectionCallbacks& callbacks, - Http1Settings settings, uint32_t max_request_headers_kb) - : ConnectionImpl(connection, HTTP_REQUEST, max_request_headers_kb), callbacks_(callbacks), - codec_settings_(settings) {} + Http1Settings settings, uint32_t max_request_headers_kb, + const uint32_t max_request_headers_count) + : ConnectionImpl(connection, HTTP_REQUEST, max_request_headers_kb, max_request_headers_count), + callbacks_(callbacks), codec_settings_(settings) {} void ServerConnectionImpl::onEncodeComplete() { ASSERT(active_request_); @@ -684,8 +697,10 @@ void ServerConnectionImpl::onBelowLowWatermark() { } } -ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, ConnectionCallbacks&) - : ConnectionImpl(connection, HTTP_RESPONSE, MAX_RESPONSE_HEADERS_KB) {} +ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, ConnectionCallbacks&, + const uint32_t max_response_headers_count) + : ConnectionImpl(connection, HTTP_RESPONSE, MAX_RESPONSE_HEADERS_KB, + max_response_headers_count) {} bool ClientConnectionImpl::cannotHaveBody() { if ((!pending_responses_.empty() && pending_responses_.front().head_request_) || diff --git a/source/common/http/http1/codec_impl.h b/source/common/http/http1/codec_impl.h index c0e7bf43186b..59867778043e 100644 --- a/source/common/http/http1/codec_impl.h +++ b/source/common/http/http1/codec_impl.h @@ -165,8 +165,8 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggablehd.type) { case NGHTTP2_HEADERS: { + // Verify that the final HeaderMap's byte size is under the limit before decoding headers. + // This assert iterates over the HeaderMap. + ASSERT(stream->headers_->byteSize().has_value() && + stream->headers_->byteSize().value() == stream->headers_->byteSizeInternal()); stream->remote_end_stream_ = frame->hd.flags & NGHTTP2_FLAG_END_STREAM; if (!stream->cookies_.empty()) { HeaderString key(Headers::get().Cookie); @@ -577,6 +581,12 @@ int ConnectionImpl::onFrameSend(const nghttp2_frame* frame) { case NGHTTP2_HEADERS: case NGHTTP2_DATA: { StreamImpl* stream = getStream(frame->hd.stream_id); + if (stream->headers_) { + // Verify that the final HeaderMap's byte size is under the limit before sending frames. + // This assert iterates over the HeaderMap. + ASSERT(stream->headers_->byteSize().has_value() && + stream->headers_->byteSize().value() == stream->headers_->byteSizeInternal()); + } stream->local_end_stream_sent_ = frame->hd.flags & NGHTTP2_FLAG_END_STREAM; break; } @@ -765,9 +775,11 @@ int ConnectionImpl::saveHeader(const nghttp2_frame* frame, HeaderString&& name, stats_.headers_cb_no_stream_.inc(); return 0; } - stream->saveHeader(std::move(name), std::move(value)); - if (stream->headers_->byteSize() > max_request_headers_kb_ * 1024) { + // Verify that the cached value in byte size exists. + ASSERT(stream->headers_->byteSize().has_value()); + if (stream->headers_->byteSize().value() > max_headers_kb_ * 1024 || + stream->headers_->size() > max_headers_count_) { // This will cause the library to reset/close the stream. stats_.header_overflow_.inc(); return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; @@ -1027,8 +1039,10 @@ ConnectionImpl::ClientHttp2Options::ClientHttp2Options(const Http2Settings& http ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, Http::ConnectionCallbacks& callbacks, Stats::Scope& stats, const Http2Settings& http2_settings, - const uint32_t max_request_headers_kb) - : ConnectionImpl(connection, stats, http2_settings, max_request_headers_kb), + const uint32_t max_response_headers_kb, + const uint32_t max_response_headers_count) + : ConnectionImpl(connection, stats, http2_settings, max_response_headers_kb, + max_response_headers_count), callbacks_(callbacks) { ClientHttp2Options client_http2_options(http2_settings); nghttp2_session_client_new2(&session_, http2_callbacks_.callbacks(), base(), @@ -1076,8 +1090,10 @@ int ClientConnectionImpl::onHeader(const nghttp2_frame* frame, HeaderString&& na ServerConnectionImpl::ServerConnectionImpl(Network::Connection& connection, Http::ServerConnectionCallbacks& callbacks, Stats::Scope& scope, const Http2Settings& http2_settings, - const uint32_t max_request_headers_kb) - : ConnectionImpl(connection, scope, http2_settings, max_request_headers_kb), + const uint32_t max_request_headers_kb, + const uint32_t max_request_headers_count) + : ConnectionImpl(connection, scope, http2_settings, max_request_headers_kb, + max_request_headers_count), callbacks_(callbacks) { Http2Options http2_options(http2_settings); nghttp2_session_server_new2(&session_, http2_callbacks_.callbacks(), base(), diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h index 460be6444a0e..ab3245d0fb66 100644 --- a/source/common/http/http2/codec_impl.h +++ b/source/common/http/http2/codec_impl.h @@ -83,9 +83,11 @@ void initializeNghttp2Logging(); class ConnectionImpl : public virtual Connection, protected Logger::Loggable { public: ConnectionImpl(Network::Connection& connection, Stats::Scope& stats, - const Http2Settings& http2_settings, const uint32_t max_request_headers_kb) + const Http2Settings& http2_settings, const uint32_t max_headers_kb, + const uint32_t max_headers_count) : stats_{ALL_HTTP2_CODEC_STATS(POOL_COUNTER_PREFIX(stats, "http2."))}, - connection_(connection), max_request_headers_kb_(max_request_headers_kb), + connection_(connection), max_headers_kb_(max_headers_kb), + max_headers_count_(max_headers_count), per_stream_buffer_limit_(http2_settings.initial_stream_window_size_), stream_error_on_invalid_http_messaging_( http2_settings.stream_error_on_invalid_http_messaging_), @@ -312,7 +314,8 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggablelog(parent_.downstream_headers_, upstream_headers_, upstream_trailers_, stream_info_); diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index 5197ae515a09..3aac107ec359 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -375,6 +375,7 @@ envoy_cc_library( "//source/common/network:utility_lib", "//source/common/protobuf", "//source/common/protobuf:utility_lib", + "//source/common/runtime:runtime_lib", "//source/extensions/transport_sockets:well_known_names", "//source/server:transport_socket_config_lib", "@envoy_api//envoy/api/v2/core:base_cc", diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 0a688e3d5934..7a6ad31a5970 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -31,6 +31,7 @@ #include "common/network/socket_option_factory.h" #include "common/protobuf/protobuf.h" #include "common/protobuf/utility.h" +#include "common/runtime/runtime_impl.h" #include "common/upstream/eds.h" #include "common/upstream/health_checker_impl.h" #include "common/upstream/logical_dns_cluster.h" @@ -530,6 +531,10 @@ ClusterInfoImpl::ClusterInfoImpl(const envoy::api::v2::Cluster& config, : runtime_(runtime), name_(config.name()), type_(config.type()), max_requests_per_connection_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_requests_per_connection, 0)), + max_response_headers_count_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( + config.common_http_protocol_options(), max_headers_count, + runtime_.snapshot().getInteger(Http::MaxResponseHeadersCountOverrideKey, + Http::DEFAULT_MAX_HEADERS_COUNT))), connect_timeout_( std::chrono::milliseconds(PROTOBUF_GET_MS_REQUIRED(config, connect_timeout))), per_connection_buffer_limit_bytes_( @@ -597,6 +602,7 @@ ClusterInfoImpl::ClusterInfoImpl(const envoy::api::v2::Cluster& config, idle_timeout_ = std::chrono::milliseconds( DurationUtil::durationToMilliseconds(config.common_http_protocol_options().idle_timeout())); } + if (config.has_eds_cluster_config()) { if (config.type() != envoy::api::v2::Cluster::EDS) { throw EnvoyException("eds_cluster_config set in a non-EDS cluster"); diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index f2ce1a7e4c3f..01d1f277a9cf 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -527,6 +527,7 @@ class ClusterInfoImpl : public ClusterInfo { } bool maintenanceMode() const override; uint64_t maxRequestsPerConnection() const override { return max_requests_per_connection_; } + uint32_t maxResponseHeadersCount() const override { return max_response_headers_count_; } const std::string& name() const override { return name_; } ResourceManager& resourceManager(ResourcePriority priority) const override; Network::TransportSocketFactory& transportSocketFactory() const override { @@ -567,6 +568,7 @@ class ClusterInfoImpl : public ClusterInfo { const std::string name_; const envoy::api::v2::Cluster::DiscoveryType type_; const uint64_t max_requests_per_connection_; + const uint32_t max_response_headers_count_; const std::chrono::milliseconds connect_timeout_; absl::optional idle_timeout_; const uint32_t per_connection_buffer_limit_bytes_; diff --git a/source/extensions/access_loggers/file/file_access_log_impl.cc b/source/extensions/access_loggers/file/file_access_log_impl.cc index 75409f34dadc..86a76dd4171d 100644 --- a/source/extensions/access_loggers/file/file_access_log_impl.cc +++ b/source/extensions/access_loggers/file/file_access_log_impl.cc @@ -24,9 +24,13 @@ void FileAccessLog::log(const Http::HeaderMap* request_headers, } if (!response_headers) { response_headers = &empty_headers; + } else { + const_cast(response_headers)->refreshByteSize(); } if (!response_trailers) { response_trailers = &empty_headers; + } else { + const_cast(response_trailers)->refreshByteSize(); } if (filter_) { diff --git a/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc b/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc index f32feaa20a13..37e3a388c3fb 100644 --- a/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc +++ b/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc @@ -171,9 +171,13 @@ void HttpGrpcAccessLog::log(const Http::HeaderMap* request_headers, } if (!response_headers) { response_headers = &empty_headers; + } else { + const_cast(response_headers)->refreshByteSize(); } if (!response_trailers) { response_trailers = &empty_headers; + } else { + const_cast(response_trailers)->refreshByteSize(); } if (filter_) { @@ -329,7 +333,7 @@ void HttpGrpcAccessLog::log(const Http::HeaderMap* request_headers, if (request_headers->EnvoyOriginalPath() != nullptr) { request_properties->set_original_path(request_headers->EnvoyOriginalPath()->value().c_str()); } - request_properties->set_request_headers_bytes(request_headers->byteSize()); + request_properties->set_request_headers_bytes(request_headers->byteSize().value()); request_properties->set_request_body_bytes(stream_info.bytesReceived()); if (request_headers->Method() != nullptr) { envoy::api::v2::core::RequestMethod method = @@ -354,7 +358,7 @@ void HttpGrpcAccessLog::log(const Http::HeaderMap* request_headers, if (stream_info.responseCode()) { response_properties->mutable_response_code()->set_value(stream_info.responseCode().value()); } - response_properties->set_response_headers_bytes(response_headers->byteSize()); + response_properties->set_response_headers_bytes(response_headers->byteSize().value()); response_properties->set_response_body_bytes(stream_info.bytesSent()); if (!response_headers_to_log_.empty()) { auto* logged_headers = response_properties->mutable_response_headers(); diff --git a/source/extensions/filters/http/rbac/rbac_filter.cc b/source/extensions/filters/http/rbac/rbac_filter.cc index f32aceb5cd3c..655792ba70b6 100644 --- a/source/extensions/filters/http/rbac/rbac_filter.cc +++ b/source/extensions/filters/http/rbac/rbac_filter.cc @@ -70,6 +70,10 @@ Http::FilterHeadersStatus RoleBasedAccessControlFilter::decodeHeaders(Http::Head if (shadow_engine.has_value()) { std::string shadow_resp_code = Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().EngineResultAllowed; + // Refresh headers byte size before checking if allowed. + // TODO(asraa): Remove this when entries in HeaderMap can no longer be modified by reference and + // HeaderMap holds an accurate internal byte size count. + headers.refreshByteSize(); if (shadow_engine->allowed(*callbacks_->connection(), headers, callbacks_->streamInfo().dynamicMetadata(), &effective_policy_id)) { ENVOY_LOG(debug, "shadow allowed"); @@ -99,6 +103,10 @@ Http::FilterHeadersStatus RoleBasedAccessControlFilter::decodeHeaders(Http::Head const auto& engine = config_->engine(callbacks_->route(), Filters::Common::RBAC::EnforcementMode::Enforced); if (engine.has_value()) { + // Refresh headers byte size before checking if allowed. + // TODO(asraa): Remove this when entries in HeaderMap can no longer be modified by reference and + // HeaderMap holds an accurate internal byte size count. + headers.refreshByteSize(); if (engine->allowed(*callbacks_->connection(), headers, callbacks_->streamInfo().dynamicMetadata(), nullptr)) { ENVOY_LOG(debug, "enforced allowed"); diff --git a/source/extensions/filters/network/http_connection_manager/BUILD b/source/extensions/filters/network/http_connection_manager/BUILD index 29f6420e6084..6a25e174c26a 100644 --- a/source/extensions/filters/network/http_connection_manager/BUILD +++ b/source/extensions/filters/network/http_connection_manager/BUILD @@ -36,6 +36,7 @@ envoy_cc_library( "//source/common/json:json_loader_lib", "//source/common/protobuf:utility_lib", "//source/common/router:rds_lib", + "//source/common/runtime:runtime_lib", "//source/extensions/filters/network:well_known_names", "//source/extensions/filters/network/common:factory_base_lib", ], diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 0014ccc55ef6..882e2b0b8623 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -140,7 +140,11 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( http1_settings_(Http::Utility::parseHttp1Settings(config.http_protocol_options())), max_request_headers_kb_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( config, max_request_headers_kb, Http::DEFAULT_MAX_REQUEST_HEADERS_KB)), - idle_timeout_(PROTOBUF_GET_OPTIONAL_MS(config, idle_timeout)), + max_request_headers_count_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( + config.common_http_protocol_options(), max_headers_count, + context.runtime().snapshot().getInteger(Http::MaxRequestHeadersCountOverrideKey, + Http::DEFAULT_MAX_HEADERS_COUNT))), + idle_timeout_(PROTOBUF_GET_OPTIONAL_MS(config.common_http_protocol_options(), idle_timeout)), stream_idle_timeout_( PROTOBUF_GET_MS_OR_DEFAULT(config, stream_idle_timeout, StreamIdleTimeoutMs)), request_timeout_(PROTOBUF_GET_MS_OR_DEFAULT(config, request_timeout, RequestTimeoutMs)), @@ -163,7 +167,12 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( 0 #endif ))) { - + // If idle_timeout_ was not configured in common_http_protocol_options, use value in deprecated + // idle_timeout field. + // TODO(asraa): Remove when idle_timeout is removed. + if (!idle_timeout_) { + idle_timeout_ = PROTOBUF_GET_OPTIONAL_MS(config, idle_timeout); + } route_config_provider_ = Router::RouteConfigProviderUtil::create(config, context_, stats_prefix_, route_config_provider_manager_); @@ -334,14 +343,15 @@ HttpConnectionManagerConfig::createCodec(Network::Connection& connection, switch (codec_type_) { case CodecType::HTTP1: return std::make_unique( - connection, callbacks, http1_settings_, maxRequestHeadersKb()); + connection, callbacks, http1_settings_, maxRequestHeadersKb(), maxRequestHeadersCount()); case CodecType::HTTP2: return std::make_unique( - connection, callbacks, context_.scope(), http2_settings_, maxRequestHeadersKb()); + connection, callbacks, context_.scope(), http2_settings_, maxRequestHeadersKb(), + maxRequestHeadersCount()); case CodecType::AUTO: - return Http::ConnectionManagerUtility::autoCreateCodec(connection, data, callbacks, - context_.scope(), http1_settings_, - http2_settings_, maxRequestHeadersKb()); + return Http::ConnectionManagerUtility::autoCreateCodec( + connection, data, callbacks, context_.scope(), http1_settings_, http2_settings_, + maxRequestHeadersKb(), maxRequestHeadersCount()); } NOT_REACHED_GCOVR_EXCL_LINE; diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index c1e7332b653c..6b1d8f28d6eb 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -101,6 +101,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, FilterChainFactory& filterFactory() override { return *this; } bool generateRequestId() override { return generate_request_id_; } uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } + uint32_t maxRequestHeadersCount() const override { return max_request_headers_count_; } absl::optional idleTimeout() const override { return idle_timeout_; } std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } std::chrono::milliseconds requestTimeout() const override { return request_timeout_; } @@ -158,6 +159,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, Http::TracingConnectionManagerConfigPtr tracing_config_; absl::optional user_agent_; const uint32_t max_request_headers_kb_; + const uint32_t max_request_headers_count_; absl::optional idle_timeout_; std::chrono::milliseconds stream_idle_timeout_; std::chrono::milliseconds request_timeout_; diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index 8fb1fde7d74e..6b5f0772f11f 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1160,7 +1160,7 @@ Http::ServerConnectionPtr AdminImpl::createCodec(Network::Connection& connection Http::ServerConnectionCallbacks& callbacks) { return Http::ConnectionManagerUtility::autoCreateCodec( connection, data, callbacks, server_.stats(), Http::Http1Settings(), Http::Http2Settings(), - maxRequestHeadersKb()); + maxRequestHeadersKb(), maxRequestHeadersCount()); } bool AdminImpl::createNetworkFilterChain(Network::Connection& connection, diff --git a/source/server/http/admin.h b/source/server/http/admin.h index 7bffdeb2b036..dbff9570777b 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -100,6 +100,7 @@ class AdminImpl : public Admin, bool generateRequestId() override { return false; } absl::optional idleTimeout() const override { return idle_timeout_; } 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 {}; } std::chrono::milliseconds requestTimeout() const override { return {}; } std::chrono::milliseconds delayedCloseTimeout() const override { return {}; } @@ -308,6 +309,7 @@ class AdminImpl : public Admin, NullRouteConfigProvider route_config_provider_; std::list handlers_; const uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB}; + const uint32_t max_request_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; absl::optional idle_timeout_; absl::optional user_agent_; Http::SlowDateProviderImpl date_provider_; diff --git a/test/common/http/codec_impl_fuzz_test.cc b/test/common/http/codec_impl_fuzz_test.cc index 86ced958d7f1..a5c7d6728144 100644 --- a/test/common/http/codec_impl_fuzz_test.cc +++ b/test/common/http/codec_impl_fuzz_test.cc @@ -350,29 +350,34 @@ void codecFuzz(const test::common::http::CodecImplFuzzTestCase& input, HttpVersi const Http2Settings client_http2settings{fromHttp2Settings(input.h2_settings().client())}; NiceMock client_callbacks; uint32_t max_request_headers_kb = Http::DEFAULT_MAX_REQUEST_HEADERS_KB; + uint32_t max_request_headers_count = Http::DEFAULT_MAX_HEADERS_COUNT; + uint32_t max_response_headers_count = Http::DEFAULT_MAX_HEADERS_COUNT; + ClientConnectionPtr client; ServerConnectionPtr server; const bool http2 = http_version == HttpVersion::Http2; if (http2) { - client = absl::make_unique(client_connection, client_callbacks, - stats_store, client_http2settings, - max_request_headers_kb); + client = std::make_unique( + client_connection, client_callbacks, stats_store, client_http2settings, + max_request_headers_kb, max_response_headers_count); } else { - client = absl::make_unique(client_connection, client_callbacks); + client = std::make_unique(client_connection, client_callbacks, + max_response_headers_count); } NiceMock server_connection; NiceMock server_callbacks; if (http2) { const Http2Settings server_http2settings{fromHttp2Settings(input.h2_settings().server())}; - server = absl::make_unique(server_connection, server_callbacks, - stats_store, server_http2settings, - max_request_headers_kb); + server = std::make_unique( + server_connection, server_callbacks, stats_store, server_http2settings, + max_request_headers_kb, max_request_headers_count); } else { const Http1Settings server_http1settings{fromHttp1Settings(input.h1_settings().server())}; server = absl::make_unique( - server_connection, server_callbacks, server_http1settings, max_request_headers_kb); + server_connection, server_callbacks, server_http1settings, max_request_headers_kb, + max_request_headers_count); } ReorderBuffer client_write_buf{*server}; diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index 07762102ba9a..641e2c4c0599 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -91,6 +91,7 @@ class FuzzConfig : public ConnectionManagerConfig { FilterChainFactory& filterFactory() override { return filter_factory_; } bool generateRequestId() override { return true; } uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } + uint32_t maxRequestHeadersCount() const override { return max_request_headers_count_; } absl::optional idleTimeout() const override { return idle_timeout_; } std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } std::chrono::milliseconds requestTimeout() const override { return request_timeout_; } @@ -133,6 +134,7 @@ class FuzzConfig : public ConnectionManagerConfig { ConnectionManagerTracingStats tracing_stats_; ConnectionManagerListenerStats listener_stats_; uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB}; + uint32_t max_request_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; absl::optional idle_timeout_; 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 889da9ed6705..433fefddb94c 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -250,6 +250,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan FilterChainFactory& filterFactory() override { return filter_factory_; } bool generateRequestId() override { return true; } uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } + uint32_t maxRequestHeadersCount() const override { return max_request_headers_count_; } absl::optional idleTimeout() const override { return idle_timeout_; } std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } std::chrono::milliseconds requestTimeout() const override { return request_timeout_; } @@ -301,6 +302,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan std::vector set_current_client_cert_details_; absl::optional user_agent_; uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB}; + uint32_t max_request_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; absl::optional idle_timeout_; std::chrono::milliseconds stream_idle_timeout_{}; std::chrono::milliseconds request_timeout_{}; @@ -4060,32 +4062,6 @@ TEST_F(HttpConnectionManagerImplTest, DisableKeepAliveWhenOverloaded) { EXPECT_EQ(1U, stats_.named_.downstream_cx_overload_disable_keepalive_.value()); } -TEST_F(HttpConnectionManagerImplTest, OverlyLongHeadersRejected) { - setup(false, ""); - - std::string response_code; - std::string response_body; - EXPECT_CALL(*codec_, dispatch(_)).Times(1).WillOnce(Invoke([&](Buffer::Instance&) -> void { - StreamDecoder* decoder = &conn_manager_->newStream(response_encoder_); - HeaderMapPtr headers{ - new TestHeaderMapImpl{{":authority", "host"}, {":path", "/"}, {":method", "GET"}}}; - headers->addCopy(LowerCaseString("Foo"), std::string(60 * 1024, 'a')); - - EXPECT_CALL(response_encoder_, encodeHeaders(_, true)) - .WillOnce(Invoke([&response_code](const HeaderMap& headers, bool) -> void { - response_code = headers.Status()->value().c_str(); - })); - decoder->decodeHeaders(std::move(headers), true); - conn_manager_->newStream(response_encoder_); - })); - - Buffer::OwnedImpl fake_input("1234"); - conn_manager_->onData(fake_input, false); // kick off request - - EXPECT_EQ("431", response_code); - EXPECT_EQ("", response_body); -} - TEST_F(HttpConnectionManagerImplTest, OverlyLongHeadersAcceptedIfConfigured) { max_request_headers_kb_ = 62; setup(false, ""); diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index d1bc974e8b8c..93ad49405e3e 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -51,6 +51,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_METHOD0(filterFactory, FilterChainFactory&()); MOCK_METHOD0(generateRequestId, bool()); MOCK_CONST_METHOD0(maxRequestHeadersKb, uint32_t()); + MOCK_CONST_METHOD0(maxRequestHeadersCount, uint32_t()); MOCK_CONST_METHOD0(idleTimeout, absl::optional()); MOCK_CONST_METHOD0(streamIdleTimeout, std::chrono::milliseconds()); MOCK_CONST_METHOD0(requestTimeout, std::chrono::milliseconds()); diff --git a/test/common/http/header_map_impl_speed_test.cc b/test/common/http/header_map_impl_speed_test.cc index b8b67b2c155f..fa78cfc88839 100644 --- a/test/common/http/header_map_impl_speed_test.cc +++ b/test/common/http/header_map_impl_speed_test.cc @@ -104,7 +104,7 @@ static void HeaderMapImplGetByteSize(benchmark::State& state) { addDummyHeaders(headers, state.range(0)); uint64_t size = 0; for (auto _ : state) { - size += headers.byteSize(); + size += headers.byteSize().value(); } benchmark::DoNotOptimize(size); } diff --git a/test/common/http/header_map_impl_test.cc b/test/common/http/header_map_impl_test.cc index 3962ed6ba0a1..1fbaaef33657 100644 --- a/test/common/http/header_map_impl_test.cc +++ b/test/common/http/header_map_impl_test.cc @@ -289,6 +289,7 @@ TEST(HeaderMapImplTest, InlineInsert) { HeaderMapImpl headers; EXPECT_TRUE(headers.empty()); EXPECT_EQ(0, headers.size()); + EXPECT_EQ(headers.byteSize().value(), 0); EXPECT_EQ(nullptr, headers.Host()); headers.insertHost().value(std::string("hello")); EXPECT_FALSE(headers.empty()); @@ -298,6 +299,19 @@ TEST(HeaderMapImplTest, InlineInsert) { EXPECT_STREQ("hello", headers.get(Headers::get().Host)->value().c_str()); } +// Utility function for testing byteSize() against a manual byte count. +uint64_t countBytesForTest(const HeaderMapImpl& headers) { + uint64_t byte_size = 0; + headers.iterate( + [](const Http::HeaderEntry& header, void* context) -> Http::HeaderMap::Iterate { + auto* byte_size = static_cast(context); + *byte_size += header.key().getStringView().size() + header.value().getStringView().size(); + return Http::HeaderMap::Iterate::Continue; + }, + &byte_size); + return byte_size; +} + TEST(HeaderMapImplTest, MoveIntoInline) { HeaderMapImpl headers; HeaderString key; @@ -315,6 +329,7 @@ TEST(HeaderMapImplTest, MoveIntoInline) { headers.addViaMove(std::move(key2), std::move(value2)); EXPECT_STREQ("cache-control", headers.CacheControl()->key().c_str()); EXPECT_STREQ("hello,there", headers.CacheControl()->value().c_str()); + EXPECT_EQ(headers.refreshByteSize(), countBytesForTest(headers)); } TEST(HeaderMapImplTest, Remove) { @@ -324,6 +339,7 @@ TEST(HeaderMapImplTest, Remove) { LowerCaseString static_key("hello"); std::string ref_value("value"); headers.addReference(static_key, ref_value); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); EXPECT_STREQ("value", headers.get(static_key)->value().c_str()); EXPECT_EQ(HeaderString::Type::Reference, headers.get(static_key)->value().type()); EXPECT_EQ(1UL, headers.size()); @@ -332,9 +348,11 @@ TEST(HeaderMapImplTest, Remove) { EXPECT_EQ(nullptr, headers.get(static_key)); EXPECT_EQ(0UL, headers.size()); EXPECT_TRUE(headers.empty()); + EXPECT_EQ(headers.refreshByteSize(), 0); // Add and remove by inline. headers.insertContentLength().value(5); + EXPECT_EQ(headers.refreshByteSize(), countBytesForTest(headers)); EXPECT_STREQ("5", headers.ContentLength()->value().c_str()); EXPECT_EQ(1UL, headers.size()); EXPECT_FALSE(headers.empty()); @@ -342,16 +360,19 @@ TEST(HeaderMapImplTest, Remove) { EXPECT_EQ(nullptr, headers.ContentLength()); EXPECT_EQ(0UL, headers.size()); EXPECT_TRUE(headers.empty()); + EXPECT_EQ(headers.refreshByteSize(), countBytesForTest(headers)); // Add inline and remove by name. headers.insertContentLength().value(5); EXPECT_STREQ("5", headers.ContentLength()->value().c_str()); EXPECT_EQ(1UL, headers.size()); EXPECT_FALSE(headers.empty()); + EXPECT_EQ(headers.refreshByteSize(), countBytesForTest(headers)); headers.remove(Headers::get().ContentLength); EXPECT_EQ(nullptr, headers.ContentLength()); EXPECT_EQ(0UL, headers.size()); EXPECT_TRUE(headers.empty()); + EXPECT_EQ(headers.refreshByteSize(), 0); } TEST(HeaderMapImplTest, RemoveRegex) { @@ -369,9 +390,11 @@ TEST(HeaderMapImplTest, RemoveRegex) { headers.addReference(key3, "value"); headers.addReference(key4, "value"); headers.addReference(key5, "value"); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); // Test removing the first header, middle headers, and the end header. headers.removePrefix(LowerCaseString("x-prefix-")); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); EXPECT_EQ(nullptr, headers.get(key1)); EXPECT_NE(nullptr, headers.get(key2)); EXPECT_EQ(nullptr, headers.get(key3)); @@ -379,7 +402,9 @@ TEST(HeaderMapImplTest, RemoveRegex) { EXPECT_EQ(nullptr, headers.get(key5)); // Remove all headers. + headers.refreshByteSize(); headers.removePrefix(LowerCaseString("")); + EXPECT_EQ(headers.byteSize().value(), 0); EXPECT_EQ(nullptr, headers.get(key2)); EXPECT_EQ(nullptr, headers.get(key4)); @@ -388,8 +413,10 @@ TEST(HeaderMapImplTest, RemoveRegex) { EXPECT_STREQ("5", headers.ContentLength()->value().c_str()); EXPECT_EQ(1UL, headers.size()); EXPECT_FALSE(headers.empty()); + EXPECT_EQ(headers.refreshByteSize(), countBytesForTest(headers)); headers.removePrefix(LowerCaseString("content")); EXPECT_EQ(nullptr, headers.ContentLength()); + EXPECT_EQ(headers.refreshByteSize(), 0); } TEST(HeaderMapImplTest, SetRemovesAllValues) { @@ -407,6 +434,7 @@ TEST(HeaderMapImplTest, SetRemovesAllValues) { headers.addReference(key2, ref_value2); headers.addReference(key1, ref_value3); headers.addReference(key1, ref_value4); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); typedef testing::MockFunction MockCb; @@ -452,6 +480,7 @@ TEST(HeaderMapImplTest, DoubleInlineAdd) { const std::string bar("bar"); headers.addReference(Headers::get().ContentLength, foo); headers.addReference(Headers::get().ContentLength, bar); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); EXPECT_STREQ("foo,bar", headers.ContentLength()->value().c_str()); EXPECT_EQ(1UL, headers.size()); } @@ -459,6 +488,7 @@ TEST(HeaderMapImplTest, DoubleInlineAdd) { HeaderMapImpl headers; headers.addReferenceKey(Headers::get().ContentLength, "foo"); headers.addReferenceKey(Headers::get().ContentLength, "bar"); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); EXPECT_STREQ("foo,bar", headers.ContentLength()->value().c_str()); EXPECT_EQ(1UL, headers.size()); } @@ -466,6 +496,7 @@ TEST(HeaderMapImplTest, DoubleInlineAdd) { HeaderMapImpl headers; headers.addReferenceKey(Headers::get().ContentLength, 5); headers.addReferenceKey(Headers::get().ContentLength, 6); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); EXPECT_STREQ("5,6", headers.ContentLength()->value().c_str()); EXPECT_EQ(1UL, headers.size()); } @@ -474,6 +505,7 @@ TEST(HeaderMapImplTest, DoubleInlineAdd) { const std::string foo("foo"); headers.addReference(Headers::get().ContentLength, foo); headers.addReferenceKey(Headers::get().ContentLength, 6); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); EXPECT_STREQ("foo,6", headers.ContentLength()->value().c_str()); EXPECT_EQ(1UL, headers.size()); } @@ -483,6 +515,7 @@ TEST(HeaderMapImplTest, DoubleInlineSet) { HeaderMapImpl headers; headers.setReferenceKey(Headers::get().ContentType, "blah"); headers.setReferenceKey(Headers::get().ContentType, "text/html"); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); EXPECT_STREQ("text/html", headers.ContentType()->value().c_str()); EXPECT_EQ(1UL, headers.size()); } @@ -491,6 +524,7 @@ TEST(HeaderMapImplTest, AddReferenceKey) { HeaderMapImpl headers; LowerCaseString foo("hello"); headers.addReferenceKey(foo, "world"); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); EXPECT_NE("world", headers.get(foo)->value().c_str()); EXPECT_STREQ("world", headers.get(foo)->value().c_str()); } @@ -499,10 +533,13 @@ TEST(HeaderMapImplTest, SetReferenceKey) { HeaderMapImpl headers; LowerCaseString foo("hello"); headers.setReferenceKey(foo, "world"); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); EXPECT_NE("world", headers.get(foo)->value().c_str()); EXPECT_STREQ("world", headers.get(foo)->value().c_str()); + headers.refreshByteSize(); headers.setReferenceKey(foo, "monde"); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); EXPECT_NE("monde", headers.get(foo)->value().c_str()); EXPECT_STREQ("monde", headers.get(foo)->value().c_str()); } @@ -512,7 +549,9 @@ TEST(HeaderMapImplTest, AddCopy) { // Start with a string value. std::unique_ptr lcKeyPtr(new LowerCaseString("hello")); + EXPECT_EQ(headers.refreshByteSize(), countBytesForTest(headers)); headers.addCopy(*lcKeyPtr, "world"); + EXPECT_EQ(headers.refreshByteSize(), countBytesForTest(headers)); const HeaderString& value = headers.get(*lcKeyPtr)->value(); @@ -532,14 +571,19 @@ TEST(HeaderMapImplTest, AddCopy) { // // addReferenceKey and addCopy can both add multiple instances of a // given header, so we need to delete the old "hello" header. + // Test that removing will return 0 byte size. + headers.refreshByteSize(); headers.remove(LowerCaseString("hello")); + EXPECT_EQ(headers.byteSize().value(), 0); // Build "hello" with string concatenation to make it unlikely that the // compiler is just reusing the same string constant for everything. lcKeyPtr = std::make_unique(std::string("he") + "llo"); EXPECT_STREQ("hello", lcKeyPtr->get().c_str()); + headers.refreshByteSize(); headers.addCopy(*lcKeyPtr, 42); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); const HeaderString& value3 = headers.get(*lcKeyPtr)->value(); @@ -565,15 +609,20 @@ TEST(HeaderMapImplTest, AddCopy) { headers.addCopy(cache_control, "max-age=1345"); EXPECT_STREQ("max-age=1345", headers.get(cache_control)->value().c_str()); EXPECT_STREQ("max-age=1345", headers.CacheControl()->value().c_str()); + EXPECT_EQ(headers.refreshByteSize(), countBytesForTest(headers)); headers.addCopy(cache_control, "public"); + EXPECT_EQ(headers.refreshByteSize(), countBytesForTest(headers)); EXPECT_STREQ("max-age=1345,public", headers.get(cache_control)->value().c_str()); headers.addCopy(cache_control, ""); + EXPECT_EQ(headers.refreshByteSize(), countBytesForTest(headers)); EXPECT_STREQ("max-age=1345,public", headers.get(cache_control)->value().c_str()); headers.addCopy(cache_control, 123); + EXPECT_EQ(headers.refreshByteSize(), countBytesForTest(headers)); EXPECT_STREQ("max-age=1345,public,123", headers.get(cache_control)->value().c_str()); headers.addCopy(cache_control, std::numeric_limits::max()); EXPECT_STREQ("max-age=1345,public,123,18446744073709551615", headers.get(cache_control)->value().c_str()); + EXPECT_EQ(headers.refreshByteSize(), countBytesForTest(headers)); } TEST(HeaderMapImplTest, Equality) { @@ -593,6 +642,7 @@ TEST(HeaderMapImplTest, LargeCharInHeader) { LowerCaseString static_key("\x90hello"); std::string ref_value("value"); headers.addReference(static_key, ref_value); + EXPECT_EQ(headers.byteSize().value(), countBytesForTest(headers)); EXPECT_STREQ("value", headers.get(static_key)->value().c_str()); } @@ -745,6 +795,7 @@ TEST(HeaderMapImplTest, PseudoHeaderOrder) { { LowerCaseString foo("hello"); Http::TestHeaderMapImpl headers{}; + EXPECT_EQ(headers.refreshByteSize(), 0); EXPECT_EQ(0UL, headers.size()); EXPECT_TRUE(headers.empty()); diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 7f367ea5b544..b72989e7cc43 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -27,12 +27,23 @@ using testing::ReturnRef; namespace Envoy { namespace Http { namespace Http1 { +namespace { +std::string createHeaderFragment(int num_headers) { + // Create a header field with num_headers headers. + std::string headers; + for (int i = 0; i < num_headers; i++) { + headers += "header" + std::to_string(i) + ": " + "\r\n"; + } + return headers; +} +} // namespace class Http1ServerConnectionImplTest : public testing::Test { public: void initialize() { - codec_ = std::make_unique(connection_, callbacks_, codec_settings_, - max_request_headers_kb_); + codec_ = + std::make_unique(connection_, callbacks_, codec_settings_, + max_request_headers_kb_, max_request_headers_count_); } NiceMock connection_; @@ -43,9 +54,12 @@ class Http1ServerConnectionImplTest : public testing::Test { void expectHeadersTest(Protocol p, bool allow_absolute_url, Buffer::OwnedImpl& buffer, TestHeaderMapImpl& expected_headers); void expect400(Protocol p, bool allow_absolute_url, Buffer::OwnedImpl& buffer); + void testRequestHeadersExceedLimit(std::string header_string); + void testRequestHeadersAccepted(std::string header_string); protected: uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB}; + uint32_t max_request_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; }; void Http1ServerConnectionImplTest::expect400(Protocol p, bool allow_absolute_url, @@ -57,8 +71,9 @@ void Http1ServerConnectionImplTest::expect400(Protocol p, bool allow_absolute_ur if (allow_absolute_url) { codec_settings_.allow_absolute_url_ = allow_absolute_url; - codec_ = std::make_unique(connection_, callbacks_, codec_settings_, - max_request_headers_kb_); + codec_ = + std::make_unique(connection_, callbacks_, codec_settings_, + max_request_headers_kb_, max_request_headers_count_); } Http::MockStreamDecoder decoder; @@ -77,8 +92,9 @@ void Http1ServerConnectionImplTest::expectHeadersTest(Protocol p, bool allow_abs // Make a new 'codec' with the right settings if (allow_absolute_url) { codec_settings_.allow_absolute_url_ = allow_absolute_url; - codec_ = std::make_unique(connection_, callbacks_, codec_settings_, - max_request_headers_kb_); + codec_ = + std::make_unique(connection_, callbacks_, codec_settings_, + max_request_headers_kb_, max_request_headers_count_); } Http::MockStreamDecoder decoder; @@ -90,6 +106,41 @@ void Http1ServerConnectionImplTest::expectHeadersTest(Protocol p, bool allow_abs EXPECT_EQ(p, codec_->protocol()); } +void Http1ServerConnectionImplTest::testRequestHeadersExceedLimit(std::string header_string) { + initialize(); + + std::string exception_reason; + NiceMock decoder; + Http::StreamEncoder* response_encoder = nullptr; + EXPECT_CALL(callbacks_, newStream(_, _)) + .WillOnce(Invoke([&](Http::StreamEncoder& encoder, bool) -> Http::StreamDecoder& { + response_encoder = &encoder; + return decoder; + })); + + Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\n"); + codec_->dispatch(buffer); + buffer = Buffer::OwnedImpl(header_string + "\r\n"); + EXPECT_THROW_WITH_MESSAGE(codec_->dispatch(buffer), EnvoyException, "headers size exceeds limit"); +} + +void Http1ServerConnectionImplTest::testRequestHeadersAccepted(std::string header_string) { + initialize(); + + NiceMock decoder; + Http::StreamEncoder* response_encoder = nullptr; + EXPECT_CALL(callbacks_, newStream(_, _)) + .WillOnce(Invoke([&](Http::StreamEncoder& encoder, bool) -> Http::StreamDecoder& { + response_encoder = &encoder; + return decoder; + })); + + Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\n"); + codec_->dispatch(buffer); + buffer = Buffer::OwnedImpl(header_string + "\r\n"); + codec_->dispatch(buffer); +} + TEST_F(Http1ServerConnectionImplTest, EmptyHeader) { initialize(); @@ -777,11 +828,15 @@ TEST_F(Http1ServerConnectionImplTest, WatermarkTest) { class Http1ClientConnectionImplTest : public testing::Test { public: - void initialize() { codec_ = std::make_unique(connection_, callbacks_); } + void initialize() { + codec_ = std::make_unique(connection_, callbacks_, + max_response_headers_count_); + } NiceMock connection_; NiceMock callbacks_; std::unique_ptr codec_; + uint32_t max_response_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; }; TEST_F(Http1ClientConnectionImplTest, SimpleGet) { @@ -1110,27 +1165,19 @@ TEST_F(Http1ClientConnectionImplTest, HighwatermarkMultipleResponses) { static_cast(codec_.get()) ->onUnderlyingConnectionBelowWriteBufferLowWatermark(); } -TEST_F(Http1ServerConnectionImplTest, TestLargeRequestHeadersRejected) { +TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersRejected) { // Default limit of 60 KiB - initialize(); - - std::string exception_reason; - NiceMock decoder; - Http::StreamEncoder* response_encoder = nullptr; - EXPECT_CALL(callbacks_, newStream(_, _)) - .WillOnce(Invoke([&](Http::StreamEncoder& encoder, bool) -> Http::StreamDecoder& { - response_encoder = &encoder; - return decoder; - })); - - Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\n"); - codec_->dispatch(buffer); std::string long_string = "big: " + std::string(60 * 1024, 'q') + "\r\n"; - buffer = Buffer::OwnedImpl(long_string); - EXPECT_THROW_WITH_MESSAGE(codec_->dispatch(buffer), EnvoyException, "headers size exceeds limit"); + testRequestHeadersExceedLimit(long_string); +} + +// Tests that the default limit for the number of request headers is 100. +TEST_F(Http1ServerConnectionImplTest, ManyRequestHeadersRejected) { + // Send a request with 101 headers. + testRequestHeadersExceedLimit(createHeaderFragment(101)); } -TEST_F(Http1ServerConnectionImplTest, TestLargeRequestHeadersSplitRejected) { +TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersSplitRejected) { // Default limit of 60 KiB initialize(); @@ -1155,10 +1202,13 @@ TEST_F(Http1ServerConnectionImplTest, TestLargeRequestHeadersSplitRejected) { EXPECT_THROW_WITH_MESSAGE(codec_->dispatch(buffer), EnvoyException, "headers size exceeds limit"); } -TEST_F(Http1ServerConnectionImplTest, TestLargeRequestHeadersAccepted) { - max_request_headers_kb_ = 65; +// Tests that the 101th request header causes overflow with the default max number of request +// headers. +TEST_F(Http1ServerConnectionImplTest, ManyRequestHeadersSplitRejected) { + // Default limit of 100. initialize(); + std::string exception_reason; NiceMock decoder; Http::StreamEncoder* response_encoder = nullptr; EXPECT_CALL(callbacks_, newStream(_, _)) @@ -1166,34 +1216,39 @@ TEST_F(Http1ServerConnectionImplTest, TestLargeRequestHeadersAccepted) { response_encoder = &encoder; return decoder; })); - Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\n"); codec_->dispatch(buffer); - std::string long_string = "big: " + std::string(64 * 1024, 'q') + "\r\n"; - buffer = Buffer::OwnedImpl(long_string); + + // Dispatch 100 headers. + buffer = Buffer::OwnedImpl(createHeaderFragment(100)); codec_->dispatch(buffer); -} -TEST_F(Http1ServerConnectionImplTest, TestLargeRequestHeadersAcceptedMaxConfigurable) { - max_request_headers_kb_ = 96; - initialize(); + // The final 101th header should induce overflow. + buffer = Buffer::OwnedImpl("header101:\r\n\r\n"); + EXPECT_THROW_WITH_MESSAGE(codec_->dispatch(buffer), EnvoyException, "headers size exceeds limit"); +} - NiceMock decoder; - Http::StreamEncoder* response_encoder = nullptr; - EXPECT_CALL(callbacks_, newStream(_, _)) - .WillOnce(Invoke([&](Http::StreamEncoder& encoder, bool) -> Http::StreamDecoder& { - response_encoder = &encoder; - return decoder; - })); +TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersAccepted) { + max_request_headers_kb_ = 65; + std::string long_string = "big: " + std::string(64 * 1024, 'q') + "\r\n"; + testRequestHeadersAccepted(long_string); +} - Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\n"); - codec_->dispatch(buffer); +TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersAcceptedMaxConfigurable) { + max_request_headers_kb_ = 96; std::string long_string = "big: " + std::string(95 * 1024, 'q') + "\r\n"; - buffer = Buffer::OwnedImpl(long_string); - codec_->dispatch(buffer); + testRequestHeadersAccepted(long_string); +} + +// Tests that the number of request headers is configurable. +TEST_F(Http1ServerConnectionImplTest, ManyRequestHeadersAccepted) { + max_request_headers_count_ = 150; + // Create a request with 150 headers. + testRequestHeadersAccepted(createHeaderFragment(150)); } -TEST_F(Http1ClientConnectionImplTest, TestLargeResponseHeadersRejected) { +// Tests that response headers of 80 kB fails. +TEST_F(Http1ClientConnectionImplTest, LargeResponseHeadersRejected) { initialize(); NiceMock response_decoder; @@ -1208,7 +1263,8 @@ TEST_F(Http1ClientConnectionImplTest, TestLargeResponseHeadersRejected) { EXPECT_THROW_WITH_MESSAGE(codec_->dispatch(buffer), EnvoyException, "headers size exceeds limit"); } -TEST_F(Http1ClientConnectionImplTest, TestLargeResponseHeadersAccepted) { +// Tests that the size of response headers for HTTP/1 must be under 80 kB. +TEST_F(Http1ClientConnectionImplTest, LargeResponseHeadersAccepted) { initialize(); NiceMock response_decoder; @@ -1223,6 +1279,39 @@ TEST_F(Http1ClientConnectionImplTest, TestLargeResponseHeadersAccepted) { codec_->dispatch(buffer); } +// Exception called when the number of response headers exceeds the default value of 100. +TEST_F(Http1ClientConnectionImplTest, ManyResponseHeadersRejected) { + initialize(); + + NiceMock response_decoder; + Http::StreamEncoder& request_encoder = codec_->newStream(response_decoder); + TestHeaderMapImpl headers{{":method", "GET"}, {":path", "/"}, {":authority", "host"}}; + request_encoder.encodeHeaders(headers, true); + + Buffer::OwnedImpl buffer("HTTP/1.1 200 OK\r\nContent-Length: 0\r\n"); + codec_->dispatch(buffer); + buffer = Buffer::OwnedImpl(createHeaderFragment(101) + "\r\n"); + EXPECT_THROW_WITH_MESSAGE(codec_->dispatch(buffer), EnvoyException, "headers size exceeds limit"); +} + +// Tests that the number of response headers is configurable. +TEST_F(Http1ClientConnectionImplTest, ManyResponseHeadersAccepted) { + max_response_headers_count_ = 152; + + initialize(); + + NiceMock response_decoder; + Http::StreamEncoder& request_encoder = codec_->newStream(response_decoder); + TestHeaderMapImpl headers{{":method", "GET"}, {":path", "/"}, {":authority", "host"}}; + request_encoder.encodeHeaders(headers, true); + + Buffer::OwnedImpl buffer("HTTP/1.1 200 OK\r\nContent-Length: 0\r\n"); + codec_->dispatch(buffer); + // Response already contains one header. + buffer = Buffer::OwnedImpl(createHeaderFragment(150) + "\r\n"); + codec_->dispatch(buffer); +} + } // namespace Http1 } // namespace Http } // namespace Envoy diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index 940d4b6ea055..0aa7a517bda3 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -65,12 +65,12 @@ class Http2CodecImplTestFixture { virtual void initialize() { Http2SettingsFromTuple(client_http2settings_, client_settings_); Http2SettingsFromTuple(server_http2settings_, server_settings_); - client_ = std::make_unique(client_connection_, client_callbacks_, - stats_store_, client_http2settings_, - max_request_headers_kb_); - server_ = std::make_unique(server_connection_, server_callbacks_, - stats_store_, server_http2settings_, - max_request_headers_kb_); + client_ = std::make_unique( + client_connection_, client_callbacks_, stats_store_, client_http2settings_, + max_request_headers_kb_, max_response_headers_count_); + server_ = std::make_unique( + server_connection_, server_callbacks_, stats_store_, server_http2settings_, + max_request_headers_kb_, max_request_headers_count_); request_encoder_ = &client_->newStream(response_decoder_); setupDefaultConnectionMocks(); @@ -162,6 +162,8 @@ class Http2CodecImplTestFixture { char corrupt_with_char_{'\0'}; uint32_t max_request_headers_kb_ = Http::DEFAULT_MAX_REQUEST_HEADERS_KB; + uint32_t max_request_headers_count_ = Http::DEFAULT_MAX_HEADERS_COUNT; + uint32_t max_response_headers_count_ = Http::DEFAULT_MAX_HEADERS_COUNT; uint32_t max_outbound_frames_ = Http2Settings::DEFAULT_MAX_OUTBOUND_FRAMES; uint32_t max_outbound_control_frames_ = Http2Settings::DEFAULT_MAX_OUTBOUND_CONTROL_FRAMES; uint32_t max_consecutive_inbound_frames_with_empty_payload_ = @@ -870,12 +872,12 @@ class Http2CodecImplStreamLimitTest : public Http2CodecImplTest {}; TEST_P(Http2CodecImplStreamLimitTest, MaxClientStreams) { Http2SettingsFromTuple(client_http2settings_, ::testing::get<0>(GetParam())); Http2SettingsFromTuple(server_http2settings_, ::testing::get<1>(GetParam())); - client_ = std::make_unique(client_connection_, client_callbacks_, - stats_store_, client_http2settings_, - max_request_headers_kb_); - server_ = std::make_unique(server_connection_, server_callbacks_, - stats_store_, server_http2settings_, - max_request_headers_kb_); + client_ = std::make_unique( + client_connection_, client_callbacks_, stats_store_, client_http2settings_, + max_request_headers_kb_, max_response_headers_count_); + server_ = std::make_unique( + server_connection_, server_callbacks_, stats_store_, server_http2settings_, + max_request_headers_kb_, max_request_headers_count_); for (int i = 0; i < 101; ++i) { request_encoder_ = &client_->newStream(response_decoder_); @@ -975,7 +977,8 @@ TEST(Http2CodecUtility, reconstituteCrumbledCookies) { } } -TEST_P(Http2CodecImplTest, TestLargeRequestHeadersInvokeResetStream) { +// Tests request headers whose size is larger than the default limit of 60K. +TEST_P(Http2CodecImplTest, LargeRequestHeadersInvokeResetStream) { initialize(); TestHeaderMapImpl request_headers; @@ -986,7 +989,8 @@ TEST_P(Http2CodecImplTest, TestLargeRequestHeadersInvokeResetStream) { request_encoder_->encodeHeaders(request_headers, false); } -TEST_P(Http2CodecImplTest, TestLargeRequestHeadersAccepted) { +// Large request headers are accepted when max limit configured. +TEST_P(Http2CodecImplTest, LargeRequestHeadersAccepted) { max_request_headers_kb_ = 64; initialize(); @@ -1000,30 +1004,78 @@ TEST_P(Http2CodecImplTest, TestLargeRequestHeadersAccepted) { request_encoder_->encodeHeaders(request_headers, false); } -TEST_P(Http2CodecImplTest, TestLargeRequestHeadersAtLimitAccepted) { +// Tests stream reset when the number of request headers exceeds the default maximum of 100. +TEST_P(Http2CodecImplTest, ManyRequestHeadersInvokeResetStream) { + initialize(); + + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + for (int i = 0; i < 100; i++) { + request_headers.addCopy(std::to_string(i), ""); + } + EXPECT_CALL(server_stream_callbacks_, onResetStream(_, _)).Times(1); + request_encoder_->encodeHeaders(request_headers, false); +} + +// Tests that max number of request headers is configurable. +TEST_P(Http2CodecImplTest, ManyRequestHeadersAccepted) { + max_request_headers_count_ = 150; + initialize(); + + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + for (int i = 0; i < 145; i++) { + request_headers.addCopy(std::to_string(i), ""); + } + EXPECT_CALL(request_decoder_, decodeHeaders_(_, _)); + EXPECT_CALL(server_stream_callbacks_, onResetStream(_, _)).Times(0); + request_encoder_->encodeHeaders(request_headers, false); +} + +// Tests that max number of response headers is configurable. +TEST_P(Http2CodecImplTest, ManyResponseHeadersAccepted) { + max_response_headers_count_ = 110; + initialize(); + + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + EXPECT_CALL(request_decoder_, decodeHeaders_(_, false)); + request_encoder_->encodeHeaders(request_headers, false); + + TestHeaderMapImpl response_headers{{":status", "200"}, {"compression", "test"}}; + for (int i = 0; i < 105; i++) { + response_headers.addCopy(std::to_string(i), ""); + } + EXPECT_CALL(response_decoder_, decodeHeaders_(_, true)); + response_encoder_->encodeHeaders(response_headers, true); +} + +TEST_P(Http2CodecImplTest, LargeRequestHeadersAtLimitAccepted) { uint32_t codec_limit_kb = 64; max_request_headers_kb_ = codec_limit_kb; initialize(); TestHeaderMapImpl request_headers; HttpTestUtility::addDefaultHeaders(request_headers); + // Refresh byte size after adding default inline headers by reference. + request_headers.refreshByteSize(); std::string key = "big"; uint32_t head_room = 77; uint32_t long_string_length = - codec_limit_kb * 1024 - request_headers.byteSize() - key.length() - head_room; + codec_limit_kb * 1024 - request_headers.byteSize().value() - key.length() - head_room; std::string long_string = std::string(long_string_length, 'q'); request_headers.addCopy(key, long_string); // The amount of data sent to the codec is not equivalent to the size of the // request headers that Envoy computes, as the codec limits based on the // entire http2 frame. The exact head room needed (76) was found through iteration. - ASSERT_EQ(request_headers.byteSize() + head_room, codec_limit_kb * 1024); + ASSERT_EQ(request_headers.byteSize().value() + head_room, codec_limit_kb * 1024); EXPECT_CALL(request_decoder_, decodeHeaders_(_, _)); request_encoder_->encodeHeaders(request_headers, true); } -TEST_P(Http2CodecImplTest, TestLargeRequestHeadersOverDefaultCodecLibraryLimit) { +TEST_P(Http2CodecImplTest, LargeRequestHeadersOverDefaultCodecLibraryLimit) { max_request_headers_kb_ = 66; initialize(); @@ -1037,7 +1089,7 @@ TEST_P(Http2CodecImplTest, TestLargeRequestHeadersOverDefaultCodecLibraryLimit) request_encoder_->encodeHeaders(request_headers, true); } -TEST_P(Http2CodecImplTest, TestLargeRequestHeadersExceedPerHeaderLimit) { +TEST_P(Http2CodecImplTest, LargeRequestHeadersExceedPerHeaderLimit) { // The name-value pair max is set by NGHTTP2_HD_MAX_NV in lib/nghttp2_hd.h to 64KB, and // creates a per-request header limit for us in h2. Note that the nghttp2 // calculated byte size will differ from envoy due to H2 compression and frames. @@ -1057,7 +1109,7 @@ TEST_P(Http2CodecImplTest, TestLargeRequestHeadersExceedPerHeaderLimit) { request_encoder_->encodeHeaders(request_headers, true); } -TEST_P(Http2CodecImplTest, TestManyLargeRequestHeadersUnderPerHeaderLimit) { +TEST_P(Http2CodecImplTest, ManyLargeRequestHeadersUnderPerHeaderLimit) { max_request_headers_kb_ = 81; initialize(); @@ -1073,7 +1125,7 @@ TEST_P(Http2CodecImplTest, TestManyLargeRequestHeadersUnderPerHeaderLimit) { request_encoder_->encodeHeaders(request_headers, true); } -TEST_P(Http2CodecImplTest, TestLargeRequestHeadersAtMaxConfigurable) { +TEST_P(Http2CodecImplTest, LargeRequestHeadersAtMaxConfigurable) { // Raising the limit past this triggers some unexpected nghttp2 error. // Further debugging required to increase past ~96 KiB. max_request_headers_kb_ = 96; diff --git a/test/common/http/http2/codec_impl_test_util.h b/test/common/http/http2/codec_impl_test_util.h index 2c4652ee0f2f..67521db0fec8 100644 --- a/test/common/http/http2/codec_impl_test_util.h +++ b/test/common/http/http2/codec_impl_test_util.h @@ -10,9 +10,9 @@ class TestServerConnectionImpl : public ServerConnectionImpl { public: TestServerConnectionImpl(Network::Connection& connection, ServerConnectionCallbacks& callbacks, Stats::Scope& scope, const Http2Settings& http2_settings, - uint32_t max_request_headers_kb) - : ServerConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers_kb) { - } + uint32_t max_request_headers_kb, uint32_t max_request_headers_count) + : ServerConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers_kb, + max_request_headers_count) {} nghttp2_session* session() { return session_; } using ServerConnectionImpl::getStream; }; @@ -21,9 +21,9 @@ class TestClientConnectionImpl : public ClientConnectionImpl { public: TestClientConnectionImpl(Network::Connection& connection, Http::ConnectionCallbacks& callbacks, Stats::Scope& scope, const Http2Settings& http2_settings, - uint32_t max_request_headers_kb) - : ClientConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers_kb) { - } + uint32_t max_request_headers_kb, uint32_t max_request_headers_count) + : ClientConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers_kb, + max_request_headers_count) {} nghttp2_session* session() { return session_; } using ClientConnectionImpl::getStream; using ConnectionImpl::sendPendingFrames; diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index 6feb80d6d37f..e791c5ceed60 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -187,6 +187,71 @@ TEST_F(HttpConnectionManagerConfigTest, DisabledStreamIdleTimeout) { EXPECT_EQ(0, config.streamIdleTimeout().count()); } +// Validate that deprecated idle_timeout is still ingested. +TEST_F(HttpConnectionManagerConfigTest, DeprecatedIdleTimeout) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + idle_timeout: 1s + route_config: + name: local_route + http_filters: + - name: envoy.router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, + date_provider_, route_config_provider_manager_); + EXPECT_EQ(1000, config.idleTimeout().value().count()); +} + +// Validate that idle_timeout set in common_http_protocol_options is used. +TEST_F(HttpConnectionManagerConfigTest, CommonHttpProtocolIdleTimeout) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + common_http_protocol_options: + idle_timeout: 1s + route_config: + name: local_route + http_filters: + - name: envoy.router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, + date_provider_, route_config_provider_manager_); + EXPECT_EQ(1000, config.idleTimeout().value().count()); +} + +// Check that the default max request header count is 100. +TEST_F(HttpConnectionManagerConfigTest, DefaultMaxRequestHeaderCount) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + route_config: + name: local_route + http_filters: + - name: envoy.router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, + date_provider_, route_config_provider_manager_); + EXPECT_EQ(100, config.maxRequestHeadersCount()); +} + +// Check that max request header count is configured. +TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeaderCountConfigurable) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + common_http_protocol_options: + max_headers_count: 200 + route_config: + name: local_route + http_filters: + - name: envoy.router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, + date_provider_, route_config_provider_manager_); + EXPECT_EQ(200, config.maxRequestHeadersCount()); +} + // Validated that by default we don't normalize paths // unless set build flag path_normalization_by_default=true TEST_F(HttpConnectionManagerConfigTest, NormalizePathDefault) { diff --git a/test/integration/autonomous_upstream.cc b/test/integration/autonomous_upstream.cc index c70d51a6bf1b..00fbce3c23df 100644 --- a/test/integration/autonomous_upstream.cc +++ b/test/integration/autonomous_upstream.cc @@ -63,7 +63,7 @@ AutonomousHttpConnection::AutonomousHttpConnection(SharedConnectionWrapper& shar Stats::Store& store, Type type, AutonomousUpstream& upstream) : FakeHttpConnection(shared_connection, store, type, upstream.timeSystem(), - Http::DEFAULT_MAX_REQUEST_HEADERS_KB), + Http::DEFAULT_MAX_REQUEST_HEADERS_KB, Http::DEFAULT_MAX_HEADERS_COUNT), upstream_(upstream) {} Http::StreamDecoder& AutonomousHttpConnection::newStream(Http::StreamEncoder& response_encoder, diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index c907e0eef22b..ea5e16e9b6c2 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -209,17 +209,20 @@ void FakeStream::finishGrpcStream(Grpc::Status::GrpcStatus status) { FakeHttpConnection::FakeHttpConnection(SharedConnectionWrapper& shared_connection, Stats::Store& store, Type type, Event::TestTimeSystem& time_system, - uint32_t max_request_headers_kb) + uint32_t max_request_headers_kb, + uint32_t max_request_headers_count) : FakeConnectionBase(shared_connection, time_system) { if (type == Type::HTTP1) { codec_ = std::make_unique( - shared_connection_.connection(), *this, Http::Http1Settings(), max_request_headers_kb); + shared_connection_.connection(), *this, Http::Http1Settings(), max_request_headers_kb, + max_request_headers_count); } else { auto settings = Http::Http2Settings(); settings.allow_connect_ = true; settings.allow_metadata_ = true; codec_ = std::make_unique( - shared_connection_.connection(), *this, store, settings, max_request_headers_kb); + shared_connection_.connection(), *this, store, settings, max_request_headers_kb, + max_request_headers_count); ASSERT(type == Type::HTTP2); } @@ -433,7 +436,8 @@ void FakeUpstream::threadRoutine() { AssertionResult FakeUpstream::waitForHttpConnection(Event::Dispatcher& client_dispatcher, FakeHttpConnectionPtr& connection, milliseconds timeout, - uint32_t max_request_headers_kb) { + uint32_t max_request_headers_kb, + uint32_t max_request_headers_count) { Event::TestTimeSystem& time_system = timeSystem(); auto end_time = time_system.monotonicTime() + timeout; { @@ -453,7 +457,8 @@ AssertionResult FakeUpstream::waitForHttpConnection(Event::Dispatcher& client_di return AssertionFailure() << "Got a new connection event, but didn't create a connection."; } connection = std::make_unique(consumeConnection(), stats_store_, http_type_, - time_system, max_request_headers_kb); + time_system, max_request_headers_kb, + max_request_headers_count); } VERIFY_ASSERTION(connection->initialize()); VERIFY_ASSERTION(connection->readDisable(false)); @@ -483,7 +488,8 @@ FakeUpstream::waitForHttpConnection(Event::Dispatcher& client_dispatcher, } else { connection = std::make_unique( upstream.consumeConnection(), upstream.stats_store_, upstream.http_type_, - upstream.timeSystem(), Http::DEFAULT_MAX_REQUEST_HEADERS_KB); + upstream.timeSystem(), Http::DEFAULT_MAX_REQUEST_HEADERS_KB, + Http::DEFAULT_MAX_HEADERS_COUNT); lock.release(); VERIFY_ASSERTION(connection->initialize()); VERIFY_ASSERTION(connection->readDisable(false)); diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index 4ec4e181a3af..9d169359879c 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -402,7 +402,8 @@ class FakeHttpConnection : public Http::ServerConnectionCallbacks, public FakeCo enum class Type { HTTP1, HTTP2 }; FakeHttpConnection(SharedConnectionWrapper& shared_connection, Stats::Store& store, Type type, - Event::TestTimeSystem& time_system, uint32_t max_request_headers_kb); + Event::TestTimeSystem& time_system, uint32_t max_request_headers_kb, + uint32_t max_request_headers_count); // By default waitForNewStream assumes the next event is a new stream and // returns AssertionFailure if an unexpected event occurs. If a caller truly @@ -531,7 +532,8 @@ class FakeUpstream : Logger::Loggable, testing::AssertionResult waitForHttpConnection(Event::Dispatcher& client_dispatcher, FakeHttpConnectionPtr& connection, std::chrono::milliseconds timeout = TestUtility::DefaultTimeout, - uint32_t max_request_headers_kb = Http::DEFAULT_MAX_REQUEST_HEADERS_KB); + uint32_t max_request_headers_kb = Http::DEFAULT_MAX_REQUEST_HEADERS_KB, + uint32_t max_request_headers_count = Http::DEFAULT_MAX_HEADERS_COUNT); ABSL_MUST_USE_RESULT testing::AssertionResult diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index 6f25e2a7f4ae..223c1651b153 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -64,6 +64,8 @@ TEST_P(Http2IntegrationTest, Retry) { testRetry(); } TEST_P(Http2IntegrationTest, RetryAttemptCount) { testRetryAttemptCountHeader(); } +TEST_P(Http2IntegrationTest, LargeRequestTrailersRejected) { testLargeRequestTrailers(66, 60); } + static std::string response_metadata_filter = R"EOF( name: response-metadata-filter config: {} @@ -891,6 +893,37 @@ void Http2RingHashIntegrationTest::sendMultipleRequests( } } +// Test cookie value parsing +TEST_P(Http2RingHashIntegrationTest, testLargeCookieParsing) { + config_helper_.addConfigModifier( + [&](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) + -> void { + auto* hash_policy = hcm.mutable_route_config() + ->mutable_virtual_hosts(0) + ->mutable_routes(0) + ->mutable_route() + ->add_hash_policy(); + auto* cookie = hash_policy->mutable_cookie(); + cookie->set_name("foo"); + }); + + Http::TestHeaderMapImpl headers({{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}}); + for (int i = 0; i < 1000; i++) { + headers.addCopy("cookie", "foo=bar" + std::to_string(i)); + } + num_upstreams_ = 2; + std::set served_by; + sendMultipleRequests(128, headers, [&](IntegrationStreamDecoder& response) { + EXPECT_EQ("200", response.headers().Status()->value().getStringView()); + EXPECT_TRUE(response.headers().get(Http::Headers::get().SetCookie) == nullptr); + served_by.insert(std::string( + response.headers().get(Http::LowerCaseString("x-served-by"))->value().getStringView())); + }); +} + TEST_P(Http2RingHashIntegrationTest, CookieRoutingNoCookieNoTtl) { config_helper_.addConfigModifier( [&](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) diff --git a/test/integration/http2_upstream_integration_test.cc b/test/integration/http2_upstream_integration_test.cc index a872a3fc3f23..85965522ae81 100644 --- a/test/integration/http2_upstream_integration_test.cc +++ b/test/integration/http2_upstream_integration_test.cc @@ -313,4 +313,66 @@ TEST_P(Http2UpstreamIntegrationTest, UpstreamConnectionCloseWithManyStreams) { } } +// Tests the default limit for the number of response headers is 100. Results in a stream reset if +// exceeds. +TEST_P(Http2UpstreamIntegrationTest, TestManyResponseHeadersRejected) { + // Default limit for response headers is 100. + initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); + + Http::TestHeaderMapImpl many_headers(default_response_headers_); + for (int i = 0; i < 100; i++) { + many_headers.addCopy("many", std::string(1, 'a')); + } + auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); + waitForNextUpstreamRequest(); + + upstream_request_->encodeHeaders(many_headers, true); + response->waitForEndStream(); + // Upstream stream reset triggered. + EXPECT_EQ("503", response->headers().Status()->value().getStringView()); +} + +// Tests bootstrap configuration of max response headers. +TEST_P(Http2UpstreamIntegrationTest, ManyResponseHeadersAccepted) { + // Set max response header count to 200. + config_helper_.addConfigModifier([](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + auto* static_resources = bootstrap.mutable_static_resources(); + auto* cluster = static_resources->mutable_clusters(0); + auto* http_protocol_options = cluster->mutable_common_http_protocol_options(); + http_protocol_options->mutable_max_headers_count()->set_value(200); + }); + Http::TestHeaderMapImpl response_headers(default_response_headers_); + for (int i = 0; i < 150; i++) { + response_headers.addCopy(std::to_string(i), std::string(1, 'a')); + } + + initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); + auto response = codec_client_->makeRequestWithBody(default_request_headers_, 1024); + waitForNextUpstreamRequest(); + + upstream_request_->encodeHeaders(response_headers, false); + upstream_request_->encodeData(512, true); + response->waitForEndStream(); + + EXPECT_TRUE(upstream_request_->complete()); + EXPECT_TRUE(response->complete()); +} + +// Tests that HTTP/2 response headers over 60 kB are rejected and result in a stream reset. +TEST_P(Http2UpstreamIntegrationTest, LargeResponseHeadersRejected) { + initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); + + Http::TestHeaderMapImpl large_headers(default_response_headers_); + large_headers.addCopy("large", std::string(60 * 1024, 'a')); + auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); + waitForNextUpstreamRequest(); + + upstream_request_->encodeHeaders(large_headers, true); + response->waitForEndStream(); + // Upstream stream reset. + EXPECT_EQ("503", response->headers().Status()->value().getStringView()); +} } // namespace Envoy diff --git a/test/integration/http2_upstream_integration_test.h b/test/integration/http2_upstream_integration_test.h index efb55c222766..d942f8861846 100644 --- a/test/integration/http2_upstream_integration_test.h +++ b/test/integration/http2_upstream_integration_test.h @@ -16,6 +16,8 @@ class Http2UpstreamIntegrationTest : public testing::TestWithParam cluster{new NiceMock()}; + cluster->max_response_headers_count_ = 200; cluster->http2_settings_.allow_connect_ = true; cluster->http2_settings_.allow_metadata_ = true; Upstream::HostDescriptionConstSharedPtr host_description{Upstream::makeTestHostDescription( @@ -239,7 +240,8 @@ void HttpIntegrationTest::setDownstreamProtocol(Http::CodecClient::Type downstre IntegrationStreamDecoderPtr HttpIntegrationTest::sendRequestAndWaitForResponse( const Http::TestHeaderMapImpl& request_headers, uint32_t request_body_size, - const Http::TestHeaderMapImpl& response_headers, uint32_t response_size, int upstream_index) { + const Http::TestHeaderMapImpl& response_headers, uint32_t response_size, int upstream_index, + std::chrono::milliseconds time) { ASSERT(codec_client_ != nullptr); // Send the request to Envoy. IntegrationStreamDecoderPtr response; @@ -248,7 +250,7 @@ IntegrationStreamDecoderPtr HttpIntegrationTest::sendRequestAndWaitForResponse( } else { response = codec_client_->makeHeaderOnlyRequest(request_headers); } - waitForNextUpstreamRequest(upstream_index); + waitForNextUpstreamRequest(upstream_index, time); // Send response headers, and end_stream if there is no response body. upstream_request_->encodeHeaders(response_headers, response_size == 0); // Send any response data, with end_stream true. @@ -278,15 +280,16 @@ void HttpIntegrationTest::cleanupUpstreamAndDownstream() { } uint64_t -HttpIntegrationTest::waitForNextUpstreamRequest(const std::vector& upstream_indices) { +HttpIntegrationTest::waitForNextUpstreamRequest(const std::vector& upstream_indices, + std::chrono::milliseconds connection_wait_timeout) { uint64_t upstream_with_request; // If there is no upstream connection, wait for it to be established. if (!fake_upstream_connection_) { AssertionResult result = AssertionFailure(); for (auto upstream_index : upstream_indices) { result = fake_upstreams_[upstream_index]->waitForHttpConnection( - *dispatcher_, fake_upstream_connection_, TestUtility::DefaultTimeout, - max_request_headers_kb_); + *dispatcher_, fake_upstream_connection_, connection_wait_timeout, max_request_headers_kb_, + max_request_headers_count_); if (result) { upstream_with_request = upstream_index; break; @@ -305,8 +308,9 @@ HttpIntegrationTest::waitForNextUpstreamRequest(const std::vector& ups return upstream_with_request; } -void HttpIntegrationTest::waitForNextUpstreamRequest(uint64_t upstream_index) { - waitForNextUpstreamRequest(std::vector({upstream_index})); +void HttpIntegrationTest::waitForNextUpstreamRequest( + uint64_t upstream_index, std::chrono::milliseconds connection_wait_timeout) { + waitForNextUpstreamRequest(std::vector({upstream_index}), connection_wait_timeout); } void HttpIntegrationTest::addFilters(std::vector filters) { @@ -821,24 +825,33 @@ void HttpIntegrationTest::testTwoRequests(bool network_backup) { EXPECT_EQ(1024U, response->body().size()); } -void HttpIntegrationTest::testLargeRequestHeaders(uint32_t size, uint32_t max_size) { - // `size` parameter is the size of the header that will be added to the - // request. The actual request byte size will exceed `size` due to keys - // and other headers. +void HttpIntegrationTest::testLargeRequestHeaders(uint32_t size, uint32_t count, uint32_t max_size, + uint32_t max_count) { + // `size` parameter dictates the size of each header that will be added to the request and `count` + // parameter is the number of headers to be added. The actual request byte size will exceed `size` + // due to the keys and other headers. The actual request header count will exceed `count` by four + // due to default headers. config_helper_.addConfigModifier( [&](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) - -> void { hcm.mutable_max_request_headers_kb()->set_value(max_size); }); + -> void { + hcm.mutable_max_request_headers_kb()->set_value(max_size); + hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value( + max_count); + }); max_request_headers_kb_ = max_size; + max_request_headers_count_ = max_count; Http::TestHeaderMapImpl big_headers{ {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, {":authority", "host"}}; + // Already added four headers. + for (unsigned int i = 0; i < count; i++) { + big_headers.addCopy(std::to_string(i), std::string(size * 1024, 'a')); + } - big_headers.addCopy("big", std::string(size * 1024, 'a')); initialize(); - codec_client_ = makeHttpConnection(lookupPort("http")); - if (size >= max_size) { + if (size >= max_size || count > max_count) { // header size includes keys too, so expect rejection when equal auto encoder_decoder = codec_client_->startRequest(big_headers); auto response = std::move(encoder_decoder.second); @@ -858,6 +871,71 @@ void HttpIntegrationTest::testLargeRequestHeaders(uint32_t size, uint32_t max_si } } +void HttpIntegrationTest::testLargeRequestTrailers(uint32_t size, uint32_t max_size) { + // `size` parameter is the size of the trailer that will be added to the + // request. The actual request byte size will exceed `size` due to keys + // and other headers. + + config_helper_.addConfigModifier( + [&](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) + -> void { hcm.mutable_max_request_headers_kb()->set_value(max_size); }); + max_request_headers_kb_ = max_size; + Http::TestHeaderMapImpl request_trailers{{"trailer", "trailer"}}; + request_trailers.addCopy("big", std::string(size * 1024, 'a')); + + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + + auto encoder_decoder = codec_client_->startRequest(default_request_headers_); + request_encoder_ = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + codec_client_->sendData(*request_encoder_, 10, false); + codec_client_->sendTrailers(*request_encoder_, request_trailers); + + if (size >= max_size && downstream_protocol_ == Http::CodecClient::Type::HTTP2) { + // For HTTP/2, expect a stream reset when the size of the trailers is larger than the maximum + // limit. + response->waitForReset(); + codec_client_->close(); + EXPECT_FALSE(response->complete()); + + } else { + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(default_response_headers_, true); + response->waitForEndStream(); + EXPECT_TRUE(response->complete()); + } +} + +void HttpIntegrationTest::testManyRequestHeaders(std::chrono::milliseconds time) { + config_helper_.addConfigModifier( + [&](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) + -> void { + hcm.mutable_max_request_headers_kb()->set_value(96); + hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value(20100); + }); + max_request_headers_kb_ = 96; + max_request_headers_count_ = 201000; + + Http::TestHeaderMapImpl big_headers{ + {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, {":authority", "host"}}; + + for (int i = 0; i < 20000; i++) { + big_headers.addCopy(std::to_string(i), std::string(0, 'a')); + } + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = + sendRequestAndWaitForResponse(big_headers, 0, default_response_headers_, 0, 0, time); + + EXPECT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().Status()->value().getStringView()); +} + void HttpIntegrationTest::testDownstreamResetBeforeResponseComplete() { initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h index 7a0de1e004f5..de045aa43514 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -114,18 +114,21 @@ class HttpIntegrationTest : public BaseIntegrationTest { // // Waits for the complete downstream response before returning. // Requires |codec_client_| to be initialized. - IntegrationStreamDecoderPtr - sendRequestAndWaitForResponse(const Http::TestHeaderMapImpl& request_headers, - uint32_t request_body_size, - const Http::TestHeaderMapImpl& response_headers, - uint32_t response_body_size, int upstream_index = 0); + IntegrationStreamDecoderPtr sendRequestAndWaitForResponse( + const Http::TestHeaderMapImpl& request_headers, uint32_t request_body_size, + const Http::TestHeaderMapImpl& response_headers, uint32_t response_body_size, + int upstream_index = 0, std::chrono::milliseconds time = TestUtility::DefaultTimeout); // Wait for the end of stream on the next upstream stream on any of the provided fake upstreams. // Sets fake_upstream_connection_ to the connection and upstream_request_ to stream. // In cases where the upstream that will receive the request is not deterministic, a second // upstream index may be provided, in which case both upstreams will be checked for requests. - uint64_t waitForNextUpstreamRequest(const std::vector& upstream_indices); - void waitForNextUpstreamRequest(uint64_t upstream_index = 0); + uint64_t waitForNextUpstreamRequest( + const std::vector& upstream_indices, + std::chrono::milliseconds connection_wait_timeout = TestUtility::DefaultTimeout); + void waitForNextUpstreamRequest( + uint64_t upstream_index = 0, + std::chrono::milliseconds connection_wait_timeout = TestUtility::DefaultTimeout); // Close |codec_client_| and |fake_upstream_connection_| cleanly. void cleanupUpstreamAndDownstream(); @@ -164,7 +167,12 @@ class HttpIntegrationTest : public BaseIntegrationTest { void testRouterUpstreamResponseBeforeRequestComplete(); void testTwoRequests(bool force_network_backup = false); - void testLargeRequestHeaders(uint32_t size, uint32_t max_size = 60); + void testLargeHeaders(Http::TestHeaderMapImpl request_headers, + Http::TestHeaderMapImpl request_trailers, uint32_t size, uint32_t max_size); + void testLargeRequestHeaders(uint32_t size, uint32_t count, uint32_t max_size = 60, + uint32_t max_count = 100); + void testLargeRequestTrailers(uint32_t size, uint32_t max_size = 60); + void testManyRequestHeaders(std::chrono::milliseconds time = TestUtility::DefaultTimeout); void testAddEncodedTrailers(); void testRetry(); @@ -200,5 +208,6 @@ class HttpIntegrationTest : public BaseIntegrationTest { // The codec type for the client-to-Envoy connection Http::CodecClient::Type downstream_protocol_{Http::CodecClient::Type::HTTP1}; uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB}; + uint32_t max_request_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; }; } // namespace Envoy diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index bdeac5bb3850..c0139cc9ede1 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -832,11 +832,131 @@ name: decode-headers-only } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersRejected) { - testLargeRequestHeaders(95, 60); + // Send one 95 kB header with limit 60 kB and 100 headers. + testLargeRequestHeaders(95, 1, 60, 100); } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersAccepted) { - testLargeRequestHeaders(95, 96); + // Send one 95 kB header with limit 96 kB and 100 headers. + testLargeRequestHeaders(95, 1, 96, 100); +} + +TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersRejected) { + // Send 101 empty headers with limit 60 kB and 100 headers. + testLargeRequestHeaders(0, 101, 60, 80); +} + +TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersAccepted) { + // Send 145 empty headers with limit 60 kB and 150 headers. + testLargeRequestHeaders(0, 140, 60, 150); +} + +TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersRejected) { + // Default header (and trailer) count limit is 100. + Http::TestHeaderMapImpl request_trailers; + for (int i = 0; i < 150; i++) { + request_trailers.addCopy("trailer", std::string(1, 'a')); + } + + initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); + auto encoder_decoder = codec_client_->startRequest(default_request_headers_); + request_encoder_ = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + codec_client_->sendData(*request_encoder_, 1, false); + codec_client_->sendTrailers(*request_encoder_, request_trailers); + + // Only relevant to Http2Downstream. + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + // Http1 Downstream ignores trailers. + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(default_response_headers_, true); + response->waitForEndStream(); + EXPECT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().Status()->value().getStringView()); + } else { + // Expect rejection. + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + response->waitForReset(); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + } +} + +TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersAccepted) { + // Set header (and trailer) count limit to 200. + uint32_t max_count = 200; + config_helper_.addConfigModifier( + [&](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) + -> void { + hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value( + max_count); + }); + max_request_headers_count_ = max_count; + Http::TestHeaderMapImpl request_trailers; + for (int i = 0; i < 150; i++) { + request_trailers.addCopy("trailer", std::string(1, 'a')); + } + + initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); + auto encoder_decoder = codec_client_->startRequest(default_request_headers_); + request_encoder_ = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + codec_client_->sendData(*request_encoder_, 1, false); + codec_client_->sendTrailers(*request_encoder_, request_trailers); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(default_response_headers_, true); + response->waitForEndStream(); + EXPECT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().Status()->value().getStringView()); +} + +TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersTimeout) { + // Set timeout for 5 seconds, and ensure that a request with 20k+ headers can be sent. + testManyRequestHeaders(std::chrono::milliseconds(5000)); +} + +TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersAccepted) { + testLargeRequestTrailers(60, 96); +} + +TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersRejected) { + testLargeRequestTrailers(66, 60); +} + +TEST_P(DownstreamProtocolIntegrationTest, ManyTrailerHeaders) { + config_helper_.addConfigModifier( + [&](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) + -> void { + hcm.mutable_max_request_headers_kb()->set_value(96); + hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value(20100); + }); + max_request_headers_kb_ = 96; + max_request_headers_count_ = 201000; + + Http::TestHeaderMapImpl request_trailers{}; + for (int i = 0; i < 20000; i++) { + request_trailers.addCopy(std::to_string(i), ""); + } + + initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); + auto encoder_decoder = + codec_client_->startRequest(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}}); + request_encoder_ = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + codec_client_->sendTrailers(*request_encoder_, request_trailers); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(default_response_headers_, true); + response->waitForEndStream(); + + EXPECT_TRUE(upstream_request_->complete()); + EXPECT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().Status()->value().getStringView()); } // Tests StopAllIterationAndBuffer. Verifies decode-headers-return-stop-all-filter calls decodeData diff --git a/test/mocks/upstream/cluster_info.cc b/test/mocks/upstream/cluster_info.cc index 5497a0cd2bbe..7f7d317a6bfa 100644 --- a/test/mocks/upstream/cluster_info.cc +++ b/test/mocks/upstream/cluster_info.cc @@ -45,6 +45,8 @@ MockClusterInfo::MockClusterInfo() ON_CALL(*this, eds_service_name()).WillByDefault(ReturnPointee(&eds_service_name_)); ON_CALL(*this, http2Settings()).WillByDefault(ReturnRef(http2_settings_)); ON_CALL(*this, extensionProtocolOptions(_)).WillByDefault(Return(extension_protocol_options_)); + ON_CALL(*this, maxResponseHeadersCount()) + .WillByDefault(ReturnPointee(&max_response_headers_count_)); ON_CALL(*this, maxRequestsPerConnection()) .WillByDefault(ReturnPointee(&max_requests_per_connection_)); ON_CALL(*this, stats()).WillByDefault(ReturnRef(stats_)); diff --git a/test/mocks/upstream/cluster_info.h b/test/mocks/upstream/cluster_info.h index dd498b8fae26..dc5b201ea27d 100644 --- a/test/mocks/upstream/cluster_info.h +++ b/test/mocks/upstream/cluster_info.h @@ -71,6 +71,7 @@ class MockClusterInfo : public ClusterInfo { MOCK_CONST_METHOD0(lbOriginalDstConfig, const absl::optional&()); MOCK_CONST_METHOD0(maintenanceMode, bool()); + MOCK_CONST_METHOD0(maxResponseHeadersCount, uint32_t()); MOCK_CONST_METHOD0(maxRequestsPerConnection, uint64_t()); MOCK_CONST_METHOD0(name, const std::string&()); MOCK_CONST_METHOD1(resourceManager, ResourceManager&(ResourcePriority priority)); @@ -91,6 +92,7 @@ class MockClusterInfo : public ClusterInfo { Http::Http2Settings http2_settings_{}; ProtocolOptionsConfigConstSharedPtr extension_protocol_options_; uint64_t max_requests_per_connection_{}; + uint32_t max_response_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; NiceMock stats_store_; ClusterStats stats_; Network::TransportSocketFactoryPtr transport_socket_factory_;