From 65ffb643eaacc60b60c3d1c12c1db895af699e8c Mon Sep 17 00:00:00 2001 From: John Morrissey Date: Mon, 22 Jun 2020 15:13:03 +0000 Subject: [PATCH] backport upstream #10960 --- .../v3/http_connection_manager.proto | 10 ++- .../v4alpha/http_connection_manager.proto | 10 ++- .../v3/http_connection_manager.proto | 10 ++- .../v4alpha/http_connection_manager.proto | 10 ++- source/common/http/conn_manager_config.h | 5 ++ source/common/http/conn_manager_impl.cc | 11 +++ source/common/http/conn_manager_impl.h | 5 +- source/common/http/conn_manager_utility.cc | 9 +++ source/common/http/conn_manager_utility.h | 3 + source/common/http/header_utility.cc | 33 ++++++++ source/common/http/header_utility.h | 5 ++ .../network/http_connection_manager/config.cc | 1 + .../network/http_connection_manager/config.h | 2 + source/server/http/admin.h | 1 + .../http/conn_manager_impl_fuzz_test.cc | 1 + test/common/http/conn_manager_impl_test.cc | 76 ++++++++++++++++++- test/common/http/conn_manager_utility_test.cc | 12 +++ test/common/http/header_utility_test.cc | 50 ++++++++++++ test/common/http/path_utility_test.cc | 4 + .../http_connection_manager/config_test.cc | 50 ++++++++++++ 20 files changed, 302 insertions(+), 6 deletions(-) diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index 06d66055a0d68..c07b301807f00 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 37] +// [#next-free-field: 38] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager"; @@ -495,6 +495,14 @@ message HttpConnectionManager { // // 3. Tracing decision (sampled, forced, etc) is set in 14th byte of the UUID. RequestIDExtension request_id_extension = 36; + + // Determines if the port part should be removed from host/authority header before any processing + // of request by HTTP filters or routing. The port would be removed only if it is equal to the :ref:`listener's` + // local port and request method is not CONNECT. This affects the upstream host header as well. + // Without setting this option, incoming requests with host `example:443` will not match against + // route with :ref:`domains` match set to `example`. Defaults to `false`. Note that port removal is not part + // of `HTTP spec ` and is provided for convenience. + bool strip_matching_host_port = 37; } message Rds { diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 226dc2727fc54..6fff979565ad5 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 37] +// [#next-free-field: 38] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"; @@ -495,6 +495,14 @@ message HttpConnectionManager { // // 3. Tracing decision (sampled, forced, etc) is set in 14th byte of the UUID. RequestIDExtension request_id_extension = 36; + + // Determines if the port part should be removed from host/authority header before any processing + // of request by HTTP filters or routing. The port would be removed only if it is equal to the :ref:`listener's` + // local port and request method is not CONNECT. This affects the upstream host header as well. + // Without setting this option, incoming requests with host `example:443` will not match against + // route with :ref:`domains` match set to `example`. Defaults to `false`. Note that port removal is not part + // of `HTTP spec ` and is provided for convenience. + bool strip_matching_host_port = 37; } message Rds { diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index 6d1044caa76be..e5e402c5967d0 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 37] +// [#next-free-field: 38] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager"; @@ -462,6 +462,14 @@ message HttpConnectionManager { RequestIDExtension request_id_extension = 36; + // Determines if the port part should be removed from host/authority header before any processing + // of request by HTTP filters or routing. The port would be removed only if it is equal to the :ref:`listener's` + // local port and request method is not CONNECT. This affects the upstream host header as well. + // Without setting this option, incoming requests with host `example:443` will not match against + // route with :ref:`domains` match set to `example`. Defaults to `false`. Note that port removal is not part + // of `HTTP spec ` and is provided for convenience. + bool strip_matching_host_port = 37; + // Should paths be normalized according to RFC 3986 before any processing of // requests by HTTP filters or routing? This affects the upstream *:path* header // as well. For paths that fail this check, Envoy will respond with 400 to diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 226dc2727fc54..6fff979565ad5 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 37] +// [#next-free-field: 38] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"; @@ -495,6 +495,14 @@ message HttpConnectionManager { // // 3. Tracing decision (sampled, forced, etc) is set in 14th byte of the UUID. RequestIDExtension request_id_extension = 36; + + // Determines if the port part should be removed from host/authority header before any processing + // of request by HTTP filters or routing. The port would be removed only if it is equal to the :ref:`listener's` + // local port and request method is not CONNECT. This affects the upstream host header as well. + // Without setting this option, incoming requests with host `example:443` will not match against + // route with :ref:`domains` match set to `example`. Defaults to `false`. Note that port removal is not part + // of `HTTP spec ` and is provided for convenience. + bool strip_matching_host_port = 37; } message Rds { diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index 774b5e9f47c58..15d9dc13254eb 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -419,6 +419,11 @@ class ConnectionManagerConfig { */ virtual bool shouldMergeSlashes() const PURE; + /** + * @return if the HttpConnectionManager should remove the port from host/authority header + */ + virtual bool shouldStripMatchingPort() const PURE; + /** * @return the action HttpConnectionManager should take when receiving client request * headers containing underscore characters. diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 2b39461878463..a6a3a99eb729f 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -741,6 +741,14 @@ const Network::Connection* ConnectionManagerImpl::ActiveStream::connection() { return &connection_manager_.read_callbacks_->connection(); } +uint32_t ConnectionManagerImpl::ActiveStream::localPort() { + auto ip = connection()->localAddress()->ip(); + if (ip == nullptr) { + return 0; + } + return ip->port(); +} + // Ordering in this function is complicated, but important. // // We want to do minimal work before selecting route and creating a filter @@ -883,6 +891,9 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he return; } + ConnectionManagerUtility::maybeNormalizeHost(*request_headers_, connection_manager_.config_, + localPort()); + if (protocol == Protocol::Http11 && request_headers_->Connection() && absl::EqualsIgnoreCase(request_headers_->Connection()->value().getStringView(), Http::Headers::get().ConnectionValues.Close)) { diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index 10bcc7522bd61..20b9cd5502a7b 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -662,12 +662,15 @@ class ConnectionManagerImpl : Logger::Loggable, void onIdleTimeout(); // Reset per-stream idle timer. void resetIdleTimer(); - // Per-stream request timeout callback + // Per-stream request timeout callback. void onRequestTimeout(); // Per-stream alive duration reached. void onStreamMaxDurationReached(); bool hasCachedRoute() { return cached_route_.has_value() && cached_route_.value(); } + // Return local port of the connection. + uint32_t localPort(); + friend std::ostream& operator<<(std::ostream& os, const ActiveStream& s) { s.dumpState(os); return os; diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index 615bcf06baa5f..df47de5395acb 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -9,6 +9,7 @@ #include "common/access_log/access_log_formatter.h" #include "common/common/empty_string.h" #include "common/common/utility.h" +#include "common/http/header_utility.h" #include "common/http/headers.h" #include "common/http/http1/codec_impl.h" #include "common/http/http2/codec_impl.h" @@ -406,5 +407,13 @@ bool ConnectionManagerUtility::maybeNormalizePath(RequestHeaderMap& request_head return is_valid_path; } +void ConnectionManagerUtility::maybeNormalizeHost(RequestHeaderMap& request_headers, + const ConnectionManagerConfig& config, + uint32_t port) { + if (config.shouldStripMatchingPort()) { + HeaderUtility::stripPortFromHost(request_headers, port); + } +} + } // namespace Http } // namespace Envoy diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h index 20381116162f1..be281efedf929 100644 --- a/source/common/http/conn_manager_utility.h +++ b/source/common/http/conn_manager_utility.h @@ -69,6 +69,9 @@ class ConnectionManagerUtility { static bool maybeNormalizePath(RequestHeaderMap& request_headers, const ConnectionManagerConfig& config); + static void maybeNormalizeHost(RequestHeaderMap& request_headers, + const ConnectionManagerConfig& config, uint32_t port); + /** * Mutate request headers if request needs to be traced. */ diff --git a/source/common/http/header_utility.cc b/source/common/http/header_utility.cc index 9772f35566831..b622a4e98c153 100644 --- a/source/common/http/header_utility.cc +++ b/source/common/http/header_utility.cc @@ -176,6 +176,39 @@ bool HeaderUtility::isEnvoyInternalRequest(const RequestHeaderMap& headers) { internal_request_header->value() == Headers::get().EnvoyInternalRequestValues.True; } +void HeaderUtility::stripPortFromHost(RequestHeaderMap& headers, uint32_t listener_port) { + + if (headers.Method() && + headers.Method()->value().getStringView() == Http::Headers::get().MethodValues.Connect) { + // According to RFC 2817 Connect method should have port part in host header. + // In this case we won't strip it even if configured to do so. + return; + } + const auto original_host = headers.Host()->value().getStringView(); + const absl::string_view::size_type port_start = original_host.rfind(':'); + if (port_start == absl::string_view::npos) { + return; + } + // According to RFC3986 v6 address is always enclosed in "[]". section 3.2.2. + const auto v6_end_index = original_host.rfind("]"); + if (v6_end_index == absl::string_view::npos || v6_end_index < port_start) { + if ((port_start + 1) > original_host.size()) { + return; + } + const absl::string_view port_str = original_host.substr(port_start + 1); + uint32_t port = 0; + if (!absl::SimpleAtoi(port_str, &port)) { + return; + } + if (port != listener_port) { + // We would strip ports only if they are the same, as local port of the listener. + return; + } + const absl::string_view host = original_host.substr(0, port_start); + headers.setHost(host); + } +} + absl::optional> HeaderUtility::requestHeadersValid(const RequestHeaderMap& headers) { // Make sure the host is valid. diff --git a/source/common/http/header_utility.h b/source/common/http/header_utility.h index e349a2ee97d0f..56dcdadbd3613 100644 --- a/source/common/http/header_utility.h +++ b/source/common/http/header_utility.h @@ -130,6 +130,11 @@ class HeaderUtility { */ static absl::optional> requestHeadersValid(const RequestHeaderMap& headers); + + /** + * @brief Remove the port part from host/authority header if it is equal to provided port + */ + static void stripPortFromHost(RequestHeaderMap& headers, uint32_t listener_port); }; } // namespace Http } // namespace Envoy diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index ed107562bccd2..ae7b9f8acc42b 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -215,6 +215,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( 0))), #endif merge_slashes_(config.merge_slashes()), + strip_matching_port_(config.strip_matching_host_port()), headers_with_underscores_action_( config.common_http_protocol_options().headers_with_underscores_action()) { // If idle_timeout_ was not configured in common_http_protocol_options, use value in deprecated diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index 59dee762513fa..786029114d571 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -156,6 +156,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, const Http::Http1Settings& http1Settings() const override { return http1_settings_; } bool shouldNormalizePath() const override { return normalize_path_; } bool shouldMergeSlashes() const override { return merge_slashes_; } + bool shouldStripMatchingPort() const override { return strip_matching_port_; } envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction headersWithUnderscoresAction() const override { return headers_with_underscores_action_; @@ -222,6 +223,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, std::chrono::milliseconds delayed_close_timeout_; const bool normalize_path_; const bool merge_slashes_; + const bool strip_matching_port_; const envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction headers_with_underscores_action_; diff --git a/source/server/http/admin.h b/source/server/http/admin.h index 82332658de566..0f5f882ee098d 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -162,6 +162,7 @@ class AdminImpl : public Admin, const Http::Http1Settings& http1Settings() const override { return http1_settings_; } bool shouldNormalizePath() const override { return true; } bool shouldMergeSlashes() const override { return true; } + bool shouldStripMatchingPort() const override { return false; } envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction headersWithUnderscoresAction() const override { return envoy::config::core::v3::HttpProtocolOptions::ALLOW; diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index 38d471ded4cc2..9b7c5f6c99ca8 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -155,6 +155,7 @@ class FuzzConfig : public ConnectionManagerConfig { const Http::Http1Settings& http1Settings() const override { return http1_settings_; } bool shouldNormalizePath() const override { return false; } bool shouldMergeSlashes() const override { return false; } + bool shouldStripMatchingPort() const override { return false; } envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction headersWithUnderscoresAction() const override { return envoy::config::core::v3::HttpProtocolOptions::ALLOW; diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index a820323b4286a..13f48e68ea0cc 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -126,7 +126,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan ON_CALL(filter_callbacks_.connection_, ssl()).WillByDefault(Return(ssl_connection_)); ON_CALL(Const(filter_callbacks_.connection_), ssl()).WillByDefault(Return(ssl_connection_)); filter_callbacks_.connection_.local_address_ = - std::make_shared("127.0.0.1"); + std::make_shared("127.0.0.1", 443); filter_callbacks_.connection_.remote_address_ = std::make_shared("0.0.0.0"); conn_manager_ = std::make_unique( @@ -346,6 +346,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan const Http::Http1Settings& http1Settings() const override { return http1_settings_; } bool shouldNormalizePath() const override { return normalize_path_; } bool shouldMergeSlashes() const override { return merge_slashes_; } + bool shouldStripMatchingPort() const override { return strip_matching_port_; } RequestIDExtensionSharedPtr requestIDExtension() override { return request_id_extension_; } envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction headersWithUnderscoresAction() const override { @@ -407,6 +408,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan Http::Http1Settings http1_settings_; bool normalize_path_ = false; bool merge_slashes_ = false; + bool strip_matching_port_ = false; envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction headers_with_underscores_action_ = envoy::config::core::v3::HttpProtocolOptions::ALLOW; NiceMock upstream_conn_; // for websocket tests @@ -821,6 +823,78 @@ TEST_F(HttpConnectionManagerImplTest, RouteShouldUseSantizedPath) { conn_manager_->onData(fake_input, false); } +// Filters observe host header w/o port's part when port's removal is configured +TEST_F(HttpConnectionManagerImplTest, FilterShouldUseNormalizedHost) { + setup(false, ""); + // Enable port removal + strip_matching_port_ = true; + const std::string original_host = "host:443"; + const std::string normalized_host = "host"; + + auto* filter = new MockStreamFilter(); + + EXPECT_CALL(filter_factory_, createFilterChain(_)) + .WillOnce(Invoke([&](FilterChainFactoryCallbacks& callbacks) -> void { + callbacks.addStreamDecoderFilter(StreamDecoderFilterSharedPtr{filter}); + })); + + EXPECT_CALL(*filter, decodeHeaders(_, true)) + .WillRepeatedly(Invoke([&](RequestHeaderMap& header_map, bool) -> FilterHeadersStatus { + EXPECT_EQ(normalized_host, header_map.Host()->value().getStringView()); + return FilterHeadersStatus::StopIteration; + })); + + EXPECT_CALL(*filter, setDecoderFilterCallbacks(_)); + + EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> void { + RequestDecoder* decoder = &conn_manager_->newStream(response_encoder_); + RequestHeaderMapPtr headers{new TestRequestHeaderMapImpl{ + {":authority", original_host}, {":path", "/"}, {":method", "GET"}}}; + decoder->decodeHeaders(std::move(headers), true); + })); + + // Kick off the incoming data. + Buffer::OwnedImpl fake_input("1234"); + conn_manager_->onData(fake_input, false); +} + +// The router observes host header w/o port, not the original host, when +// remove_port is configured +TEST_F(HttpConnectionManagerImplTest, RouteShouldUseNormalizedHost) { + setup(false, ""); + // Enable port removal + strip_matching_port_ = true; + const std::string original_host = "host:443"; + const std::string normalized_host = "host"; + + EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> void { + RequestDecoder* decoder = &conn_manager_->newStream(response_encoder_); + RequestHeaderMapPtr headers{new TestRequestHeaderMapImpl{ + {":authority", original_host}, {":path", "/"}, {":method", "GET"}}}; + decoder->decodeHeaders(std::move(headers), true); + })); + + const std::string fake_cluster_name = "fake_cluster"; + + std::shared_ptr fake_cluster = + std::make_shared>(); + std::shared_ptr route = std::make_shared>(); + EXPECT_CALL(route->route_entry_, clusterName()).WillRepeatedly(ReturnRef(fake_cluster_name)); + + EXPECT_CALL(*route_config_provider_.route_config_, route(_, _, _)) + .WillOnce(Invoke( + [&](const Http::RequestHeaderMap& header_map, const StreamInfo::StreamInfo&, uint64_t) { + EXPECT_EQ(normalized_host, header_map.Host()->value().getStringView()); + return route; + })); + EXPECT_CALL(filter_factory_, createFilterChain(_)) + .WillOnce(Invoke([&](FilterChainFactoryCallbacks&) -> void {})); + + // Kick off the incoming data. + Buffer::OwnedImpl fake_input("1234"); + conn_manager_->onData(fake_input, false); +} + TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlow) { setup(false, ""); diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 60ece5bfdf445..773c2931680a4 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -134,6 +134,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_METHOD(const Http::Http1Settings&, http1Settings, (), (const)); MOCK_METHOD(bool, shouldNormalizePath, (), (const)); MOCK_METHOD(bool, shouldMergeSlashes, (), (const)); + MOCK_METHOD(bool, shouldStripMatchingPort, (), (const)); MOCK_METHOD(envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction, headersWithUnderscoresAction, (), (const)); @@ -1425,6 +1426,17 @@ TEST_F(ConnectionManagerUtilityTest, MergeSlashesWithoutNormalization) { EXPECT_EQ(header_map.Path()->value().getStringView(), "/xyz/../abc"); } +// maybeNormalizeHost() removes port part from host header. +TEST_F(ConnectionManagerUtilityTest, RemovePort) { + ON_CALL(config_, shouldStripMatchingPort()).WillByDefault(Return(true)); + TestRequestHeaderMapImpl original_headers; + original_headers.setHost("host:443"); + + TestRequestHeaderMapImpl header_map(original_headers); + ConnectionManagerUtility::maybeNormalizeHost(header_map, config_, 443); + EXPECT_EQ(header_map.Host()->value().getStringView(), "host"); +} + // test preserve_external_request_id true does not reset the passed requestId if passed TEST_F(ConnectionManagerUtilityTest, PreserveExternalRequestId) { connection_.remote_address_ = std::make_shared("134.2.2.11"); diff --git a/test/common/http/header_utility_test.cc b/test/common/http/header_utility_test.cc index 55d2056aab78f..a367f2b1e3408 100644 --- a/test/common/http/header_utility_test.cc +++ b/test/common/http/header_utility_test.cc @@ -20,6 +20,56 @@ envoy::config::route::v3::HeaderMatcher parseHeaderMatcherFromYaml(const std::st return header_matcher; } +class HeaderUtilityTest : public testing::Test { +public: + const HeaderEntry& hostHeaderEntry(const std::string& host_value, bool set_connect = false) { + headers_.setHost(host_value); + if (set_connect) { + headers_.setMethod(Http::Headers::get().MethodValues.Connect); + } + return *headers_.Host(); + } + RequestHeaderMapImpl headers_; +}; + +// Port's part from host header get removed +TEST_F(HeaderUtilityTest, RemovePortsFromHost) { + const std::vector> host_headers{ + {"localhost", "localhost"}, // w/o port part + {"localhost:443", "localhost"}, // name w/ port + {"", ""}, // empty + {":443", ""}, // just port + {"192.168.1.1", "192.168.1.1"}, // ipv4 + {"192.168.1.1:443", "192.168.1.1"}, // ipv4 w/ port + {"[fc00::1]:443", "[fc00::1]"}, // ipv6 w/ port + {"[fc00::1]", "[fc00::1]"}, // ipv6 + {":", ":"}, // malformed string #1 + {"]:", "]:"}, // malformed string #2 + {":abc", ":abc"}, // malformed string #3 + {"localhost:80", "localhost:80"}, // port not matching w/ hostname + {"192.168.1.1:80", "192.168.1.1:80"}, // port not matching w/ ipv4 + {"[fc00::1]:80", "[fc00::1]:80"} // port not matching w/ ipv6 + }; + + for (const auto& host_pair : host_headers) { + auto& host_header = hostHeaderEntry(host_pair.first); + HeaderUtility::stripPortFromHost(headers_, 443); + EXPECT_EQ(host_header.value().getStringView(), host_pair.second); + } +} + +// Port's part from host header won't be removed if method is "connect" +TEST_F(HeaderUtilityTest, RemovePortsFromHostConnect) { + const std::vector> host_headers{ + {"localhost:443", "localhost:443"}, + }; + for (const auto& host_pair : host_headers) { + auto& host_header = hostHeaderEntry(host_pair.first, true); + HeaderUtility::stripPortFromHost(headers_, 443); + EXPECT_EQ(host_header.value().getStringView(), host_pair.second); + } +} + TEST(HeaderDataConstructorTest, NoSpecifierSet) { const std::string yaml = R"EOF( name: test-header diff --git a/test/common/http/path_utility_test.cc b/test/common/http/path_utility_test.cc index 0cd17e324c6d0..d7033290fd122 100644 --- a/test/common/http/path_utility_test.cc +++ b/test/common/http/path_utility_test.cc @@ -18,6 +18,10 @@ class PathUtilityTest : public testing::Test { headers_.setPath(path_value); return *headers_.Path(); } + const HeaderEntry& hostHeaderEntry(const std::string& host_value) { + headers_.setHost(host_value); + return *headers_.Host(); + } RequestHeaderMapImpl headers_; }; diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index 54b438220e790..bc8ffb6f7fe77 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -950,6 +950,56 @@ TEST_F(HttpConnectionManagerConfigTest, MergeSlashesFalse) { EXPECT_FALSE(config.shouldMergeSlashes()); } +// Validated that by default we don't remove port. +TEST_F(HttpConnectionManagerConfigTest, RemovePortDefault) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + route_config: + name: local_route + http_filters: + - name: envoy.filters.http.router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_, http_tracer_manager_); + EXPECT_FALSE(config.shouldStripMatchingPort()); +} + +// Validated that when configured, we remove port. +TEST_F(HttpConnectionManagerConfigTest, RemovePortTrue) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + route_config: + name: local_route + strip_matching_host_port: true + http_filters: + - name: envoy.filters.http.router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_, http_tracer_manager_); + EXPECT_TRUE(config.shouldStripMatchingPort()); +} + +// Validated that when explicitly set false, we don't remove port. +TEST_F(HttpConnectionManagerConfigTest, RemovePortFalse) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + route_config: + name: local_route + strip_matching_host_port: false + http_filters: + - name: envoy.filters.http.router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_, http_tracer_manager_); + EXPECT_FALSE(config.shouldStripMatchingPort()); +} + // Validated that by default we allow requests with header names containing underscores. TEST_F(HttpConnectionManagerConfigTest, HeadersWithUnderscoresAllowedByDefault) { const std::string yaml_string = R"EOF(