From 6a2e11235bba6812d67a59e9d54e61521f25c40e Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Mon, 3 May 2021 09:14:48 -0400 Subject: [PATCH] http3: respecting header number limits (#15970) Respecting upstream and downstream caps on number of headers for HTTP/3 Risk Level: n/a (http/3) Testing: turned up integration tests Docs Changes: n/a Release Notes: n/a #14829 among others. Signed-off-by: Alyssa Wilk --- .../http_conn_man/response_code_details.rst | 3 ++ source/common/http/codec_client.cc | 2 +- source/common/quic/codec_impl.cc | 6 ++-- source/common/quic/codec_impl.h | 5 +-- .../common/quic/envoy_quic_client_stream.cc | 21 ++++++++---- source/common/quic/envoy_quic_client_stream.h | 3 +- .../common/quic/envoy_quic_server_stream.cc | 8 ++++- source/common/quic/envoy_quic_stream.h | 17 ---------- source/common/quic/envoy_quic_utils.h | 32 +++++++++++++++++-- .../quic_filter_manager_connection_impl.h | 7 ++++ .../network/http_connection_manager/config.cc | 2 +- .../quic/envoy_quic_client_session_test.cc | 2 +- .../quic/envoy_quic_server_session_test.cc | 2 +- test/common/quic/envoy_quic_utils_test.cc | 7 ++-- test/integration/fake_upstream.cc | 3 +- test/integration/http_integration.cc | 2 +- .../multiplexed_upstream_integration_test.cc | 6 ++-- test/integration/protocol_integration_test.cc | 19 +++++++---- 18 files changed, 97 insertions(+), 50 deletions(-) diff --git a/docs/root/configuration/http/http_conn_man/response_code_details.rst b/docs/root/configuration/http/http_conn_man/response_code_details.rst index ebca3a321f2e..24d94164ca78 100644 --- a/docs/root/configuration/http/http_conn_man/response_code_details.rst +++ b/docs/root/configuration/http/http_conn_man/response_code_details.rst @@ -112,3 +112,6 @@ All http3 details are rooted at *http3.* http3.invalid_header_field, One of the HTTP/3 headers was invalid http3.headers_too_large, The size of headers (or trailers) exceeded the configured limits http3.unexpected_underscore, Envoy was configured to drop or reject requests with header keys beginning with underscores. + http3.too_many_headers, Either incoming request or response headers contained too many headers. + http3.too_many_trailers, Either incoming request or response trailers contained too many entries. + diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc index 7ea1916839b6..3353ff81d090 100644 --- a/source/common/http/codec_client.cc +++ b/source/common/http/codec_client.cc @@ -193,7 +193,7 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne auto& quic_session = dynamic_cast(*connection_); codec_ = std::make_unique( quic_session, *this, host->cluster().http3CodecStats(), host->cluster().http3Options(), - Http::DEFAULT_MAX_REQUEST_HEADERS_KB); + Http::DEFAULT_MAX_REQUEST_HEADERS_KB, host->cluster().maxResponseHeadersCount()); // Initialize the session after max request header size is changed in above http client // connection creation. quic_session.Initialize(); diff --git a/source/common/quic/codec_impl.cc b/source/common/quic/codec_impl.cc index 5cdae4dd8e96..6ec3c355e023 100644 --- a/source/common/quic/codec_impl.cc +++ b/source/common/quic/codec_impl.cc @@ -22,7 +22,7 @@ QuicHttpServerConnectionImpl::QuicHttpServerConnectionImpl( EnvoyQuicServerSession& quic_session, Http::ServerConnectionCallbacks& callbacks, Http::Http3::CodecStats& stats, const envoy::config::core::v3::Http3ProtocolOptions& http3_options, - const uint32_t max_request_headers_kb, + const uint32_t max_request_headers_kb, const uint32_t max_request_headers_count, envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction headers_with_underscores_action) : QuicHttpConnectionImplBase(quic_session, stats), quic_server_session_(quic_session) { @@ -30,6 +30,7 @@ QuicHttpServerConnectionImpl::QuicHttpServerConnectionImpl( quic_session.setHttp3Options(http3_options); quic_session.setHeadersWithUnderscoreAction(headers_with_underscores_action); quic_session.setHttpConnectionCallbacks(callbacks); + quic_session.setMaxIncomingHeadersCount(max_request_headers_count); quic_session.set_max_inbound_header_list_size(max_request_headers_kb * 1024u); } @@ -69,11 +70,12 @@ QuicHttpClientConnectionImpl::QuicHttpClientConnectionImpl( EnvoyQuicClientSession& session, Http::ConnectionCallbacks& callbacks, Http::Http3::CodecStats& stats, const envoy::config::core::v3::Http3ProtocolOptions& http3_options, - const uint32_t max_request_headers_kb) + const uint32_t max_request_headers_kb, const uint32_t max_response_headers_count) : QuicHttpConnectionImplBase(session, stats), quic_client_session_(session) { session.setCodecStats(stats); session.setHttp3Options(http3_options); session.setHttpConnectionCallbacks(callbacks); + session.setMaxIncomingHeadersCount(max_response_headers_count); session.set_max_inbound_header_list_size(max_request_headers_kb * 1024); } diff --git a/source/common/quic/codec_impl.h b/source/common/quic/codec_impl.h index efd38ea18c42..1dca7b65931a 100644 --- a/source/common/quic/codec_impl.h +++ b/source/common/quic/codec_impl.h @@ -43,7 +43,7 @@ class QuicHttpServerConnectionImpl : public QuicHttpConnectionImplBase, EnvoyQuicServerSession& quic_session, Http::ServerConnectionCallbacks& callbacks, Http::Http3::CodecStats& stats, const envoy::config::core::v3::Http3ProtocolOptions& http3_options, - const uint32_t max_request_headers_kb, + const uint32_t max_request_headers_kb, const uint32_t max_request_headers_count, envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction headers_with_underscores_action); @@ -63,7 +63,8 @@ class QuicHttpClientConnectionImpl : public QuicHttpConnectionImplBase, QuicHttpClientConnectionImpl(EnvoyQuicClientSession& session, Http::ConnectionCallbacks& callbacks, Http::Http3::CodecStats& stats, const envoy::config::core::v3::Http3ProtocolOptions& http3_options, - const uint32_t max_request_headers_kb); + const uint32_t max_request_headers_kb, + const uint32_t max_response_headers_count); // Http::ClientConnection Http::RequestEncoder& newStream(Http::ResponseDecoder& response_decoder) override; diff --git a/source/common/quic/envoy_quic_client_stream.cc b/source/common/quic/envoy_quic_client_stream.cc index b107850ee230..185c7bc588d2 100644 --- a/source/common/quic/envoy_quic_client_stream.cc +++ b/source/common/quic/envoy_quic_client_stream.cc @@ -134,7 +134,8 @@ void EnvoyQuicClientStream::OnInitialHeadersComplete(bool fin, size_t frame_len, } quic::QuicSpdyStream::OnInitialHeadersComplete(fin, frame_len, header_list); if (!headers_decompressed() || header_list.empty()) { - onStreamError(!http3_options_.override_stream_error_on_invalid_http_message().value()); + onStreamError(!http3_options_.override_stream_error_on_invalid_http_message().value(), + quic::QUIC_BAD_APPLICATION_PAYLOAD); return; } @@ -143,16 +144,18 @@ void EnvoyQuicClientStream::OnInitialHeadersComplete(bool fin, size_t frame_len, end_stream_decoded_ = true; } std::unique_ptr headers = - quicHeadersToEnvoyHeaders(header_list, *this); + quicHeadersToEnvoyHeaders( + header_list, *this, filterManagerConnection()->maxIncomingHeadersCount(), details_); if (headers == nullptr) { - onStreamError(close_connection_upon_invalid_header_); + onStreamError(close_connection_upon_invalid_header_, quic::QUIC_STREAM_EXCESSIVE_LOAD); return; } const absl::optional optional_status = Http::Utility::getResponseStatusNoThrow(*headers); if (!optional_status.has_value()) { details_ = Http3ResponseCodeDetailValues::invalid_http_header; - onStreamError(!http3_options_.override_stream_error_on_invalid_http_message().value()); + onStreamError(!http3_options_.override_stream_error_on_invalid_http_message().value(), + quic::QUIC_BAD_APPLICATION_PAYLOAD); return; } const uint64_t status = optional_status.value(); @@ -239,6 +242,11 @@ void EnvoyQuicClientStream::maybeDecodeTrailers() { if (sequencer()->IsClosed() && !FinishedReadingTrailers()) { // Only decode trailers after finishing decoding body. end_stream_decoded_ = true; + if (received_trailers().size() > filterManagerConnection()->maxIncomingHeadersCount()) { + details_ = Http3ResponseCodeDetailValues::too_many_trailers; + onStreamError(close_connection_upon_invalid_header_, quic::QUIC_STREAM_EXCESSIVE_LOAD); + return; + } response_decoder_->decodeTrailers( spdyHeaderBlockToEnvoyHeaders(received_trailers())); MarkTrailersConsumed(); @@ -299,7 +307,8 @@ QuicFilterManagerConnectionImpl* EnvoyQuicClientStream::filterManagerConnection( return dynamic_cast(session()); } -void EnvoyQuicClientStream::onStreamError(absl::optional should_close_connection) { +void EnvoyQuicClientStream::onStreamError(absl::optional should_close_connection, + quic::QuicRstStreamErrorCode rst_code) { if (details_.empty()) { details_ = Http3ResponseCodeDetailValues::invalid_http_header; } @@ -313,7 +322,7 @@ void EnvoyQuicClientStream::onStreamError(absl::optional should_close_conn if (close_connection_upon_invalid_header) { stream_delegate()->OnStreamError(quic::QUIC_HTTP_FRAME_ERROR, "Invalid headers"); } else { - Reset(quic::QUIC_BAD_APPLICATION_PAYLOAD); + Reset(rst_code); } } diff --git a/source/common/quic/envoy_quic_client_stream.h b/source/common/quic/envoy_quic_client_stream.h index 48e4940b53ac..a722accc6bfc 100644 --- a/source/common/quic/envoy_quic_client_stream.h +++ b/source/common/quic/envoy_quic_client_stream.h @@ -78,7 +78,8 @@ class EnvoyQuicClientStream : public quic::QuicSpdyClientStream, // Either reset the stream or close the connection according to // should_close_connection and configured http3 options. - void onStreamError(absl::optional should_close_connection); + void onStreamError(absl::optional should_close_connection, + quic::QuicRstStreamErrorCode rst_code); Http::ResponseDecoder* response_decoder_{nullptr}; diff --git a/source/common/quic/envoy_quic_server_stream.cc b/source/common/quic/envoy_quic_server_stream.cc index e6a2197a3be4..094b1069fb05 100644 --- a/source/common/quic/envoy_quic_server_stream.cc +++ b/source/common/quic/envoy_quic_server_stream.cc @@ -160,7 +160,8 @@ void EnvoyQuicServerStream::OnInitialHeadersComplete(bool fin, size_t frame_len, end_stream_decoded_ = true; } std::unique_ptr headers = - quicHeadersToEnvoyHeaders(header_list, *this); + quicHeadersToEnvoyHeaders( + header_list, *this, filterManagerConnection()->maxIncomingHeadersCount(), details_); if (headers == nullptr) { onStreamError(close_connection_upon_invalid_header_); return; @@ -242,6 +243,11 @@ void EnvoyQuicServerStream::maybeDecodeTrailers() { if (sequencer()->IsClosed() && !FinishedReadingTrailers()) { // Only decode trailers after finishing decoding body. end_stream_decoded_ = true; + if (received_trailers().size() > filterManagerConnection()->maxIncomingHeadersCount()) { + details_ = Http3ResponseCodeDetailValues::too_many_trailers; + onStreamError(close_connection_upon_invalid_header_); + return; + } request_decoder_->decodeTrailers( spdyHeaderBlockToEnvoyHeaders(received_trailers())); MarkTrailersConsumed(); diff --git a/source/common/quic/envoy_quic_stream.h b/source/common/quic/envoy_quic_stream.h index bc8c9d0e7d98..56e744d56d80 100644 --- a/source/common/quic/envoy_quic_stream.h +++ b/source/common/quic/envoy_quic_stream.h @@ -13,23 +13,6 @@ namespace Envoy { namespace Quic { -// Changes or additions to details should be reflected in -// docs/root/configuration/http/http_conn_man/response_code_details_details.rst -class Http3ResponseCodeDetailValues { -public: - // Invalid HTTP header field was received and stream is going to be - // closed. - static constexpr absl::string_view invalid_http_header = "http3.invalid_header_field"; - // The size of headers (or trailers) exceeded the configured limits. - static constexpr absl::string_view headers_too_large = "http3.headers_too_large"; - // Envoy was configured to drop requests with header keys beginning with underscores. - static constexpr absl::string_view invalid_underscore = "http3.unexpected_underscore"; - // The peer refused the stream. - static constexpr absl::string_view remote_refused = "http3.remote_refuse"; - // The peer reset the stream. - static constexpr absl::string_view remote_reset = "http3.remote_reset"; -}; - // Base class for EnvoyQuicServer|ClientStream. class EnvoyQuicStream : public virtual Http::StreamEncoder, public Http::Stream, diff --git a/source/common/quic/envoy_quic_utils.h b/source/common/quic/envoy_quic_utils.h index f30c582b7d53..f71d2fd0c58f 100644 --- a/source/common/quic/envoy_quic_utils.h +++ b/source/common/quic/envoy_quic_utils.h @@ -32,6 +32,27 @@ namespace Envoy { namespace Quic { +// Changes or additions to details should be reflected in +// docs/root/configuration/http/http_conn_man/response_code_details.rst +class Http3ResponseCodeDetailValues { +public: + // Invalid HTTP header field was received and stream is going to be + // closed. + static constexpr absl::string_view invalid_http_header = "http3.invalid_header_field"; + // The size of headers (or trailers) exceeded the configured limits. + static constexpr absl::string_view headers_too_large = "http3.headers_too_large"; + // Envoy was configured to drop requests with header keys beginning with underscores. + static constexpr absl::string_view invalid_underscore = "http3.unexpected_underscore"; + // The peer refused the stream. + static constexpr absl::string_view remote_refused = "http3.remote_refuse"; + // The peer reset the stream. + static constexpr absl::string_view remote_reset = "http3.remote_reset"; + // Too many trailers were sent. + static constexpr absl::string_view too_many_trailers = "http3.too_many_trailers"; + // Too many headers were sent. + static constexpr absl::string_view too_many_headers = "http3.too_many_headers"; +}; + // TODO(danzh): this is called on each write. Consider to return an address instance on the stack if // the heap allocation is too expensive. Network::Address::InstanceConstSharedPtr @@ -48,14 +69,21 @@ class HeaderValidator { // The returned header map has all keys in lower case. template -std::unique_ptr quicHeadersToEnvoyHeaders(const quic::QuicHeaderList& header_list, - HeaderValidator& validator) { +std::unique_ptr +quicHeadersToEnvoyHeaders(const quic::QuicHeaderList& header_list, HeaderValidator& validator, + uint32_t max_headers_allowed, absl::string_view& details) { auto headers = T::create(); for (const auto& entry : header_list) { + if (max_headers_allowed == 0) { + details = Http3ResponseCodeDetailValues::too_many_headers; + return nullptr; + } + max_headers_allowed--; Http::HeaderUtility::HeaderValidationResult result = validator.validateHeader(entry.first, entry.second); switch (result) { case Http::HeaderUtility::HeaderValidationResult::REJECT: + // The validator sets the details to Http3ResponseCodeDetailValues::invalid_underscore return nullptr; case Http::HeaderUtility::HeaderValidationResult::DROP: continue; diff --git a/source/common/quic/quic_filter_manager_connection_impl.h b/source/common/quic/quic_filter_manager_connection_impl.h index 91cfd98b8e23..c33bc5959d7f 100644 --- a/source/common/quic/quic_filter_manager_connection_impl.h +++ b/source/common/quic/quic_filter_manager_connection_impl.h @@ -142,6 +142,12 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase, codec_stats_ = std::reference_wrapper(stats); } + uint32_t maxIncomingHeadersCount() { return max_headers_count_; } + + void setMaxIncomingHeadersCount(uint32_t max_headers_count) { + max_headers_count_ = max_headers_count; + } + protected: // Propagate connection close to network_connection_callbacks_. void onConnectionCloseEvent(const quic::QuicConnectionCloseFrame& frame, @@ -179,6 +185,7 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase, StreamInfo::StreamInfoImpl stream_info_; std::string transport_failure_reason_; uint32_t bytes_to_send_{0}; + uint32_t max_headers_count_{std::numeric_limits::max()}; // Keeps the buffer state of the connection, and react upon the changes of how many bytes are // buffered cross all streams' send buffer. The state is evaluated and may be changed upon each // stream write. QUICHE doesn't buffer data in connection, all the data is buffered in stream's diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 70f4d08f2970..0cf301d93ead 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -606,7 +606,7 @@ HttpConnectionManagerConfig::createCodec(Network::Connection& connection, return std::make_unique( dynamic_cast(connection), callbacks, Http::Http3::CodecStats::atomicGet(http3_codec_stats_, context_.scope()), http3_options_, - maxRequestHeadersKb(), headersWithUnderscoresAction()); + maxRequestHeadersKb(), maxRequestHeadersCount(), headersWithUnderscoresAction()); #else // Should be blocked by configuration checking at an earlier point. NOT_REACHED_GCOVR_EXCL_LINE; diff --git a/test/common/quic/envoy_quic_client_session_test.cc b/test/common/quic/envoy_quic_client_session_test.cc index 8617d8953e83..9facb2058a79 100644 --- a/test/common/quic/envoy_quic_client_session_test.cc +++ b/test/common/quic/envoy_quic_client_session_test.cc @@ -118,7 +118,7 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam { stats_({ALL_HTTP3_CODEC_STATS(POOL_COUNTER_PREFIX(scope_, "http3."), POOL_GAUGE_PREFIX(scope_, "http3."))}), http_connection_(envoy_quic_session_, http_connection_callbacks_, stats_, http3_options_, - 64 * 1024) { + 64 * 1024, 100) { EXPECT_EQ(time_system_.systemTime(), envoy_quic_session_.streamInfo().startTime()); EXPECT_EQ(EMPTY_STRING, envoy_quic_session_.nextProtocol()); EXPECT_EQ(Http::Protocol::Http3, http_connection_.protocol()); diff --git a/test/common/quic/envoy_quic_server_session_test.cc b/test/common/quic/envoy_quic_server_session_test.cc index fe441dbc25cb..415b5c609823 100644 --- a/test/common/quic/envoy_quic_server_session_test.cc +++ b/test/common/quic/envoy_quic_server_session_test.cc @@ -204,7 +204,7 @@ class EnvoyQuicServerSessionTest : public testing::TestWithParam { EXPECT_CALL(*read_filter_, onNewConnection()).WillOnce(Invoke([this]() { // Create ServerConnection instance and setup callbacks for it. http_connection_ = std::make_unique( - envoy_quic_session_, http_connection_callbacks_, stats_, http3_options_, 64 * 1024, + envoy_quic_session_, http_connection_callbacks_, stats_, http3_options_, 64 * 1024, 100, envoy::config::core::v3::HttpProtocolOptions::ALLOW); EXPECT_EQ(Http::Protocol::Http3, http_connection_->protocol()); // Stop iteration to avoid calling getRead/WriteBuffer(). diff --git a/test/common/quic/envoy_quic_utils_test.cc b/test/common/quic/envoy_quic_utils_test.cc index 2ba153ba9a50..7d032b425371 100644 --- a/test/common/quic/envoy_quic_utils_test.cc +++ b/test/common/quic/envoy_quic_utils_test.cc @@ -87,8 +87,9 @@ TEST(EnvoyQuicUtilsTest, HeadersConversion) { } return Http::HeaderUtility::HeaderValidationResult::ACCEPT; }); + absl::string_view details; auto envoy_headers2 = - quicHeadersToEnvoyHeaders(quic_headers, validator); + quicHeadersToEnvoyHeaders(quic_headers, validator, 100, details); EXPECT_EQ(*envoy_headers, *envoy_headers2); quic::QuicHeaderList quic_headers2; @@ -105,8 +106,8 @@ TEST(EnvoyQuicUtilsTest, HeadersConversion) { } return Http::HeaderUtility::HeaderValidationResult::ACCEPT; }); - EXPECT_EQ(nullptr, - quicHeadersToEnvoyHeaders(quic_headers2, validator)); + EXPECT_EQ(nullptr, quicHeadersToEnvoyHeaders(quic_headers2, validator, + 100, details)); } } // namespace Quic diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index 94ab19fd1969..3364b23afbad 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -346,7 +346,8 @@ FakeHttpConnection::FakeHttpConnection( Http::Http3::CodecStats& stats = fake_upstream.http3CodecStats(); codec_ = std::make_unique( dynamic_cast(shared_connection_.connection()), *this, stats, - fake_upstream.http3Options(), max_request_headers_kb, headers_with_underscores_action); + fake_upstream.http3Options(), max_request_headers_kb, max_request_headers_count, + headers_with_underscores_action); #else ASSERT(false, "running a QUIC integration test without compiling QUIC"); #endif diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 99827c4020d8..ea2bd3eb32bd 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -1251,7 +1251,7 @@ void HttpIntegrationTest::testManyRequestHeaders(std::chrono::milliseconds time) // This test uses an Http::HeaderMapImpl instead of an Http::TestHeaderMapImpl to avoid // time-consuming asserts when using a large number of headers. setMaxRequestHeadersKb(96); - setMaxRequestHeadersCount(10005); + setMaxRequestHeadersCount(10010); config_helper_.addConfigModifier( [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& diff --git a/test/integration/multiplexed_upstream_integration_test.cc b/test/integration/multiplexed_upstream_integration_test.cc index 88bd515b1679..107f67b1e39b 100644 --- a/test/integration/multiplexed_upstream_integration_test.cc +++ b/test/integration/multiplexed_upstream_integration_test.cc @@ -322,7 +322,6 @@ TEST_P(Http2UpstreamIntegrationTest, ManyLargeSimultaneousRequestWithRandomBacku } TEST_P(Http2UpstreamIntegrationTest, UpstreamConnectionCloseWithManyStreams) { - EXCLUDE_UPSTREAM_HTTP3; // Times out waiting for reset. config_helper_.setBufferLimits(1024, 1024); // Set buffer limits upstream and downstream. const uint32_t num_requests = 20; std::vector encoders; @@ -368,6 +367,9 @@ TEST_P(Http2UpstreamIntegrationTest, UpstreamConnectionCloseWithManyStreams) { ASSERT_TRUE(upstream_requests[i]->waitForEndStream(*dispatcher_)); upstream_requests[i]->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, false); upstream_requests[i]->encodeData(100, false); + // Make sure at least the headers go through, to ensure stream reset rather + // than disconnect. + responses[i]->waitForHeaders(); } // Close the connection. ASSERT_TRUE(fake_upstream_connection_->close()); @@ -441,8 +443,6 @@ name: router // Tests the default limit for the number of response headers is 100. Results in a stream reset if // exceeds. TEST_P(Http2UpstreamIntegrationTest, TestManyResponseHeadersRejected) { - EXCLUDE_UPSTREAM_HTTP3; // no 503. - // Default limit for response headers is 100. initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 03cd14677936..3e34c3e2c9a5 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -1327,10 +1327,14 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) { // header size to avoid QUIC_HEADERS_TOO_LARGE stream error. config_helper_.addConfigModifier( [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& - hcm) -> void { hcm.mutable_max_request_headers_kb()->set_value(96); }); + hcm) -> void { + hcm.mutable_max_request_headers_kb()->set_value(96); + hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value(8000); + }); } if (upstreamProtocol() == FakeHttpConnection::Type::HTTP3) { setMaxRequestHeadersKb(96); + setMaxRequestHeadersCount(8000); } initialize(); @@ -1593,8 +1597,6 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyLargeRequestHeadersAccepted) { } TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersRejected) { - // QUICHE doesn't limit number of headers. - EXCLUDE_DOWNSTREAM_HTTP3 // Send 101 empty headers with limit 60 kB and 100 headers. testLargeRequestHeaders(0, 101, 60, 80); } @@ -1605,14 +1607,15 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersAccepted) { } TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersRejected) { - // QUICHE doesn't limit number of headers. - EXCLUDE_DOWNSTREAM_HTTP3 - // The default configured header (and trailer) count limit is 100. + // Default header (and trailer) count limit is 100. config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); config_helper_.addConfigModifier(setEnableUpstreamTrailersHttp1()); Http::TestRequestTrailerMapImpl request_trailers; for (int i = 0; i < 150; i++) { - request_trailers.addCopy("trailer", std::string(1, 'a')); + // TODO(alyssawilk) QUIC fails without the trailers being distinct because + // the checks are done before transformation. Either make the transformation + // use commas, or do QUIC checks before and after. + request_trailers.addCopy(absl::StrCat("trailer", i), std::string(1, 'a')); } initialize(); @@ -1620,6 +1623,8 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersRejected) { auto encoder_decoder = codec_client_->startRequest(default_request_headers_); request_encoder_ = &encoder_decoder.first; auto response = std::move(encoder_decoder.second); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + EXPECT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); codec_client_->sendData(*request_encoder_, 1, false); codec_client_->sendTrailers(*request_encoder_, request_trailers);