Skip to content

Commit

Permalink
http3: respecting header number limits (envoyproxy#15970)
Browse files Browse the repository at this point in the history
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
envoyproxy#14829 among others.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Gokul Nair <gnair@twitter.com>
  • Loading branch information
alyssawilk authored and Gokul Nair committed May 6, 2021
1 parent c452d32 commit 6ace0e0
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 7 deletions.
2 changes: 1 addition & 1 deletion source/common/http/codec_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne
auto& quic_session = dynamic_cast<Quic::EnvoyQuicClientSession&>(*connection_);
codec_ = std::make_unique<Quic::QuicHttpClientConnectionImpl>(
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();
Expand Down
6 changes: 4 additions & 2 deletions source/common/quic/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ 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) {
quic_session.setCodecStats(stats);
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);
}

Expand Down Expand Up @@ -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);
}

Expand Down
10 changes: 6 additions & 4 deletions test/integration/protocol_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 6ace0e0

Please sign in to comment.