diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 467bf5a8406d..8cd2b65409aa 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -68,6 +68,11 @@ minor_behavior_changes: ``envoy.reloadable_features.lua_respond_with_send_local_reply`` to false. bug_fixes: +- area: http + change: | + Fixed HTTP/2 CONNECT to be RFC compliant, rather than following the abandoned extended connect draft. + This behavioral change can be reverted by setting runtime guard ``envoy.reloadable_features.use_rfc_connect`` to false. + - area: runtime change: | Fixed a bug where was ``envoy.restart_features.no_runtime_singleton`` was inverted. diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index b2860cbc3002..439601d36426 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -300,9 +300,15 @@ Status ConnectionImpl::ClientStreamImpl::encodeHeaders(const RequestHeaderMap& h // configurable if necessary. // https://tools.ietf.org/html/draft-kinnear-httpbis-http2-transport-02 modified_headers = createHeaderMap(headers); - modified_headers->setProtocol(Headers::get().ProtocolValues.Bytestream); - if (!headers.Path()) { - modified_headers->setPath("/"); + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.use_rfc_connect")) { + modified_headers->removeScheme(); + modified_headers->removePath(); + modified_headers->removeProtocol(); + } else { + modified_headers->setProtocol(Headers::get().ProtocolValues.Bytestream); + if (!headers.Path()) { + modified_headers->setPath("/"); + } } encodeHeadersBase(*modified_headers, end_stream); } else { diff --git a/source/common/quic/envoy_quic_client_stream.cc b/source/common/quic/envoy_quic_client_stream.cc index fa227423c5a8..b3ae995f4e53 100644 --- a/source/common/quic/envoy_quic_client_stream.cc +++ b/source/common/quic/envoy_quic_client_stream.cc @@ -44,14 +44,17 @@ Http::Status EnvoyQuicClientStream::encodeHeaders(const Http::RequestHeaderMap& auto spdy_headers = envoyHeadersToSpdyHeaderBlock(headers); if (headers.Method()) { if (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"] = "/"; + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.use_rfc_connect")) { + spdy_headers.erase(":scheme"); + spdy_headers.erase(":path"); + spdy_headers.erase(":protocol"); + } else { + // Legacy support for abandoned + // 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"] = "/"; + } } } else if (headers.Method()->value() == "HEAD") { sent_head_request_ = true; diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 0c78a1b8c934..2b4c11ca1eb6 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -70,6 +70,7 @@ RUNTIME_GUARD(envoy_reloadable_features_top_level_ecds_stats); RUNTIME_GUARD(envoy_reloadable_features_udp_listener_updates_filter_chain_in_place); RUNTIME_GUARD(envoy_reloadable_features_update_expected_rq_timeout_on_retry); RUNTIME_GUARD(envoy_reloadable_features_update_grpc_response_error_tag); +RUNTIME_GUARD(envoy_reloadable_features_use_rfc_connect); RUNTIME_GUARD(envoy_reloadable_features_validate_connect); RUNTIME_GUARD(envoy_restart_features_explicit_wildcard_resource); RUNTIME_GUARD(envoy_restart_features_remove_runtime_singleton); diff --git a/source/common/tcp_proxy/upstream.cc b/source/common/tcp_proxy/upstream.cc index a798047efa0d..fe9579234ae2 100644 --- a/source/common/tcp_proxy/upstream.cc +++ b/source/common/tcp_proxy/upstream.cc @@ -278,11 +278,14 @@ void Http2Upstream::setRequestEncoder(Http::RequestEncoder& request_encoder, boo auto headers = Http::createHeaderMap({ {Http::Headers::get().Method, config_.usePost() ? "POST" : "CONNECT"}, {Http::Headers::get().Host, config_.hostname()}, - {Http::Headers::get().Path, "/"}, - {Http::Headers::get().Scheme, scheme}, }); - if (!config_.usePost()) { + if (config_.usePost()) { + headers->addReference(Http::Headers::get().Path, "/"); + headers->addReference(Http::Headers::get().Scheme, scheme); + } else if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.use_rfc_connect")) { + headers->addReference(Http::Headers::get().Path, "/"); + headers->addReference(Http::Headers::get().Scheme, scheme); headers->addReference(Http::Headers::get().Protocol, Http::Headers::get().ProtocolValues.Bytestream); } diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index 92c31ec3b25d..345def7f4047 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -3613,7 +3613,8 @@ TEST_P(Http2CodecImplTest, ResetStreamCausesOutboundFlood) { } // CONNECT without upgrade type gets tagged with "bytestream" -TEST_P(Http2CodecImplTest, ConnectTest) { +TEST_P(Http2CodecImplTest, ConnectTestOld) { + scoped_runtime_.mergeValues({{"envoy.reloadable_features.use_rfc_connect", "false"}}); client_http2_options_.set_allow_connect(true); server_http2_options_.set_allow_connect(true); initialize(); @@ -3639,6 +3640,31 @@ TEST_P(Http2CodecImplTest, ConnectTest) { driveToCompletion(); } +TEST_P(Http2CodecImplTest, ConnectTest) { + client_http2_options_.set_allow_connect(true); + server_http2_options_.set_allow_connect(true); + initialize(); + MockStreamCallbacks callbacks; + request_encoder_->getStream().addCallbacks(callbacks); + + TestRequestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + request_headers.setReferenceKey(Headers::get().Method, Http::Headers::get().MethodValues.Connect); + request_headers.setReferenceKey(Headers::get().Protocol, "bytestream"); + TestRequestHeaderMapImpl expected_headers; + expected_headers.setReferenceKey(Headers::get().Host, "host"); + expected_headers.setReferenceKey(Headers::get().Method, + Http::Headers::get().MethodValues.Connect); + EXPECT_CALL(request_decoder_, decodeHeaders_(HeaderMapEqual(&expected_headers), false)); + EXPECT_TRUE(request_encoder_->encodeHeaders(request_headers, false).ok()); + driveToCompletion(); + + EXPECT_CALL(callbacks, onResetStream(StreamResetReason::ConnectError, _)); + EXPECT_CALL(server_stream_callbacks_, onResetStream(StreamResetReason::ConnectError, _)); + response_encoder_->getStream().resetStream(StreamResetReason::ConnectError); + driveToCompletion(); +} + TEST_P(Http2CodecImplTest, ShouldWaitForDeferredBodyToProcessBeforeProcessingTrailers) { // We must initialize before dtor, otherwise we'll touch uninitialized // members in dtor. diff --git a/test/common/tcp_proxy/BUILD b/test/common/tcp_proxy/BUILD index 23b6a081fa26..45be486491e1 100644 --- a/test/common/tcp_proxy/BUILD +++ b/test/common/tcp_proxy/BUILD @@ -81,5 +81,6 @@ envoy_cc_test( "//test/mocks/http:http_mocks", "//test/mocks/tcp:tcp_mocks", "//test/mocks/upstream:cluster_manager_mocks", + "//test/test_common:test_runtime_lib", ], ) diff --git a/test/common/tcp_proxy/upstream_test.cc b/test/common/tcp_proxy/upstream_test.cc index cffa9c0ec9a5..b42a43cb5cf1 100644 --- a/test/common/tcp_proxy/upstream_test.cc +++ b/test/common/tcp_proxy/upstream_test.cc @@ -9,6 +9,7 @@ #include "test/mocks/tcp/mocks.h" #include "test/test_common/environment.h" #include "test/test_common/network_utility.h" +#include "test/test_common/test_runtime.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -223,7 +224,10 @@ template class HttpUpstreamRequestEncoderTest : public testing::Tes TYPED_TEST_SUITE(HttpUpstreamRequestEncoderTest, Implementations); -TYPED_TEST(HttpUpstreamRequestEncoderTest, RequestEncoder) { +TYPED_TEST(HttpUpstreamRequestEncoderTest, RequestEncoderOld) { + TestScopedRuntime scoped_runtime; + scoped_runtime.mergeValues({{"envoy.reloadable_features.use_rfc_connect", "false"}}); + this->setupUpstream(); std::unique_ptr expected_headers; expected_headers = Http::createHeaderMap({ @@ -243,6 +247,18 @@ TYPED_TEST(HttpUpstreamRequestEncoderTest, RequestEncoder) { this->upstream_->setRequestEncoder(this->encoder_, false); } +TYPED_TEST(HttpUpstreamRequestEncoderTest, RequestEncoder) { + this->setupUpstream(); + std::unique_ptr expected_headers; + expected_headers = Http::createHeaderMap({ + {Http::Headers::get().Method, "CONNECT"}, + {Http::Headers::get().Host, this->config_->hostname()}, + }); + + EXPECT_CALL(this->encoder_, encodeHeaders(HeaderMapEqualRef(expected_headers.get()), false)); + this->upstream_->setRequestEncoder(this->encoder_, false); +} + TYPED_TEST(HttpUpstreamRequestEncoderTest, RequestEncoderUsePost) { this->config_message_.set_use_post(true); this->setupUpstream(); @@ -287,14 +303,6 @@ TYPED_TEST(HttpUpstreamRequestEncoderTest, RequestEncoderHeaders) { {Http::Headers::get().Host, this->config_->hostname()}, }); - if (this->is_http2_) { - expected_headers->setReferenceKey(Http::Headers::get().Path, "/"); - expected_headers->setReferenceKey(Http::Headers::get().Scheme, - Http::Headers::get().SchemeValues.Http); - expected_headers->setReferenceKey(Http::Headers::get().Protocol, - Http::Headers::get().ProtocolValues.Bytestream); - } - expected_headers->setCopy(Http::LowerCaseString("header0"), "value0"); expected_headers->addCopy(Http::LowerCaseString("header1"), "value1"); expected_headers->addCopy(Http::LowerCaseString("header1"), "value2"); @@ -323,25 +331,9 @@ TYPED_TEST(HttpUpstreamRequestEncoderTest, ConfigReuse) { {Http::Headers::get().Host, this->config_->hostname()}, }); - if (this->is_http2_) { - expected_headers->setReferenceKey(Http::Headers::get().Path, "/"); - expected_headers->setReferenceKey(Http::Headers::get().Scheme, - Http::Headers::get().SchemeValues.Http); - expected_headers->setReferenceKey(Http::Headers::get().Protocol, - Http::Headers::get().ProtocolValues.Bytestream); - } - expected_headers->setCopy(Http::LowerCaseString("key"), "value1"); expected_headers->addCopy(Http::LowerCaseString("key"), "value2"); - if (this->is_http2_) { - expected_headers->setReferenceKey(Http::Headers::get().Path, "/"); - expected_headers->setReferenceKey(Http::Headers::get().Scheme, - Http::Headers::get().SchemeValues.Http); - expected_headers->setReferenceKey(Http::Headers::get().Protocol, - Http::Headers::get().ProtocolValues.Bytestream); - } - expected_headers->setCopy(Http::LowerCaseString("key"), "value1"); expected_headers->addCopy(Http::LowerCaseString("key"), "value2"); @@ -382,14 +374,6 @@ TYPED_TEST(HttpUpstreamRequestEncoderTest, RequestEncoderHeadersWithDownstreamIn {Http::Headers::get().Host, this->config_->hostname()}, }); - if (this->is_http2_) { - expected_headers->setReferenceKey(Http::Headers::get().Path, "/"); - expected_headers->setReferenceKey(Http::Headers::get().Scheme, - Http::Headers::get().SchemeValues.Http); - expected_headers->setReferenceKey(Http::Headers::get().Protocol, - Http::Headers::get().ProtocolValues.Bytestream); - } - expected_headers->setCopy(Http::LowerCaseString("header0"), "value0"); expected_headers->addCopy(Http::LowerCaseString("downstream_local_port"), "80"); auto ip_versions = TestEnvironment::getIpVersionsForTest(); diff --git a/test/extensions/filters/http/compressor/compressor_integration_tests.cc b/test/extensions/filters/http/compressor/compressor_integration_tests.cc index 9b5e33f4c347..982b94b47645 100644 --- a/test/extensions/filters/http/compressor/compressor_integration_tests.cc +++ b/test/extensions/filters/http/compressor/compressor_integration_tests.cc @@ -268,12 +268,7 @@ TEST_P(CompressorProxyingConnectIntegrationTest, ProxyConnect) { RELEASE_ASSERT(result, result.message()); ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); EXPECT_EQ(upstream_request_->headers().get(Http::Headers::get().Method)[0]->value(), "CONNECT"); - if (upstreamProtocol() == Http::CodecType::HTTP1) { - EXPECT_TRUE(upstream_request_->headers().get(Http::Headers::get().Protocol).empty()); - } else { - EXPECT_EQ(upstream_request_->headers().get(Http::Headers::get().Protocol)[0]->value(), - "bytestream"); - } + EXPECT_TRUE(upstream_request_->headers().get(Http::Headers::get().Protocol).empty()); // Send response headers upstream_request_->encodeHeaders(default_response_headers_, false); diff --git a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc index 21246374d0da..855a86f82c0d 100644 --- a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc +++ b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc @@ -303,16 +303,11 @@ TEST_P(LocalJwksIntegrationTest, ConnectRequestWithRegExMatch) { request_encoder_ = &encoder_decoder.first; auto response = std::move(encoder_decoder.second); - if (downstreamProtocol() == Http::CodecType::HTTP1) { - // Because CONNECT requests for HTTP/1 do not include a path, they will fail - // to find a route match and return a 404. - ASSERT_TRUE(response->waitForEndStream()); - ASSERT_TRUE(response->complete()); - EXPECT_EQ("404", response->headers().getStatusValue()); - } else { - ASSERT_TRUE(response->waitForReset()); - ASSERT_TRUE(codec_client_->waitForDisconnect()); - } + // Because CONNECT requests do not include a path, they will fail + // to find a route match and return a 404. + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("404", response->headers().getStatusValue()); } // The test case with a fake upstream for remote Jwks server. diff --git a/test/integration/BUILD b/test/integration/BUILD index ce546984aa28..bfc75c609241 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -1397,7 +1397,7 @@ envoy_cc_test( data = [ "//test/config/integration/certs", ], - shard_count = 3, + shard_count = 7, deps = [ ":http_integration_lib", ":http_protocol_integration_lib", diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 5474ed514bb7..37ee63bf5804 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -2733,19 +2733,15 @@ TEST_P(DownstreamProtocolIntegrationTest, ConnectIsBlocked) { request_encoder_ = &encoder_decoder.first; auto response = std::move(encoder_decoder.second); - if (downstreamProtocol() == Http::CodecType::HTTP1) { - // Because CONNECT requests for HTTP/1 do not include a path, they will fail - // to find a route match and return a 404. - ASSERT_TRUE(response->waitForEndStream()); - EXPECT_EQ("404", response->headers().getStatusValue()); - EXPECT_TRUE(response->complete()); - } else { - ASSERT_TRUE(response->waitForReset()); - ASSERT_TRUE(codec_client_->waitForDisconnect()); - } + // Because CONNECT requests do not include a path, they will fail + // to find a route match and return a 404. + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_EQ("404", response->headers().getStatusValue()); + EXPECT_TRUE(response->complete()); } TEST_P(DownstreamProtocolIntegrationTest, ExtendedConnectIsBlocked) { + config_helper_.addRuntimeOverride("envoy.reloadable_features.use_rfc_connect", "false"); if (downstreamProtocol() == Http::CodecType::HTTP1) { return; } @@ -2785,7 +2781,8 @@ TEST_P(DownstreamProtocolIntegrationTest, ConnectStreamRejection) { auto response = codec_client_->makeHeaderOnlyRequest( Http::TestRequestHeaderMapImpl{{":method", "CONNECT"}, {":authority", "sni.lyft.com"}}); - ASSERT_TRUE(response->waitForReset()); + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_EQ("404", response->headers().getStatusValue()); EXPECT_FALSE(codec_client_->disconnected()); } diff --git a/test/integration/tcp_tunneling_integration_test.cc b/test/integration/tcp_tunneling_integration_test.cc index f5efad6eaa7f..1b6aeca071ad 100644 --- a/test/integration/tcp_tunneling_integration_test.cc +++ b/test/integration/tcp_tunneling_integration_test.cc @@ -32,7 +32,7 @@ class ConnectTerminationIntegrationTest : public HttpProtocolIntegrationTest { auto* route_config = hcm.mutable_route_config(); ASSERT_EQ(1, route_config->virtual_hosts_size()); route_config->mutable_virtual_hosts(0)->clear_domains(); - route_config->mutable_virtual_hosts(0)->add_domains("host:80"); + route_config->mutable_virtual_hosts(0)->add_domains("foo.lyft.com:80"); } }); HttpIntegrationTest::initialize(); @@ -66,7 +66,7 @@ class ConnectTerminationIntegrationTest : public HttpProtocolIntegrationTest { {":path", "/"}, {":protocol", "bytestream"}, {":scheme", "https"}, - {":authority", "host:80"}}; + {":authority", "foo.lyft.com:80"}}; void clearExtendedConnectHeaders() { connect_headers_.removeProtocol(); connect_headers_.removePath(); @@ -221,7 +221,7 @@ TEST_P(ConnectTerminationIntegrationTest, BuggyHeaders) { {":path", "/"}, {":protocol", "bytestream"}, {":scheme", "https"}, - {":authority", "host:80"}}); + {":authority", "foo.lyft.com:80"}}); // If the connection is established (created, set to half close, and then the // FIN arrives), make sure the FIN arrives, and send a FIN from upstream. if (fake_upstreams_[0]->waitForRawConnection(fake_raw_upstream_connection_) && @@ -294,19 +294,64 @@ class ProxyingConnectIntegrationTest : public HttpProtocolIntegrationTest { } Http::TestRequestHeaderMapImpl connect_headers_{{":method", "CONNECT"}, - {":path", "/"}, - {":protocol", "bytestream"}, - {":scheme", "https"}, - {":authority", "host:80"}}; + {":authority", "foo.lyft.com:80"}}; IntegrationStreamDecoderPtr response_; }; -INSTANTIATE_TEST_SUITE_P(Protocols, ProxyingConnectIntegrationTest, - testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( - {Http::CodecType::HTTP1, Http::CodecType::HTTP2, - Http::CodecType::HTTP3}, - {Http::CodecType::HTTP1})), - HttpProtocolIntegrationTest::protocolTestParamsToString); +INSTANTIATE_TEST_SUITE_P( + Protocols, ProxyingConnectIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( + {Http::CodecType::HTTP1, Http::CodecType::HTTP2, Http::CodecType::HTTP3}, + {Http::CodecType::HTTP1, Http::CodecType::HTTP2, Http::CodecType::HTTP3})), + HttpProtocolIntegrationTest::protocolTestParamsToString); + +TEST_P(ProxyingConnectIntegrationTest, ProxyConnectLegacy) { + config_helper_.addRuntimeOverride("envoy.reloadable_features.use_rfc_connect", "false"); + + initialize(); + + Http::TestRequestHeaderMapImpl legacy_connect_headers{{":method", "CONNECT"}, + {":path", "/"}, + {":protocol", "bytestream"}, + {":scheme", "https"}, + {":authority", "foo.lyft.com:80"}}; + // Send request headers. + codec_client_ = makeHttpConnection(lookupPort("http")); + auto encoder_decoder = codec_client_->startRequest(legacy_connect_headers); + request_encoder_ = &encoder_decoder.first; + response_ = std::move(encoder_decoder.second); + + // Wait for them to arrive upstream. + AssertionResult result = + fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_); + RELEASE_ASSERT(result, result.message()); + result = fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_); + RELEASE_ASSERT(result, result.message()); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); + EXPECT_EQ(upstream_request_->headers().get(Http::Headers::get().Method)[0]->value(), "CONNECT"); + if (upstreamProtocol() == Http::CodecType::HTTP1) { + EXPECT_TRUE(upstream_request_->headers().get(Http::Headers::get().Protocol).empty()); + } else { + EXPECT_EQ(upstream_request_->headers().getProtocolValue(), "bytestream"); + } + + // Send response headers + upstream_request_->encodeHeaders(default_response_headers_, false); + + // Wait for them to arrive downstream. + response_->waitForHeaders(); + EXPECT_EQ("200", response_->headers().getStatusValue()); + + // Make sure that even once the response has started, that data can continue to go upstream. + codec_client_->sendData(*request_encoder_, "hello", false); + ASSERT_TRUE(upstream_request_->waitForData(*dispatcher_, 5)); + + // Also test upstream to downstream data. + upstream_request_->encodeData(12, false); + response_->waitForBodyData(12); + + cleanupUpstreamAndDownstream(); +} TEST_P(ProxyingConnectIntegrationTest, ProxyConnect) { initialize(); @@ -328,8 +373,9 @@ TEST_P(ProxyingConnectIntegrationTest, ProxyConnect) { if (upstreamProtocol() == Http::CodecType::HTTP1) { EXPECT_TRUE(upstream_request_->headers().get(Http::Headers::get().Protocol).empty()); } else { - EXPECT_EQ(upstream_request_->headers().get(Http::Headers::get().Protocol)[0]->value(), - "bytestream"); + EXPECT_EQ("", upstream_request_->headers().getSchemeValue()); + EXPECT_EQ("", upstream_request_->headers().getProtocolValue()); + EXPECT_EQ("", upstream_request_->headers().getSchemeValue()); } // Send response headers @@ -378,15 +424,10 @@ TEST_P(ProxyingConnectIntegrationTest, ProxyConnectWithPortStripping) { RELEASE_ASSERT(result, result.message()); ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); EXPECT_EQ(upstream_request_->headers().getMethodValue(), "CONNECT"); - EXPECT_EQ(upstream_request_->headers().getHostValue(), "host:80"); - if (upstreamProtocol() == Http::CodecType::HTTP1) { - EXPECT_TRUE(upstream_request_->headers().getProtocolValue().empty()); - } else { - EXPECT_EQ(upstream_request_->headers().getProtocolValue(), "bytestream"); - } + EXPECT_EQ(upstream_request_->headers().getHostValue(), "foo.lyft.com:80"); auto stripped_host = upstream_request_->headers().get(Http::LowerCaseString("host-in-envoy")); ASSERT_EQ(stripped_host.size(), 1); - EXPECT_EQ(stripped_host[0]->value(), "host"); + EXPECT_EQ(stripped_host[0]->value(), "foo.lyft.com"); // Send response headers upstream_request_->encodeHeaders(default_response_headers_, false); @@ -424,12 +465,6 @@ TEST_P(ProxyingConnectIntegrationTest, ProxyConnectWithIP) { RELEASE_ASSERT(result, result.message()); ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); EXPECT_EQ(upstream_request_->headers().get(Http::Headers::get().Method)[0]->value(), "CONNECT"); - if (upstreamProtocol() == Http::CodecType::HTTP1) { - EXPECT_TRUE(upstream_request_->headers().get(Http::Headers::get().Protocol).empty()); - } else { - EXPECT_EQ(upstream_request_->headers().get(Http::Headers::get().Protocol)[0]->value(), - "bytestream"); - } // Send response headers upstream_request_->encodeHeaders(default_response_headers_, false); @@ -461,7 +496,7 @@ class TcpTunnelingIntegrationTest : public HttpProtocolIntegrationTest { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy proxy_config; proxy_config.set_stat_prefix("tcp_stats"); proxy_config.set_cluster("cluster_0"); - proxy_config.mutable_tunneling_config()->set_hostname("host.com:80"); + proxy_config.mutable_tunneling_config()->set_hostname("foo.lyft.com:80"); auto* listener = bootstrap.mutable_static_resources()->add_listeners(); listener->set_name("tcp_proxy"); @@ -562,7 +597,7 @@ TEST_P(TcpTunnelingIntegrationTest, BasicUsePost) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy proxy_config; proxy_config.set_stat_prefix("tcp_stats"); proxy_config.set_cluster("cluster_0"); - proxy_config.mutable_tunneling_config()->set_hostname("host.com:80"); + proxy_config.mutable_tunneling_config()->set_hostname("foo.lyft.com:80"); proxy_config.mutable_tunneling_config()->set_use_post(true); auto* listeners = bootstrap.mutable_static_resources()->mutable_listeners(); @@ -599,7 +634,7 @@ TEST_P(TcpTunnelingIntegrationTest, BasicHeaderEvaluationTunnelingConfig) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy proxy_config; proxy_config.set_stat_prefix("tcp_stats"); proxy_config.set_cluster("cluster_0"); - proxy_config.mutable_tunneling_config()->set_hostname("host.com:80"); + proxy_config.mutable_tunneling_config()->set_hostname("foo.lyft.com:80"); auto new_header = proxy_config.mutable_tunneling_config()->mutable_headers_to_add()->Add(); new_header->mutable_header()->set_key("downstream-local-ip"); new_header->mutable_header()->set_value("%DOWNSTREAM_LOCAL_ADDRESS_WITHOUT_PORT%"); @@ -651,7 +686,7 @@ TEST_P(TcpTunnelingIntegrationTest, HeaderEvaluatorConfigUpdate) { config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void { proxy_config.set_stat_prefix("tcp_stats"); proxy_config.set_cluster("cluster_0"); - proxy_config.mutable_tunneling_config()->set_hostname("host.com:80"); + proxy_config.mutable_tunneling_config()->set_hostname("foo.lyft.com:80"); auto address_header = proxy_config.mutable_tunneling_config()->mutable_headers_to_add()->Add(); address_header->mutable_header()->set_key("config-version"); address_header->mutable_header()->set_value("1"); @@ -852,7 +887,7 @@ TEST_P(TcpTunnelingIntegrationTest, UpstreamConnectingDownstreamDisconnect) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy proxy_config; proxy_config.set_stat_prefix("tcp_stats"); proxy_config.set_cluster("cluster_0"); - proxy_config.mutable_tunneling_config()->set_hostname("host.com:80"); + proxy_config.mutable_tunneling_config()->set_hostname("foo.lyft.com:80"); // Enable retries. The crash is due to retrying after the downstream connection is closed, which // can't occur if retries are not enabled. @@ -1200,7 +1235,7 @@ TEST_P(TcpTunnelingIntegrationTest, TransferEncodingHeaderIgnoredHttp1) { std::string data; ASSERT_TRUE(fake_upstream_connection->waitForData( FakeRawConnection::waitForInexactMatch("\r\n\r\n"), &data)); - ASSERT_THAT(data, testing::HasSubstr("CONNECT host.com:80 HTTP/1.1")); + ASSERT_THAT(data, testing::HasSubstr("CONNECT foo.lyft.com:80 HTTP/1.1")); // Send upgrade headers downstream, fully establishing the connection. ASSERT_TRUE( @@ -1289,11 +1324,11 @@ TEST_P(TcpTunnelingIntegrationTest, UpstreamDisconnectBeforeResponseReceived) { tcp_client_->close(); } -INSTANTIATE_TEST_SUITE_P(IpAndHttpVersions, TcpTunnelingIntegrationTest, - testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( - {Http::CodecType::HTTP1}, - {Http::CodecType::HTTP1, Http::CodecType::HTTP2, - Http::CodecType::HTTP3})), - HttpProtocolIntegrationTest::protocolTestParamsToString); +INSTANTIATE_TEST_SUITE_P( + IpAndHttpVersions, TcpTunnelingIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( + {Http::CodecType::HTTP1, Http::CodecType::HTTP2, Http::CodecType::HTTP3}, + {Http::CodecType::HTTP1, Http::CodecType::HTTP2, Http::CodecType::HTTP3})), + HttpProtocolIntegrationTest::protocolTestParamsToString); } // namespace } // namespace Envoy