From bb02ae695153af5162d2b2618a6789f32179b97b Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Tue, 9 Mar 2021 14:49:48 -0500 Subject: [PATCH 01/21] enable protocol integration test Signed-off-by: Dan Zhang --- .../integration/quic_http_integration_test.cc | 79 +++----------- .../quic_protocol_integration_test.cc | 17 ++- test/integration/BUILD | 6 + test/integration/fake_upstream.cc | 4 +- test/integration/filters/BUILD | 1 + test/integration/filters/pause_filter.cc | 45 ++++++-- test/integration/http_integration.cc | 86 ++++++++++++++- test/integration/http_integration.h | 9 ++ test/integration/http_protocol_integration.cc | 16 ++- test/integration/http_protocol_integration.h | 9 +- test/integration/protocol_integration_test.cc | 86 ++++++++++++--- test/integration/utility.cc | 103 ++++++++++++++++-- test/integration/utility.h | 12 ++ 13 files changed, 369 insertions(+), 104 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc b/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc index 0ac6c7e34ed7..f884cef33c5e 100644 --- a/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc +++ b/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc @@ -9,7 +9,6 @@ #include "test/config/utility.h" #include "test/integration/http_integration.h" -#include "test/integration/ssl_utility.h" #include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" @@ -56,51 +55,7 @@ void updateResource(AtomicFileUpdater& updater, double pressure) { updater.update(absl::StrCat(pressure)); } -std::unique_ptr -createQuicClientTransportSocketFactory(const Ssl::ClientSslTransportOptions& options, Api::Api& api, - const std::string& san_to_match) { - std::string yaml_plain = R"EOF( - common_tls_context: - validation_context: - trusted_ca: - filename: "{{ test_rundir }}/test/config/integration/certs/cacert.pem" -)EOF"; - envoy::extensions::transport_sockets::quic::v3::QuicUpstreamTransport - quic_transport_socket_config; - auto* tls_context = quic_transport_socket_config.mutable_upstream_tls_context(); - TestUtility::loadFromYaml(TestEnvironment::substitute(yaml_plain), *tls_context); - auto* common_context = tls_context->mutable_common_tls_context(); - - if (options.alpn_) { - common_context->add_alpn_protocols("h3"); - } - if (options.san_) { - common_context->mutable_validation_context()->add_match_subject_alt_names()->set_exact( - san_to_match); - } - for (const std::string& cipher_suite : options.cipher_suites_) { - common_context->mutable_tls_params()->add_cipher_suites(cipher_suite); - } - if (!options.sni_.empty()) { - tls_context->set_sni(options.sni_); - } - - common_context->mutable_tls_params()->set_tls_minimum_protocol_version(options.tls_version_); - common_context->mutable_tls_params()->set_tls_maximum_protocol_version(options.tls_version_); - - envoy::config::core::v3::TransportSocket message; - message.mutable_typed_config()->PackFrom(quic_transport_socket_config); - auto& config_factory = Config::Utility::getAndCheckFactory< - Server::Configuration::UpstreamTransportSocketConfigFactory>(message); - NiceMock mock_factory_ctx; - ON_CALL(mock_factory_ctx, api()).WillByDefault(testing::ReturnRef(api)); - return std::unique_ptr( - static_cast( - config_factory - .createTransportSocketFactory(quic_transport_socket_config, mock_factory_ctx) - .release())); -} - +// A test that sets up its own client connection with customized quic version and connection ID. class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVersionTest { public: QuicHttpIntegrationTest() @@ -143,9 +98,12 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVers getNextConnectionId(), server_addr_, conn_helper_, alarm_factory_, quic::ParsedQuicVersionVector{supported_versions_[0]}, local_addr, *dispatcher_, nullptr); quic_connection_ = connection.get(); + ASSERT(quic_transport_socket_factory_ != nullptr); auto session = std::make_unique( - quic_config_, supported_versions_, std::move(connection), server_id_, crypto_config_.get(), - &push_promise_index_, *dispatcher_, 0); + quic_config_, supported_versions_, std::move(connection), + quic::QuicServerId{transport_socket_factory_->clientContextConfig().serverNameIndication(), + static_cast(server_addr_->ip()->port()), false}, + crypto_config_.get(), &push_promise_index_, *dispatcher_, 0); session->Initialize(); return session; } @@ -160,14 +118,11 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVers absl::optional http2_options) override { IntegrationCodecClientPtr codec = HttpIntegrationTest::makeRawHttpConnection(std::move(conn), http2_options); - if (codec->disconnected()) { - // Connection may get closed during version negotiation or handshake. - ENVOY_LOG(error, "Fail to connect to server with error: {}", - codec->connection()->transportFailureReason()); - } else { + if (!codec->disconnected()) { codec->setCodecClientCallbacks(client_codec_callback_); + EXPECT_EQ(transport_socket_factory_->clientContextConfig().serverNameIndication(), + codec->connection()->requestedServerName()); } - EXPECT_EQ(server_id_.host(), codec->connection()->requestedServerName()); return codec; } @@ -241,14 +196,16 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVers updateResource(file_updater_1_, 0); updateResource(file_updater_2_, 0); HttpIntegrationTest::initialize(); + // Latch quic_transport_socket_factory_ which is instantiated in initialize(). + transport_socket_factory_ = + static_cast(quic_transport_socket_factory_.get()); registerTestServerPorts({"http"}); + + ASSERT(&transport_socket_factory_->clientContextConfig()); + crypto_config_ = std::make_unique(std::make_unique( - stats_store_, - createQuicClientTransportSocketFactory( - Ssl::ClientSslTransportOptions().setAlpn(true).setSan(true), *api_, san_to_match_) - ->clientContextConfig(), - timeSystem())); + stats_store_, transport_socket_factory_->clientContextConfig(), timeSystem())); } void testMultipleQuicConnections() { @@ -297,8 +254,6 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVers protected: quic::QuicConfig quic_config_; - quic::QuicServerId server_id_{"lyft.com", 443, false}; - std::string san_to_match_{"spiffe://lyft.com/backend-team"}; quic::QuicClientPushPromiseIndex push_promise_index_; quic::ParsedQuicVersionVector supported_versions_; std::unique_ptr crypto_config_; @@ -307,12 +262,12 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVers CodecClientCallbacksForTest client_codec_callback_; Network::Address::InstanceConstSharedPtr server_addr_; EnvoyQuicClientConnection* quic_connection_{nullptr}; - bool set_reuse_port_{false}; const std::string injected_resource_filename_1_; const std::string injected_resource_filename_2_; AtomicFileUpdater file_updater_1_; AtomicFileUpdater file_updater_2_; std::list designated_connection_ids_; + Quic::QuicClientTransportSocketFactory* transport_socket_factory_{nullptr}; }; INSTANTIATE_TEST_SUITE_P(QuicHttpIntegrationTests, QuicHttpIntegrationTest, 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..c43c0410250d 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,11 +2,22 @@ namespace Envoy { -// We do not yet run QUIC downstream tests. -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(DownstreamProtocolIntegrationTest); +// This will run with HTTP/3 downstream, and HTTP/2 upstream. +INSTANTIATE_TEST_SUITE_P(Protocols, DownstreamProtocolIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( + {Http::CodecClient::Type::HTTP3}, + {FakeHttpConnection::Type::HTTP1, FakeHttpConnection::Type::HTTP2})), + HttpProtocolIntegrationTest::protocolTestParamsToString); + +// This will run with HTTP/3 downstream, and HTTP/2 upstream. +INSTANTIATE_TEST_SUITE_P(DownstreamProtocols, ProtocolIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( + {Http::CodecClient::Type::HTTP3}, + {FakeHttpConnection::Type::HTTP1, FakeHttpConnection::Type::HTTP2})), + HttpProtocolIntegrationTest::protocolTestParamsToString); // This will run with HTTP/1 and HTTP/2 downstream, and HTTP/3 upstream. -INSTANTIATE_TEST_SUITE_P(Protocols, ProtocolIntegrationTest, +INSTANTIATE_TEST_SUITE_P(UpstreamProtocols, ProtocolIntegrationTest, testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( {Http::CodecClient::Type::HTTP1, Http::CodecClient::Type::HTTP2}, {FakeHttpConnection::Type::HTTP3})), diff --git a/test/integration/BUILD b/test/integration/BUILD index 23667952cc21..5be22277c135 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -570,6 +570,7 @@ envoy_cc_test_library( "//test/test_common:registry_lib", "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/transport_sockets/quic/v3:pkg_cc_proto", ], ) @@ -635,14 +636,19 @@ envoy_cc_test_library( "//source/common/common:assert_lib", "//source/common/common:utility_lib", "//source/common/http:codec_client_lib", + "//source/common/http/http3:quic_client_connection_factory_lib", "//source/common/stats:isolated_store_lib", + "//source/extensions/quic_listeners/quiche:quic_transport_socket_factory_lib", "//test/common/upstream:utility_lib", "//test/mocks/event:event_mocks", + "//test/mocks/server:transport_socket_factory_context_mocks", "//test/mocks/upstream:cluster_info_mocks", + "//test/test_common:environment_lib", "//test/test_common:network_utility_lib", "//test/test_common:printers_lib", "//test/test_common:simulated_time_system_lib", "//test/test_common:test_time_lib", + "@envoy_api//envoy/extensions/transport_sockets/quic/v3:pkg_cc_proto", ], ) diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index 55a03439adf4..f6fbd30c2051 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -345,7 +345,9 @@ FakeHttpConnection::FakeHttpConnection( codec_ = std::unique_ptr( Config::Utility::getAndCheckFactoryByName( Http::QuicCodecNames::get().Quiche) - .createQuicServerConnection(shared_connection_.connection(), *this)); + .createQuicServerConnection(shared_connection_.connection(), *this, + max_request_headers_kb, max_request_headers_count, + headers_with_underscores_action)); } shared_connection_.connection().addReadFilter( Network::ReadFilterSharedPtr{new ReadFilter(*this)}); diff --git a/test/integration/filters/BUILD b/test/integration/filters/BUILD index 1d78984323f3..0e4f379ac838 100644 --- a/test/integration/filters/BUILD +++ b/test/integration/filters/BUILD @@ -182,6 +182,7 @@ envoy_cc_test_library( "//include/envoy/registry", "//source/common/network:connection_lib", "//source/extensions/filters/http/common:pass_through_filter_lib", + "//source/extensions/quic_listeners/quiche:quic_filter_manager_connection_lib", "//test/extensions/filters/http/common:empty_http_filter_config_lib", ], ) diff --git a/test/integration/filters/pause_filter.cc b/test/integration/filters/pause_filter.cc index b7f9aa1a3c36..221474e0fe77 100644 --- a/test/integration/filters/pause_filter.cc +++ b/test/integration/filters/pause_filter.cc @@ -5,6 +5,7 @@ #include "common/network/connection_impl.h" #include "extensions/filters/http/common/pass_through_filter.h" +#include "extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h" #include "test/extensions/filters/http/common/empty_http_filter_config.h" @@ -29,8 +30,9 @@ class TestPauseFilter : public Http::PassThroughFilter { number_of_decode_calls_ref_++; // If this is the second stream to decode headers and we're at high watermark. force low // watermark state - if (number_of_decode_calls_ref_ == 2 && connection()->aboveHighWatermark()) { - connection()->onWriteBufferLowWatermark(); + if (number_of_decode_calls_ref_ == 2 && + decoder_callbacks_->connection()->aboveHighWatermark()) { + mockConnectionBelowLowWatermark(); } } return PassThroughFilter::decodeData(buf, end_stream); @@ -42,19 +44,46 @@ class TestPauseFilter : public Http::PassThroughFilter { number_of_encode_calls_ref_++; // If this is the first stream to encode headers and we're not at high watermark, force high // watermark state. - if (number_of_encode_calls_ref_ == 1 && !connection()->aboveHighWatermark()) { - connection()->onWriteBufferHighWatermark(); + if (number_of_encode_calls_ref_ == 1 && + !decoder_callbacks_->connection()->aboveHighWatermark()) { + mockConnectionAboveHighWatermark(); } } return PassThroughFilter::encodeData(buf, end_stream); } - Network::ConnectionImpl* connection() { + void mockConnectionAboveHighWatermark() { // As long as we're doing horrible things let's do *all* the horrible things. - // Assert the connection we have is a ConnectionImpl and const cast it so we - // can force watermark changes. + // Assert the connection we have is a ConnectionImpl or QuicFilterManagerConnectionImpl and + // const cast it so we can force watermark changes. auto conn_impl = dynamic_cast(decoder_callbacks_->connection()); - return const_cast(conn_impl); + if (conn_impl != nullptr) { + const_cast(conn_impl)->onWriteBufferHighWatermark(); + return; + } + // If transport protocol is QUIC, simulate connection buffer above watermark differently. + auto quic_connection = const_cast( + dynamic_cast( + decoder_callbacks_->connection())); + quic_connection->write_buffer_watermark_simulation_.checkHighWatermark( + quic_connection->write_buffer_watermark_simulation_.highWatermark() + 1u); + } + + void mockConnectionBelowLowWatermark() { + // As long as we're doing horrible things let's do *all* the horrible things. + // Assert the connection we have is a ConnectionImpl or QuicFilterManagerConnectionImpl and + // const cast it so we can force watermark changes. + auto conn_impl = dynamic_cast(decoder_callbacks_->connection()); + if (conn_impl != nullptr) { + const_cast(conn_impl)->onWriteBufferHighWatermark(); + return; + } + // If transport protocol is QUIC, simulate connection buffer below watermark differently. + auto quic_connection = const_cast( + dynamic_cast( + decoder_callbacks_->connection())); + quic_connection->write_buffer_watermark_simulation_.checkLowWatermark( + quic_connection->write_buffer_watermark_simulation_.highWatermark() / 2 - 1u); } absl::Mutex& encode_lock_; diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 1c046889e8f1..84ccb870c2c7 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -11,6 +11,7 @@ #include "envoy/config/bootstrap/v3/bootstrap.pb.h" #include "envoy/event/dispatcher.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" +#include "envoy/extensions/transport_sockets/quic/v3/quic_transport.pb.h" #include "envoy/http/header_map.h" #include "envoy/network/address.h" #include "envoy/registry/registry.h" @@ -20,6 +21,7 @@ #include "common/common/fmt.h" #include "common/common/thread_annotations.h" #include "common/http/headers.h" +#include "common/http/http3/quic_client_connection_factory.h" #include "common/network/socket_option_impl.h" #include "common/network/utility.h" #include "common/protobuf/utility.h" @@ -32,6 +34,7 @@ #include "test/common/upstream/utility.h" #include "test/integration/autonomous_upstream.h" +#include "test/integration/ssl_utility.h" #include "test/integration/test_host_predicate_config.h" #include "test/integration/utility.h" #include "test/mocks/upstream/cluster_info.h" @@ -40,8 +43,21 @@ #include "test/test_common/registry.h" #include "absl/time/time.h" +#include "base_integration_test.h" #include "gtest/gtest.h" +#if defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-parameter" +#pragma GCC diagnostic ignored "-Winvalid-offsetof" +#endif + +#include "quiche/quic/core/quic_utils.h" + +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif + namespace Envoy { namespace { @@ -72,6 +88,7 @@ IntegrationCodecClient::IntegrationCodecClient( CodecClient::Type type) : CodecClientProd(type, std::move(conn), host_description, dispatcher, random), dispatcher_(dispatcher), callbacks_(*this), codec_callbacks_(*this) { + std::cerr << "=========== IntegrationCodecClient add connection callback " << &callbacks_ << "\n"; connection_->addConnectionCallbacks(callbacks_); setCodecConnectionCallbacks(codec_callbacks_); dispatcher.run(Event::Dispatcher::RunType::Block); @@ -212,6 +229,23 @@ void IntegrationCodecClient::ConnectionCallbacks::onEvent(Network::ConnectionEve } } +Network::ClientConnectionPtr HttpIntegrationTest::makeClientConnectionWithOptions( + uint32_t port, const Network::ConnectionSocket::OptionsSharedPtr& options) { + if (downstream_protocol_ <= Http::CodecClient::Type::HTTP2) { + return BaseIntegrationTest::makeClientConnectionWithOptions(port, options); + } + // Setting socket options is not supported for HTTP3. + ASSERT(!options); + Network::Address::InstanceConstSharedPtr server_addr = Network::Utility::resolveUrl( + fmt::format("udp://{}:{}", Network::Test::getLoopbackAddressUrlString(version_), port)); + Network::Address::InstanceConstSharedPtr local_addr = + Network::Test::getCanonicalLoopbackAddress(version_); + return Config::Utility::getAndCheckFactoryByName( + Http::QuicCodecNames::get().Quiche) + .createQuicNetworkConnection(server_addr, local_addr, *quic_transport_socket_factory_, + stats_store_, *dispatcher_, timeSystem()); +} + IntegrationCodecClientPtr HttpIntegrationTest::makeHttpConnection(uint32_t port) { return makeHttpConnection(makeClientConnection(port)); } @@ -232,8 +266,16 @@ IntegrationCodecClientPtr HttpIntegrationTest::makeRawHttpConnection( Upstream::HostDescriptionConstSharedPtr host_description{Upstream::makeTestHostDescription( cluster, fmt::format("tcp://{}:80", Network::Test::getLoopbackAddressUrlString(version_)), timeSystem())}; - return std::make_unique(*dispatcher_, random_, std::move(conn), - host_description, downstream_protocol_); + // This call may fail in QUICHE because of INVALID_VERSION. QUIC connection doesn't support + // in-connection version negotiation. + auto codec = std::make_unique(*dispatcher_, random_, std::move(conn), + host_description, downstream_protocol_); + if (downstream_protocol_ == Http::CodecClient::Type::HTTP3 && codec->disconnected()) { + // Connection may get closed during version negotiation or handshake. + ENVOY_LOG(error, "Fail to connect to server with error: {}", + codec->connection()->transportFailureReason()); + } + return codec; } IntegrationCodecClientPtr @@ -275,6 +317,44 @@ void HttpIntegrationTest::useAccessLog( HttpIntegrationTest::~HttpIntegrationTest() { cleanupUpstreamAndDownstream(); } +void HttpIntegrationTest::initialize() { + if (downstream_protocol_ != Http::CodecClient::Type::HTTP3) { + return BaseIntegrationTest::initialize(); + } + NiceMock mock_factory_ctx; + ON_CALL(mock_factory_ctx, api()).WillByDefault(testing::ReturnRef(*api_)); + + quic_transport_socket_factory_ = + IntegrationUtil::createQuicClientTransportSocketFactory(mock_factory_ctx, san_to_match_); + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + envoy::extensions::transport_sockets::quic::v3::QuicDownstreamTransport + quic_transport_socket_config; + auto tls_context = quic_transport_socket_config.mutable_downstream_tls_context(); + ConfigHelper::initializeTls(ConfigHelper::ServerSslOptions().setRsaCert(true).setTlsV13(true), + *tls_context->mutable_common_tls_context()); + for (auto& listener : *bootstrap.mutable_static_resources()->mutable_listeners()) { + if (listener.udp_listener_config().udp_listener_name() == "quiche_quic_listener") { + auto* filter_chain = listener.mutable_filter_chains(0); + auto* transport_socket = filter_chain->mutable_transport_socket(); + transport_socket->mutable_typed_config()->PackFrom(quic_transport_socket_config); + + listener.set_reuse_port(set_reuse_port_); + } + } + }); + + config_helper_.addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { + hcm.mutable_drain_timeout()->clear_seconds(); + hcm.mutable_drain_timeout()->set_nanos(500 * 1000 * 1000); + EXPECT_EQ(hcm.codec_type(), envoy::extensions::filters::network::http_connection_manager:: + v3::HttpConnectionManager::HTTP3); + }); + BaseIntegrationTest::initialize(); + registerTestServerPorts({"http"}); +} + void HttpIntegrationTest::setDownstreamProtocol(Http::CodecClient::Type downstream_protocol) { downstream_protocol_ = downstream_protocol; config_helper_.setClientCodec(typeToCodecType(downstream_protocol_)); @@ -497,7 +577,6 @@ void HttpIntegrationTest::testRouterNotFound() { void HttpIntegrationTest::testRouterNotFoundWithBody() { config_helper_.setDefaultHostAndRoute("foo.com", "/found"); initialize(); - BufferingStreamDecoderPtr response = IntegrationUtil::makeSingleRequest( lookupPort("http"), "POST", "/notfound", "foo", downstream_protocol_, version_); ASSERT_TRUE(response->complete()); @@ -846,6 +925,7 @@ void HttpIntegrationTest::testEnvoyHandling100Continue(bool additional_continue_ ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); // Send the rest of the request. + std::cerr << "============== finish sendData"; codec_client_->sendData(*request_encoder_, 10, true); ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); // Verify the Expect header is stripped. diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h index 5ee7de657997..791e7d7c7bc3 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -105,6 +105,8 @@ class HttpIntegrationTest : public BaseIntegrationTest { const std::string& config = ConfigHelper::httpProxyConfig()); ~HttpIntegrationTest() override; + void initialize() override; + protected: void useAccessLog(absl::string_view format = "", std::vector formatters = {}); @@ -114,6 +116,9 @@ class HttpIntegrationTest : public BaseIntegrationTest { virtual IntegrationCodecClientPtr makeRawHttpConnection( Network::ClientConnectionPtr&& conn, absl::optional http2_options); + // Makes a downstream network connection object based on client codec version. + Network::ClientConnectionPtr makeClientConnectionWithOptions( + uint32_t port, const Network::ConnectionSocket::OptionsSharedPtr& options) override; // Makes a http connection object with asserting a connected state. IntegrationCodecClientPtr makeHttpConnection(Network::ClientConnectionPtr&& conn); @@ -260,6 +265,10 @@ class HttpIntegrationTest : public BaseIntegrationTest { uint32_t max_request_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; std::string access_log_name_; testing::NiceMock random_; + + bool set_reuse_port_{false}; + std::string san_to_match_{"spiffe://lyft.com/backend-team"}; + Network::TransportSocketFactoryPtr quic_transport_socket_factory_; }; // Helper class for integration tests using raw HTTP/2 frames diff --git a/test/integration/http_protocol_integration.cc b/test/integration/http_protocol_integration.cc index 9420f33f1fb6..11c182fe70ff 100644 --- a/test/integration/http_protocol_integration.cc +++ b/test/integration/http_protocol_integration.cc @@ -30,12 +30,22 @@ absl::string_view upstreamToString(FakeHttpConnection::Type type) { return "UnknownUpstream"; } +absl::string_view downstreamToString(Http::CodecClient::Type type) { + switch (type) { + case Http::CodecClient::Type::HTTP1: + return "HttpDownstream_"; + case Http::CodecClient::Type::HTTP2: + return "Http2Downstream_"; + case Http::CodecClient::Type::HTTP3: + return "Http3Downstream_"; + } + return "UnknownDownstream"; +} + std::string HttpProtocolIntegrationTest::protocolTestParamsToString( const ::testing::TestParamInfo& params) { return absl::StrCat((params.param.version == Network::Address::IpVersion::v4 ? "IPv4_" : "IPv6_"), - (params.param.downstream_protocol == Http::CodecClient::Type::HTTP2 - ? "Http2Downstream_" - : "HttpDownstream_"), + downstreamToString(params.param.downstream_protocol), upstreamToString(params.param.upstream_protocol)); } diff --git a/test/integration/http_protocol_integration.h b/test/integration/http_protocol_integration.h index 6233e54865f1..911c12661642 100644 --- a/test/integration/http_protocol_integration.h +++ b/test/integration/http_protocol_integration.h @@ -31,9 +31,9 @@ class HttpProtocolIntegrationTest : public testing::TestWithParam& p); HttpProtocolIntegrationTest() - : HttpIntegrationTest(GetParam().downstream_protocol, GetParam().version) {} + : HttpIntegrationTest(GetParam().downstream_protocol, GetParam().version, + GetParam().downstream_protocol == Http::CodecClient::Type::HTTP3 + ? ConfigHelper::quicHttpProxyConfig() + : ConfigHelper::httpProxyConfig()) {} void SetUp() override { setDownstreamProtocol(GetParam().downstream_protocol); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index c8bdb3ed8010..b42610966004 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -59,6 +59,11 @@ void setDoNotValidateRouteConfig( return; \ } +#define EXCLUDE_DOWNSTREAM_HTTP3 \ + if (downstreamProtocol() == Http::CodecClient::Type::HTTP3) { \ + return; \ + } + TEST_P(ProtocolIntegrationTest, TrailerSupportHttp1) { config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); config_helper_.addConfigModifier(setEnableUpstreamTrailersHttp1()); @@ -188,6 +193,10 @@ name: health_check // Verifies behavior for https://github.com/envoyproxy/envoy/pull/11248 TEST_P(ProtocolIntegrationTest, AddBodyToRequestAndWaitForIt) { + // QUICHE can't guarantee headers and FIN to be delivered together, so + // headers-only request can't be detected at L7 filters. + EXCLUDE_DOWNSTREAM_HTTP3; + // filters are prepended, so add them in reverse order config_helper_.addFilter(R"EOF( name: wait-for-whole-request-and-response-filter @@ -235,6 +244,11 @@ TEST_P(ProtocolIntegrationTest, AddBodyToResponseAndWaitForIt) { } TEST_P(ProtocolIntegrationTest, ContinueHeadersOnlyInjectBodyFilter) { + // Headers-only request is translated into headers and empty body by QUICHE + // because FIN bit in IETF QUIC stream is decoupled with http HEADERS frame. + // decodeHeaders() is always called with end_stream = false if QUICHE is + // using IETF version. + EXCLUDE_DOWNSTREAM_HTTP3 EXCLUDE_UPSTREAM_HTTP3; config_helper_.addFilter(R"EOF( name: continue-headers-only-inject-body-filter @@ -312,7 +326,7 @@ name: add-trailers-filter } EXPECT_TRUE(response->complete()); EXPECT_EQ("503", response->headers().getStatusValue()); - if (downstream_protocol_ == Http::CodecClient::Type::HTTP2) { + if (downstream_protocol_ >= Http::CodecClient::Type::HTTP2) { EXPECT_EQ("encode", response->trailers()->getGrpcMessageValue()); } } @@ -370,6 +384,9 @@ TEST_P(DownstreamProtocolIntegrationTest, DownstreamRequestWithFaultyFilter) { } TEST_P(DownstreamProtocolIntegrationTest, FaultyFilterWithConnect) { + // TODO(danzh) re-enable after plumbing through http2 option + // "allow_connect". + EXCLUDE_DOWNSTREAM_HTTP3; // Faulty filter that removed host in a CONNECT request. config_helper_.addConfigModifier( [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& @@ -431,7 +448,6 @@ TEST_P(ProtocolIntegrationTest, LongHeaderValueWithSpaces) { // reads, which the buffer rounds up to about 20KB when allocating slices in // Buffer::OwnedImpl::reserve(). const std::string long_header_value_with_inner_lws = "v" + std::string(32 * 1024, ' ') + "v"; - initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); auto response = codec_client_->makeHeaderOnlyRequest( @@ -731,6 +747,10 @@ 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) { + if (upstreamProtocol() == FakeHttpConnection::Type::HTTP2 && + downstreamProtocol() == Http::CodecClient::Type::HTTP3) { + return; + } const Upstream::HealthyLoad healthy_priority_load({0u, 100u}); const Upstream::DegradedLoad degraded_priority_load({0u, 100u}); NiceMock retry_priority(healthy_priority_load, @@ -940,7 +960,7 @@ TEST_P(DownstreamProtocolIntegrationTest, HittingDecoderFilterLimit) { // the 413-and-connection-close may be sent while the body is still being // sent, resulting in a write error and the connection being closed before the // response is read. - if (downstream_protocol_ == Http::CodecClient::Type::HTTP2) { + if (downstream_protocol_ >= Http::CodecClient::Type::HTTP2) { ASSERT_TRUE(response->complete()); } if (response->complete()) { @@ -1081,10 +1101,22 @@ TEST_P(ProtocolIntegrationTest, HeadersWithUnderscoresDropped) { EXPECT_EQ("200", response->headers().getStatusValue()); EXPECT_THAT(response->headers(), HeaderHasValueRef("bar_baz", "fooz")); Stats::Store& stats = test_server_->server().stats(); - std::string stat_name = (downstreamProtocol() == Http::CodecClient::Type::HTTP1) - ? "http1.dropped_headers_with_underscores" - : "http2.dropped_headers_with_underscores"; - EXPECT_EQ(1L, TestUtility::findCounter(stats, stat_name)->value()); + std::string stat_name; + switch (downstreamProtocol()) { + case Http::CodecClient::Type::HTTP1: + stat_name = "http1.dropped_headers_with_underscores"; + break; + case Http::CodecClient::Type::HTTP2: + stat_name = "http2.dropped_headers_with_underscores"; + break; + case Http::CodecClient::Type::HTTP3: + break; + default: + RELEASE_ASSERT(false, fmt::format("Unknown downstream protocol {}", downstream_protocol_)); + }; + if (downstream_protocol_ != Http::CodecClient::Type::HTTP3) { + EXPECT_EQ(1L, TestUtility::findCounter(stats, stat_name)->value()); + } } // Verify that by default headers with underscores in their names remain in both requests and @@ -1192,10 +1224,10 @@ TEST_P(ProtocolIntegrationTest, 304WithBody) { test_server_->waitForCounterGe("cluster.cluster_0.upstream_cx_protocol_error", 1); } - // Only for HTTP/2, where streams are ended with an explicit end-stream so we + // Only for HTTP/2 and Http/3, where streams are ended with an explicit end-stream so we // can differentiate between 304-with-advertised-but-absent-body and // 304-with-body, is there a protocol error on the active stream. - if (downstream_protocol_ == Http::CodecClient::Type::HTTP2 && + if (downstream_protocol_ >= Http::CodecClient::Type::HTTP2 && upstreamProtocol() >= FakeHttpConnection::Type::HTTP2) { response->waitForReset(); } @@ -1273,7 +1305,6 @@ TEST_P(ProtocolIntegrationTest, MissingStatus) { // Validate that lots of tiny cookies doesn't cause a DoS (single cookie header). TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) { initialize(); - codec_client_ = makeHttpConnection(lookupPort("http")); Http::TestRequestHeaderMapImpl request_headers{{":method", "POST"}, {":path", "/test/long/url"}, @@ -1346,6 +1377,8 @@ TEST_P(DownstreamProtocolIntegrationTest, InvalidContentLength) { } TEST_P(DownstreamProtocolIntegrationTest, InvalidContentLengthAllowed) { + // TODO(danzh) Add override_stream_error_on_invalid_http_message to http3 protocol options. + EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addConfigModifier( [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& hcm) -> void { @@ -1385,6 +1418,7 @@ TEST_P(DownstreamProtocolIntegrationTest, InvalidContentLengthAllowed) { } TEST_P(DownstreamProtocolIntegrationTest, MultipleContentLengths) { + initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); auto encoder_decoder = @@ -1407,6 +1441,8 @@ TEST_P(DownstreamProtocolIntegrationTest, MultipleContentLengths) { // TODO(PiotrSikora): move this HTTP/2 only variant to http2_integration_test.cc. TEST_P(DownstreamProtocolIntegrationTest, MultipleContentLengthsAllowed) { + // override_stream_error_on_invalid_http_message not supported yet. + EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addConfigModifier( [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& hcm) -> void { @@ -1416,6 +1452,7 @@ TEST_P(DownstreamProtocolIntegrationTest, MultipleContentLengthsAllowed) { }); initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); auto encoder_decoder = codec_client_->startRequest(Http::TestRequestHeaderMapImpl{{":method", "POST"}, @@ -1465,6 +1502,7 @@ name: local-reply-during-encode } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlRejected) { + EXCLUDE_DOWNSTREAM_HTTP3 // Send one 95 kB URL with limit 60 kB headers. testLargeRequestUrl(95, 60); } @@ -1475,6 +1513,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlAccepted) { } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersRejected) { + EXCLUDE_DOWNSTREAM_HTTP3 // Send one 95 kB header with limit 60 kB and 100 headers. testLargeRequestHeaders(95, 1, 60, 100); } @@ -1485,6 +1524,8 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersAccepted) { } 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); } @@ -1495,6 +1536,8 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersAccepted) { } TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersRejected) { + // QUICHE doesn't limit number of headers. + EXCLUDE_DOWNSTREAM_HTTP3 // Default header (and trailer) count limit is 100. config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); config_helper_.addConfigModifier(setEnableUpstreamTrailersHttp1()); @@ -1554,6 +1597,7 @@ 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_DOWNSTREAM_HTTP3 // Set timeout for 5 seconds, and ensure that a request with 10k+ headers can be sent. testManyRequestHeaders(std::chrono::milliseconds(5000)); } @@ -1564,6 +1608,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersAccepted) { } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersRejected) { + EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); testLargeRequestTrailers(66, 60); } @@ -1571,6 +1616,9 @@ 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) { + // Enable after setting QUICHE max_inbound_header_list_size_ from HCM + // config. + EXCLUDE_DOWNSTREAM_HTTP3 max_request_headers_kb_ = 96; max_request_headers_count_ = 20005; @@ -1618,9 +1666,9 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyTrailerHeaders) { // ------------------------------------------ // H1 H1 Envoy will reject (HTTP/1 codec behavior) // H1 H2 Envoy will reject (HTTP/1 codec behavior) -// H2 H1 Envoy will forward but backend will reject (HTTP/1 +// H2, H3 H1 Envoy will forward but backend will reject (HTTP/1 // codec behavior) -// H2 H2 Success +// H2, H3 H2 Success TEST_P(ProtocolIntegrationTest, LargeRequestMethod) { // There will be no upstream connections for HTTP/1 downstream, we need to // test the full mesh regardless. @@ -1643,7 +1691,7 @@ TEST_P(ProtocolIntegrationTest, LargeRequestMethod) { EXPECT_TRUE(response->complete()); EXPECT_EQ("400", response->headers().getStatusValue()); } else { - ASSERT(downstreamProtocol() == Http::CodecClient::Type::HTTP2); + ASSERT(downstreamProtocol() >= Http::CodecClient::Type::HTTP2); if (upstreamProtocol() == FakeHttpConnection::Type::HTTP1) { auto response = codec_client_->makeHeaderOnlyRequest(request_headers); ASSERT_TRUE( @@ -1663,6 +1711,9 @@ TEST_P(ProtocolIntegrationTest, LargeRequestMethod) { // Tests StopAllIterationAndBuffer. Verifies decode-headers-return-stop-all-filter calls decodeData // once after iteration is resumed. TEST_P(DownstreamProtocolIntegrationTest, TestDecodeHeadersReturnsStopAll) { + // Enable after setting QUICHE stream initial flow control window from http2 + // options. + EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addFilter(R"EOF( name: call-decodedata-once-filter )EOF"); @@ -1714,6 +1765,7 @@ name: passthrough-filter // Tests StopAllIterationAndWatermark. decode-headers-return-stop-all-watermark-filter sets buffer // limit to 100. Verifies data pause when limit is reached, and resume after iteration continues. TEST_P(DownstreamProtocolIntegrationTest, TestDecodeHeadersReturnsStopAllWatermark) { + EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addFilter(R"EOF( name: decode-headers-return-stop-all-filter )EOF"); @@ -1772,6 +1824,9 @@ name: passthrough-filter // Test two filters that return StopAllIterationAndBuffer back-to-back. TEST_P(DownstreamProtocolIntegrationTest, TestTwoFiltersDecodeHeadersReturnsStopAll) { + // TODO(danzh) Re-enable after codec buffer can be set according to http2 + // options. + EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addFilter(R"EOF( name: decode-headers-return-stop-all-filter )EOF"); @@ -1820,6 +1875,8 @@ name: passthrough-filter // Tests encodeHeaders() returns StopAllIterationAndBuffer. TEST_P(DownstreamProtocolIntegrationTest, TestEncodeHeadersReturnsStopAll) { + // TODO(danzh) Re-enable after codec buffer can be set according to quic options. + EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addFilter(R"EOF( name: encode-headers-return-stop-all-filter )EOF"); @@ -1852,6 +1909,7 @@ name: encode-headers-return-stop-all-filter // Tests encodeHeaders() returns StopAllIterationAndWatermark. TEST_P(DownstreamProtocolIntegrationTest, TestEncodeHeadersReturnsStopAllWatermark) { + EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addFilter(R"EOF( name: encode-headers-return-stop-all-filter )EOF"); @@ -2138,6 +2196,8 @@ TEST_P(DownstreamProtocolIntegrationTest, ConnectStreamRejection) { {":method", "CONNECT"}, {":path", "/"}, {":authority", "host"}}); response->waitForReset(); + // TODO(danzh) plumb through stream_error_on_invalid_http_message. + EXCLUDE_DOWNSTREAM_HTTP3; EXPECT_FALSE(codec_client_->disconnected()); } diff --git a/test/integration/utility.cc b/test/integration/utility.cc index 24b71b58ceff..d6b3bf5dd7a1 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -6,21 +6,27 @@ #include #include "envoy/event/dispatcher.h" +#include "envoy/extensions/transport_sockets/quic/v3/quic_transport.pb.h" #include "envoy/network/connection.h" #include "common/api/api_impl.h" #include "common/buffer/buffer_impl.h" #include "common/common/assert.h" #include "common/common/fmt.h" +#include "common/config/utility.h" #include "common/http/header_map_impl.h" #include "common/http/headers.h" +#include "common/http/http3/quic_client_connection_factory.h" +#include "common/http/http3/well_known_names.h" #include "common/network/utility.h" #include "common/upstream/upstream_impl.h" #include "test/common/upstream/utility.h" #include "test/mocks/common.h" +#include "test/mocks/server/transport_socket_factory_context.h" #include "test/mocks/stats/mocks.h" #include "test/mocks/upstream/cluster_info.h" +#include "test/test_common/environment.h" #include "test/test_common/network_utility.h" #include "test/test_common/printers.h" #include "test/test_common/utility.h" @@ -75,12 +81,71 @@ void BufferingStreamDecoder::onResetStream(Http::StreamResetReason, absl::string ADD_FAILURE(); } +struct ConnectionCallbacks : public Network::ConnectionCallbacks { + ConnectionCallbacks(Event::Dispatcher& dispatcher) : dispatcher_(dispatcher) {} + + // Network::ConnectionCallbacks + void onEvent(Network::ConnectionEvent event) override { + if (event == Network::ConnectionEvent::Connected) { + connected_ = true; + dispatcher_.exit(); + } else if (event == Network::ConnectionEvent::RemoteClose) { + dispatcher_.exit(); + } else { + if (!connected_) { + // Before handshake gets established, any connection failure should exit the loop. I.e. a + // QUIC connection may fail of INVALID_VERSION if both this client doesn't support any of + // the versions the server advertised before handshake established. In this case the + // connection is closed locally and this is in a blocking event loop. + dispatcher_.exit(); + } + } + } + + void onAboveWriteBufferHighWatermark() override {} + void onBelowWriteBufferLowWatermark() override {} + + Event::Dispatcher& dispatcher_; + bool connected_{false}; +}; + +Network::TransportSocketFactoryPtr IntegrationUtil::createQuicClientTransportSocketFactory( + Server::Configuration::TransportSocketFactoryContext& context, + const std::string& san_to_match) { + std::string yaml_plain = R"EOF( + common_tls_context: + validation_context: + trusted_ca: + filename: "{{ test_rundir }}/test/config/integration/certs/cacert.pem" +)EOF"; + envoy::extensions::transport_sockets::quic::v3::QuicUpstreamTransport + quic_transport_socket_config; + auto* tls_context = quic_transport_socket_config.mutable_upstream_tls_context(); + TestUtility::loadFromYaml(TestEnvironment::substitute(yaml_plain), *tls_context); + auto* common_context = tls_context->mutable_common_tls_context(); + + common_context->add_alpn_protocols("h3"); + common_context->mutable_validation_context()->add_match_subject_alt_names()->set_exact( + san_to_match); + tls_context->set_sni("lyft.com"); + + common_context->mutable_tls_params()->set_tls_minimum_protocol_version( + envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLS_AUTO); + common_context->mutable_tls_params()->set_tls_maximum_protocol_version( + envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLS_AUTO); + + envoy::config::core::v3::TransportSocket message; + message.mutable_typed_config()->PackFrom(quic_transport_socket_config); + auto& config_factory = Config::Utility::getAndCheckFactory< + Server::Configuration::UpstreamTransportSocketConfigFactory>(message); + return config_factory.createTransportSocketFactory(quic_transport_socket_config, context); +} + BufferingStreamDecoderPtr IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPtr& addr, const std::string& method, const std::string& url, const std::string& body, Http::CodecClient::Type type, const std::string& host, const std::string& content_type) { - NiceMock mock_stats_store; NiceMock random; Event::GlobalTimeSystem time_system; @@ -88,14 +153,36 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt Api::Impl api(Thread::threadFactoryForTest(), mock_stats_store, time_system, Filesystem::fileSystemForTest(), random_generator); Event::DispatcherPtr dispatcher(api.allocateDispatcher("test_thread")); + ConnectionCallbacks connection_callbacks(*dispatcher); std::shared_ptr cluster{new NiceMock()}; - Upstream::HostDescriptionConstSharedPtr host_description{ - Upstream::makeTestHostDescription(cluster, "tcp://127.0.0.1:80", time_system)}; - Http::CodecClientProd client( - type, - dispatcher->createClientConnection(addr, Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr), - host_description, *dispatcher, random); + Upstream::HostDescriptionConstSharedPtr host_description{Upstream::makeTestHostDescription( + cluster, + fmt::format("{}://127.0.0.1:80", (type == Http::CodecClient::Type::HTTP3 ? "udp" : "tcp")), + time_system)}; + + NiceMock mock_factory_ctx; + ON_CALL(mock_factory_ctx, api()).WillByDefault(testing::ReturnRef(api)); + Network::TransportSocketFactoryPtr transport_socket_factory = + createQuicClientTransportSocketFactory(mock_factory_ctx, "spiffe://lyft.com/backend-team"); + Network::ClientConnectionPtr connection = + (type == Http::CodecClient::Type::HTTP3 + ? Config::Utility::getAndCheckFactoryByName( + Http::QuicCodecNames::get().Quiche) + .createQuicNetworkConnection(addr, Network::Address::InstanceConstSharedPtr(), + *transport_socket_factory, mock_stats_store, + *dispatcher, time_system) + : dispatcher->createClientConnection(addr, Network::Address::InstanceConstSharedPtr(), + Network::Test::createRawBufferSocket(), nullptr)); + if (type == Http::CodecClient::Type::HTTP3) { + connection->addConnectionCallbacks(connection_callbacks); + } + Http::CodecClientProd client(type, std::move(connection), host_description, *dispatcher, random); + + if (type == Http::CodecClient::Type::HTTP3) { + // Quic connection needs to finish handshake. + dispatcher->run(Event::Dispatcher::RunType::Block); + } + BufferingStreamDecoderPtr response(new BufferingStreamDecoder([&]() -> void { client.close(); dispatcher->exit(); diff --git a/test/integration/utility.h b/test/integration/utility.h index afe3df9a3de2..a9cf91c48126 100644 --- a/test/integration/utility.h +++ b/test/integration/utility.h @@ -9,6 +9,7 @@ #include "envoy/http/codec.h" #include "envoy/http/header_map.h" #include "envoy/network/filter.h" +#include "envoy/server/factory_context.h" #include "common/common/assert.h" #include "common/common/utility.h" @@ -184,6 +185,17 @@ class IntegrationUtil { const std::string& body, Http::CodecClient::Type type, Network::Address::IpVersion ip_version, const std::string& host = "host", const std::string& content_type = ""); + + /** + * Create transport socket factory for Quic client transport socket. + * @param context supplies the port to connect to on localhost. + * @param san_to_match configs |context| to match Subject Alternative Name during certificate + * verification. + * @return TransportSocketFactoryPtr the client transport socket factory. + */ + static Network::TransportSocketFactoryPtr createQuicClientTransportSocketFactory( + Server::Configuration::TransportSocketFactoryContext& context, + const std::string& san_to_match); }; // A set of connection callbacks which tracks connection state. From 09fc660cb9528d2c5f211a02d8b702b22406dd87 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Tue, 9 Mar 2021 15:12:39 -0500 Subject: [PATCH 02/21] visibility Signed-off-by: Dan Zhang --- source/extensions/quic_listeners/quiche/BUILD | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source/extensions/quic_listeners/quiche/BUILD b/source/extensions/quic_listeners/quiche/BUILD index da68122404ad..6596a2b2cade 100644 --- a/source/extensions/quic_listeners/quiche/BUILD +++ b/source/extensions/quic_listeners/quiche/BUILD @@ -179,6 +179,7 @@ envoy_cc_library( srcs = ["quic_filter_manager_connection_impl.cc"], hdrs = ["quic_filter_manager_connection_impl.h"], tags = ["nofips"], +visibility = ["//test:__subpackages__"], deps = [ ":envoy_quic_connection_lib", ":envoy_quic_simulated_watermark_buffer_lib", @@ -387,6 +388,7 @@ envoy_cc_extension( "//source/server:__subpackages__", ], security_posture = "unknown", +visibility = ["//test:__subpackages__"], tags = ["nofips"], deps = [ "//include/envoy/network:transport_socket_interface", From 6ed3c94db342d30a9f68875507e3186937868317 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Wed, 10 Mar 2021 01:53:40 -0500 Subject: [PATCH 03/21] make protocol test pass Signed-off-by: Dan Zhang --- source/extensions/quic_listeners/quiche/BUILD | 4 ++-- .../quic_listeners/quiche/active_quic_listener.cc | 1 + .../quiche/envoy_quic_client_stream.cc | 14 +++++++++++++- .../quiche/envoy_quic_server_stream.cc | 12 +++++++++--- .../quic_listeners/quiche/envoy_quic_utils.cc | 8 ++++++++ .../quic_listeners/quiche/envoy_quic_utils.h | 5 ++++- .../quiche/platform/quiche_flags_impl.cc | 5 +++-- .../quiche/quic_filter_manager_connection_impl.h | 7 +++++++ test/integration/fake_upstream.cc | 4 +--- test/integration/protocol_integration_test.cc | 14 +++++++++++++- 10 files changed, 61 insertions(+), 13 deletions(-) diff --git a/source/extensions/quic_listeners/quiche/BUILD b/source/extensions/quic_listeners/quiche/BUILD index 6596a2b2cade..850c7c2c0b5d 100644 --- a/source/extensions/quic_listeners/quiche/BUILD +++ b/source/extensions/quic_listeners/quiche/BUILD @@ -179,7 +179,7 @@ envoy_cc_library( srcs = ["quic_filter_manager_connection_impl.cc"], hdrs = ["quic_filter_manager_connection_impl.h"], tags = ["nofips"], -visibility = ["//test:__subpackages__"], + visibility = ["//test:__subpackages__"], deps = [ ":envoy_quic_connection_lib", ":envoy_quic_simulated_watermark_buffer_lib", @@ -388,8 +388,8 @@ envoy_cc_extension( "//source/server:__subpackages__", ], security_posture = "unknown", -visibility = ["//test:__subpackages__"], tags = ["nofips"], + visibility = ["//test:__subpackages__"], deps = [ "//include/envoy/network:transport_socket_interface", "//include/envoy/server:transport_socket_config_interface", diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index 7a3843865246..805ce5a1cc62 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -39,6 +39,7 @@ ActiveQuicListener::ActiveQuicListener( &listener_config), dispatcher_(dispatcher), version_manager_(quic::CurrentSupportedVersions()), kernel_worker_routing_(kernel_worker_routing) { + SetQuicReloadableFlag(quic_single_ack_in_packet2, true); if (Runtime::LoaderSingleton::getExisting()) { enabled_.emplace(Runtime::FeatureFlag(enabled, Runtime::LoaderSingleton::get())); } diff --git a/source/extensions/quic_listeners/quiche/envoy_quic_client_stream.cc b/source/extensions/quic_listeners/quiche/envoy_quic_client_stream.cc index d57840cda4e4..1664e68ca0ed 100644 --- a/source/extensions/quic_listeners/quiche/envoy_quic_client_stream.cc +++ b/source/extensions/quic_listeners/quiche/envoy_quic_client_stream.cc @@ -62,7 +62,19 @@ Http::Status EnvoyQuicClientStream::encodeHeaders(const Http::RequestHeaderMap& ? static_cast(this) : (dynamic_cast(session())->headers_stream()); const uint64_t bytes_to_send_old = writing_stream->BufferedDataBytes(); - WriteHeaders(envoyHeadersToSpdyHeaderBlock(headers), end_stream, nullptr); + auto spdy_headers = envoyHeadersToSpdyHeaderBlock(headers); + if (headers.Method() && headers.Method()->value() == "CONNECT") { + // It is a bytestream connect and should have :path and :protocol set accordingly + // As HTTP/1.1 does not require a path for CONNECT, we may have to add one + // if shifting codecs. For now, default to "/" - this can be made + // configurable if necessary. + // https://tools.ietf.org/html/draft-kinnear-httpbis-http2-transport-02 + spdy_headers[":protocol"] = Http::Headers::get().ProtocolValues.Bytestream; + if (!headers.Path()) { + spdy_headers[":path"] = "/"; + } + } + WriteHeaders(std::move(spdy_headers), end_stream, nullptr); local_end_stream_ = end_stream; const uint64_t bytes_to_send_new = writing_stream->BufferedDataBytes(); ASSERT(bytes_to_send_old <= bytes_to_send_new); diff --git a/source/extensions/quic_listeners/quiche/envoy_quic_server_stream.cc b/source/extensions/quic_listeners/quiche/envoy_quic_server_stream.cc index 99dc7a70ddf0..602358d1dff8 100644 --- a/source/extensions/quic_listeners/quiche/envoy_quic_server_stream.cc +++ b/source/extensions/quic_listeners/quiche/envoy_quic_server_stream.cc @@ -26,6 +26,7 @@ #include "common/buffer/buffer_impl.h" #include "common/http/header_map_impl.h" #include "common/common/assert.h" +#include "common/http/header_utility.h" namespace Envoy { namespace Quic { @@ -160,9 +161,14 @@ void EnvoyQuicServerStream::OnInitialHeadersComplete(bool fin, size_t frame_len, if (fin) { end_stream_decoded_ = true; } - request_decoder_->decodeHeaders( - quicHeadersToEnvoyHeaders(header_list), - /*end_stream=*/fin); + std::unique_ptr headers = + quicHeadersToEnvoyHeaders(header_list); + if (!Http::HeaderUtility::authorityIsValid(headers->Host()->value().getStringView())) { + stream_delegate()->OnStreamError(quic::QUIC_HTTP_FRAME_ERROR, "Invalid headers"); + return; + } + request_decoder_->decodeHeaders(std::move(headers), + /*end_stream=*/fin); ConsumeHeaderList(); } diff --git a/source/extensions/quic_listeners/quiche/envoy_quic_utils.cc b/source/extensions/quic_listeners/quiche/envoy_quic_utils.cc index abb70d9093f2..f0f658eb5585 100644 --- a/source/extensions/quic_listeners/quiche/envoy_quic_utils.cc +++ b/source/extensions/quic_listeners/quiche/envoy_quic_utils.cc @@ -4,6 +4,7 @@ #include "envoy/config/core/v3/base.pb.h" #include "common/network/socket_option_factory.h" +#include "common/network/utility.h" namespace Envoy { namespace Quic { @@ -118,6 +119,13 @@ Network::ConnectionSocketPtr createConnectionSocket(Network::Address::InstanceConstSharedPtr& peer_addr, Network::Address::InstanceConstSharedPtr& local_addr, const Network::ConnectionSocket::OptionsSharedPtr& options) { + if (local_addr == nullptr) { + if (peer_addr->ip()->ipv4() != nullptr) { + local_addr = Network::Utility::getCanonicalIpv4LoopbackAddress(); + } else { + local_addr = Network::Utility::getIpv6LoopbackAddress(); + } + } auto connection_socket = std::make_unique( Network::Socket::Type::Datagram, local_addr, peer_addr); connection_socket->addOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions()); diff --git a/source/extensions/quic_listeners/quiche/envoy_quic_utils.h b/source/extensions/quic_listeners/quiche/envoy_quic_utils.h index 2e9c247d4722..ef68543df62d 100644 --- a/source/extensions/quic_listeners/quiche/envoy_quic_utils.h +++ b/source/extensions/quic_listeners/quiche/envoy_quic_utils.h @@ -61,7 +61,10 @@ std::unique_ptr spdyHeaderBlockToEnvoyHeaders(const spdy::SpdyHeaderBlock& he for (auto entry : header_block) { // TODO(danzh): Avoid temporary strings and addCopy() with string_view. std::string key(entry.first); - headers->addCopy(Http::LowerCaseString(key), entry.second); + std::vector values = absl::StrSplit(entry.second, '\0'); + for (const absl::string_view& value : values) { + headers->addCopy(Http::LowerCaseString(key), value); + } } return headers; } diff --git a/source/extensions/quic_listeners/quiche/platform/quiche_flags_impl.cc b/source/extensions/quic_listeners/quiche/platform/quiche_flags_impl.cc index 78fd70d6cd9f..aad20c06f72c 100644 --- a/source/extensions/quic_listeners/quiche/platform/quiche_flags_impl.cc +++ b/source/extensions/quic_listeners/quiche/platform/quiche_flags_impl.cc @@ -45,7 +45,9 @@ FlagRegistry& FlagRegistry::getInstance() { return *instance; } -FlagRegistry::FlagRegistry() : flags_(makeFlagMap()) {} +FlagRegistry::FlagRegistry() : flags_(makeFlagMap()) { + FLAGS_quic_reloadable_flag_quic_single_ack_in_packet2->setValue(true); +} void FlagRegistry::resetFlags() const { for (auto& kv : flags_) { @@ -134,7 +136,6 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_http2_testonly_default_false, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_http2_testonly_default_true, true) QUIC_FLAG(FLAGS_quic_restart_flag_http2_testonly_default_false, false) QUIC_FLAG(FLAGS_quic_restart_flag_http2_testonly_default_true, true) - #undef QUIC_FLAG #define STRINGIFY(X) #X diff --git a/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h b/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h index 6d07c2d07d0e..3e3eedba3bbd 100644 --- a/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h +++ b/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h @@ -12,6 +12,10 @@ #include "extensions/quic_listeners/quiche/envoy_quic_simulated_watermark_buffer.h" namespace Envoy { + +class TestPauseFilter; +class RandomPauseFilter; + namespace Quic { // Act as a Network::Connection to HCM and a FilterManager to FilterFactoryCb. @@ -123,6 +127,9 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase { EnvoyQuicConnection* quic_connection_{nullptr}; private: + friend class Envoy::RandomPauseFilter; + friend class Envoy::TestPauseFilter; + // Called when aggregated buffered bytes across all the streams exceeds high watermark. void onSendBufferHighWatermark(); // Called when aggregated buffered bytes across all the streams declines to low watermark. diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index f6fbd30c2051..55a03439adf4 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -345,9 +345,7 @@ FakeHttpConnection::FakeHttpConnection( codec_ = std::unique_ptr( Config::Utility::getAndCheckFactoryByName( Http::QuicCodecNames::get().Quiche) - .createQuicServerConnection(shared_connection_.connection(), *this, - max_request_headers_kb, max_request_headers_count, - headers_with_underscores_action)); + .createQuicServerConnection(shared_connection_.connection(), *this)); } shared_connection_.connection().addReadFilter( Network::ReadFilterSharedPtr{new ReadFilter(*this)}); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index b42610966004..ebe99bf39da2 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -442,6 +442,7 @@ TEST_P(DownstreamProtocolIntegrationTest, MissingHeadersLocalReply) { // Regression test for https://github.com/envoyproxy/envoy/issues/10270 TEST_P(ProtocolIntegrationTest, LongHeaderValueWithSpaces) { + EXCLUDE_DOWNSTREAM_HTTP3 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 @@ -1077,6 +1078,8 @@ TEST_P(ProtocolIntegrationTest, MaxStreamDurationWithRetryPolicyWhenRetryUpstrea // Verify that headers with underscores in their names are dropped from client requests // but remain in upstream responses. TEST_P(ProtocolIntegrationTest, HeadersWithUnderscoresDropped) { + // TODO(danzh) treat underscore in headers according to the config. + EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addConfigModifier( [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& hcm) -> void { @@ -1143,6 +1146,7 @@ TEST_P(ProtocolIntegrationTest, HeadersWithUnderscoresRemainByDefault) { // Verify that request with headers containing underscores is rejected when configured. TEST_P(DownstreamProtocolIntegrationTest, HeadersWithUnderscoresCauseRequestRejectedByDefault) { + EXCLUDE_DOWNSTREAM_HTTP3 useAccessLog("%RESPONSE_FLAGS% %RESPONSE_CODE_DETAILS%"); config_helper_.addConfigModifier( [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& @@ -1304,6 +1308,7 @@ TEST_P(ProtocolIntegrationTest, MissingStatus) { // Validate that lots of tiny cookies doesn't cause a DoS (single cookie header). TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) { + EXCLUDE_DOWNSTREAM_HTTP3 initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); Http::TestRequestHeaderMapImpl request_headers{{":method", "POST"}, @@ -1325,6 +1330,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) { // Validate that lots of tiny cookies doesn't cause a DoS (many cookie headers). TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingMany) { + EXCLUDE_DOWNSTREAM_HTTP3 // Set header count limit to 2010. uint32_t max_count = 2010; config_helper_.addConfigModifier( @@ -1353,6 +1359,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingMany) { } TEST_P(DownstreamProtocolIntegrationTest, InvalidContentLength) { + EXCLUDE_DOWNSTREAM_HTTP3 initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -1418,7 +1425,7 @@ TEST_P(DownstreamProtocolIntegrationTest, InvalidContentLengthAllowed) { } TEST_P(DownstreamProtocolIntegrationTest, MultipleContentLengths) { - + EXCLUDE_DOWNSTREAM_HTTP3 initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); auto encoder_decoder = @@ -1508,6 +1515,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlRejected) { } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlAccepted) { + EXCLUDE_DOWNSTREAM_HTTP3 // Send one 95 kB URL with limit 96 kB headers. testLargeRequestUrl(95, 96); } @@ -1519,6 +1527,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersRejected) { } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersAccepted) { + EXCLUDE_DOWNSTREAM_HTTP3 // Send one 95 kB header with limit 96 kB and 100 headers. testLargeRequestHeaders(95, 1, 96, 100); } @@ -1603,6 +1612,7 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersTimeout) { } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersAccepted) { + EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); testLargeRequestTrailers(60, 96); } @@ -1673,6 +1683,7 @@ 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_DOWNSTREAM_HTTP3 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}, @@ -2133,6 +2144,7 @@ TEST_P(DownstreamProtocolIntegrationTest, BasicMaxStreamTimeoutLegacy) { // Make sure that invalid authority headers get blocked at or before the HCM. TEST_P(DownstreamProtocolIntegrationTest, InvalidAuthority) { + EXCLUDE_DOWNSTREAM_HTTP3 initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); From 186a69da5288327db9f49b330f9beb89c900332e Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Thu, 11 Mar 2021 00:18:38 -0500 Subject: [PATCH 04/21] add more test Signed-off-by: Dan Zhang --- .../quiche/active_quic_listener.cc | 2 + .../quiche/client_connection_factory_impl.cc | 3 ++ .../quiche/platform/quiche_flags_impl.cc | 5 +-- .../quic_filter_manager_connection_impl.h | 6 +-- .../quiche/envoy_quic_utils_test.cc | 15 ++++++- .../quic_listeners/quiche/integration/BUILD | 1 + test/integration/filters/BUILD | 13 ++++++ test/integration/filters/pause_filter.cc | 45 ++++--------------- test/integration/http_integration.cc | 22 +++------ test/integration/http_protocol_integration.h | 2 +- 10 files changed, 50 insertions(+), 64 deletions(-) diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index 805ce5a1cc62..78cd127ec9be 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -39,7 +39,9 @@ ActiveQuicListener::ActiveQuicListener( &listener_config), dispatcher_(dispatcher), version_manager_(quic::CurrentSupportedVersions()), kernel_worker_routing_(kernel_worker_routing) { + // This flag fix a QUICHE issue which may crashe Envoy during connection close. SetQuicReloadableFlag(quic_single_ack_in_packet2, true); + if (Runtime::LoaderSingleton::getExisting()) { enabled_.emplace(Runtime::FeatureFlag(enabled, Runtime::LoaderSingleton::get())); } diff --git a/source/extensions/quic_listeners/quiche/client_connection_factory_impl.cc b/source/extensions/quic_listeners/quiche/client_connection_factory_impl.cc index 9b8f40e34c73..4396e68dd044 100644 --- a/source/extensions/quic_listeners/quiche/client_connection_factory_impl.cc +++ b/source/extensions/quic_listeners/quiche/client_connection_factory_impl.cc @@ -11,6 +11,9 @@ QuicClientConnectionFactoryImpl::createQuicNetworkConnection( Network::Address::InstanceConstSharedPtr local_addr, Network::TransportSocketFactory& transport_socket_factory, Stats::Scope& stats_scope, Event::Dispatcher& dispatcher, TimeSource& time_source) { + // This flag fix a QUICHE issue which may crashe Envoy during connection close. + SetQuicReloadableFlag(quic_single_ack_in_packet2, true); + // TODO(#14829): reject config if anything but QuicClientTransportSocketConfigFactory configured. // raw buffer socket is configured. auto* quic_socket_factory = diff --git a/source/extensions/quic_listeners/quiche/platform/quiche_flags_impl.cc b/source/extensions/quic_listeners/quiche/platform/quiche_flags_impl.cc index aad20c06f72c..78fd70d6cd9f 100644 --- a/source/extensions/quic_listeners/quiche/platform/quiche_flags_impl.cc +++ b/source/extensions/quic_listeners/quiche/platform/quiche_flags_impl.cc @@ -45,9 +45,7 @@ FlagRegistry& FlagRegistry::getInstance() { return *instance; } -FlagRegistry::FlagRegistry() : flags_(makeFlagMap()) { - FLAGS_quic_reloadable_flag_quic_single_ack_in_packet2->setValue(true); -} +FlagRegistry::FlagRegistry() : flags_(makeFlagMap()) {} void FlagRegistry::resetFlags() const { for (auto& kv : flags_) { @@ -136,6 +134,7 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_http2_testonly_default_false, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_http2_testonly_default_true, true) QUIC_FLAG(FLAGS_quic_restart_flag_http2_testonly_default_false, false) QUIC_FLAG(FLAGS_quic_restart_flag_http2_testonly_default_true, true) + #undef QUIC_FLAG #define STRINGIFY(X) #X diff --git a/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h b/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h index 3e3eedba3bbd..4be7a776f068 100644 --- a/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h +++ b/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h @@ -13,8 +13,7 @@ namespace Envoy { -class TestPauseFilter; -class RandomPauseFilter; +class TestPauseFilterForQuic; namespace Quic { @@ -127,8 +126,7 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase { EnvoyQuicConnection* quic_connection_{nullptr}; private: - friend class Envoy::RandomPauseFilter; - friend class Envoy::TestPauseFilter; + friend class Envoy::TestPauseFilterForQuic; // Called when aggregated buffered bytes across all the streams exceeds high watermark. void onSendBufferHighWatermark(); diff --git a/test/extensions/quic_listeners/quiche/envoy_quic_utils_test.cc b/test/extensions/quic_listeners/quiche/envoy_quic_utils_test.cc index ad5dd5df4870..d8f5af4c4005 100644 --- a/test/extensions/quic_listeners/quiche/envoy_quic_utils_test.cc +++ b/test/extensions/quic_listeners/quiche/envoy_quic_utils_test.cc @@ -50,13 +50,24 @@ TEST(EnvoyQuicUtilsTest, HeadersConversion) { headers_block[":authority"] = "www.google.com"; headers_block[":path"] = "/index.hml"; headers_block[":scheme"] = "https"; + headers_block.AppendValueOrAddHeader("key", "value1"); + headers_block.AppendValueOrAddHeader("key", "value2"); auto envoy_headers = spdyHeaderBlockToEnvoyHeaders(headers_block); - EXPECT_EQ(headers_block.size(), envoy_headers->size()); + EXPECT_EQ(headers_block.size() + 1u, envoy_headers->size()); EXPECT_EQ("www.google.com", envoy_headers->getHostValue()); EXPECT_EQ("/index.hml", envoy_headers->getPathValue()); EXPECT_EQ("https", envoy_headers->getSchemeValue()); + EXPECT_EQ("value1", envoy_headers->get(Http::LowerCaseString("key"))[0]->value().getStringView()); + EXPECT_EQ("value2", envoy_headers->get(Http::LowerCaseString("key"))[1]->value().getStringView()); - quic::QuicHeaderList quic_headers = quic::test::AsHeaderList(headers_block); + quic::QuicHeaderList quic_headers; + quic_headers.OnHeaderBlockStart(); + quic_headers.OnHeader(":authority", "www.google.com"); + quic_headers.OnHeader(":path", "/index.hml"); + quic_headers.OnHeader(":scheme", "https"); + quic_headers.OnHeader("key", "value1"); + quic_headers.OnHeader("key", "value2"); + quic_headers.OnHeaderBlockEnd(0, 0); auto envoy_headers2 = quicHeadersToEnvoyHeaders(quic_headers); EXPECT_EQ(*envoy_headers, *envoy_headers2); } diff --git a/test/extensions/quic_listeners/quiche/integration/BUILD b/test/extensions/quic_listeners/quiche/integration/BUILD index 5086389b21dc..d6fec0d41872 100644 --- a/test/extensions/quic_listeners/quiche/integration/BUILD +++ b/test/extensions/quic_listeners/quiche/integration/BUILD @@ -27,6 +27,7 @@ envoy_cc_test( "//source/extensions/quic_listeners/quiche:quic_factory_lib", "//source/extensions/quic_listeners/quiche:quic_transport_socket_factory_lib", "//test/integration:protocol_integration_test_lib", + "//test/integration/filters:pause_filter_for_quic_lib", ], ) diff --git a/test/integration/filters/BUILD b/test/integration/filters/BUILD index 0e4f379ac838..9ec65dd4d270 100644 --- a/test/integration/filters/BUILD +++ b/test/integration/filters/BUILD @@ -182,6 +182,19 @@ envoy_cc_test_library( "//include/envoy/registry", "//source/common/network:connection_lib", "//source/extensions/filters/http/common:pass_through_filter_lib", + "//test/extensions/filters/http/common:empty_http_filter_config_lib", + ], +) + +envoy_cc_test_library( + name = "pause_filter_for_quic_lib", + srcs = [ + "pause_filter_for_quic.cc", + ], + deps = [ + "//include/envoy/http:filter_interface", + "//include/envoy/registry", + "//source/extensions/filters/http/common:pass_through_filter_lib", "//source/extensions/quic_listeners/quiche:quic_filter_manager_connection_lib", "//test/extensions/filters/http/common:empty_http_filter_config_lib", ], diff --git a/test/integration/filters/pause_filter.cc b/test/integration/filters/pause_filter.cc index 221474e0fe77..b7f9aa1a3c36 100644 --- a/test/integration/filters/pause_filter.cc +++ b/test/integration/filters/pause_filter.cc @@ -5,7 +5,6 @@ #include "common/network/connection_impl.h" #include "extensions/filters/http/common/pass_through_filter.h" -#include "extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h" #include "test/extensions/filters/http/common/empty_http_filter_config.h" @@ -30,9 +29,8 @@ class TestPauseFilter : public Http::PassThroughFilter { number_of_decode_calls_ref_++; // If this is the second stream to decode headers and we're at high watermark. force low // watermark state - if (number_of_decode_calls_ref_ == 2 && - decoder_callbacks_->connection()->aboveHighWatermark()) { - mockConnectionBelowLowWatermark(); + if (number_of_decode_calls_ref_ == 2 && connection()->aboveHighWatermark()) { + connection()->onWriteBufferLowWatermark(); } } return PassThroughFilter::decodeData(buf, end_stream); @@ -44,46 +42,19 @@ class TestPauseFilter : public Http::PassThroughFilter { number_of_encode_calls_ref_++; // If this is the first stream to encode headers and we're not at high watermark, force high // watermark state. - if (number_of_encode_calls_ref_ == 1 && - !decoder_callbacks_->connection()->aboveHighWatermark()) { - mockConnectionAboveHighWatermark(); + if (number_of_encode_calls_ref_ == 1 && !connection()->aboveHighWatermark()) { + connection()->onWriteBufferHighWatermark(); } } return PassThroughFilter::encodeData(buf, end_stream); } - void mockConnectionAboveHighWatermark() { + Network::ConnectionImpl* connection() { // As long as we're doing horrible things let's do *all* the horrible things. - // Assert the connection we have is a ConnectionImpl or QuicFilterManagerConnectionImpl and - // const cast it so we can force watermark changes. + // Assert the connection we have is a ConnectionImpl and const cast it so we + // can force watermark changes. auto conn_impl = dynamic_cast(decoder_callbacks_->connection()); - if (conn_impl != nullptr) { - const_cast(conn_impl)->onWriteBufferHighWatermark(); - return; - } - // If transport protocol is QUIC, simulate connection buffer above watermark differently. - auto quic_connection = const_cast( - dynamic_cast( - decoder_callbacks_->connection())); - quic_connection->write_buffer_watermark_simulation_.checkHighWatermark( - quic_connection->write_buffer_watermark_simulation_.highWatermark() + 1u); - } - - void mockConnectionBelowLowWatermark() { - // As long as we're doing horrible things let's do *all* the horrible things. - // Assert the connection we have is a ConnectionImpl or QuicFilterManagerConnectionImpl and - // const cast it so we can force watermark changes. - auto conn_impl = dynamic_cast(decoder_callbacks_->connection()); - if (conn_impl != nullptr) { - const_cast(conn_impl)->onWriteBufferHighWatermark(); - return; - } - // If transport protocol is QUIC, simulate connection buffer below watermark differently. - auto quic_connection = const_cast( - dynamic_cast( - decoder_callbacks_->connection())); - quic_connection->write_buffer_watermark_simulation_.checkLowWatermark( - quic_connection->write_buffer_watermark_simulation_.highWatermark() / 2 - 1u); + return const_cast(conn_impl); } absl::Mutex& encode_lock_; diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 84ccb870c2c7..f17cbc53edce 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -46,18 +46,6 @@ #include "base_integration_test.h" #include "gtest/gtest.h" -#if defined(__GNUC__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wunused-parameter" -#pragma GCC diagnostic ignored "-Winvalid-offsetof" -#endif - -#include "quiche/quic/core/quic_utils.h" - -#if defined(__GNUC__) -#pragma GCC diagnostic pop -#endif - namespace Envoy { namespace { @@ -88,7 +76,6 @@ IntegrationCodecClient::IntegrationCodecClient( CodecClient::Type type) : CodecClientProd(type, std::move(conn), host_description, dispatcher, random), dispatcher_(dispatcher), callbacks_(*this), codec_callbacks_(*this) { - std::cerr << "=========== IntegrationCodecClient add connection callback " << &callbacks_ << "\n"; connection_->addConnectionCallbacks(callbacks_); setCodecConnectionCallbacks(codec_callbacks_); dispatcher.run(Event::Dispatcher::RunType::Block); @@ -925,7 +912,6 @@ void HttpIntegrationTest::testEnvoyHandling100Continue(bool additional_continue_ ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); // Send the rest of the request. - std::cerr << "============== finish sendData"; codec_client_->sendData(*request_encoder_, 10, true); ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); // Verify the Expect header is stripped. @@ -1058,11 +1044,13 @@ void HttpIntegrationTest::testTwoRequests(bool network_backup) { // created while the socket appears to be in the high watermark state, and regression tests that // flow control will be corrected as the socket "becomes unblocked" if (network_backup) { - config_helper_.addFilter(R"EOF( - name: pause-filter + config_helper_.addFilter( + fmt::format(R"EOF( + name: pause-filter{} typed_config: "@type": type.googleapis.com/google.protobuf.Empty - )EOF"); + )EOF", + downstreamProtocol() == Http::CodecClient::Type::HTTP3 ? "-for-quic" : "")); } initialize(); diff --git a/test/integration/http_protocol_integration.h b/test/integration/http_protocol_integration.h index 911c12661642..f65b15455116 100644 --- a/test/integration/http_protocol_integration.h +++ b/test/integration/http_protocol_integration.h @@ -31,8 +31,8 @@ class HttpProtocolIntegrationTest : public testing::TestWithParam Date: Thu, 11 Mar 2021 00:19:23 -0500 Subject: [PATCH 05/21] quic_pause_filter Signed-off-by: Dan Zhang --- .../filters/pause_filter_for_quic.cc | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 test/integration/filters/pause_filter_for_quic.cc diff --git a/test/integration/filters/pause_filter_for_quic.cc b/test/integration/filters/pause_filter_for_quic.cc new file mode 100644 index 000000000000..90f95e3f337e --- /dev/null +++ b/test/integration/filters/pause_filter_for_quic.cc @@ -0,0 +1,91 @@ +#include + +#include "envoy/registry/registry.h" + +#include "extensions/filters/http/common/pass_through_filter.h" +#include "extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h" + +#include "test/extensions/filters/http/common/empty_http_filter_config.h" + +namespace Envoy { + +// This filter exists to synthetically test network backup by faking TCP +// connection back-up when an encode is finished, and unblocking it when the +// next stream starts to decode headers. +// Allows regression tests for https://github.com/envoyproxy/envoy/issues/4541 +class TestPauseFilterForQuic : public Http::PassThroughFilter { +public: + // Pass in a some global filter state to ensure the Network::Connection is + // blocked and unblocked exactly once. + TestPauseFilterForQuic(absl::Mutex& encode_lock, uint32_t& number_of_encode_calls_ref, + uint32_t& number_of_decode_calls_ref) + : encode_lock_(encode_lock), number_of_encode_calls_ref_(number_of_encode_calls_ref), + number_of_decode_calls_ref_(number_of_decode_calls_ref) {} + + Http::FilterDataStatus decodeData(Buffer::Instance& buf, bool end_stream) override { + if (end_stream) { + absl::WriterMutexLock m(&encode_lock_); + number_of_decode_calls_ref_++; + // If this is the second stream to decode headers and we're at high watermark. force low + // watermark state + if (number_of_decode_calls_ref_ == 2 && + decoder_callbacks_->connection()->aboveHighWatermark()) { + auto quic_connection = const_cast( + dynamic_cast( + decoder_callbacks_->connection())); + quic_connection->write_buffer_watermark_simulation_.checkHighWatermark( + quic_connection->write_buffer_watermark_simulation_.highWatermark() + 1u); + } + } + return PassThroughFilter::decodeData(buf, end_stream); + } + + Http::FilterDataStatus encodeData(Buffer::Instance& buf, bool end_stream) override { + if (end_stream) { + absl::WriterMutexLock m(&encode_lock_); + number_of_encode_calls_ref_++; + // If this is the first stream to encode headers and we're not at high watermark, force high + // watermark state. + if (number_of_encode_calls_ref_ == 1 && + !decoder_callbacks_->connection()->aboveHighWatermark()) { + auto quic_connection = const_cast( + dynamic_cast( + decoder_callbacks_->connection())); + quic_connection->write_buffer_watermark_simulation_.checkHighWatermark( + quic_connection->write_buffer_watermark_simulation_.highWatermark() + 1u); + } + } + return PassThroughFilter::encodeData(buf, end_stream); + } + + absl::Mutex& encode_lock_; + uint32_t& number_of_encode_calls_ref_; + uint32_t& number_of_decode_calls_ref_; +}; + +class TestPauseFilterConfigForQuic : public Extensions::HttpFilters::Common::EmptyHttpFilterConfig { +public: + TestPauseFilterConfigForQuic() : EmptyHttpFilterConfig("pause-filter-for-quic") {} + + Http::FilterFactoryCb createFilter(const std::string&, + Server::Configuration::FactoryContext&) override { + return [&](Http::FilterChainFactoryCallbacks& callbacks) -> void { + // ABSL_GUARDED_BY insists the lock be held when the guarded variables are passed by + // reference. + absl::WriterMutexLock m(&encode_lock_); + callbacks.addStreamFilter(std::make_shared<::Envoy::TestPauseFilterForQuic>( + encode_lock_, number_of_encode_calls_, number_of_decode_calls_)); + }; + } + + absl::Mutex encode_lock_; + uint32_t number_of_encode_calls_ ABSL_GUARDED_BY(encode_lock_) = 0; + uint32_t number_of_decode_calls_ ABSL_GUARDED_BY(encode_lock_) = 0; +}; + +// perform static registration +static Registry::RegisterFactory + register_; + +} // namespace Envoy From d5cbd0097e093ddc30c3bc8dcb00f34a30c72b72 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Sat, 13 Mar 2021 00:22:10 -0500 Subject: [PATCH 06/21] address comments Signed-off-by: Dan Zhang --- source/common/http/utility.h | 1 + source/extensions/quic_listeners/quiche/BUILD | 2 +- .../quiche/active_quic_listener.cc | 2 +- .../quiche/client_connection_factory_impl.cc | 2 +- .../quic_listeners/quiche/envoy_quic_utils.cc | 6 +- test/integration/BUILD | 104 ++++++++---------- test/integration/ssl_utility.cc | 25 +++-- test/integration/ssl_utility.h | 12 +- test/integration/utility.cc | 22 +--- 9 files changed, 78 insertions(+), 98 deletions(-) diff --git a/source/common/http/utility.h b/source/common/http/utility.h index 526fb701a4fd..7d2a065bcb37 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -39,6 +39,7 @@ class AlpnNameValues { const std::string Http11 = "http/1.1"; const std::string Http2 = "h2"; const std::string Http2c = "h2c"; + const std::string Http3 = "h3"; }; using AlpnNames = ConstSingleton; diff --git a/source/extensions/quic_listeners/quiche/BUILD b/source/extensions/quic_listeners/quiche/BUILD index 850c7c2c0b5d..488c58548936 100644 --- a/source/extensions/quic_listeners/quiche/BUILD +++ b/source/extensions/quic_listeners/quiche/BUILD @@ -386,10 +386,10 @@ envoy_cc_extension( # Needed to verify that a quic specific configuration is used for quic transport socket. extra_visibility = [ "//source/server:__subpackages__", + "//test:__subpackages__", ], security_posture = "unknown", tags = ["nofips"], - visibility = ["//test:__subpackages__"], deps = [ "//include/envoy/network:transport_socket_interface", "//include/envoy/server:transport_socket_config_interface", diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index 78cd127ec9be..cbab473f6373 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -39,7 +39,7 @@ ActiveQuicListener::ActiveQuicListener( &listener_config), dispatcher_(dispatcher), version_manager_(quic::CurrentSupportedVersions()), kernel_worker_routing_(kernel_worker_routing) { - // This flag fix a QUICHE issue which may crashe Envoy during connection close. + // This flag fix a QUICHE issue which may crash Envoy during connection close. SetQuicReloadableFlag(quic_single_ack_in_packet2, true); if (Runtime::LoaderSingleton::getExisting()) { diff --git a/source/extensions/quic_listeners/quiche/client_connection_factory_impl.cc b/source/extensions/quic_listeners/quiche/client_connection_factory_impl.cc index 4396e68dd044..5a9a6154f02a 100644 --- a/source/extensions/quic_listeners/quiche/client_connection_factory_impl.cc +++ b/source/extensions/quic_listeners/quiche/client_connection_factory_impl.cc @@ -11,7 +11,7 @@ QuicClientConnectionFactoryImpl::createQuicNetworkConnection( Network::Address::InstanceConstSharedPtr local_addr, Network::TransportSocketFactory& transport_socket_factory, Stats::Scope& stats_scope, Event::Dispatcher& dispatcher, TimeSource& time_source) { - // This flag fix a QUICHE issue which may crashe Envoy during connection close. + // This flag fix a QUICHE issue which may crash Envoy during connection close. SetQuicReloadableFlag(quic_single_ack_in_packet2, true); // TODO(#14829): reject config if anything but QuicClientTransportSocketConfigFactory configured. diff --git a/source/extensions/quic_listeners/quiche/envoy_quic_utils.cc b/source/extensions/quic_listeners/quiche/envoy_quic_utils.cc index f0f658eb5585..0533311978b4 100644 --- a/source/extensions/quic_listeners/quiche/envoy_quic_utils.cc +++ b/source/extensions/quic_listeners/quiche/envoy_quic_utils.cc @@ -120,11 +120,7 @@ createConnectionSocket(Network::Address::InstanceConstSharedPtr& peer_addr, Network::Address::InstanceConstSharedPtr& local_addr, const Network::ConnectionSocket::OptionsSharedPtr& options) { if (local_addr == nullptr) { - if (peer_addr->ip()->ipv4() != nullptr) { - local_addr = Network::Utility::getCanonicalIpv4LoopbackAddress(); - } else { - local_addr = Network::Utility::getIpv6LoopbackAddress(); - } + local_addr = Network::Utility::getLocalAddress(peer_addr->ip()->version()); } auto connection_socket = std::make_unique( Network::Socket::Type::Datagram, local_addr, peer_addr); diff --git a/test/integration/BUILD b/test/integration/BUILD index 5be22277c135..afa7113d934c 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -619,39 +619,6 @@ envoy_cc_test_library( ], ) -envoy_cc_test_library( - name = "utility_lib", - srcs = [ - "utility.cc", - ], - hdrs = [ - "utility.h", - ], - deps = [ - "//include/envoy/api:api_interface", - "//include/envoy/http:codec_interface", - "//include/envoy/http:header_map_interface", - "//include/envoy/network:filter_interface", - "//source/common/api:api_lib", - "//source/common/common:assert_lib", - "//source/common/common:utility_lib", - "//source/common/http:codec_client_lib", - "//source/common/http/http3:quic_client_connection_factory_lib", - "//source/common/stats:isolated_store_lib", - "//source/extensions/quic_listeners/quiche:quic_transport_socket_factory_lib", - "//test/common/upstream:utility_lib", - "//test/mocks/event:event_mocks", - "//test/mocks/server:transport_socket_factory_context_mocks", - "//test/mocks/upstream:cluster_info_mocks", - "//test/test_common:environment_lib", - "//test/test_common:network_utility_lib", - "//test/test_common:printers_lib", - "//test/test_common:simulated_time_system_lib", - "//test/test_common:test_time_lib", - "@envoy_api//envoy/extensions/transport_sockets/quic/v3:pkg_cc_proto", - ], -) - envoy_cc_test_library( name = "integration_tcp_client_lib", srcs = [ @@ -731,7 +698,6 @@ envoy_cc_test_library( ":autonomous_upstream_lib", ":fake_upstream_lib", ":integration_tcp_client_lib", - ":server_lib", ":utility_lib", "//source/common/config:api_version_lib", "//source/common/config:version_converter_lib", @@ -757,26 +723,57 @@ envoy_cc_test_library( ) envoy_cc_test_library( - name = "server_lib", + name = "autonomous_upstream_lib", + srcs = [ + "autonomous_upstream.cc", + ], + hdrs = [ + "autonomous_upstream.h", + ], + deps = [ + ":fake_upstream_lib", + ], +) + +envoy_cc_test_library( + name = "utility_lib", srcs = [ "server.cc", + "ssl_utility.cc", + "utility.cc", ], hdrs = [ "server.h", + "ssl_utility.h", + "utility.h", ], + data = ["//test/common/runtime:filesystem_test_data"], deps = [ ":server_stats_interface", ":tcp_dump", - ":utility_lib", + "//include/envoy/api:api_interface", + "//include/envoy/http:codec_interface", + "//include/envoy/http:header_map_interface", + "//include/envoy/network:filter_interface", "//include/envoy/server:options_interface", "//include/envoy/server:process_context_interface", "//include/envoy/stats:stats_interface", + "//source/common/api:api_lib", "//source/common/common:assert_lib", "//source/common/common:lock_guard_lib", "//source/common/common:logger_lib", "//source/common/common:thread_lib", + "//source/common/common:utility_lib", + "//source/common/http:codec_client_lib", + "//source/common/http/http3:quic_client_connection_factory_lib", + "//source/common/json:json_loader_lib", + "//source/common/network:utility_lib", "//source/common/stats:allocator_lib", + "//source/common/stats:isolated_store_lib", "//source/common/thread_local:thread_local_lib", + "//source/extensions/quic_listeners/quiche:quic_transport_socket_factory_lib", + "//source/extensions/transport_sockets/tls:config", + "//source/extensions/transport_sockets/tls:context_lib", "//source/server:drain_manager_lib", "//source/server:hot_restart_nop_lib", "//source/server:listener_hooks_lib", @@ -784,46 +781,39 @@ envoy_cc_test_library( "//source/server:process_context_lib", "//source/server:server_lib", "//test/common/runtime:utility_lib", + "//test/common/upstream:utility_lib", + "//test/config:utility_lib", "//test/mocks:common_lib", + "//test/mocks/event:event_mocks", "//test/mocks/runtime:runtime_mocks", + "//test/mocks/server:transport_socket_factory_context_mocks", + "//test/mocks/upstream:cluster_info_mocks", "//test/test_common:environment_lib", + "//test/test_common:network_utility_lib", + "//test/test_common:printers_lib", + "//test/test_common:simulated_time_system_lib", + "//test/test_common:test_time_lib", "//test/test_common:test_time_system_interface", "//test/test_common:utility_lib", "@com_google_absl//absl/synchronization", "@envoy_api//envoy/config/listener/v3:pkg_cc_proto", - ], -) - -envoy_cc_test_library( - name = "autonomous_upstream_lib", - srcs = [ - "autonomous_upstream.cc", - ], - hdrs = [ - "autonomous_upstream.h", - ], - deps = [ - ":fake_upstream_lib", + "@envoy_api//envoy/extensions/transport_sockets/quic/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/transport_sockets/tls/v3:pkg_cc_proto", ], ) envoy_cc_test_library( name = "integration_lib", - srcs = [ - "ssl_utility.cc", - ], hdrs = [ "integration.h", - "ssl_utility.h", ], - data = ["//test/common/runtime:filesystem_test_data"], deps = [ ":autonomous_upstream_lib", ":base_integration_test_lib", ":fake_upstream_lib", ":integration_stream_decoder_lib", ":integration_tcp_client_lib", - ":server_lib", + ":utility_lib", "//include/envoy/api:api_interface", "//include/envoy/buffer:buffer_interface", "//include/envoy/event:dispatcher_interface", @@ -866,8 +856,6 @@ envoy_cc_test_library( "//source/extensions/access_loggers/file:config", "//source/extensions/transport_sockets/raw_buffer:config", "//source/extensions/transport_sockets/tap:config", - "//source/extensions/transport_sockets/tls:config", - "//source/extensions/transport_sockets/tls:context_lib", "//source/server:connection_handler_lib", "//source/server:drain_manager_lib", "//source/server:hot_restart_nop_lib", @@ -879,7 +867,6 @@ envoy_cc_test_library( "//test/common/upstream:utility_lib", "//test/config:utility_lib", "//test/mocks/buffer:buffer_mocks", - "//test/mocks/server:transport_socket_factory_context_mocks", "//test/mocks/stats:stats_mocks", "//test/mocks/upstream:retry_priority_factory_mocks", "//test/mocks/upstream:retry_priority_mocks", @@ -890,7 +877,6 @@ envoy_cc_test_library( "//test/test_common:test_time_lib", "//test/test_common:test_time_system_interface", "//test/test_common:utility_lib", - "@envoy_api//envoy/extensions/transport_sockets/tls/v3:pkg_cc_proto", ], ) diff --git a/test/integration/ssl_utility.cc b/test/integration/ssl_utility.cc index aab3dd5d4adf..3141c1c0826f 100644 --- a/test/integration/ssl_utility.cc +++ b/test/integration/ssl_utility.cc @@ -1,7 +1,5 @@ #include "test/integration/ssl_utility.h" -#include "envoy/extensions/transport_sockets/tls/v3/cert.pb.h" - #include "common/http/utility.h" #include "common/json/json_loader.h" #include "common/network/utility.h" @@ -23,9 +21,9 @@ using testing::ReturnRef; namespace Envoy { namespace Ssl { -Network::TransportSocketFactoryPtr -createClientSslTransportSocketFactory(const ClientSslTransportOptions& options, - ContextManager& context_manager, Api::Api& api) { +void initializeUpstreamTlsContextConfig( + const ClientSslTransportOptions& options, + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext& tls_context) { std::string yaml_plain = R"EOF( common_tls_context: validation_context: @@ -50,17 +48,19 @@ createClientSslTransportSocketFactory(const ClientSslTransportOptions& options, )EOF"; } - envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml_plain), tls_context); auto* common_context = tls_context.mutable_common_tls_context(); if (options.alpn_) { common_context->add_alpn_protocols(Http::Utility::AlpnNames::get().Http2); common_context->add_alpn_protocols(Http::Utility::AlpnNames::get().Http11); + common_context->add_alpn_protocols(Http::Utility::AlpnNames::get().Http3); } - if (options.san_) { - common_context->mutable_validation_context() - ->add_hidden_envoy_deprecated_verify_subject_alt_name("spiffe://lyft.com/backend-team"); + if (!options.san_.empty()) { + // common_context->mutable_validation_context() + // ->add_hidden_envoy_deprecated_verify_subject_alt_name(options.san_); + common_context->mutable_validation_context()->add_match_subject_alt_names()->set_exact( + options.san_); } for (const std::string& cipher_suite : options.cipher_suites_) { common_context->mutable_tls_params()->add_cipher_suites(cipher_suite); @@ -71,6 +71,13 @@ createClientSslTransportSocketFactory(const ClientSslTransportOptions& options, common_context->mutable_tls_params()->set_tls_minimum_protocol_version(options.tls_version_); common_context->mutable_tls_params()->set_tls_maximum_protocol_version(options.tls_version_); +} + +Network::TransportSocketFactoryPtr +createClientSslTransportSocketFactory(ClientSslTransportOptions& options, + ContextManager& context_manager, Api::Api& api) { + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; + initializeUpstreamTlsContextConfig(options.setSan("spiffe://lyft.com/backend-team"), tls_context); NiceMock mock_factory_ctx; ON_CALL(mock_factory_ctx, api()).WillByDefault(ReturnRef(api)); diff --git a/test/integration/ssl_utility.h b/test/integration/ssl_utility.h index afaf57eae00c..e908e90661e2 100644 --- a/test/integration/ssl_utility.h +++ b/test/integration/ssl_utility.h @@ -16,8 +16,8 @@ struct ClientSslTransportOptions { return *this; } - ClientSslTransportOptions& setSan(bool san) { - san_ = san; + ClientSslTransportOptions& setSan(absl::string_view san) { + san_ = std::string(san); return *this; } @@ -48,7 +48,7 @@ struct ClientSslTransportOptions { } bool alpn_{}; - bool san_{}; + std::string san_; bool client_ecdsa_cert_{}; std::vector cipher_suites_{}; std::string sigalgs_; @@ -57,8 +57,12 @@ struct ClientSslTransportOptions { envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLS_AUTO}; }; +void initializeUpstreamTlsContextConfig( + const ClientSslTransportOptions& options, + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext& tls_context); + Network::TransportSocketFactoryPtr -createClientSslTransportSocketFactory(const ClientSslTransportOptions& options, +createClientSslTransportSocketFactory(ClientSslTransportOptions& options, ContextManager& context_manager, Api::Api& api); Network::TransportSocketFactoryPtr createUpstreamSslContext(ContextManager& context_manager, diff --git a/test/integration/utility.cc b/test/integration/utility.cc index d6b3bf5dd7a1..ed83df1520f6 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -22,6 +22,7 @@ #include "common/upstream/upstream_impl.h" #include "test/common/upstream/utility.h" +#include "test/integration/ssl_utility.h" #include "test/mocks/common.h" #include "test/mocks/server/transport_socket_factory_context.h" #include "test/mocks/stats/mocks.h" @@ -112,27 +113,12 @@ struct ConnectionCallbacks : public Network::ConnectionCallbacks { Network::TransportSocketFactoryPtr IntegrationUtil::createQuicClientTransportSocketFactory( Server::Configuration::TransportSocketFactoryContext& context, const std::string& san_to_match) { - std::string yaml_plain = R"EOF( - common_tls_context: - validation_context: - trusted_ca: - filename: "{{ test_rundir }}/test/config/integration/certs/cacert.pem" -)EOF"; envoy::extensions::transport_sockets::quic::v3::QuicUpstreamTransport quic_transport_socket_config; auto* tls_context = quic_transport_socket_config.mutable_upstream_tls_context(); - TestUtility::loadFromYaml(TestEnvironment::substitute(yaml_plain), *tls_context); - auto* common_context = tls_context->mutable_common_tls_context(); - - common_context->add_alpn_protocols("h3"); - common_context->mutable_validation_context()->add_match_subject_alt_names()->set_exact( - san_to_match); - tls_context->set_sni("lyft.com"); - - common_context->mutable_tls_params()->set_tls_minimum_protocol_version( - envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLS_AUTO); - common_context->mutable_tls_params()->set_tls_maximum_protocol_version( - envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLS_AUTO); + initializeUpstreamTlsContextConfig( + Ssl::ClientSslTransportOptions().setAlpn(true).setSan(san_to_match).setSni("lyft.com"), + *tls_context); envoy::config::core::v3::TransportSocket message; message.mutable_typed_config()->PackFrom(quic_transport_socket_config); From cf083113b19f4531fbf65a3866591553331bad69 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Mon, 15 Mar 2021 14:02:50 -0400 Subject: [PATCH 07/21] fix const reference Signed-off-by: Dan Zhang --- test/integration/ssl_utility.cc | 8 ++++++-- test/integration/ssl_utility.h | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/test/integration/ssl_utility.cc b/test/integration/ssl_utility.cc index 3141c1c0826f..1f684a0608e5 100644 --- a/test/integration/ssl_utility.cc +++ b/test/integration/ssl_utility.cc @@ -74,10 +74,14 @@ void initializeUpstreamTlsContextConfig( } Network::TransportSocketFactoryPtr -createClientSslTransportSocketFactory(ClientSslTransportOptions& options, +createClientSslTransportSocketFactory(const ClientSslTransportOptions& options, ContextManager& context_manager, Api::Api& api) { +ClientSslTransportOptions options_with_san = options; + if (options.san_.empty()) { + options_with_san.setSan("spiffe://lyft.com/backend-team"); + } envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; - initializeUpstreamTlsContextConfig(options.setSan("spiffe://lyft.com/backend-team"), tls_context); + initializeUpstreamTlsContextConfig(options_with_san, tls_context); NiceMock mock_factory_ctx; ON_CALL(mock_factory_ctx, api()).WillByDefault(ReturnRef(api)); diff --git a/test/integration/ssl_utility.h b/test/integration/ssl_utility.h index e908e90661e2..5c4904080478 100644 --- a/test/integration/ssl_utility.h +++ b/test/integration/ssl_utility.h @@ -62,7 +62,7 @@ void initializeUpstreamTlsContextConfig( envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext& tls_context); Network::TransportSocketFactoryPtr -createClientSslTransportSocketFactory(ClientSslTransportOptions& options, +createClientSslTransportSocketFactory(const ClientSslTransportOptions& options, ContextManager& context_manager, Api::Api& api); Network::TransportSocketFactoryPtr createUpstreamSslContext(ContextManager& context_manager, From 56827916386e7312bbedfc9aaebb611ddff08a2f Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Tue, 16 Mar 2021 17:26:37 -0400 Subject: [PATCH 08/21] fix in filters Signed-off-by: Dan Zhang --- test/integration/filters/add_body_filter.cc | 12 +++++++++++- test/integration/http_integration.cc | 1 + test/integration/protocol_integration_test.cc | 18 ++++++------------ 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/test/integration/filters/add_body_filter.cc b/test/integration/filters/add_body_filter.cc index 18a11de9bd3f..00378476b2be 100644 --- a/test/integration/filters/add_body_filter.cc +++ b/test/integration/filters/add_body_filter.cc @@ -31,11 +31,21 @@ class AddBodyStreamFilter : public Http::PassThroughFilter { return Http::FilterHeadersStatus::Continue; } + Http::FilterDataStatus decodeData(Buffer::Instance& data, bool end_stream) override { + // Ensure that decodeData 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 + // decodeData() with an empty body. + if (end_stream && decoder_callbacks_->connection()->streamInfo().protocol() && data.length() == 0u) { + data.add("body"); + } + return Http::FilterDataStatus::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()); + ASSERT(!end_stream || decoder_callbacks_->connection()->streamInfo().protocol()); data.add("body"); return Http::FilterDataStatus::Continue; } diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 58a0d28b52b2..aa164ba44f8c 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -321,6 +321,7 @@ void HttpIntegrationTest::initialize() { *tls_context->mutable_common_tls_context()); for (auto& listener : *bootstrap.mutable_static_resources()->mutable_listeners()) { if (listener.udp_listener_config().udp_listener_name() == "quiche_quic_listener") { + // if (listener.udp_listener_config().listener_config().typed_config().type_url() == "type.googleapis.com/envoy.config.listener.v3.QuicProtocolOptions") { auto* filter_chain = listener.mutable_filter_chains(0); auto* transport_socket = filter_chain->mutable_transport_socket(); transport_socket->mutable_typed_config()->PackFrom(quic_transport_socket_config); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index d8a9907af63d..f46e589120d0 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -193,10 +193,6 @@ name: health_check // Verifies behavior for https://github.com/envoyproxy/envoy/pull/11248 TEST_P(ProtocolIntegrationTest, AddBodyToRequestAndWaitForIt) { - // QUICHE can't guarantee headers and FIN to be delivered together, so - // headers-only request can't be detected at L7 filters. - EXCLUDE_DOWNSTREAM_HTTP3; - // filters are prepended, so add them in reverse order config_helper_.addFilter(R"EOF( name: wait-for-whole-request-and-response-filter @@ -1114,6 +1110,7 @@ TEST_P(ProtocolIntegrationTest, HeadersWithUnderscoresDropped) { stat_name = "http2.dropped_headers_with_underscores"; break; case Http::CodecClient::Type::HTTP3: + // TODO(danzh) add stats for H3. break; default: RELEASE_ASSERT(false, fmt::format("Unknown downstream protocol {}", downstream_protocol_)); @@ -1362,6 +1359,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingMany) { } TEST_P(DownstreamProtocolIntegrationTest, InvalidContentLength) { + // TODO(danzh) Add content length validation. EXCLUDE_DOWNSTREAM_HTTP3 initialize(); @@ -1428,6 +1426,7 @@ TEST_P(DownstreamProtocolIntegrationTest, InvalidContentLengthAllowed) { } TEST_P(DownstreamProtocolIntegrationTest, MultipleContentLengths) { + // TODO(danzh) Add content length validation. EXCLUDE_DOWNSTREAM_HTTP3 initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -1462,7 +1461,6 @@ TEST_P(DownstreamProtocolIntegrationTest, MultipleContentLengthsAllowed) { }); initialize(); - codec_client_ = makeHttpConnection(lookupPort("http")); auto encoder_decoder = codec_client_->startRequest(Http::TestRequestHeaderMapImpl{{":method", "POST"}, @@ -1512,7 +1510,6 @@ name: local-reply-during-encode } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlRejected) { - EXCLUDE_DOWNSTREAM_HTTP3 // Send one 95 kB URL with limit 60 kB headers. testLargeRequestUrl(95, 60); } @@ -1525,7 +1522,6 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlAccepted) { } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersRejected) { - EXCLUDE_DOWNSTREAM_HTTP3 // Send one 95 kB header with limit 60 kB and 100 headers. testLargeRequestHeaders(95, 1, 60, 100); } @@ -1731,8 +1727,7 @@ TEST_P(ProtocolIntegrationTest, LargeRequestMethod) { // Tests StopAllIterationAndBuffer. Verifies decode-headers-return-stop-all-filter calls decodeData // once after iteration is resumed. TEST_P(DownstreamProtocolIntegrationTest, TestDecodeHeadersReturnsStopAll) { - // Enable after setting QUICHE stream initial flow control window from http2 - // options. + // Enable after setting QUICHE stream initial flow control window from http3 options. EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addFilter(R"EOF( name: call-decodedata-once-filter @@ -1785,6 +1780,7 @@ name: passthrough-filter // Tests StopAllIterationAndWatermark. decode-headers-return-stop-all-watermark-filter sets buffer // limit to 100. Verifies data pause when limit is reached, and resume after iteration continues. TEST_P(DownstreamProtocolIntegrationTest, TestDecodeHeadersReturnsStopAllWatermark) { + // TODO(danzh) Re-enable after codec buffer can be set according to http3 options. EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addFilter(R"EOF( name: decode-headers-return-stop-all-filter @@ -1844,8 +1840,7 @@ name: passthrough-filter // Test two filters that return StopAllIterationAndBuffer back-to-back. TEST_P(DownstreamProtocolIntegrationTest, TestTwoFiltersDecodeHeadersReturnsStopAll) { - // TODO(danzh) Re-enable after codec buffer can be set according to http2 - // options. + // TODO(danzh) Re-enable after codec buffer can be set according to http3 options. EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addFilter(R"EOF( name: decode-headers-return-stop-all-filter @@ -2172,7 +2167,6 @@ TEST_P(DownstreamProtocolIntegrationTest, BasicMaxStreamTimeoutLegacy) { // Make sure that invalid authority headers get blocked at or before the HCM. TEST_P(DownstreamProtocolIntegrationTest, InvalidAuthority) { - EXCLUDE_DOWNSTREAM_HTTP3 initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); From 725d93e2345f2ec253dd277309bb4f81ad3ebd2b Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Tue, 16 Mar 2021 17:44:12 -0400 Subject: [PATCH 09/21] stop using udp_listener_name Signed-off-by: Dan Zhang --- test/integration/filters/add_body_filter.cc | 5 +++-- test/integration/http_integration.cc | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/test/integration/filters/add_body_filter.cc b/test/integration/filters/add_body_filter.cc index 00378476b2be..51faeb1e7b19 100644 --- a/test/integration/filters/add_body_filter.cc +++ b/test/integration/filters/add_body_filter.cc @@ -35,8 +35,9 @@ class AddBodyStreamFilter : public Http::PassThroughFilter { // Ensure that decodeData 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 // decodeData() with an empty body. - if (end_stream && decoder_callbacks_->connection()->streamInfo().protocol() && data.length() == 0u) { - data.add("body"); + if (end_stream && decoder_callbacks_->connection()->streamInfo().protocol() && + data.length() == 0u) { + data.add("body"); } return Http::FilterDataStatus::Continue; } diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 8e0a3e5fbc16..162623e649c8 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -320,8 +320,8 @@ void HttpIntegrationTest::initialize() { ConfigHelper::initializeTls(ConfigHelper::ServerSslOptions().setRsaCert(true).setTlsV13(true), *tls_context->mutable_common_tls_context()); for (auto& listener : *bootstrap.mutable_static_resources()->mutable_listeners()) { - if (listener.udp_listener_config().udp_listener_name() == "quiche_quic_listener") { - // if (listener.udp_listener_config().listener_config().typed_config().type_url() == "type.googleapis.com/envoy.config.listener.v3.QuicProtocolOptions") { + if (listener.udp_listener_config().listener_config().typed_config().type_url() == + "type.googleapis.com/envoy.config.listener.v3.QuicProtocolOptions") { auto* filter_chain = listener.mutable_filter_chains(0); auto* transport_socket = filter_chain->mutable_transport_socket(); transport_socket->mutable_typed_config()->PackFrom(quic_transport_socket_config); From 354a32244695c90d53f7b8f30ef3b8046da5f021 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Tue, 16 Mar 2021 20:32:37 -0400 Subject: [PATCH 10/21] fix IPv6 test failure Signed-off-by: Dan Zhang --- test/integration/protocol_integration_test.cc | 1 + test/integration/utility.cc | 13 +++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index db3e00c983bd..e5ae9db75003 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -1309,6 +1309,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) { EXCLUDE_DOWNSTREAM_HTTP3 EXCLUDE_UPSTREAM_HTTP3; initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); Http::TestRequestHeaderMapImpl request_headers{{":method", "POST"}, {":path", "/test/long/url"}, diff --git a/test/integration/utility.cc b/test/integration/utility.cc index 4d5307f95bfb..98a214635f42 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -18,6 +18,7 @@ #include "common/http/headers.h" #include "common/http/http3/quic_client_connection_factory.h" #include "common/http/http3/well_known_names.h" +#include "common/network/address_impl.h" #include "common/network/utility.h" #include "common/upstream/upstream_impl.h" @@ -158,8 +159,16 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt Http::QuicCodecNames::get().Quiche); persistent_info = connection_factory.createNetworkConnectionInfo( *dispatcher, *transport_socket_factory, mock_stats_store, time_system, addr); - connection = connection_factory.createQuicNetworkConnection( - *persistent_info, *dispatcher, addr, Network::Address::InstanceConstSharedPtr()); + + Network::Address::InstanceConstSharedPtr local_address; + if (addr->ip()->version() == Network::Address::IpVersion::v4) { + local_address = Network::Utility::getLocalAddress(Network::Address::IpVersion::v4); + } else { + // Docker only works with loopback v6 address. + local_address = std::make_shared("::1"); + } + connection = connection_factory.createQuicNetworkConnection(*persistent_info, *dispatcher, addr, + local_address); connection->addConnectionCallbacks(connection_callbacks); } else { connection = From 2b9462601842bee3b53a79c32af2f04d9a7b748c Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Tue, 16 Mar 2021 22:51:34 -0400 Subject: [PATCH 11/21] fix setSan Signed-off-by: Dan Zhang --- .../transport_sockets/tls/integration/ssl_integration_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc b/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc index 940d480899ae..4b2de9460e15 100644 --- a/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc +++ b/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc @@ -121,7 +121,7 @@ TEST_P(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferHttp2) { TEST_P(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferVerifySAN) { ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr { - return makeSslClientConnection(ClientSslTransportOptions().setSan(true)); + return makeSslClientConnection(ClientSslTransportOptions()); }; testRouterRequestAndResponseWithBody(1024, 512, false, false, &creator); checkStats(); @@ -130,7 +130,7 @@ TEST_P(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferVerifySAN) { TEST_P(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferHttp2VerifySAN) { setDownstreamProtocol(Http::CodecClient::Type::HTTP2); ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr { - return makeSslClientConnection(ClientSslTransportOptions().setAlpn(true).setSan(true)); + return makeSslClientConnection(ClientSslTransportOptions().setAlpn(true)); }; testRouterRequestAndResponseWithBody(1024, 512, false, false, &creator); checkStats(); From 77cb93746b78960dbe0e20d56b599b0d6de3c174 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Wed, 17 Mar 2021 12:45:49 -0400 Subject: [PATCH 12/21] fix asan and gcc Signed-off-by: Dan Zhang --- .../integration/quic_http_integration_test.cc | 15 +++++---------- test/integration/http_integration.h | 6 ++++-- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc b/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc index f884cef33c5e..904bd4c12ef3 100644 --- a/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc +++ b/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc @@ -26,6 +26,7 @@ #pragma GCC diagnostic pop #endif +#include "extensions/quic_listeners/quiche/client_connection_factory_impl.h" #include "extensions/quic_listeners/quiche/envoy_quic_client_session.h" #include "extensions/quic_listeners/quiche/envoy_quic_client_connection.h" #include "extensions/quic_listeners/quiche/envoy_quic_proof_verifier.h" @@ -98,12 +99,11 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVers getNextConnectionId(), server_addr_, conn_helper_, alarm_factory_, quic::ParsedQuicVersionVector{supported_versions_[0]}, local_addr, *dispatcher_, nullptr); quic_connection_ = connection.get(); - ASSERT(quic_transport_socket_factory_ != nullptr); + ASSERT(quic_connection_persistent_info_ != nullptr); + auto& persistent_info = static_cast(*quic_connection_persistent_info_); auto session = std::make_unique( - quic_config_, supported_versions_, std::move(connection), - quic::QuicServerId{transport_socket_factory_->clientContextConfig().serverNameIndication(), - static_cast(server_addr_->ip()->port()), false}, - crypto_config_.get(), &push_promise_index_, *dispatcher_, 0); + quic_config_, supported_versions_, std::move(connection), persistent_info.server_id_, + persistent_info.crypto_config_.get(), &push_promise_index_, *dispatcher_, 0); session->Initialize(); return session; } @@ -202,10 +202,6 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVers registerTestServerPorts({"http"}); ASSERT(&transport_socket_factory_->clientContextConfig()); - - crypto_config_ = - std::make_unique(std::make_unique( - stats_store_, transport_socket_factory_->clientContextConfig(), timeSystem())); } void testMultipleQuicConnections() { @@ -256,7 +252,6 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVers quic::QuicConfig quic_config_; quic::QuicClientPushPromiseIndex push_promise_index_; quic::ParsedQuicVersionVector supported_versions_; - std::unique_ptr crypto_config_; EnvoyQuicConnectionHelper conn_helper_; EnvoyQuicAlarmFactory alarm_factory_; CodecClientCallbacksForTest client_codec_callback_; diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h index 77c5a7ebd201..5ec5bb2e5dc0 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -249,6 +249,10 @@ class HttpIntegrationTest : public BaseIntegrationTest { // Prefix listener stat with IP:port, including IP version dependent loopback address. std::string listenerStatPrefix(const std::string& stat_name); + Network::TransportSocketFactoryPtr quic_transport_socket_factory_; + // Must outlive |codec_client_| because it may not close connection till the end of its life + // scope. + std::unique_ptr quic_connection_persistent_info_; // The client making requests to Envoy. IntegrationCodecClientPtr codec_client_; // A placeholder for the first upstream connection. @@ -268,8 +272,6 @@ class HttpIntegrationTest : public BaseIntegrationTest { bool set_reuse_port_{false}; std::string san_to_match_{"spiffe://lyft.com/backend-team"}; - std::unique_ptr quic_connection_persistent_info_; - Network::TransportSocketFactoryPtr quic_transport_socket_factory_; }; // Helper class for integration tests using raw HTTP/2 frames From 4e9e56ac9b7810705a362c24a07483694b4743a7 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Wed, 17 Mar 2021 17:28:12 -0400 Subject: [PATCH 13/21] test size to large Signed-off-by: Dan Zhang --- test/extensions/quic_listeners/quiche/integration/BUILD | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/integration/BUILD b/test/extensions/quic_listeners/quiche/integration/BUILD index 2cbb38cf9273..c3cab4a52f76 100644 --- a/test/extensions/quic_listeners/quiche/integration/BUILD +++ b/test/extensions/quic_listeners/quiche/integration/BUILD @@ -10,12 +10,12 @@ envoy_package() envoy_cc_test( name = "quic_protocol_integration_test", - size = "medium", + size = "large", srcs = [ "quic_protocol_integration_test.cc", ], data = ["//test/config/integration/certs"], - shard_count = 6, + shard_count = 8, tags = [ "fails_on_clang_cl", "fails_on_windows", From 88398b8c6d959ad0a930d8d9e2f73560e1c7426a Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Wed, 17 Mar 2021 20:51:22 -0400 Subject: [PATCH 14/21] test size to large Signed-off-by: Dan Zhang --- test/extensions/quic_listeners/quiche/integration/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/quic_listeners/quiche/integration/BUILD b/test/extensions/quic_listeners/quiche/integration/BUILD index c3cab4a52f76..824db7a326ea 100644 --- a/test/extensions/quic_listeners/quiche/integration/BUILD +++ b/test/extensions/quic_listeners/quiche/integration/BUILD @@ -33,7 +33,7 @@ envoy_cc_test( envoy_cc_test( name = "quic_http_integration_test", - size = "medium", + size = "large", srcs = ["quic_http_integration_test.cc"], data = ["//test/config/integration/certs"], # TODO(envoyproxy/windows-dev): Diagnose failure shown only on clang-cl build, see: From 5c1172342758438a00524b45be0bb1b1cc7d10eb Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Thu, 18 Mar 2021 12:57:54 -0400 Subject: [PATCH 15/21] move config setup to config helper Signed-off-by: Dan Zhang --- test/config/utility.cc | 22 +++++++++++++++- test/config/utility.h | 7 +++-- .../integration/quic_http_integration_test.cc | 12 --------- .../quic_protocol_integration_test.cc | 5 ++-- test/integration/http_integration.cc | 26 +------------------ test/integration/http_protocol_integration.h | 8 +++--- 6 files changed, 33 insertions(+), 47 deletions(-) diff --git a/test/config/utility.cc b/test/config/utility.cc index 2ce73d362eb1..b04182c8f284 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -145,7 +145,10 @@ name: "envoy.filters.listener.tls_inspector" )EOF"; } -std::string ConfigHelper::httpProxyConfig() { +std::string ConfigHelper::httpProxyConfig(bool downstream_use_quic) { + if (downstream_use_quic) { + return quicHttpProxyConfig(); + } return absl::StrCat(baseConfig(), fmt::format(R"EOF( filter_chains: filters: @@ -1050,6 +1053,23 @@ void ConfigHelper::addSslConfig(const ServerSslOptions& options) { filter_chain->mutable_transport_socket()->mutable_typed_config()->PackFrom(tls_context); } +void ConfigHelper::addQuicDownstreamTransportSocketConfig(bool reuse_port) { + envoy::extensions::transport_sockets::quic::v3::QuicDownstreamTransport + quic_transport_socket_config; + auto tls_context = quic_transport_socket_config.mutable_downstream_tls_context(); + ConfigHelper::initializeTls(ConfigHelper::ServerSslOptions().setRsaCert(true).setTlsV13(true), + *tls_context->mutable_common_tls_context()); + for (auto& listener : *bootstrap_.mutable_static_resources()->mutable_listeners()) { + if (listener.udp_listener_config().listener_config().typed_config().type_url() == + "type.googleapis.com/envoy.config.listener.v3.QuicProtocolOptions") { + auto* filter_chain = listener.mutable_filter_chains(0); + auto* transport_socket = filter_chain->mutable_transport_socket(); + transport_socket->mutable_typed_config()->PackFrom(quic_transport_socket_config); + listener.set_reuse_port(reuse_port); + } + } +} + bool ConfigHelper::setAccessLog( const std::string& filename, absl::string_view format, std::vector formatters) { diff --git a/test/config/utility.h b/test/config/utility.h index 1a748f913b3a..73efd034e87e 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -90,7 +90,7 @@ class ConfigHelper { // By default, this runs with an L7 proxy config, but config can be set to TCP_PROXY_CONFIG // to test L4 proxying. ConfigHelper(const Network::Address::IpVersion version, Api::Api& api, - const std::string& config = httpProxyConfig()); + const std::string& config = httpProxyConfig(false)); static void initializeTls(const ServerSslOptions& options, @@ -111,7 +111,7 @@ class ConfigHelper { // A basic configuration for L4 proxying. static std::string tcpProxyConfig(); // A basic configuration for L7 proxying. - static std::string httpProxyConfig(); + static std::string httpProxyConfig(bool downstream_use_quic = false); // A basic configuration for L7 proxying with QUIC transport. static std::string quicHttpProxyConfig(); // A string for a basic buffer filter, which can be used with addFilter() @@ -217,6 +217,9 @@ class ConfigHelper { void addSslConfig(const ServerSslOptions& options); void addSslConfig() { addSslConfig({}); } + // Add the default SSL configuration for QUIC downstream. + void addQuicDownstreamTransportSocketConfig(bool resuse_port); + // Set the HTTP access log for the first HCM (if present) to a given file. The default is // the platform's null device. bool setAccessLog(const std::string& filename, absl::string_view format = "", diff --git a/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc b/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc index 904bd4c12ef3..7d3b7f70b5aa 100644 --- a/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc +++ b/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc @@ -137,18 +137,6 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVers void initialize() override { config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { - envoy::extensions::transport_sockets::quic::v3::QuicDownstreamTransport - quic_transport_socket_config; - auto tls_context = quic_transport_socket_config.mutable_downstream_tls_context(); - ConfigHelper::initializeTls(ConfigHelper::ServerSslOptions().setRsaCert(true).setTlsV13(true), - *tls_context->mutable_common_tls_context()); - auto* filter_chain = - bootstrap.mutable_static_resources()->mutable_listeners(0)->mutable_filter_chains(0); - auto* transport_socket = filter_chain->mutable_transport_socket(); - transport_socket->mutable_typed_config()->PackFrom(quic_transport_socket_config); - - bootstrap.mutable_static_resources()->mutable_listeners(0)->set_reuse_port(set_reuse_port_); - const std::string overload_config = fmt::format(R"EOF( refresh_interval: 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 af4940d989fa..4d94b6783cad 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,27 +2,26 @@ namespace Envoy { -// This will run with HTTP/3 downstream, and HTTP/2 upstream. +// These will run with HTTP/3 downstream, and Http and HTTP/2 upstream. INSTANTIATE_TEST_SUITE_P(Protocols, DownstreamProtocolIntegrationTest, testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( {Http::CodecClient::Type::HTTP3}, {FakeHttpConnection::Type::HTTP1, FakeHttpConnection::Type::HTTP2})), HttpProtocolIntegrationTest::protocolTestParamsToString); -// This will run with HTTP/3 downstream, and HTTP/2 upstream. INSTANTIATE_TEST_SUITE_P(DownstreamProtocols, ProtocolIntegrationTest, testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( {Http::CodecClient::Type::HTTP3}, {FakeHttpConnection::Type::HTTP1, FakeHttpConnection::Type::HTTP2})), HttpProtocolIntegrationTest::protocolTestParamsToString); +// These will run with HTTP/1 and HTTP/2 downstream, and HTTP/3 upstream. INSTANTIATE_TEST_SUITE_P(UpstreamProtocols, DownstreamProtocolIntegrationTest, testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( {Http::CodecClient::Type::HTTP1, 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(UpstreamProtocols, ProtocolIntegrationTest, testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( {Http::CodecClient::Type::HTTP1, Http::CodecClient::Type::HTTP2}, diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 162623e649c8..92bc8bcf5cfd 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -313,32 +313,8 @@ void HttpIntegrationTest::initialize() { quic_transport_socket_factory_ = IntegrationUtil::createQuicClientTransportSocketFactory(mock_factory_ctx, san_to_match_); - config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { - envoy::extensions::transport_sockets::quic::v3::QuicDownstreamTransport - quic_transport_socket_config; - auto tls_context = quic_transport_socket_config.mutable_downstream_tls_context(); - ConfigHelper::initializeTls(ConfigHelper::ServerSslOptions().setRsaCert(true).setTlsV13(true), - *tls_context->mutable_common_tls_context()); - for (auto& listener : *bootstrap.mutable_static_resources()->mutable_listeners()) { - if (listener.udp_listener_config().listener_config().typed_config().type_url() == - "type.googleapis.com/envoy.config.listener.v3.QuicProtocolOptions") { - auto* filter_chain = listener.mutable_filter_chains(0); - auto* transport_socket = filter_chain->mutable_transport_socket(); - transport_socket->mutable_typed_config()->PackFrom(quic_transport_socket_config); - - listener.set_reuse_port(set_reuse_port_); - } - } - }); + config_helper_.addQuicDownstreamTransportSocketConfig(set_reuse_port_); - config_helper_.addConfigModifier( - [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& - hcm) { - hcm.mutable_drain_timeout()->clear_seconds(); - hcm.mutable_drain_timeout()->set_nanos(500 * 1000 * 1000); - EXPECT_EQ(hcm.codec_type(), envoy::extensions::filters::network::http_connection_manager:: - v3::HttpConnectionManager::HTTP3); - }); BaseIntegrationTest::initialize(); registerTestServerPorts({"http"}); diff --git a/test/integration/http_protocol_integration.h b/test/integration/http_protocol_integration.h index f65b15455116..05a91eed1bb1 100644 --- a/test/integration/http_protocol_integration.h +++ b/test/integration/http_protocol_integration.h @@ -49,10 +49,10 @@ class HttpProtocolIntegrationTest : public testing::TestWithParam& p); HttpProtocolIntegrationTest() - : HttpIntegrationTest(GetParam().downstream_protocol, GetParam().version, - GetParam().downstream_protocol == Http::CodecClient::Type::HTTP3 - ? ConfigHelper::quicHttpProxyConfig() - : ConfigHelper::httpProxyConfig()) {} + : HttpIntegrationTest( + GetParam().downstream_protocol, GetParam().version, + ConfigHelper::httpProxyConfig(/*downstream_is_quic=*/GetParam().downstream_protocol == + Http::CodecClient::Type::HTTP3)) {} void SetUp() override { setDownstreamProtocol(GetParam().downstream_protocol); From 4c2b490f14ac5c5a3c0888d4e6fa6dd6b64e105b Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Fri, 19 Mar 2021 15:49:44 -0400 Subject: [PATCH 16/21] fix setSan Signed-off-by: Dan Zhang --- .../tls/integration/ssl_integration_test.cc | 4 ++-- test/integration/integration_admin_test.h | 2 +- test/integration/ssl_utility.cc | 6 +----- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc b/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc index 4b2de9460e15..b7a1fe7526a2 100644 --- a/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc +++ b/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc @@ -121,7 +121,7 @@ TEST_P(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferHttp2) { TEST_P(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferVerifySAN) { ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr { - return makeSslClientConnection(ClientSslTransportOptions()); + return makeSslClientConnection(ClientSslTransportOptions().setSan(san_to_match_)); }; testRouterRequestAndResponseWithBody(1024, 512, false, false, &creator); checkStats(); @@ -130,7 +130,7 @@ TEST_P(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferVerifySAN) { TEST_P(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferHttp2VerifySAN) { setDownstreamProtocol(Http::CodecClient::Type::HTTP2); ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr { - return makeSslClientConnection(ClientSslTransportOptions().setAlpn(true)); + return makeSslClientConnection(ClientSslTransportOptions().setAlpn(true).setSan(san_to_match_)); }; testRouterRequestAndResponseWithBody(1024, 512, false, false, &creator); checkStats(); diff --git a/test/integration/integration_admin_test.h b/test/integration/integration_admin_test.h index a63649e7ed71..487ac3b3c63e 100644 --- a/test/integration/integration_admin_test.h +++ b/test/integration/integration_admin_test.h @@ -76,7 +76,7 @@ class IntegrationAdminTest : public HttpProtocolIntegrationTest { } // Validate that the stats JSON has expected histograms element. - EXPECT_EQ(expected_hist_count, histogram_count); + EXPECT_EQ(expected_hist_count, histogram_count) << stats_json; } }; diff --git a/test/integration/ssl_utility.cc b/test/integration/ssl_utility.cc index 40fe484fd7e8..0d86dcc7137e 100644 --- a/test/integration/ssl_utility.cc +++ b/test/integration/ssl_utility.cc @@ -76,12 +76,8 @@ void initializeUpstreamTlsContextConfig( Network::TransportSocketFactoryPtr createClientSslTransportSocketFactory(const ClientSslTransportOptions& options, ContextManager& context_manager, Api::Api& api) { - ClientSslTransportOptions options_with_san = options; - if (options.san_.empty()) { - options_with_san.setSan("spiffe://lyft.com/backend-team"); - } envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; - initializeUpstreamTlsContextConfig(options_with_san, tls_context); + initializeUpstreamTlsContextConfig(options, tls_context); NiceMock mock_factory_ctx; ON_CALL(mock_factory_ctx, api()).WillByDefault(ReturnRef(api)); From 97292b268702e6a8bb69f29a6363db748198fd26 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Sun, 21 Mar 2021 23:49:58 -0400 Subject: [PATCH 17/21] change send single request Signed-off-by: Dan Zhang --- test/integration/http_integration.cc | 2 +- test/integration/utility.cc | 106 +++++++++++++++------------ test/integration/utility.h | 4 +- 3 files changed, 61 insertions(+), 51 deletions(-) diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 92bc8bcf5cfd..edb612f9b962 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -311,7 +311,7 @@ void HttpIntegrationTest::initialize() { ON_CALL(mock_factory_ctx, api()).WillByDefault(testing::ReturnRef(*api_)); quic_transport_socket_factory_ = - IntegrationUtil::createQuicClientTransportSocketFactory(mock_factory_ctx, san_to_match_); + IntegrationUtil::createQuicUpstreamTransportSocketFactory(mock_factory_ctx, san_to_match_); config_helper_.addQuicDownstreamTransportSocketConfig(set_reuse_port_); diff --git a/test/integration/utility.cc b/test/integration/utility.cc index 98a214635f42..996d24b95cf1 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -111,7 +111,7 @@ struct ConnectionCallbacks : public Network::ConnectionCallbacks { bool connected_{false}; }; -Network::TransportSocketFactoryPtr IntegrationUtil::createQuicClientTransportSocketFactory( +Network::TransportSocketFactoryPtr IntegrationUtil::createQuicUpstreamTransportSocketFactory( Server::Configuration::TransportSocketFactoryContext& context, const std::string& san_to_match) { envoy::extensions::transport_sockets::quic::v3::QuicUpstreamTransport @@ -128,6 +128,37 @@ Network::TransportSocketFactoryPtr IntegrationUtil::createQuicClientTransportSoc return config_factory.createTransportSocketFactory(quic_transport_socket_config, context); } +BufferingStreamDecoderPtr +sendRequestAndWaitForResponse(Event::Dispatcher& dispatcher, const std::string& method, + const std::string& url, const std::string& body, + const std::string& host, const std::string& content_type, + Http::CodecClientProd& client) { + BufferingStreamDecoderPtr response(new BufferingStreamDecoder([&]() -> void { + client.close(); + dispatcher.exit(); + })); + Http::RequestEncoder& encoder = client.newStream(*response); + encoder.getStream().addCallbacks(*response); + + Http::TestRequestHeaderMapImpl headers; + headers.setMethod(method); + headers.setPath(url); + headers.setHost(host); + headers.setReferenceScheme(Http::Headers::get().SchemeValues.Http); + if (!content_type.empty()) { + headers.setContentType(content_type); + } + const auto status = encoder.encodeHeaders(headers, body.empty()); + ASSERT(status.ok()); + if (!body.empty()) { + Buffer::OwnedImpl body_buffer(body); + encoder.encodeData(body_buffer, true); + } + + dispatcher.run(Event::Dispatcher::RunType::Block); + return response; +} + BufferingStreamDecoderPtr IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPtr& addr, const std::string& method, const std::string& url, @@ -147,64 +178,43 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt fmt::format("{}://127.0.0.1:80", (type == Http::CodecClient::Type::HTTP3 ? "udp" : "tcp")), time_system)}; + if (type <= Http::CodecClient::Type::HTTP2) { + Http::CodecClientProd client( + type, + dispatcher->createClientConnection(addr, Network::Address::InstanceConstSharedPtr(), + Network::Test::createRawBufferSocket(), nullptr), + host_description, *dispatcher, random); + return sendRequestAndWaitForResponse(*dispatcher, method, url, body, host, content_type, + client); + } + NiceMock mock_factory_ctx; ON_CALL(mock_factory_ctx, api()).WillByDefault(testing::ReturnRef(api)); Network::TransportSocketFactoryPtr transport_socket_factory = - createQuicClientTransportSocketFactory(mock_factory_ctx, "spiffe://lyft.com/backend-team"); + createQuicUpstreamTransportSocketFactory(mock_factory_ctx, "spiffe://lyft.com/backend-team"); std::unique_ptr persistent_info; - Network::ClientConnectionPtr connection; - if (type == Http::CodecClient::Type::HTTP3) { - Http::QuicClientConnectionFactory& connection_factory = - Config::Utility::getAndCheckFactoryByName( - Http::QuicCodecNames::get().Quiche); - persistent_info = connection_factory.createNetworkConnectionInfo( - *dispatcher, *transport_socket_factory, mock_stats_store, time_system, addr); - - Network::Address::InstanceConstSharedPtr local_address; - if (addr->ip()->version() == Network::Address::IpVersion::v4) { - local_address = Network::Utility::getLocalAddress(Network::Address::IpVersion::v4); - } else { - // Docker only works with loopback v6 address. - local_address = std::make_shared("::1"); - } - connection = connection_factory.createQuicNetworkConnection(*persistent_info, *dispatcher, addr, - local_address); - connection->addConnectionCallbacks(connection_callbacks); + Http::QuicClientConnectionFactory& connection_factory = + Config::Utility::getAndCheckFactoryByName( + Http::QuicCodecNames::get().Quiche); + persistent_info = connection_factory.createNetworkConnectionInfo( + *dispatcher, *transport_socket_factory, mock_stats_store, time_system, addr); + + Network::Address::InstanceConstSharedPtr local_address; + if (addr->ip()->version() == Network::Address::IpVersion::v4) { + local_address = Network::Utility::getLocalAddress(Network::Address::IpVersion::v4); } else { - connection = - dispatcher->createClientConnection(addr, Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + // Docker only works with loopback v6 address. + local_address = std::make_shared("::1"); } + Network::ClientConnectionPtr connection = connection_factory.createQuicNetworkConnection( + *persistent_info, *dispatcher, addr, local_address); + connection->addConnectionCallbacks(connection_callbacks); Http::CodecClientProd client(type, std::move(connection), host_description, *dispatcher, random); if (type == Http::CodecClient::Type::HTTP3) { // Quic connection needs to finish handshake. dispatcher->run(Event::Dispatcher::RunType::Block); } - - BufferingStreamDecoderPtr response(new BufferingStreamDecoder([&]() -> void { - client.close(); - dispatcher->exit(); - })); - Http::RequestEncoder& encoder = client.newStream(*response); - encoder.getStream().addCallbacks(*response); - - Http::TestRequestHeaderMapImpl headers; - headers.setMethod(method); - headers.setPath(url); - headers.setHost(host); - headers.setReferenceScheme(Http::Headers::get().SchemeValues.Http); - if (!content_type.empty()) { - headers.setContentType(content_type); - } - const auto status = encoder.encodeHeaders(headers, body.empty()); - ASSERT(status.ok()); - if (!body.empty()) { - Buffer::OwnedImpl body_buffer(body); - encoder.encodeData(body_buffer, true); - } - - dispatcher->run(Event::Dispatcher::RunType::Block); - return response; + return sendRequestAndWaitForResponse(*dispatcher, method, url, body, host, content_type, client); } BufferingStreamDecoderPtr diff --git a/test/integration/utility.h b/test/integration/utility.h index 3c82b61f7e61..9590685c8b0a 100644 --- a/test/integration/utility.h +++ b/test/integration/utility.h @@ -191,13 +191,13 @@ class IntegrationUtil { const std::string& content_type = ""); /** - * Create transport socket factory for Quic client transport socket. + * Create transport socket factory for Quic upstream transport socket. * @param context supplies the port to connect to on localhost. * @param san_to_match configs |context| to match Subject Alternative Name during certificate * verification. * @return TransportSocketFactoryPtr the client transport socket factory. */ - static Network::TransportSocketFactoryPtr createQuicClientTransportSocketFactory( + static Network::TransportSocketFactoryPtr createQuicUpstreamTransportSocketFactory( Server::Configuration::TransportSocketFactoryContext& context, const std::string& san_to_match); }; From 280fc25b4a78ad2fa41ff395bb03d41ceb4439de Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Mon, 22 Mar 2021 16:12:23 -0400 Subject: [PATCH 18/21] address comments Signed-off-by: Dan Zhang --- .../quiche/platform/quic_logging_impl.h | 2 -- test/config/utility.cc | 1 + .../quiche/envoy_quic_utils_test.cc | 3 ++ .../integration/quic_http_integration_test.cc | 5 --- test/integration/http_integration.cc | 13 +++++--- test/integration/protocol_integration_test.cc | 12 ++++++- test/integration/ssl_utility.cc | 2 -- test/integration/utility.cc | 31 +++++++++++-------- test/integration/utility.h | 5 ++- 9 files changed, 44 insertions(+), 30 deletions(-) diff --git a/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h b/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h index 905cc62bbf25..939226420b06 100644 --- a/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h +++ b/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h @@ -127,7 +127,6 @@ namespace quic { using QuicLogLevel = spdlog::level::level_enum; static const QuicLogLevel TRACE = spdlog::level::trace; -static const QuicLogLevel DEBUG = spdlog::level::debug; static const QuicLogLevel INFO = spdlog::level::info; static const QuicLogLevel WARNING = spdlog::level::warn; static const QuicLogLevel ERROR = spdlog::level::err; @@ -184,7 +183,6 @@ inline spdlog::logger& GetLogger() { #define QUICHE_IS_LOG_LEVEL_ENABLED_IMPL(severity) \ inline bool isLogLevelEnabled##severity() { return quic::severity >= GetLogger().level(); } QUICHE_IS_LOG_LEVEL_ENABLED_IMPL(TRACE) -QUICHE_IS_LOG_LEVEL_ENABLED_IMPL(DEBUG) QUICHE_IS_LOG_LEVEL_ENABLED_IMPL(INFO) QUICHE_IS_LOG_LEVEL_ENABLED_IMPL(WARNING) QUICHE_IS_LOG_LEVEL_ENABLED_IMPL(ERROR) diff --git a/test/config/utility.cc b/test/config/utility.cc index e1c8ef324f68..9f25cc985e08 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -1062,6 +1062,7 @@ void ConfigHelper::addQuicDownstreamTransportSocketConfig(bool reuse_port) { for (auto& listener : *bootstrap_.mutable_static_resources()->mutable_listeners()) { if (listener.udp_listener_config().listener_config().typed_config().type_url() == "type.googleapis.com/envoy.config.listener.v3.QuicProtocolOptions") { + ASSERT(listener.filter_chains_size() > 0); auto* filter_chain = listener.mutable_filter_chains(0); auto* transport_socket = filter_chain->mutable_transport_socket(); transport_socket->mutable_typed_config()->PackFrom(quic_transport_socket_config); diff --git a/test/extensions/quic_listeners/quiche/envoy_quic_utils_test.cc b/test/extensions/quic_listeners/quiche/envoy_quic_utils_test.cc index d8f5af4c4005..dade0cc4c340 100644 --- a/test/extensions/quic_listeners/quiche/envoy_quic_utils_test.cc +++ b/test/extensions/quic_listeners/quiche/envoy_quic_utils_test.cc @@ -50,9 +50,12 @@ TEST(EnvoyQuicUtilsTest, HeadersConversion) { headers_block[":authority"] = "www.google.com"; headers_block[":path"] = "/index.hml"; headers_block[":scheme"] = "https"; + // "value1" and "value2" should be coalesced into one header by QUICHE and splitted again while + // converting to Envoy headers.. headers_block.AppendValueOrAddHeader("key", "value1"); headers_block.AppendValueOrAddHeader("key", "value2"); auto envoy_headers = spdyHeaderBlockToEnvoyHeaders(headers_block); + // Envoy header block is 1 header larger because QUICHE header block does coalescing. EXPECT_EQ(headers_block.size() + 1u, envoy_headers->size()); EXPECT_EQ("www.google.com", envoy_headers->getHostValue()); EXPECT_EQ("/index.hml", envoy_headers->getPathValue()); diff --git a/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc b/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc index 7d3b7f70b5aa..8b0ba7e96639 100644 --- a/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc +++ b/test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc @@ -108,11 +108,6 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVers return session; } - // This call may fail because of INVALID_VERSION, because QUIC connection doesn't support - // in-connection version negotiation. - // TODO(#8479) Propagate INVALID_VERSION error to caller and let caller to use server advertised - // version list to create a new connection with mutually supported version and make client codec - // again. IntegrationCodecClientPtr makeRawHttpConnection( Network::ClientConnectionPtr&& conn, absl::optional http2_options) override { diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index edb612f9b962..25a9bf65b7b5 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -258,6 +258,9 @@ IntegrationCodecClientPtr HttpIntegrationTest::makeRawHttpConnection( host_description, downstream_protocol_); if (downstream_protocol_ == Http::CodecClient::Type::HTTP3 && codec->disconnected()) { // Connection may get closed during version negotiation or handshake. + // TODO(#8479) QUIC connection doesn't support in-connection version negotiationPropagate + // INVALID_VERSION error to caller and let caller to use server advertised version list to + // create a new connection with mutually supported version and make client codec again. ENVOY_LOG(error, "Fail to connect to server with error: {}", codec->connection()->transportFailureReason()); } @@ -307,12 +310,13 @@ void HttpIntegrationTest::initialize() { if (downstream_protocol_ != Http::CodecClient::Type::HTTP3) { return BaseIntegrationTest::initialize(); } - NiceMock mock_factory_ctx; - ON_CALL(mock_factory_ctx, api()).WillByDefault(testing::ReturnRef(*api_)); - + // Needs to be instantiated before base class calls initialize() which starts a QUIC listener + // according to the config. quic_transport_socket_factory_ = - IntegrationUtil::createQuicUpstreamTransportSocketFactory(mock_factory_ctx, san_to_match_); + IntegrationUtil::createQuicUpstreamTransportSocketFactory(*api_, san_to_match_); + // Needed to config QUIC transport socket factory, and needs to be added before base class calls + // initialize(). config_helper_.addQuicDownstreamTransportSocketConfig(set_reuse_port_); BaseIntegrationTest::initialize(); @@ -320,6 +324,7 @@ void HttpIntegrationTest::initialize() { Network::Address::InstanceConstSharedPtr server_addr = Network::Utility::resolveUrl(fmt::format( "udp://{}:{}", Network::Test::getLoopbackAddressUrlString(version_), lookupPort("http"))); + // Needs to outlive all QUIC connections. quic_connection_persistent_info_ = Config::Utility::getAndCheckFactoryByName( Http::QuicCodecNames::get().Quiche) diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 3e88379c7550..4b8055873826 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -59,6 +59,7 @@ void setDoNotValidateRouteConfig( return; \ } +// TODO(#2557) fix all the failures. #define EXCLUDE_DOWNSTREAM_HTTP3 \ if (downstreamProtocol() == Http::CodecClient::Type::HTTP3) { \ return; \ @@ -746,6 +747,7 @@ TEST_P(DownstreamProtocolIntegrationTest, RetryAttemptCountHeader) { TEST_P(DownstreamProtocolIntegrationTest, RetryPriority) { if (upstreamProtocol() == FakeHttpConnection::Type::HTTP2 && downstreamProtocol() == Http::CodecClient::Type::HTTP3) { + // TODO(alyssawilk) investigate why this combination doesn't work. return; } EXCLUDE_UPSTREAM_HTTP3; @@ -1144,6 +1146,7 @@ TEST_P(ProtocolIntegrationTest, HeadersWithUnderscoresRemainByDefault) { // Verify that request with headers containing underscores is rejected when configured. TEST_P(DownstreamProtocolIntegrationTest, HeadersWithUnderscoresCauseRequestRejectedByDefault) { + // TODO(danzh) pass headers_with_underscores_action config into QUIC stream. EXCLUDE_DOWNSTREAM_HTTP3 useAccessLog("%RESPONSE_FLAGS% %RESPONSE_CODE_DETAILS%"); config_helper_.addConfigModifier( @@ -1306,6 +1309,7 @@ TEST_P(ProtocolIntegrationTest, MissingStatus) { // Validate that lots of tiny cookies doesn't cause a DoS (single cookie header). TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) { + // TODO(danzh) re-enable this test after quic headers size become configurable. EXCLUDE_DOWNSTREAM_HTTP3 EXCLUDE_UPSTREAM_HTTP3; initialize(); @@ -1330,6 +1334,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) { // Validate that lots of tiny cookies doesn't cause a DoS (many cookie headers). TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingMany) { + // TODO(danzh) re-enable this test after quic headers size become configurable. EXCLUDE_DOWNSTREAM_HTTP3 EXCLUDE_UPSTREAM_HTTP3; // Set header count limit to 2010. @@ -1542,6 +1547,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlRejected) { } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlAccepted) { + // TODO(danzh) re-enable this test after quic headers size become configurable. EXCLUDE_DOWNSTREAM_HTTP3 EXCLUDE_UPSTREAM_HTTP3; // Send one 95 kB URL with limit 96 kB headers. @@ -1636,6 +1642,7 @@ 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) { + // TODO(danzh) re-enable this test after quic headers size become configurable. EXCLUDE_DOWNSTREAM_HTTP3 EXCLUDE_UPSTREAM_HTTP3; // Set timeout for 5 seconds, and ensure that a request with 10k+ headers can be sent. @@ -1643,6 +1650,7 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersTimeout) { } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersAccepted) { + // TODO(danzh) re-enable this test after quic headers size become configurable. EXCLUDE_DOWNSTREAM_HTTP3 EXCLUDE_UPSTREAM_HTTP3; config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); @@ -1650,6 +1658,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersAccepted) { } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersRejected) { + // TODO(danzh) investigate why it failed for H3. EXCLUDE_DOWNSTREAM_HTTP3 EXCLUDE_UPSTREAM_HTTP3; config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); @@ -1754,7 +1763,7 @@ TEST_P(ProtocolIntegrationTest, LargeRequestMethod) { // Tests StopAllIterationAndBuffer. Verifies decode-headers-return-stop-all-filter calls decodeData // once after iteration is resumed. TEST_P(DownstreamProtocolIntegrationTest, TestDecodeHeadersReturnsStopAll) { - // Enable after setting QUICHE stream initial flow control window from http3 options. + // TODO(danzh) Enable after setting QUICHE stream initial flow control window from http3 options. EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addFilter(R"EOF( name: call-decodedata-once-filter @@ -1961,6 +1970,7 @@ name: encode-headers-return-stop-all-filter // Tests encodeHeaders() returns StopAllIterationAndWatermark. TEST_P(DownstreamProtocolIntegrationTest, TestEncodeHeadersReturnsStopAllWatermark) { + // TODO(danzh) Re-enable after codec buffer can be set according to http3 options. EXCLUDE_DOWNSTREAM_HTTP3 config_helper_.addFilter(R"EOF( name: encode-headers-return-stop-all-filter diff --git a/test/integration/ssl_utility.cc b/test/integration/ssl_utility.cc index c46a226695fe..0afb4b6d80fb 100644 --- a/test/integration/ssl_utility.cc +++ b/test/integration/ssl_utility.cc @@ -65,8 +65,6 @@ void initializeUpstreamTlsContextConfig( common_context->add_alpn_protocols(Http::Utility::AlpnNames::get().Http3); } if (!options.san_.empty()) { - // common_context->mutable_validation_context() - // ->add_hidden_envoy_deprecated_verify_subject_alt_name(options.san_); common_context->mutable_validation_context()->add_match_subject_alt_names()->set_exact( options.san_); } diff --git a/test/integration/utility.cc b/test/integration/utility.cc index 996d24b95cf1..4cb675a1f4db 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -83,15 +83,21 @@ void BufferingStreamDecoder::onResetStream(Http::StreamResetReason, absl::string ADD_FAILURE(); } -struct ConnectionCallbacks : public Network::ConnectionCallbacks { - ConnectionCallbacks(Event::Dispatcher& dispatcher) : dispatcher_(dispatcher) {} +// A callback for a QUIC client connection to unblock the test after handshake succeeds. QUIC +// network connection initiates handshake and raises Connected event when it's done. Tests should +// proceed with sending requests afterwards. +class TestConnectionCallbacks : public Network::ConnectionCallbacks { +public: + TestConnectionCallbacks(Event::Dispatcher& dispatcher) : dispatcher_(dispatcher) {} // Network::ConnectionCallbacks void onEvent(Network::ConnectionEvent event) override { if (event == Network::ConnectionEvent::Connected) { + // Handshake finished, unblock the test to continue. connected_ = true; dispatcher_.exit(); } else if (event == Network::ConnectionEvent::RemoteClose) { + // If the peer closes the connection, no need to wait anymore. dispatcher_.exit(); } else { if (!connected_) { @@ -107,13 +113,16 @@ struct ConnectionCallbacks : public Network::ConnectionCallbacks { void onAboveWriteBufferHighWatermark() override {} void onBelowWriteBufferLowWatermark() override {} +private: Event::Dispatcher& dispatcher_; bool connected_{false}; }; -Network::TransportSocketFactoryPtr IntegrationUtil::createQuicUpstreamTransportSocketFactory( - Server::Configuration::TransportSocketFactoryContext& context, - const std::string& san_to_match) { +Network::TransportSocketFactoryPtr +IntegrationUtil::createQuicUpstreamTransportSocketFactory(Api::Api& api, + const std::string& san_to_match) { + NiceMock context; + ON_CALL(context, api()).WillByDefault(testing::ReturnRef(api)); envoy::extensions::transport_sockets::quic::v3::QuicUpstreamTransport quic_transport_socket_config; auto* tls_context = quic_transport_socket_config.mutable_upstream_tls_context(); @@ -171,7 +180,7 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt Api::Impl api(Thread::threadFactoryForTest(), mock_stats_store, time_system, Filesystem::fileSystemForTest(), random_generator); Event::DispatcherPtr dispatcher(api.allocateDispatcher("test_thread")); - ConnectionCallbacks connection_callbacks(*dispatcher); + TestConnectionCallbacks connection_callbacks(*dispatcher); std::shared_ptr cluster{new NiceMock()}; Upstream::HostDescriptionConstSharedPtr host_description{Upstream::makeTestHostDescription( cluster, @@ -188,10 +197,8 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt client); } - NiceMock mock_factory_ctx; - ON_CALL(mock_factory_ctx, api()).WillByDefault(testing::ReturnRef(api)); Network::TransportSocketFactoryPtr transport_socket_factory = - createQuicUpstreamTransportSocketFactory(mock_factory_ctx, "spiffe://lyft.com/backend-team"); + createQuicUpstreamTransportSocketFactory(api, "spiffe://lyft.com/backend-team"); std::unique_ptr persistent_info; Http::QuicClientConnectionFactory& connection_factory = Config::Utility::getAndCheckFactoryByName( @@ -210,10 +217,8 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt *persistent_info, *dispatcher, addr, local_address); connection->addConnectionCallbacks(connection_callbacks); Http::CodecClientProd client(type, std::move(connection), host_description, *dispatcher, random); - if (type == Http::CodecClient::Type::HTTP3) { - // Quic connection needs to finish handshake. - dispatcher->run(Event::Dispatcher::RunType::Block); - } + // Quic connection needs to finish handshake. + dispatcher->run(Event::Dispatcher::RunType::Block); return sendRequestAndWaitForResponse(*dispatcher, method, url, body, host, content_type, client); } diff --git a/test/integration/utility.h b/test/integration/utility.h index 9590685c8b0a..d05a222821ba 100644 --- a/test/integration/utility.h +++ b/test/integration/utility.h @@ -197,9 +197,8 @@ class IntegrationUtil { * verification. * @return TransportSocketFactoryPtr the client transport socket factory. */ - static Network::TransportSocketFactoryPtr createQuicUpstreamTransportSocketFactory( - Server::Configuration::TransportSocketFactoryContext& context, - const std::string& san_to_match); + static Network::TransportSocketFactoryPtr + createQuicUpstreamTransportSocketFactory(Api::Api& api, const std::string& san_to_match); }; // A set of connection callbacks which tracks connection state. From 3c029786121ee6fedc87f8da39e9f4ae47a10d29 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Mon, 22 Mar 2021 18:33:08 -0400 Subject: [PATCH 19/21] fix comment Signed-off-by: Dan Zhang --- .../extensions/quic_listeners/quiche/envoy_quic_utils_test.cc | 2 +- test/integration/utility.cc | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/envoy_quic_utils_test.cc b/test/extensions/quic_listeners/quiche/envoy_quic_utils_test.cc index dade0cc4c340..4ac8561f39c0 100644 --- a/test/extensions/quic_listeners/quiche/envoy_quic_utils_test.cc +++ b/test/extensions/quic_listeners/quiche/envoy_quic_utils_test.cc @@ -50,7 +50,7 @@ TEST(EnvoyQuicUtilsTest, HeadersConversion) { headers_block[":authority"] = "www.google.com"; headers_block[":path"] = "/index.hml"; headers_block[":scheme"] = "https"; - // "value1" and "value2" should be coalesced into one header by QUICHE and splitted again while + // "value1" and "value2" should be coalesced into one header by QUICHE and split again while // converting to Envoy headers.. headers_block.AppendValueOrAddHeader("key", "value1"); headers_block.AppendValueOrAddHeader("key", "value2"); diff --git a/test/integration/utility.cc b/test/integration/utility.cc index 4cb675a1f4db..d461516e038b 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -93,7 +93,9 @@ class TestConnectionCallbacks : public Network::ConnectionCallbacks { // Network::ConnectionCallbacks void onEvent(Network::ConnectionEvent event) override { if (event == Network::ConnectionEvent::Connected) { - // Handshake finished, unblock the test to continue. + // Handshake finished, unblock the test to continue. This is needed because we call + // Dispatcher::run() with Block to wait for the handshake to finish before proceeding. + // TODO(danzh) find an alternative approach with behaviors more in parallel with SSL. connected_ = true; dispatcher_.exit(); } else if (event == Network::ConnectionEvent::RemoteClose) { From c31225b4fc7d0293efd9e94762c9abe76b9a78e4 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Tue, 23 Mar 2021 14:55:30 -0400 Subject: [PATCH 20/21] comment about \0 Signed-off-by: Dan Zhang --- source/extensions/quic_listeners/quiche/envoy_quic_utils.h | 1 + 1 file changed, 1 insertion(+) diff --git a/source/extensions/quic_listeners/quiche/envoy_quic_utils.h b/source/extensions/quic_listeners/quiche/envoy_quic_utils.h index ef68543df62d..9d59e6b0e4e1 100644 --- a/source/extensions/quic_listeners/quiche/envoy_quic_utils.h +++ b/source/extensions/quic_listeners/quiche/envoy_quic_utils.h @@ -61,6 +61,7 @@ std::unique_ptr spdyHeaderBlockToEnvoyHeaders(const spdy::SpdyHeaderBlock& he for (auto entry : header_block) { // TODO(danzh): Avoid temporary strings and addCopy() with string_view. std::string key(entry.first); + // QUICHE coalesces mulitple trailer values with the same key with '\0'. std::vector values = absl::StrSplit(entry.second, '\0'); for (const absl::string_view& value : values) { headers->addCopy(Http::LowerCaseString(key), value); From 2dd1ae5e3dae50441b23421d424c6924c95bb2ad Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Tue, 23 Mar 2021 18:01:12 -0400 Subject: [PATCH 21/21] fix typo Signed-off-by: Dan Zhang --- source/extensions/quic_listeners/quiche/envoy_quic_utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/quic_listeners/quiche/envoy_quic_utils.h b/source/extensions/quic_listeners/quiche/envoy_quic_utils.h index 9d59e6b0e4e1..209b1b0c21ae 100644 --- a/source/extensions/quic_listeners/quiche/envoy_quic_utils.h +++ b/source/extensions/quic_listeners/quiche/envoy_quic_utils.h @@ -61,7 +61,7 @@ std::unique_ptr spdyHeaderBlockToEnvoyHeaders(const spdy::SpdyHeaderBlock& he for (auto entry : header_block) { // TODO(danzh): Avoid temporary strings and addCopy() with string_view. std::string key(entry.first); - // QUICHE coalesces mulitple trailer values with the same key with '\0'. + // QUICHE coalesces multiple trailer values with the same key with '\0'. std::vector values = absl::StrSplit(entry.second, '\0'); for (const absl::string_view& value : values) { headers->addCopy(Http::LowerCaseString(key), value);