From 6ace0e0318489bbb2240681a669d60f6cfa5a9e7 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 Signed-off-by: Gokul Nair --- source/common/http/codec_client.cc | 2 +- source/common/quic/codec_impl.cc | 6 ++++-- test/integration/protocol_integration_test.cc | 10 ++++++---- 3 files changed, 11 insertions(+), 7 deletions(-) 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/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 7b093a89eee8..bcd0285ecb47 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -1322,10 +1322,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(); @@ -1598,9 +1602,7 @@ 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;