From 60e4494f4c29ac260dba8d2ba3323e91b84286a0 Mon Sep 17 00:00:00 2001 From: Auni Ahsan Date: Mon, 28 Jan 2019 12:58:52 -0500 Subject: [PATCH 01/13] Increase limit to 64 Signed-off-by: Auni Ahsan --- .../v2/http_connection_manager.proto | 4 +- source/common/http/codec_client.cc | 3 +- source/common/http/conn_manager_utility.cc | 14 +++--- source/common/http/conn_manager_utility.h | 3 +- source/common/http/http2/codec_impl.cc | 14 ++++-- source/common/http/http2/codec_impl.h | 11 +++-- .../network/http_connection_manager/config.cc | 5 +- test/common/http/http2/codec_impl_test.cc | 48 +++++++++++++------ test/integration/fake_upstream.cc | 2 +- test/integration/http2_integration_test.cc | 4 +- test/integration/integration_test.cc | 4 +- 11 files changed, 69 insertions(+), 43 deletions(-) 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 60e7728a86e0c..be519e7da07f0 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 @@ -133,11 +133,11 @@ message HttpConnectionManager { // The maximum request headers size for incoming connections. The default max // is 60K, based on default settings for http codecs. For HTTP1, the current // limit set by http_parser is 80K. for HTTP2, the default allowed header - // block in nghttp2 is 64K. The max configurable setting is 63K in order to + // block in nghttp2 is 64K. The max configurable setting is 64K in order to // stay under both codec limits. // Requests that exceed this size will receive a 431 response. google.protobuf.UInt32Value max_request_headers_kb = 29 - [(validate.rules).uint32.gt = 0, (validate.rules).uint32.lte = 63]; + [(validate.rules).uint32.gt = 0, (validate.rules).uint32.lte = 64]; // The idle timeout for connections managed by the connection manager. The // idle timeout is defined as the period in which there are no active diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc index 43ae66e859d8a..8d8402942d705 100644 --- a/source/common/http/codec_client.cc +++ b/source/common/http/codec_client.cc @@ -145,7 +145,8 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne } case Type::HTTP2: { codec_ = std::make_unique( - *connection_, *this, host->cluster().statsScope(), host->cluster().http2Settings()); + *connection_, *this, host->cluster().statsScope(), host->cluster().http2Settings(), + Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE_KB); break; } } diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index 7e28efd675f7f..8275334db994a 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -36,15 +36,13 @@ std::string ConnectionManagerUtility::determineNextProtocol(Network::Connection& return ""; } -ServerConnectionPtr ConnectionManagerUtility::autoCreateCodec(Network::Connection& connection, - const Buffer::Instance& data, - ServerConnectionCallbacks& callbacks, - Stats::Scope& scope, - const Http1Settings& http1_settings, - const Http2Settings& http2_settings) { +ServerConnectionPtr ConnectionManagerUtility::autoCreateCodec( + Network::Connection& connection, const Buffer::Instance& data, + ServerConnectionCallbacks& callbacks, Stats::Scope& scope, const Http1Settings& http1_settings, + const Http2Settings& http2_settings, uint32_t max_request_headers_kb) { if (determineNextProtocol(connection, data) == Http2::ALPN_STRING) { - return ServerConnectionPtr{ - new Http2::ServerConnectionImpl(connection, callbacks, scope, http2_settings)}; + return ServerConnectionPtr{new Http2::ServerConnectionImpl( + connection, callbacks, scope, http2_settings, max_request_headers_kb)}; } else { return ServerConnectionPtr{ new Http1::ServerConnectionImpl(connection, callbacks, http1_settings)}; diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h index 7ba81d8eeb52e..702c43910d7e0 100644 --- a/source/common/http/conn_manager_utility.h +++ b/source/common/http/conn_manager_utility.h @@ -36,7 +36,8 @@ class ConnectionManagerUtility { static ServerConnectionPtr autoCreateCodec(Network::Connection& connection, const Buffer::Instance& data, ServerConnectionCallbacks& callbacks, Stats::Scope& scope, - const Http1Settings& http1_settings, const Http2Settings& http2_settings); + const Http1Settings& http1_settings, + const Http2Settings& http2_settings, uint32_t max_request_headers_kb); /** * Mutates request headers in various ways. This functionality is broken out because of its diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index a3ad27a815120..e7e8638e9b5a2 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -641,7 +641,7 @@ int ConnectionImpl::saveHeader(const nghttp2_frame* frame, HeaderString&& name, } stream->saveHeader(std::move(name), std::move(value)); - if (stream->headers_->byteSize() > StreamImpl::MAX_HEADER_SIZE) { + if (stream->headers_->byteSize() > max_request_headers_kb_ * 1024) { // This will cause the library to reset/close the stream. stats_.header_overflow_.inc(); return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; @@ -877,8 +877,10 @@ ConnectionImpl::ClientHttp2Options::ClientHttp2Options(const Http2Settings& http ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, Http::ConnectionCallbacks& callbacks, - Stats::Scope& stats, const Http2Settings& http2_settings) - : ConnectionImpl(connection, stats, http2_settings), callbacks_(callbacks) { + Stats::Scope& stats, const Http2Settings& http2_settings, + uint32_t max_request_headers) + : ConnectionImpl(connection, stats, http2_settings, max_request_headers), + callbacks_(callbacks) { ClientHttp2Options client_http2_options(http2_settings); nghttp2_session_client_new2(&session_, http2_callbacks_.callbacks(), base(), client_http2_options.options()); @@ -924,8 +926,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) - : ConnectionImpl(connection, scope, http2_settings), callbacks_(callbacks) { + Stats::Scope& scope, const Http2Settings& http2_settings, + uint32_t max_request_headers) + : ConnectionImpl(connection, scope, http2_settings, max_request_headers), + callbacks_(callbacks) { Http2Options http2_options(http2_settings); nghttp2_session_server_new2(&session_, http2_callbacks_.callbacks(), base(), http2_options.options()); diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h index 143b306d6ce87..f3b733515c986 100644 --- a/source/common/http/http2/codec_impl.h +++ b/source/common/http/http2/codec_impl.h @@ -74,9 +74,9 @@ class Utility { class ConnectionImpl : public virtual Connection, protected Logger::Loggable { public: ConnectionImpl(Network::Connection& connection, Stats::Scope& stats, - const Http2Settings& http2_settings) + const Http2Settings& http2_settings, uint32_t max_request_headers_kb) : stats_{ALL_HTTP2_CODEC_STATS(POOL_COUNTER_PREFIX(stats, "http2."))}, - connection_(connection), + connection_(connection), max_request_headers_kb_(max_request_headers_kb), per_stream_buffer_limit_(http2_settings.initial_stream_window_size_), dispatching_(false), raised_goaway_(false), pending_deferred_reset_(false) {} @@ -291,6 +291,7 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable { 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_); + stats_store_, client_http2settings_, + max_request_headers_); server_ = std::make_unique(server_connection_, server_callbacks_, - stats_store_, server_http2settings_); + stats_store_, server_http2settings_, + max_request_headers_); request_encoder_ = &client_->newStream(response_decoder_); setupDefaultConnectionMocks(); @@ -144,6 +148,7 @@ class Http2CodecImplTest : public TestBaseWithParam { StreamEncoder* response_encoder_{}; MockStreamCallbacks server_stream_callbacks_; bool corrupt_data_ = false; + uint32_t max_request_headers_ = Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE_KB; }; TEST_P(Http2CodecImplTest, ShutdownNotice) { @@ -775,9 +780,11 @@ 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_); + stats_store_, client_http2settings_, + max_request_headers_); server_ = std::make_unique(server_connection_, server_callbacks_, - stats_store_, server_http2settings_); + stats_store_, server_http2settings_, + max_request_headers_); for (int i = 0; i < 101; ++i) { request_encoder_ = &client_->newStream(response_decoder_); @@ -877,17 +884,28 @@ TEST(Http2CodecUtility, reconstituteCrumbledCookies) { } } -// For issue #1421 regression test that Envoy's H2 codec applies header limits early. -TEST_P(Http2CodecImplTest, TestCodecHeaderLimits) { +TEST_P(Http2CodecImplTest, TestLargeHeadersInvokeResetStream) { initialize(); TestHeaderMapImpl request_headers; HttpTestUtility::addDefaultHeaders(request_headers); - std::string long_string = std::string(1024, 'q'); - for (int i = 0; i < 63; ++i) { - request_headers.addCopy(fmt::format("{}", i), long_string); - } - EXPECT_CALL(server_stream_callbacks_, onResetStream(_)); + std::string long_string = std::string(63 * 1024, 'q'); + request_headers.addCopy("big", long_string); + EXPECT_CALL(server_stream_callbacks_, onResetStream(_)).Times(1); + request_encoder_->encodeHeaders(request_headers, false); +} + +TEST_P(Http2CodecImplTest, TestLargeHeadersAcceptedIfConfigured) { + max_request_headers_ = 64; + initialize(); + + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + std::string long_string = std::string(63 * 1024, 'q'); + request_headers.addCopy("big", long_string); + + EXPECT_CALL(request_decoder_, decodeHeaders_(_, _)); + EXPECT_CALL(server_stream_callbacks_, onResetStream(_)).Times(0); request_encoder_->encodeHeaders(request_headers, false); } diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index d97f56258b3fb..0a14167e45525 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -218,7 +218,7 @@ FakeHttpConnection::FakeHttpConnection(SharedConnectionWrapper& shared_connectio settings.allow_connect_ = true; settings.allow_metadata_ = true; codec_ = std::make_unique(shared_connection_.connection(), - *this, store, settings); + *this, store, settings, Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE_KB); ASSERT(type == Type::HTTP2); } diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index 4287216c1392c..666abf707efc7 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -467,9 +467,9 @@ TEST_P(Http2IntegrationTest, RetryPriority) { testRetryPriority(); } TEST_P(Http2IntegrationTest, GrpcRetry) { testGrpcRetry(); } -TEST_P(Http2IntegrationTest, LargeHeadersInvokeResetStream) { testLargeRequestHeaders(62, 60); } +TEST_P(Http2IntegrationTest, LargeHeadersInvokeResetStream) { testLargeRequestHeaders(63, 60); } -TEST_P(Http2IntegrationTest, LargeHeadersAcceptedIfConfigured) { testLargeRequestHeaders(62, 63); } +TEST_P(Http2IntegrationTest, LargeHeadersAcceptedIfConfigured) { testLargeRequestHeaders(63, 64); } TEST_P(Http2IntegrationTest, EncodingHeaderOnlyResponse) { testHeadersOnlyFilterEncoding(); } diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index a2eac922b4a2b..8b2be331f9f68 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -490,9 +490,9 @@ TEST_P(IntegrationTest, InvalidContentLength) { testInvalidContentLength(); } TEST_P(IntegrationTest, MultipleContentLengths) { testMultipleContentLengths(); } -TEST_P(IntegrationTest, LargeHeadersRejected) { testLargeRequestHeaders(62, 60); } +TEST_P(IntegrationTest, LargeHeadersRejected) { testLargeRequestHeaders(63, 60); } -TEST_P(IntegrationTest, LargeHeadersAccepted) { testLargeRequestHeaders(62, 63); } +TEST_P(IntegrationTest, LargeHeadersAcceptedIfConfigured) { testLargeRequestHeaders(63, 64); } TEST_P(IntegrationTest, UpstreamProtocolError) { initialize(); From cb954f785243e252c7318b9f991bbd29ad691167 Mon Sep 17 00:00:00 2001 From: Auni Ahsan Date: Wed, 30 Jan 2019 09:56:00 -0500 Subject: [PATCH 02/13] Kill size_ and SIZE_ Signed-off-by: Auni Ahsan --- include/envoy/http/codec.h | 2 +- source/common/http/codec_client.cc | 2 +- .../filters/network/http_connection_manager/config.cc | 6 +++--- .../filters/network/http_connection_manager/config.h | 4 ++-- source/server/http/admin.h | 4 ++-- test/common/http/conn_manager_impl_fuzz_test.cc | 4 ++-- test/common/http/conn_manager_impl_test.cc | 6 +++--- test/common/http/http2/codec_impl_test.cc | 2 +- test/integration/fake_upstream.cc | 2 +- 9 files changed, 16 insertions(+), 16 deletions(-) diff --git a/include/envoy/http/codec.h b/include/envoy/http/codec.h index 7f4701a63ea21..584057e2b339f 100644 --- a/include/envoy/http/codec.h +++ b/include/envoy/http/codec.h @@ -14,7 +14,7 @@ namespace Envoy { namespace Http { // Legacy default value of 60K is safely under both codec default limits. -static const uint32_t DEFAULT_MAX_REQUEST_HEADERS_SIZE_KB = 60; +static const uint32_t DEFAULT_MAX_REQUEST_HEADERS_SIZE = 60; class Stream; diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc index 8d8402942d705..df6a04dfe5174 100644 --- a/source/common/http/codec_client.cc +++ b/source/common/http/codec_client.cc @@ -146,7 +146,7 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne case Type::HTTP2: { codec_ = std::make_unique( *connection_, *this, host->cluster().statsScope(), host->cluster().http2Settings(), - Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE_KB); + Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE); break; } } diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 45bfa257bdddd..c9fb5a483f9e4 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -140,7 +140,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( route_config_provider_manager_(route_config_provider_manager), http2_settings_(Http::Utility::parseHttp2Settings(config.http2_protocol_options())), http1_settings_(Http::Utility::parseHttp1Settings(config.http_protocol_options())), - max_request_headers_size_kb_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( + max_request_headers_kb_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( config, max_request_headers_kb, Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE_KB)), idle_timeout_(PROTOBUF_GET_OPTIONAL_MS(config, idle_timeout)), stream_idle_timeout_( @@ -326,11 +326,11 @@ HttpConnectionManagerConfig::createCodec(Network::Connection& connection, new Http::Http1::ServerConnectionImpl(connection, callbacks, http1_settings_)}; case CodecType::HTTP2: return Http::ServerConnectionPtr{new Http::Http2::ServerConnectionImpl( - connection, callbacks, context_.scope(), http2_settings_, max_request_headers_size_kb_)}; + connection, callbacks, context_.scope(), http2_settings_, max_request_headers_kb_)}; case CodecType::AUTO: return Http::ConnectionManagerUtility::autoCreateCodec( connection, data, callbacks, context_.scope(), http1_settings_, http2_settings_, - max_request_headers_size_kb_); + max_request_headers_kb_); } 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 05c3c982241df..9c9a909dadb67 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -101,7 +101,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, FilterChainFactory& filterFactory() override { return *this; } bool reverseEncodeOrder() override { return reverse_encode_order_; } bool generateRequestId() override { return generate_request_id_; } - uint32_t maxRequestHeadersSizeKb() const override { return max_request_headers_size_kb_; } + uint32_t maxRequestHeadersSizeKb() const override { return max_request_headers_kb_; } 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,7 +158,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, std::string server_name_; Http::TracingConnectionManagerConfigPtr tracing_config_; absl::optional user_agent_; - uint32_t max_request_headers_size_kb_; + uint32_t max_request_headers_kb_; absl::optional idle_timeout_; std::chrono::milliseconds stream_idle_timeout_; std::chrono::milliseconds request_timeout_; diff --git a/source/server/http/admin.h b/source/server/http/admin.h index 05b3c1fa840c5..c6d427f38cd7d 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -100,7 +100,7 @@ class AdminImpl : public Admin, bool reverseEncodeOrder() override { return false; } bool generateRequestId() override { return false; } absl::optional idleTimeout() const override { return idle_timeout_; } - uint32_t maxRequestHeadersSizeKb() const override { return max_request_headers_size_kb_; } + uint32_t maxRequestHeadersSizeKb() const override { return max_request_headers_kb_; } std::chrono::milliseconds streamIdleTimeout() const override { return {}; } std::chrono::milliseconds requestTimeout() const override { return {}; } std::chrono::milliseconds delayedCloseTimeout() const override { return {}; } @@ -305,7 +305,7 @@ class AdminImpl : public Admin, Http::ConnectionManagerTracingStats tracing_stats_; NullRouteConfigProvider route_config_provider_; std::list handlers_; - uint32_t max_request_headers_size_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE_KB}; + uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE}; absl::optional idle_timeout_; absl::optional user_agent_; Http::SlowDateProviderImpl date_provider_; diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index d2c53be04898b..bbde746fb9aa9 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -91,7 +91,7 @@ class FuzzConfig : public ConnectionManagerConfig { FilterChainFactory& filterFactory() override { return filter_factory_; } bool reverseEncodeOrder() override { return true; } bool generateRequestId() override { return true; } - uint32_t maxRequestHeadersSizeKb() const override { return max_request_headers_size_kb_; } + uint32_t maxRequestHeadersSizeKb() const override { return max_request_headers_kb_; } 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_; } @@ -132,7 +132,7 @@ class FuzzConfig : public ConnectionManagerConfig { ConnectionManagerStats stats_; ConnectionManagerTracingStats tracing_stats_; ConnectionManagerListenerStats listener_stats_; - uint32_t max_request_headers_size_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE_KB}; + uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE}; 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 527bb7b86578f..013a1f199de96 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -230,7 +230,7 @@ class HttpConnectionManagerImplTest : public TestBase, public ConnectionManagerC FilterChainFactory& filterFactory() override { return filter_factory_; } bool reverseEncodeOrder() override { return reverse_encode_order_; } bool generateRequestId() override { return true; } - uint32_t maxRequestHeadersSizeKb() const override { return max_request_headers_size_kb_; } + uint32_t maxRequestHeadersSizeKb() const override { return max_request_headers_kb_; } 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_; } @@ -281,7 +281,7 @@ class HttpConnectionManagerImplTest : public TestBase, public ConnectionManagerC Http::ForwardClientCertType forward_client_cert_{Http::ForwardClientCertType::Sanitize}; std::vector set_current_client_cert_details_; absl::optional user_agent_; - uint32_t max_request_headers_size_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE_KB}; + uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE}; absl::optional idle_timeout_; std::chrono::milliseconds stream_idle_timeout_{}; std::chrono::milliseconds request_timeout_{}; @@ -3624,7 +3624,7 @@ TEST_F(HttpConnectionManagerImplTest, OverlyLongHeadersRejected) { } TEST_F(HttpConnectionManagerImplTest, OverlyLongHeadersAcceptedIfConfigured) { - max_request_headers_size_kb_ = 62; + max_request_headers_kb_ = 62; setup(false, ""); EXPECT_CALL(*codec_, dispatch(_)).Times(1).WillOnce(Invoke([&](Buffer::Instance&) -> void { diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index 9c88606e59637..d066544b8a930 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -148,7 +148,7 @@ class Http2CodecImplTest : public TestBaseWithParam { StreamEncoder* response_encoder_{}; MockStreamCallbacks server_stream_callbacks_; bool corrupt_data_ = false; - uint32_t max_request_headers_ = Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE_KB; + uint32_t max_request_headers_ = Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE; }; TEST_P(Http2CodecImplTest, ShutdownNotice) { diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index 0a14167e45525..a0986a2ea2002 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -218,7 +218,7 @@ FakeHttpConnection::FakeHttpConnection(SharedConnectionWrapper& shared_connectio settings.allow_connect_ = true; settings.allow_metadata_ = true; codec_ = std::make_unique(shared_connection_.connection(), - *this, store, settings, Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE_KB); + *this, store, settings, Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE); ASSERT(type == Type::HTTP2); } From 383407b455d0b50e42dd016716d8983ca7579240 Mon Sep 17 00:00:00 2001 From: Auni Ahsan Date: Wed, 30 Jan 2019 12:28:17 -0500 Subject: [PATCH 03/13] more renames Signed-off-by: Auni Ahsan --- include/envoy/http/codec.h | 2 +- source/common/http/codec_client.cc | 2 +- source/common/http/conn_manager_config.h | 2 +- source/common/http/conn_manager_impl.cc | 4 ++-- .../filters/network/http_connection_manager/config.cc | 2 +- .../filters/network/http_connection_manager/config.h | 4 ++-- source/server/http/admin.h | 6 +++--- test/common/http/conn_manager_impl_fuzz_test.cc | 4 ++-- test/common/http/conn_manager_impl_test.cc | 4 ++-- test/common/http/conn_manager_utility_test.cc | 2 +- test/common/http/http2/codec_impl_test.cc | 2 +- .../filters/network/http_connection_manager/config_test.cc | 4 ++-- test/integration/fake_upstream.cc | 2 +- 13 files changed, 20 insertions(+), 20 deletions(-) diff --git a/include/envoy/http/codec.h b/include/envoy/http/codec.h index 584057e2b339f..568a735339e7f 100644 --- a/include/envoy/http/codec.h +++ b/include/envoy/http/codec.h @@ -14,7 +14,7 @@ namespace Envoy { namespace Http { // Legacy default value of 60K is safely under both codec default limits. -static const uint32_t DEFAULT_MAX_REQUEST_HEADERS_SIZE = 60; +static const uint32_t DEFAULT_MAX_REQUEST_HEADERS_KB = 60; class Stream; diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc index df6a04dfe5174..3f8defb71ee6d 100644 --- a/source/common/http/codec_client.cc +++ b/source/common/http/codec_client.cc @@ -146,7 +146,7 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne case Type::HTTP2: { codec_ = std::make_unique( *connection_, *this, host->cluster().statsScope(), host->cluster().http2Settings(), - Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE); + Http::DEFAULT_MAX_REQUEST_HEADERS_KB); break; } } diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index ee7b486aaf973..32d7d8b7d2f69 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -231,7 +231,7 @@ class ConnectionManagerConfig { /** * @return maximum request headers size the connection manager will accept. */ - virtual uint32_t maxRequestHeadersSizeKb() const PURE; + virtual uint32_t maxRequestHeadersKb() const PURE; /** * @return per-stream idle timeout for incoming connection manager connections. Zero indicates a diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index c941694efb99d..7682ec44fe0f7 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -617,9 +617,9 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, } } - ASSERT(connection_manager_.config_.maxRequestHeadersSizeKb() > 0); + ASSERT(connection_manager_.config_.maxRequestHeadersKb() > 0); if (request_headers_->byteSize() > - (connection_manager_.config_.maxRequestHeadersSizeKb() * 1024)) { + (connection_manager_.config_.maxRequestHeadersKb() * 1024)) { sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::RequestHeaderFieldsTooLarge, "", nullptr, is_head_request_, absl::nullopt); return; diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index c9fb5a483f9e4..d2fc17194a017 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -141,7 +141,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( http2_settings_(Http::Utility::parseHttp2Settings(config.http2_protocol_options())), 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_SIZE_KB)), + config, max_request_headers_kb, Http::DEFAULT_MAX_REQUEST_HEADERS_KB)), idle_timeout_(PROTOBUF_GET_OPTIONAL_MS(config, idle_timeout)), stream_idle_timeout_( PROTOBUF_GET_MS_OR_DEFAULT(config, stream_idle_timeout, StreamIdleTimeoutMs)), diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index 9c9a909dadb67..e81f925ce1a0d 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -95,13 +95,13 @@ class HttpConnectionManagerConfig : Logger::Loggable, const std::list& accessLogs() override { return access_logs_; } Http::ServerConnectionPtr createCodec(Network::Connection& connection, const Buffer::Instance& data, - Http::ServerConnectionCallbacks& callbacks) override; + Http::ServerConnectionCallbacks& callbacks, uint32_t max_request_headers) override; Http::DateProvider& dateProvider() override { return date_provider_; } std::chrono::milliseconds drainTimeout() override { return drain_timeout_; } FilterChainFactory& filterFactory() override { return *this; } bool reverseEncodeOrder() override { return reverse_encode_order_; } bool generateRequestId() override { return generate_request_id_; } - uint32_t maxRequestHeadersSizeKb() const override { return max_request_headers_kb_; } + uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } 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_; } diff --git a/source/server/http/admin.h b/source/server/http/admin.h index c6d427f38cd7d..e81efdf975c1a 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -93,14 +93,14 @@ class AdminImpl : public Admin, const std::list& accessLogs() override { return access_logs_; } Http::ServerConnectionPtr createCodec(Network::Connection& connection, const Buffer::Instance& data, - Http::ServerConnectionCallbacks& callbacks) override; + Http::ServerConnectionCallbacks& callbacks, uint32_t max_request_headers_kb) override; Http::DateProvider& dateProvider() override { return date_provider_; } std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); } Http::FilterChainFactory& filterFactory() override { return *this; } bool reverseEncodeOrder() override { return false; } bool generateRequestId() override { return false; } absl::optional idleTimeout() const override { return idle_timeout_; } - uint32_t maxRequestHeadersSizeKb() const override { return max_request_headers_kb_; } + uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } std::chrono::milliseconds streamIdleTimeout() const override { return {}; } std::chrono::milliseconds requestTimeout() const override { return {}; } std::chrono::milliseconds delayedCloseTimeout() const override { return {}; } @@ -305,7 +305,7 @@ class AdminImpl : public Admin, Http::ConnectionManagerTracingStats tracing_stats_; NullRouteConfigProvider route_config_provider_; std::list handlers_; - uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE}; + uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB}; absl::optional idle_timeout_; absl::optional user_agent_; Http::SlowDateProviderImpl date_provider_; diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index bbde746fb9aa9..40e85bfc23cbf 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -91,7 +91,7 @@ class FuzzConfig : public ConnectionManagerConfig { FilterChainFactory& filterFactory() override { return filter_factory_; } bool reverseEncodeOrder() override { return true; } bool generateRequestId() override { return true; } - uint32_t maxRequestHeadersSizeKb() const override { return max_request_headers_kb_; } + uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } 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_; } @@ -132,7 +132,7 @@ class FuzzConfig : public ConnectionManagerConfig { ConnectionManagerStats stats_; ConnectionManagerTracingStats tracing_stats_; ConnectionManagerListenerStats listener_stats_; - uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE}; + uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB}; 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 013a1f199de96..39885de7c4e0c 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -230,7 +230,7 @@ class HttpConnectionManagerImplTest : public TestBase, public ConnectionManagerC FilterChainFactory& filterFactory() override { return filter_factory_; } bool reverseEncodeOrder() override { return reverse_encode_order_; } bool generateRequestId() override { return true; } - uint32_t maxRequestHeadersSizeKb() const override { return max_request_headers_kb_; } + uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } 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_; } @@ -281,7 +281,7 @@ class HttpConnectionManagerImplTest : public TestBase, public ConnectionManagerC Http::ForwardClientCertType forward_client_cert_{Http::ForwardClientCertType::Sanitize}; std::vector set_current_client_cert_details_; absl::optional user_agent_; - uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE}; + uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB}; absl::optional idle_timeout_; std::chrono::milliseconds stream_idle_timeout_{}; std::chrono::milliseconds request_timeout_{}; diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 6ede90b769f41..6fa6608d219da 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -51,7 +51,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_METHOD0(filterFactory, FilterChainFactory&()); MOCK_METHOD0(reverseEncodeOrder, bool()); MOCK_METHOD0(generateRequestId, bool()); - MOCK_CONST_METHOD0(maxRequestHeadersSizeKb, uint32_t()); + MOCK_CONST_METHOD0(maxRequestHeadersKb, 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/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index d066544b8a930..b18ccd2d037fd 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -148,7 +148,7 @@ class Http2CodecImplTest : public TestBaseWithParam { StreamEncoder* response_encoder_{}; MockStreamCallbacks server_stream_callbacks_; bool corrupt_data_ = false; - uint32_t max_request_headers_ = Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE; + uint32_t max_request_headers_ = Http::DEFAULT_MAX_REQUEST_HEADERS_KB; }; TEST_P(Http2CodecImplTest, ShutdownNotice) { 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 4fe6b5d7cc92d..2e42f07f03dcb 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -163,7 +163,7 @@ TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeadersSizeDefault) { HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, date_provider_, route_config_provider_manager_); - EXPECT_EQ(60, config.maxRequestHeadersSizeKb()); + EXPECT_EQ(60, config.maxRequestHeadersKb()); } TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeadersSizeConfigured) { @@ -178,7 +178,7 @@ TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeadersSizeConfigured) { HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, date_provider_, route_config_provider_manager_); - EXPECT_EQ(16, config.maxRequestHeadersSizeKb()); + EXPECT_EQ(16, config.maxRequestHeadersKb()); } // Validated that an explicit zero stream idle timeout disables. diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index a0986a2ea2002..799af6e1e9b58 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -218,7 +218,7 @@ FakeHttpConnection::FakeHttpConnection(SharedConnectionWrapper& shared_connectio settings.allow_connect_ = true; settings.allow_metadata_ = true; codec_ = std::make_unique(shared_connection_.connection(), - *this, store, settings, Http::DEFAULT_MAX_REQUEST_HEADERS_SIZE); + *this, store, settings, Http::DEFAULT_MAX_REQUEST_HEADERS_KB); ASSERT(type == Type::HTTP2); } From e34afec16a4a32229d85c6e061b1dc241f410ad3 Mon Sep 17 00:00:00 2001 From: Auni Ahsan Date: Wed, 30 Jan 2019 12:49:55 -0500 Subject: [PATCH 04/13] everything builds Signed-off-by: Auni Ahsan --- .../filters/network/http_connection_manager/config.cc | 4 ++-- .../filters/network/http_connection_manager/config.h | 2 +- source/server/http/admin.cc | 2 +- source/server/http/admin.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index d2fc17194a017..0e1b580ac9d7d 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -326,11 +326,11 @@ HttpConnectionManagerConfig::createCodec(Network::Connection& connection, new Http::Http1::ServerConnectionImpl(connection, callbacks, http1_settings_)}; case CodecType::HTTP2: return Http::ServerConnectionPtr{new Http::Http2::ServerConnectionImpl( - connection, callbacks, context_.scope(), http2_settings_, max_request_headers_kb_)}; + connection, callbacks, context_.scope(), http2_settings_, maxRequestHeadersKb())}; case CodecType::AUTO: return Http::ConnectionManagerUtility::autoCreateCodec( connection, data, callbacks, context_.scope(), http1_settings_, http2_settings_, - max_request_headers_kb_); + maxRequestHeadersKb()); } 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 e81f925ce1a0d..f49793313da16 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -95,7 +95,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, const std::list& accessLogs() override { return access_logs_; } Http::ServerConnectionPtr createCodec(Network::Connection& connection, const Buffer::Instance& data, - Http::ServerConnectionCallbacks& callbacks, uint32_t max_request_headers) override; + Http::ServerConnectionCallbacks& callbacks) override; Http::DateProvider& dateProvider() override { return date_provider_; } std::chrono::milliseconds drainTimeout() override { return drain_timeout_; } FilterChainFactory& filterFactory() override { return *this; } diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index c16b7ae5396ba..a70fc2fa8b39e 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1106,7 +1106,7 @@ Http::ServerConnectionPtr AdminImpl::createCodec(Network::Connection& connection const Buffer::Instance& data, Http::ServerConnectionCallbacks& callbacks) { return Http::ConnectionManagerUtility::autoCreateCodec( - connection, data, callbacks, server_.stats(), Http::Http1Settings(), Http::Http2Settings()); + connection, data, callbacks, server_.stats(), Http::Http1Settings(), Http::Http2Settings(), maxRequestHeadersKb()); } bool AdminImpl::createNetworkFilterChain(Network::Connection& connection, diff --git a/source/server/http/admin.h b/source/server/http/admin.h index e81efdf975c1a..477f0e75e28a4 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -93,7 +93,7 @@ class AdminImpl : public Admin, const std::list& accessLogs() override { return access_logs_; } Http::ServerConnectionPtr createCodec(Network::Connection& connection, const Buffer::Instance& data, - Http::ServerConnectionCallbacks& callbacks, uint32_t max_request_headers_kb) override; + Http::ServerConnectionCallbacks& callbacks) override; Http::DateProvider& dateProvider() override { return date_provider_; } std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); } Http::FilterChainFactory& filterFactory() override { return *this; } From 4466c8cb3f88f9f4335572b0eff328f0af612877 Mon Sep 17 00:00:00 2001 From: Auni Ahsan Date: Wed, 30 Jan 2019 12:55:37 -0500 Subject: [PATCH 05/13] fix format Signed-off-by: Auni Ahsan --- source/common/http/conn_manager_impl.cc | 3 +-- source/common/http/conn_manager_utility.h | 4 ++-- .../filters/network/http_connection_manager/config.cc | 6 +++--- source/server/http/admin.cc | 3 ++- test/integration/fake_upstream.cc | 5 +++-- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 7682ec44fe0f7..8805eacffe33e 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -618,8 +618,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, } ASSERT(connection_manager_.config_.maxRequestHeadersKb() > 0); - if (request_headers_->byteSize() > - (connection_manager_.config_.maxRequestHeadersKb() * 1024)) { + if (request_headers_->byteSize() > (connection_manager_.config_.maxRequestHeadersKb() * 1024)) { sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::RequestHeaderFieldsTooLarge, "", nullptr, is_head_request_, absl::nullopt); return; diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h index 702c43910d7e0..a1ecd6bcafddd 100644 --- a/source/common/http/conn_manager_utility.h +++ b/source/common/http/conn_manager_utility.h @@ -36,8 +36,8 @@ class ConnectionManagerUtility { static ServerConnectionPtr autoCreateCodec(Network::Connection& connection, const Buffer::Instance& data, ServerConnectionCallbacks& callbacks, Stats::Scope& scope, - const Http1Settings& http1_settings, - const Http2Settings& http2_settings, uint32_t max_request_headers_kb); + const Http1Settings& http1_settings, const Http2Settings& http2_settings, + uint32_t max_request_headers_kb); /** * Mutates request headers in various ways. This functionality is broken out because of its diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 0e1b580ac9d7d..dcc0a58531f92 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -328,9 +328,9 @@ HttpConnectionManagerConfig::createCodec(Network::Connection& connection, return Http::ServerConnectionPtr{new Http::Http2::ServerConnectionImpl( connection, callbacks, context_.scope(), http2_settings_, maxRequestHeadersKb())}; 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()); } NOT_REACHED_GCOVR_EXCL_LINE; diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index a70fc2fa8b39e..ade0137129bfc 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1106,7 +1106,8 @@ Http::ServerConnectionPtr AdminImpl::createCodec(Network::Connection& connection const Buffer::Instance& data, Http::ServerConnectionCallbacks& callbacks) { return Http::ConnectionManagerUtility::autoCreateCodec( - connection, data, callbacks, server_.stats(), Http::Http1Settings(), Http::Http2Settings(), maxRequestHeadersKb()); + connection, data, callbacks, server_.stats(), Http::Http1Settings(), Http::Http2Settings(), + maxRequestHeadersKb()); } bool AdminImpl::createNetworkFilterChain(Network::Connection& connection, diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index 799af6e1e9b58..0a48b7e0c526f 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -217,8 +217,9 @@ FakeHttpConnection::FakeHttpConnection(SharedConnectionWrapper& shared_connectio auto settings = Http::Http2Settings(); settings.allow_connect_ = true; settings.allow_metadata_ = true; - codec_ = std::make_unique(shared_connection_.connection(), - *this, store, settings, Http::DEFAULT_MAX_REQUEST_HEADERS_KB); + codec_ = std::make_unique( + shared_connection_.connection(), *this, store, settings, + Http::DEFAULT_MAX_REQUEST_HEADERS_KB); ASSERT(type == Type::HTTP2); } From e2015a60710f0a2d9879fd53231cb13a2609af0f Mon Sep 17 00:00:00 2001 From: Auni Ahsan Date: Wed, 30 Jan 2019 12:59:00 -0500 Subject: [PATCH 06/13] Remove old check and stat Signed-off-by: Auni Ahsan --- docs/root/configuration/http_conn_man/stats.rst | 2 +- source/common/http/http2/codec_impl.h | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/docs/root/configuration/http_conn_man/stats.rst b/docs/root/configuration/http_conn_man/stats.rst index 188b79d1873cb..3b65ddfb660f2 100644 --- a/docs/root/configuration/http_conn_man/stats.rst +++ b/docs/root/configuration/http_conn_man/stats.rst @@ -109,7 +109,7 @@ All http2 statistics are rooted at *http2.* :header: Name, Type, Description :widths: 1, 1, 2 - header_overflow, Counter, Total number of connections reset due to the headers being larger than `Envoy::Http::Http2::ConnectionImpl::StreamImpl::MAX_HEADER_SIZE` (63k) + header_overflow, Counter, Total number of connections reset due to the headers being larger than configured value. headers_cb_no_stream, Counter, Total number of errors where a header callback is called without an associated stream. This tracks an unexpected occurrence due to an as yet undiagnosed bug rx_messaging_error, Counter, Total number of invalid received frames that violated `section 8 `_ of the HTTP/2 spec. This will result in a *tx_reset* rx_reset, Counter, Total number of reset stream frames received by Envoy diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h index f3b733515c986..f8c497e86bc99 100644 --- a/source/common/http/http2/codec_impl.h +++ b/source/common/http/http2/codec_impl.h @@ -185,11 +185,6 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable Date: Wed, 30 Jan 2019 15:22:17 -0500 Subject: [PATCH 07/13] Move test connection and server into util Signed-off-by: Auni Ahsan --- test/common/http/http2/BUILD | 11 +++++++ .../common/http/http2/codec_impl_fuzz_test.cc | 26 ++++------------ test/common/http/http2/codec_impl_test.cc | 21 +------------ test/common/http/http2/codec_impl_test_util.h | 31 +++++++++++++++++++ 4 files changed, 49 insertions(+), 40 deletions(-) create mode 100644 test/common/http/http2/codec_impl_test_util.h diff --git a/test/common/http/http2/BUILD b/test/common/http/http2/BUILD index 27677cfd9b454..436b29f83860d 100644 --- a/test/common/http/http2/BUILD +++ b/test/common/http/http2/BUILD @@ -4,6 +4,7 @@ load( "//bazel:envoy_build_system.bzl", "envoy_cc_fuzz_test", "envoy_cc_test", + "envoy_cc_test_library", "envoy_package", "envoy_proto_library", ) @@ -22,6 +23,7 @@ envoy_cc_fuzz_test( corpus = "codec_impl_corpus", deps = [ ":codec_impl_fuzz_proto_cc", + ":codec_impl_test_util", "//source/common/http:header_map_lib", "//source/common/http/http2:codec_lib", "//test/fuzz:utility_lib", @@ -35,6 +37,7 @@ envoy_cc_test( srcs = ["codec_impl_test.cc"], shard_count = 5, deps = [ + ":codec_impl_test_util", "//source/common/event:dispatcher_lib", "//source/common/http:exception_lib", "//source/common/http:header_map_lib", @@ -48,6 +51,14 @@ envoy_cc_test( ], ) +envoy_cc_test_library( + name = "codec_impl_test_util", + hdrs = ["codec_impl_test_util.h"], + deps = [ + "//source/common/http/http2:codec_lib", + ], +) + envoy_cc_test( name = "conn_pool_test", srcs = ["conn_pool_test.cc"], diff --git a/test/common/http/http2/codec_impl_fuzz_test.cc b/test/common/http/http2/codec_impl_fuzz_test.cc index c023d11207420..32f5af6e981c3 100644 --- a/test/common/http/http2/codec_impl_fuzz_test.cc +++ b/test/common/http/http2/codec_impl_fuzz_test.cc @@ -9,6 +9,8 @@ #include +#include "codec_impl_test_util.h" + #include "common/common/assert.h" #include "common/common/logger.h" #include "common/http/header_map_impl.h" @@ -30,24 +32,6 @@ namespace Envoy { namespace Http { namespace Http2 { -class TestServerConnectionImpl : public ServerConnectionImpl { -public: - TestServerConnectionImpl(Network::Connection& connection, ServerConnectionCallbacks& callbacks, - Stats::Scope& scope, const Http2Settings& http2_settings) - : ServerConnectionImpl(connection, callbacks, scope, http2_settings) {} - nghttp2_session* session() { return session_; } - using ServerConnectionImpl::getStream; -}; - -class TestClientConnectionImpl : public ClientConnectionImpl { -public: - TestClientConnectionImpl(Network::Connection& connection, Http::ConnectionCallbacks& callbacks, - Stats::Scope& scope, const Http2Settings& http2_settings) - : ClientConnectionImpl(connection, callbacks, scope, http2_settings) {} - nghttp2_session* session() { return session_; } - using ClientConnectionImpl::getStream; -}; - // Convert from test proto Http2Settings to Http2Settings. Http2Settings fromHttp2Settings(const test::common::http::http2::Http2Settings& settings) { Http2Settings h2_settings; @@ -260,14 +244,16 @@ DEFINE_PROTO_FUZZER(const test::common::http::http2::CodecImplFuzzTestCase& inpu NiceMock client_connection; const Http2Settings client_http2settings{fromHttp2Settings(input.client_settings())}; NiceMock client_callbacks; + uint32_t max_request_headers_kb = Http::DEFAULT_MAX_REQUEST_HEADERS_KB; + TestClientConnectionImpl client(client_connection, client_callbacks, stats_store, - client_http2settings); + client_http2settings, max_request_headers_kb); NiceMock server_connection; const Http2Settings server_http2settings{fromHttp2Settings(input.server_settings())}; NiceMock server_callbacks; TestServerConnectionImpl server(server_connection, server_callbacks, stats_store, - server_http2settings); + server_http2settings, max_request_headers_kb); ReorderBuffer client_write_buf{server}; ReorderBuffer server_write_buf{client}; diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index b18ccd2d037fd..26d3444cb3c59 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -15,6 +15,7 @@ #include "test/test_common/test_base.h" #include "test/test_common/utility.h" +#include "codec_impl_test_util.h" #include "gmock/gmock.h" using testing::_; @@ -33,26 +34,6 @@ namespace Http2 { using Http2SettingsTuple = ::testing::tuple; using Http2SettingsTestParam = ::testing::tuple; -class TestServerConnectionImpl : public ServerConnectionImpl { -public: - TestServerConnectionImpl(Network::Connection& connection, ServerConnectionCallbacks& callbacks, - Stats::Scope& scope, const Http2Settings& http2_settings, - uint32_t max_request_headers) - : ServerConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers) {} - nghttp2_session* session() { return session_; } - using ServerConnectionImpl::getStream; -}; - -class TestClientConnectionImpl : public ClientConnectionImpl { -public: - TestClientConnectionImpl(Network::Connection& connection, Http::ConnectionCallbacks& callbacks, - Stats::Scope& scope, const Http2Settings& http2_settings, - uint32_t max_request_headers) - : ClientConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers) {} - nghttp2_session* session() { return session_; } - using ClientConnectionImpl::getStream; -}; - class Http2CodecImplTest : public TestBaseWithParam { public: struct ConnectionWrapper { diff --git a/test/common/http/http2/codec_impl_test_util.h b/test/common/http/http2/codec_impl_test_util.h new file mode 100644 index 0000000000000..7460c634503fb --- /dev/null +++ b/test/common/http/http2/codec_impl_test_util.h @@ -0,0 +1,31 @@ +#include "envoy/http/codec.h" + +#include "common/http/http2/codec_impl.h" + +namespace Envoy { +namespace Http { +namespace Http2 { + +class TestServerConnectionImpl : public ServerConnectionImpl { +public: + TestServerConnectionImpl(Network::Connection& connection, ServerConnectionCallbacks& callbacks, + Stats::Scope& scope, const Http2Settings& http2_settings, + uint32_t max_request_headers) + : ServerConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers) {} + nghttp2_session* session() { return session_; } + using ServerConnectionImpl::getStream; +}; + +class TestClientConnectionImpl : public ClientConnectionImpl { +public: + TestClientConnectionImpl(Network::Connection& connection, Http::ConnectionCallbacks& callbacks, + Stats::Scope& scope, const Http2Settings& http2_settings, + uint32_t max_request_headers) + : ClientConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers) {} + nghttp2_session* session() { return session_; } + using ClientConnectionImpl::getStream; +}; + +} // namespace Http2 +} // namespace Http +} // namespace Envoy From 50e57369045ac1a5c81f3e8e5edf75eb5b20ecba Mon Sep 17 00:00:00 2001 From: Auni Ahsan Date: Thu, 31 Jan 2019 12:41:01 -0500 Subject: [PATCH 08/13] correct docs Signed-off-by: Auni Ahsan --- docs/root/configuration/http_conn_man/stats.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/configuration/http_conn_man/stats.rst b/docs/root/configuration/http_conn_man/stats.rst index 3b65ddfb660f2..f763de2efaea2 100644 --- a/docs/root/configuration/http_conn_man/stats.rst +++ b/docs/root/configuration/http_conn_man/stats.rst @@ -109,7 +109,7 @@ All http2 statistics are rooted at *http2.* :header: Name, Type, Description :widths: 1, 1, 2 - header_overflow, Counter, Total number of connections reset due to the headers being larger than configured value. + header_overflow, Counter, Total number of connections reset due to the headers being larger than the configured value. headers_cb_no_stream, Counter, Total number of errors where a header callback is called without an associated stream. This tracks an unexpected occurrence due to an as yet undiagnosed bug rx_messaging_error, Counter, Total number of invalid received frames that violated `section 8 `_ of the HTTP/2 spec. This will result in a *tx_reset* rx_reset, Counter, Total number of reset stream frames received by Envoy From 632b612db8285c333184988edb8ce01d7cf0ac74 Mon Sep 17 00:00:00 2001 From: Auni Ahsan Date: Thu, 31 Jan 2019 12:41:07 -0500 Subject: [PATCH 09/13] move remaining non-kb max request header vars Signed-off-by: Auni Ahsan --- source/common/http/http2/codec_impl.cc | 8 ++++---- source/common/http/http2/codec_impl.h | 4 ++-- test/common/http/http2/codec_impl_test.cc | 12 ++++++------ test/common/http/http2/codec_impl_test_util.h | 8 ++++---- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index e7e8638e9b5a2..822ed5ff891e8 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -878,8 +878,8 @@ ConnectionImpl::ClientHttp2Options::ClientHttp2Options(const Http2Settings& http ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, Http::ConnectionCallbacks& callbacks, Stats::Scope& stats, const Http2Settings& http2_settings, - uint32_t max_request_headers) - : ConnectionImpl(connection, stats, http2_settings, max_request_headers), + uint32_t max_request_headers_kb) + : ConnectionImpl(connection, stats, http2_settings, max_request_headers_kb), callbacks_(callbacks) { ClientHttp2Options client_http2_options(http2_settings); nghttp2_session_client_new2(&session_, http2_callbacks_.callbacks(), base(), @@ -927,8 +927,8 @@ int ClientConnectionImpl::onHeader(const nghttp2_frame* frame, HeaderString&& na ServerConnectionImpl::ServerConnectionImpl(Network::Connection& connection, Http::ServerConnectionCallbacks& callbacks, Stats::Scope& scope, const Http2Settings& http2_settings, - uint32_t max_request_headers) - : ConnectionImpl(connection, scope, http2_settings, max_request_headers), + uint32_t max_request_headers_kb) + : ConnectionImpl(connection, scope, http2_settings, max_request_headers_kb), 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 f8c497e86bc99..a91ec5251ef05 100644 --- a/source/common/http/http2/codec_impl.h +++ b/source/common/http/http2/codec_impl.h @@ -316,7 +316,7 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl { public: ClientConnectionImpl(Network::Connection& connection, ConnectionCallbacks& callbacks, Stats::Scope& stats, const Http2Settings& http2_settings, - uint32_t max_request_headers); + uint32_t max_request_headers_kb); // Http::ClientConnection Http::StreamEncoder& newStream(StreamDecoder& response_decoder) override; @@ -337,7 +337,7 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl { public: ServerConnectionImpl(Network::Connection& connection, ServerConnectionCallbacks& callbacks, Stats::Scope& scope, const Http2Settings& http2_settings, - uint32_t max_request_headers); + uint32_t max_request_headers_kb); private: // ConnectionImpl diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index 26d3444cb3c59..fd75392355c29 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -57,10 +57,10 @@ class Http2CodecImplTest : public TestBaseWithParam { Http2SettingsFromTuple(server_http2settings_, ::testing::get<1>(GetParam())); client_ = std::make_unique(client_connection_, client_callbacks_, stats_store_, client_http2settings_, - max_request_headers_); + max_request_headers_kb_); server_ = std::make_unique(server_connection_, server_callbacks_, stats_store_, server_http2settings_, - max_request_headers_); + max_request_headers_kb_); request_encoder_ = &client_->newStream(response_decoder_); setupDefaultConnectionMocks(); @@ -129,7 +129,7 @@ class Http2CodecImplTest : public TestBaseWithParam { StreamEncoder* response_encoder_{}; MockStreamCallbacks server_stream_callbacks_; bool corrupt_data_ = false; - uint32_t max_request_headers_ = Http::DEFAULT_MAX_REQUEST_HEADERS_KB; + uint32_t max_request_headers_kb_ = Http::DEFAULT_MAX_REQUEST_HEADERS_KB; }; TEST_P(Http2CodecImplTest, ShutdownNotice) { @@ -762,10 +762,10 @@ TEST_P(Http2CodecImplStreamLimitTest, MaxClientStreams) { Http2SettingsFromTuple(server_http2settings_, ::testing::get<1>(GetParam())); client_ = std::make_unique(client_connection_, client_callbacks_, stats_store_, client_http2settings_, - max_request_headers_); + max_request_headers_kb_); server_ = std::make_unique(server_connection_, server_callbacks_, stats_store_, server_http2settings_, - max_request_headers_); + max_request_headers_kb_); for (int i = 0; i < 101; ++i) { request_encoder_ = &client_->newStream(response_decoder_); @@ -877,7 +877,7 @@ TEST_P(Http2CodecImplTest, TestLargeHeadersInvokeResetStream) { } TEST_P(Http2CodecImplTest, TestLargeHeadersAcceptedIfConfigured) { - max_request_headers_ = 64; + max_request_headers_kb_ = 64; initialize(); TestHeaderMapImpl request_headers; diff --git a/test/common/http/http2/codec_impl_test_util.h b/test/common/http/http2/codec_impl_test_util.h index 7460c634503fb..840f442b19fb4 100644 --- a/test/common/http/http2/codec_impl_test_util.h +++ b/test/common/http/http2/codec_impl_test_util.h @@ -10,8 +10,8 @@ class TestServerConnectionImpl : public ServerConnectionImpl { public: TestServerConnectionImpl(Network::Connection& connection, ServerConnectionCallbacks& callbacks, Stats::Scope& scope, const Http2Settings& http2_settings, - uint32_t max_request_headers) - : ServerConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers) {} + uint32_t max_request_headers_kb) + : ServerConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers_kb) {} nghttp2_session* session() { return session_; } using ServerConnectionImpl::getStream; }; @@ -20,8 +20,8 @@ class TestClientConnectionImpl : public ClientConnectionImpl { public: TestClientConnectionImpl(Network::Connection& connection, Http::ConnectionCallbacks& callbacks, Stats::Scope& scope, const Http2Settings& http2_settings, - uint32_t max_request_headers) - : ClientConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers) {} + uint32_t max_request_headers_kb) + : ClientConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers_kb) {} nghttp2_session* session() { return session_; } using ClientConnectionImpl::getStream; }; From 63b671e142622f58505e64a98d97b51260d8c894 Mon Sep 17 00:00:00 2001 From: Auni Ahsan Date: Fri, 1 Feb 2019 11:39:30 -0500 Subject: [PATCH 10/13] fix format Signed-off-by: Auni Ahsan --- test/common/http/http2/codec_impl_test_util.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/common/http/http2/codec_impl_test_util.h b/test/common/http/http2/codec_impl_test_util.h index 840f442b19fb4..b642701265c44 100644 --- a/test/common/http/http2/codec_impl_test_util.h +++ b/test/common/http/http2/codec_impl_test_util.h @@ -11,7 +11,8 @@ class TestServerConnectionImpl : public ServerConnectionImpl { 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) {} + : ServerConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers_kb) { + } nghttp2_session* session() { return session_; } using ServerConnectionImpl::getStream; }; @@ -21,7 +22,8 @@ class TestClientConnectionImpl : public ClientConnectionImpl { 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) {} + : ClientConnectionImpl(connection, callbacks, scope, http2_settings, max_request_headers_kb) { + } nghttp2_session* session() { return session_; } using ClientConnectionImpl::getStream; }; From d9b3d7789e200f5cd95325db1b46e1f6bcdd2882 Mon Sep 17 00:00:00 2001 From: Auni Ahsan Date: Fri, 1 Feb 2019 14:09:28 -0500 Subject: [PATCH 11/13] add at-limit test Signed-off-by: Auni Ahsan --- test/common/http/http2/codec_impl_test.cc | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index fd75392355c29..2399511a129e3 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -890,6 +890,29 @@ TEST_P(Http2CodecImplTest, TestLargeHeadersAcceptedIfConfigured) { request_encoder_->encodeHeaders(request_headers, false); } +TEST_P(Http2CodecImplTest, TestLargeHeadersAtLimitAccepted) { + uint32_t codec_limit_kb = 64; + max_request_headers_kb_ = codec_limit_kb; + initialize(); + + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + 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; + 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); + + EXPECT_CALL(request_decoder_, decodeHeaders_(_, _)); + request_encoder_->encodeHeaders(request_headers, true); +} + TEST_P(Http2CodecImplTest, TestCodecHeaderCompression) { initialize(); From 0e6dc368246258ce48a821d6afd96b8d9c94bb75 Mon Sep 17 00:00:00 2001 From: Auni Ahsan Date: Mon, 4 Feb 2019 12:12:59 -0500 Subject: [PATCH 12/13] docs change Signed-off-by: Auni Ahsan --- docs/root/configuration/http_conn_man/stats.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/configuration/http_conn_man/stats.rst b/docs/root/configuration/http_conn_man/stats.rst index f763de2efaea2..174fa99bdb105 100644 --- a/docs/root/configuration/http_conn_man/stats.rst +++ b/docs/root/configuration/http_conn_man/stats.rst @@ -109,7 +109,7 @@ All http2 statistics are rooted at *http2.* :header: Name, Type, Description :widths: 1, 1, 2 - header_overflow, Counter, Total number of connections reset due to the headers being larger than the configured value. + header_overflow, Counter, Total number of connections reset due to the headers being larger than the :ref:`configured value `. headers_cb_no_stream, Counter, Total number of errors where a header callback is called without an associated stream. This tracks an unexpected occurrence due to an as yet undiagnosed bug rx_messaging_error, Counter, Total number of invalid received frames that violated `section 8 `_ of the HTTP/2 spec. This will result in a *tx_reset* rx_reset, Counter, Total number of reset stream frames received by Envoy From 937e3d01bdc2f3b13a2ba53ecd73292403ce9a92 Mon Sep 17 00:00:00 2001 From: Auni Ahsan Date: Mon, 4 Feb 2019 12:24:32 -0500 Subject: [PATCH 13/13] Const-ify max_request_headers fields Signed-off-by: Auni Ahsan --- source/common/http/conn_manager_utility.cc | 2 +- source/common/http/conn_manager_utility.h | 2 +- source/common/http/http2/codec_impl.cc | 4 ++-- source/common/http/http2/codec_impl.h | 8 ++++---- .../filters/network/http_connection_manager/config.h | 2 +- source/server/http/admin.h | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index 8275334db994a..2ebfe881d4d79 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -39,7 +39,7 @@ 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, uint32_t max_request_headers_kb) { + const Http2Settings& http2_settings, const uint32_t max_request_headers_kb) { if (determineNextProtocol(connection, data) == Http2::ALPN_STRING) { return ServerConnectionPtr{new Http2::ServerConnectionImpl( connection, callbacks, scope, http2_settings, max_request_headers_kb)}; diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h index a1ecd6bcafddd..0d313f185e649 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, - uint32_t max_request_headers_kb); + const uint32_t max_request_headers_kb); /** * Mutates request headers in various ways. This functionality is broken out because of its diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index 822ed5ff891e8..c7dd029d8ce0c 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -878,7 +878,7 @@ ConnectionImpl::ClientHttp2Options::ClientHttp2Options(const Http2Settings& http ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, Http::ConnectionCallbacks& callbacks, Stats::Scope& stats, const Http2Settings& http2_settings, - uint32_t max_request_headers_kb) + const uint32_t max_request_headers_kb) : ConnectionImpl(connection, stats, http2_settings, max_request_headers_kb), callbacks_(callbacks) { ClientHttp2Options client_http2_options(http2_settings); @@ -927,7 +927,7 @@ int ClientConnectionImpl::onHeader(const nghttp2_frame* frame, HeaderString&& na ServerConnectionImpl::ServerConnectionImpl(Network::Connection& connection, Http::ServerConnectionCallbacks& callbacks, Stats::Scope& scope, const Http2Settings& http2_settings, - uint32_t max_request_headers_kb) + const uint32_t max_request_headers_kb) : ConnectionImpl(connection, scope, http2_settings, max_request_headers_kb), callbacks_(callbacks) { Http2Options http2_options(http2_settings); diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h index a91ec5251ef05..713abb886aefc 100644 --- a/source/common/http/http2/codec_impl.h +++ b/source/common/http/http2/codec_impl.h @@ -74,7 +74,7 @@ class Utility { class ConnectionImpl : public virtual Connection, protected Logger::Loggable { public: ConnectionImpl(Network::Connection& connection, Stats::Scope& stats, - const Http2Settings& http2_settings, uint32_t max_request_headers_kb) + const Http2Settings& http2_settings, const uint32_t max_request_headers_kb) : stats_{ALL_HTTP2_CODEC_STATS(POOL_COUNTER_PREFIX(stats, "http2."))}, connection_(connection), max_request_headers_kb_(max_request_headers_kb), per_stream_buffer_limit_(http2_settings.initial_stream_window_size_), dispatching_(false), @@ -286,7 +286,7 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable, std::string server_name_; Http::TracingConnectionManagerConfigPtr tracing_config_; absl::optional user_agent_; - uint32_t max_request_headers_kb_; + const uint32_t max_request_headers_kb_; absl::optional idle_timeout_; std::chrono::milliseconds stream_idle_timeout_; std::chrono::milliseconds request_timeout_; diff --git a/source/server/http/admin.h b/source/server/http/admin.h index 477f0e75e28a4..8587c9ec52df9 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -305,7 +305,7 @@ class AdminImpl : public Admin, Http::ConnectionManagerTracingStats tracing_stats_; NullRouteConfigProvider route_config_provider_; std::list handlers_; - uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB}; + const uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB}; absl::optional idle_timeout_; absl::optional user_agent_; Http::SlowDateProviderImpl date_provider_;