diff --git a/source/extensions/quic_listeners/quiche/envoy_quic_client_session.cc b/source/extensions/quic_listeners/quiche/envoy_quic_client_session.cc index db778f6e0184..c37d0ceb3dbf 100644 --- a/source/extensions/quic_listeners/quiche/envoy_quic_client_session.cc +++ b/source/extensions/quic_listeners/quiche/envoy_quic_client_session.cc @@ -14,7 +14,10 @@ EnvoyQuicClientSession::EnvoyQuicClientSession( : QuicFilterManagerConnectionImpl(*connection, dispatcher, send_buffer_limit), quic::QuicSpdyClientSession(config, supported_versions, connection.release(), server_id, crypto_config, push_promise_index), - host_name_(server_id.host()) {} + host_name_(server_id.host()) { + // HTTP/3 header limits should be configurable, but for now hard-code to Envoy defaults. + set_max_inbound_header_list_size(Http::DEFAULT_MAX_REQUEST_HEADERS_KB * 1000); +} EnvoyQuicClientSession::~EnvoyQuicClientSession() { ASSERT(!connection()->connected()); diff --git a/source/extensions/quic_listeners/quiche/envoy_quic_server_session.cc b/source/extensions/quic_listeners/quiche/envoy_quic_server_session.cc index d827855e60f0..bdfb2315b172 100644 --- a/source/extensions/quic_listeners/quiche/envoy_quic_server_session.cc +++ b/source/extensions/quic_listeners/quiche/envoy_quic_server_session.cc @@ -19,7 +19,10 @@ EnvoyQuicServerSession::EnvoyQuicServerSession( : quic::QuicServerSessionBase(config, supported_versions, connection.get(), visitor, helper, crypto_config, compressed_certs_cache), QuicFilterManagerConnectionImpl(*connection, dispatcher, send_buffer_limit), - quic_connection_(std::move(connection)), listener_config_(listener_config) {} + quic_connection_(std::move(connection)), listener_config_(listener_config) { + // HTTP/3 header limits should be configurable, but for now hard-code to Envoy defaults. + set_max_inbound_header_list_size(Http::DEFAULT_MAX_REQUEST_HEADERS_KB * 1000); +} EnvoyQuicServerSession::~EnvoyQuicServerSession() { ASSERT(!quic_connection_->connected()); diff --git a/test/extensions/quic_listeners/quiche/integration/quic_protocol_integration_test.cc b/test/extensions/quic_listeners/quiche/integration/quic_protocol_integration_test.cc index 019f2007f282..b28aa28131d9 100644 --- a/test/extensions/quic_listeners/quiche/integration/quic_protocol_integration_test.cc +++ b/test/extensions/quic_listeners/quiche/integration/quic_protocol_integration_test.cc @@ -2,8 +2,11 @@ namespace Envoy { -// We do not yet run QUIC downstream tests. -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(DownstreamProtocolIntegrationTest); +// TODO(danzh) this should run with QUIC downstream instead of HTTP2. +INSTANTIATE_TEST_SUITE_P(Protocols, DownstreamProtocolIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( + {Http::CodecClient::Type::HTTP2}, {FakeHttpConnection::Type::HTTP3})), + HttpProtocolIntegrationTest::protocolTestParamsToString); // This will run with HTTP/1 and HTTP/2 downstream, and HTTP/3 upstream. INSTANTIATE_TEST_SUITE_P(Protocols, ProtocolIntegrationTest, diff --git a/test/integration/filters/add_body_filter.cc b/test/integration/filters/add_body_filter.cc index c319f0f5f729..18a11de9bd3f 100644 --- a/test/integration/filters/add_body_filter.cc +++ b/test/integration/filters/add_body_filter.cc @@ -13,7 +13,7 @@ namespace Envoy { -// A test filter that inserts body to a header only request/response. +// A test filter that adds body data to a request/response without body payload. class AddBodyStreamFilter : public Http::PassThroughFilter { public: constexpr static char name[] = "add-body-filter"; @@ -24,11 +24,22 @@ class AddBodyStreamFilter : public Http::PassThroughFilter { Buffer::OwnedImpl body("body"); headers.setContentLength(body.length()); decoder_callbacks_->addDecodedData(body, false); + } else { + headers.removeContentLength(); } return Http::FilterHeadersStatus::Continue; } + Http::FilterDataStatus encodeData(Buffer::Instance& data, bool end_stream) override { + // Ensure that encodeData is only called for HTTP/3 (where protocol is set at the + // connection level). In HTTP/3 the FIN arrives separately so we will get + // encodeData() with an empty body. + ASSERT(end_stream == false || decoder_callbacks_->connection()->streamInfo().protocol()); + data.add("body"); + return Http::FilterDataStatus::Continue; + } + Http::FilterHeadersStatus encodeHeaders(Http::ResponseHeaderMap& headers, bool end_stream) override { if (end_stream) { diff --git a/test/integration/filters/continue_headers_only_inject_body_filter.cc b/test/integration/filters/continue_headers_only_inject_body_filter.cc index a7f1b952e4ce..5f1538bdbbaa 100644 --- a/test/integration/filters/continue_headers_only_inject_body_filter.cc +++ b/test/integration/filters/continue_headers_only_inject_body_filter.cc @@ -28,17 +28,42 @@ class ContinueHeadersOnlyInjectBodyFilter : public Http::PassThroughFilter { return Http::FilterHeadersStatus::ContinueAndDontEndStream; } - Http::FilterHeadersStatus encodeHeaders(Http::ResponseHeaderMap& headers, bool) override { + Http::FilterHeadersStatus encodeHeaders(Http::ResponseHeaderMap& headers, + bool end_stream) override { headers.setContentLength(body_.length()); - encoder_callbacks_->dispatcher().post([this]() -> void { + encoder_callbacks_->dispatcher().post([this, end_stream]() -> void { + posted_ = true; Buffer::OwnedImpl buffer(body_); - encoder_callbacks_->injectEncodedDataToFilterChain(buffer, true); + encoder_callbacks_->injectEncodedDataToFilterChain(buffer, end_stream); + if (encoded_) { + encoder_callbacks_->continueEncoding(); + } }); - return Http::FilterHeadersStatus::ContinueAndDontEndStream; + if (end_stream) { + return Http::FilterHeadersStatus::ContinueAndDontEndStream; + } else { + return Http::FilterHeadersStatus::Continue; + } + } + + Http::FilterDataStatus encodeData(Buffer::Instance&, bool) override { + encoded_ = true; + if (!posted_) { + return Http::FilterDataStatus::StopIterationAndBuffer; + } + return Http::FilterDataStatus::Continue; } private: constexpr static absl::string_view body_ = "body"; + // For HTTP/3 upstream, the headers and fin will arrive separately. + // Make sure that the body is added, and then continue encoding occurs once. + // If encodeData hits before the post, encodeData will stop iteration to ensure + // the fin is not passed on, and when the post happens it will resume + // encoding. + // If the post happens first, encodeData can simply continue. + bool posted_{}; + bool encoded_{}; }; static Registry::RegisterFactory, diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index d88460720d50..6c585a3e51a9 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -212,7 +212,6 @@ TEST_P(ProtocolIntegrationTest, AddBodyToRequestAndWaitForIt) { } TEST_P(ProtocolIntegrationTest, AddBodyToResponseAndWaitForIt) { - EXCLUDE_UPSTREAM_HTTP3; // filters are prepended, so add them in reverse order config_helper_.addFilter(R"EOF( name: add-body-filter @@ -235,7 +234,6 @@ TEST_P(ProtocolIntegrationTest, AddBodyToResponseAndWaitForIt) { } TEST_P(ProtocolIntegrationTest, ContinueHeadersOnlyInjectBodyFilter) { - EXCLUDE_UPSTREAM_HTTP3; config_helper_.addFilter(R"EOF( name: continue-headers-only-inject-body-filter typed_config: @@ -338,6 +336,7 @@ TEST_P(ProtocolIntegrationTest, ResponseWithHostHeader) { // Tests missing headers needed for H/1 codec first line. TEST_P(DownstreamProtocolIntegrationTest, DownstreamRequestWithFaultyFilter) { + EXCLUDE_UPSTREAM_HTTP3; // buffer bug. useAccessLog("%RESPONSE_CODE_DETAILS%"); config_helper_.addFilter("{ name: invalid-header-filter, typed_config: { \"@type\": " "type.googleapis.com/google.protobuf.Empty } }"); @@ -370,6 +369,7 @@ TEST_P(DownstreamProtocolIntegrationTest, DownstreamRequestWithFaultyFilter) { } TEST_P(DownstreamProtocolIntegrationTest, FaultyFilterWithConnect) { + EXCLUDE_UPSTREAM_HTTP3; // buffer bug. // Faulty filter that removed host in a CONNECT request. config_helper_.addConfigModifier( [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& @@ -425,7 +425,6 @@ TEST_P(DownstreamProtocolIntegrationTest, MissingHeadersLocalReply) { // Regression test for https://github.com/envoyproxy/envoy/issues/10270 TEST_P(ProtocolIntegrationTest, LongHeaderValueWithSpaces) { - EXCLUDE_UPSTREAM_HTTP3; // Header with at least 20kb of spaces surrounded by non-whitespace characters to ensure that // dispatching is split across 2 dispatch calls. This threshold comes from Envoy preferring 16KB // reads, which the buffer rounds up to about 20KB when allocating slices in @@ -463,7 +462,7 @@ TEST_P(ProtocolIntegrationTest, LongHeaderValueWithSpaces) { } TEST_P(ProtocolIntegrationTest, Retry) { - EXCLUDE_UPSTREAM_HTTP3; + EXCLUDE_UPSTREAM_HTTP3; // flakes with CHECK failed: (max_plaintext_size_) >= (PacketSize()). initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); auto response = codec_client_->makeRequestWithBody( @@ -501,6 +500,12 @@ TEST_P(ProtocolIntegrationTest, Retry) { TestUtility::findCounter(stats, "cluster.cluster_0.http2.tx_reset"); ASSERT_NE(nullptr, counter); EXPECT_EQ(1L, counter->value()); + } else if (upstreamProtocol() == FakeHttpConnection::Type::HTTP3) { + // TODO(alyssawilk) http3 stats. + Stats::CounterSharedPtr counter = + TestUtility::findCounter(stats, "cluster.cluster_0.upstream_rq_tx_reset"); + ASSERT_NE(nullptr, counter); + EXPECT_EQ(1L, counter->value()); } else { Stats::CounterSharedPtr counter = TestUtility::findCounter(stats, "cluster.cluster_0.http1.dropped_headers_with_underscores"); @@ -509,7 +514,7 @@ TEST_P(ProtocolIntegrationTest, Retry) { } TEST_P(ProtocolIntegrationTest, RetryStreaming) { - EXCLUDE_UPSTREAM_HTTP3; + EXCLUDE_UPSTREAM_HTTP3; // flakes with CHECK failed: (max_plaintext_size_) >= (PacketSize()). initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); auto encoder_decoder = @@ -687,6 +692,7 @@ TEST_P(ProtocolIntegrationTest, RetryStreamingCancelDueToBufferOverflow) { // Tests that the x-envoy-attempt-count header is properly set on the upstream request and the // downstream response, and updated after the request is retried. TEST_P(DownstreamProtocolIntegrationTest, RetryAttemptCountHeader) { + EXCLUDE_UPSTREAM_HTTP3; auto host = config_helper_.createVirtualHost("host", "/test_retry"); host.set_include_request_attempt_count(true); host.set_include_attempt_count_in_response(true); @@ -731,6 +737,7 @@ TEST_P(DownstreamProtocolIntegrationTest, RetryAttemptCountHeader) { // The retry priority will always target P1, which would otherwise never be hit due to P0 being // healthy. TEST_P(DownstreamProtocolIntegrationTest, RetryPriority) { + EXCLUDE_UPSTREAM_HTTP3; const Upstream::HealthyLoad healthy_priority_load({0u, 100u}); const Upstream::DegradedLoad degraded_priority_load({0u, 100u}); NiceMock retry_priority(healthy_priority_load, @@ -803,6 +810,7 @@ TEST_P(DownstreamProtocolIntegrationTest, RetryPriority) { // the same host. With a total of two upstream hosts, this should result in us continuously sending // requests to the same host. TEST_P(DownstreamProtocolIntegrationTest, RetryHostPredicateFilter) { + EXCLUDE_UPSTREAM_HTTP3; TestHostPredicateFactory predicate_factory; Registry::InjectFactory inject_factory(predicate_factory); @@ -1009,7 +1017,7 @@ TEST_P(ProtocolIntegrationTest, HittingEncoderFilterLimit) { // connection error is not detected under these circumstances. #if !defined(__APPLE__) TEST_P(ProtocolIntegrationTest, 100ContinueAndClose) { - EXCLUDE_UPSTREAM_HTTP3; // TODO(#14829) 503 + EXCLUDE_UPSTREAM_HTTP3; testEnvoyHandling100Continue(false, "", true); } #endif @@ -1039,7 +1047,7 @@ TEST_P(ProtocolIntegrationTest, EnvoyProxyingLateMultiple1xx) { TEST_P(ProtocolIntegrationTest, TwoRequests) { testTwoRequests(); } TEST_P(ProtocolIntegrationTest, TwoRequestsWithForcedBackup) { - EXCLUDE_UPSTREAM_HTTP3; + EXCLUDE_UPSTREAM_HTTP3; // Hits assert in switchStreamBlockState. testTwoRequests(true); } @@ -1271,6 +1279,7 @@ TEST_P(ProtocolIntegrationTest, MissingStatus) { // Validate that lots of tiny cookies doesn't cause a DoS (single cookie header). TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) { + EXCLUDE_UPSTREAM_HTTP3; initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -1293,6 +1302,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) { // Validate that lots of tiny cookies doesn't cause a DoS (many cookie headers). TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingMany) { + EXCLUDE_UPSTREAM_HTTP3; // Set header count limit to 2010. uint32_t max_count = 2010; config_helper_.addConfigModifier( @@ -1469,6 +1479,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlRejected) { } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlAccepted) { + EXCLUDE_UPSTREAM_HTTP3; // Send one 95 kB URL with limit 96 kB headers. testLargeRequestUrl(95, 96); } @@ -1479,6 +1490,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersRejected) { } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersAccepted) { + EXCLUDE_UPSTREAM_HTTP3; // Send one 95 kB header with limit 96 kB and 100 headers. testLargeRequestHeaders(95, 1, 96, 100); } @@ -1494,6 +1506,7 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersAccepted) { } TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersRejected) { + EXCLUDE_UPSTREAM_HTTP3; // buffer bug. // Default header (and trailer) count limit is 100. config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); config_helper_.addConfigModifier(setEnableUpstreamTrailersHttp1()); @@ -1521,6 +1534,7 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersRejected) { } TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersAccepted) { + EXCLUDE_UPSTREAM_HTTP3; // Set header (and trailer) count limit to 200. uint32_t max_count = 200; config_helper_.addConfigModifier( @@ -1553,16 +1567,19 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersAccepted) { // This test uses an Http::HeaderMapImpl instead of an Http::TestHeaderMapImpl to avoid // time-consuming byte size validations that will cause this test to timeout. TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersTimeout) { + EXCLUDE_UPSTREAM_HTTP3; // Set timeout for 5 seconds, and ensure that a request with 10k+ headers can be sent. testManyRequestHeaders(std::chrono::milliseconds(5000)); } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersAccepted) { + EXCLUDE_UPSTREAM_HTTP3; config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); testLargeRequestTrailers(60, 96); } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersRejected) { + EXCLUDE_UPSTREAM_HTTP3; // TODO(danzh) QuicMemSliceImpl flake config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); testLargeRequestTrailers(66, 60); } @@ -1570,6 +1587,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersRejected) { // This test uses an Http::HeaderMapImpl instead of an Http::TestHeaderMapImpl to avoid // time-consuming byte size verification that will cause this test to timeout. TEST_P(DownstreamProtocolIntegrationTest, ManyTrailerHeaders) { + EXCLUDE_UPSTREAM_HTTP3; max_request_headers_kb_ = 96; max_request_headers_count_ = 20005; @@ -1624,7 +1642,6 @@ TEST_P(ProtocolIntegrationTest, LargeRequestMethod) { // There will be no upstream connections for HTTP/1 downstream, we need to // test the full mesh regardless. testing_upstream_intentionally_ = true; - EXCLUDE_UPSTREAM_HTTP3; // TODO(#14829) Rejected with QUIC_STREAM_EXCESSIVE_LOAD const std::string long_method = std::string(48 * 1024, 'a'); const Http::TestRequestHeaderMapImpl request_headers{{":method", long_method}, {":path", "/test/long/url"}, @@ -1846,7 +1863,12 @@ name: encode-headers-return-stop-all-filter response->waitForEndStream(); ASSERT_TRUE(response->complete()); - EXPECT_EQ(count_ * size_ + added_decoded_data_size_, response->body().size()); + // Data is added in encodeData for all protocols, and encodeTrailers for HTTP/2 and above. + int times_added = (upstreamProtocol() == FakeHttpConnection::Type::HTTP1 || + downstreamProtocol() == Http::CodecClient::Type::HTTP1) + ? 1 + : 2; + EXPECT_EQ(count_ * size_ + added_decoded_data_size_ * times_added, response->body().size()); } // Tests encodeHeaders() returns StopAllIterationAndWatermark. @@ -1886,13 +1908,17 @@ name: encode-headers-return-stop-all-filter response->waitForEndStream(); ASSERT_TRUE(response->complete()); - EXPECT_EQ(count_ * size_ + added_decoded_data_size_, response->body().size()); + // Data is added in encodeData for all protocols, and encodeTrailers for HTTP/2 and above. + int times_added = (upstreamProtocol() == FakeHttpConnection::Type::HTTP1 || + downstreamProtocol() == Http::CodecClient::Type::HTTP1) + ? 1 + : 2; + EXPECT_EQ(count_ * size_ + added_decoded_data_size_ * times_added, response->body().size()); } // Per https://github.com/envoyproxy/envoy/issues/7488 make sure we don't // combine set-cookie headers TEST_P(ProtocolIntegrationTest, MultipleSetCookies) { - EXCLUDE_UPSTREAM_HTTP3; // Multiple cookies not yet supported. initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -1913,7 +1939,7 @@ TEST_P(ProtocolIntegrationTest, MultipleSetCookies) { // Resets the downstream stream immediately and verifies that we clean up everything. TEST_P(ProtocolIntegrationTest, TestDownstreamResetIdleTimeout) { - EXCLUDE_UPSTREAM_HTTP3; + EXCLUDE_UPSTREAM_HTTP3; // TODO(danzh) QuicMemSliceImpl flake useAccessLog("%RESPONSE_FLAGS% %RESPONSE_CODE_DETAILS%"); config_helper_.setDownstreamHttpIdleTimeout(std::chrono::milliseconds(100)); @@ -2143,6 +2169,7 @@ TEST_P(DownstreamProtocolIntegrationTest, ConnectStreamRejection) { // Regression test for https://github.com/envoyproxy/envoy/issues/12131 TEST_P(DownstreamProtocolIntegrationTest, Test100AndDisconnect) { + EXCLUDE_UPSTREAM_HTTP3; initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); @@ -2157,6 +2184,7 @@ TEST_P(DownstreamProtocolIntegrationTest, Test100AndDisconnect) { } TEST_P(DownstreamProtocolIntegrationTest, Test100AndDisconnectLegacy) { + EXCLUDE_UPSTREAM_HTTP3; config_helper_.addRuntimeOverride("envoy.reloadable_features.allow_500_after_100", "false"); initialize();