From 3f51e9dc602c2b10b69bff477e3ffbadbe9fa80c Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 10 Jun 2024 15:49:29 +0000 Subject: [PATCH] runtime: removing http_allow_partial_urls_in_referer Signed-off-by: Alyssa Wilk --- changelogs/current.yaml | 3 + source/common/http/utility.cc | 8 --- source/common/runtime/runtime_features.cc | 1 - test/common/http/conn_manager_utility_test.cc | 55 ------------------- 4 files changed, 3 insertions(+), 64 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 3c3e6ba20947..f85f1d3b1b7e 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -179,6 +179,9 @@ removed_config_or_runtime: - area: http change: | Removed ``envoy.reloadable_features.lowercase_scheme`` runtime flag and lagacy code paths. +- area: http + change: | + Removed ``envoy.reloadable_features.http_allow_partial_urls_in_referer`` runtime flag and lagacy code paths. - area: upstream change: | Removed ``envoy.reloadable_features.convert_legacy_lb_config`` runtime flag and lagacy code paths. diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index a1a4711b3a5e..c80cb18f0d8e 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -1524,14 +1524,6 @@ bool Utility::isValidRefererValue(absl::string_view value) { // a host to be present if there is a schema. Utility::Url url; - if (!Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.http_allow_partial_urls_in_referer")) { - if (url.initialize(value, false)) { - return true; - } - return false; - } - if (url.initialize(value, false)) { return !(url.containsFragment() || url.containsUserinfo()); } diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 50bc1655a634..0faa49e167cc 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -63,7 +63,6 @@ RUNTIME_GUARD(envoy_reloadable_features_http2_discard_host_header); RUNTIME_GUARD(envoy_reloadable_features_http2_use_oghttp2); RUNTIME_GUARD(envoy_reloadable_features_http2_use_visitor_for_data); RUNTIME_GUARD(envoy_reloadable_features_http2_validate_authority_with_quiche); -RUNTIME_GUARD(envoy_reloadable_features_http_allow_partial_urls_in_referer); RUNTIME_GUARD(envoy_reloadable_features_http_filter_avoid_reentrant_local_reply); // Delay deprecation and decommission until UHV is enabled. RUNTIME_GUARD(envoy_reloadable_features_http_reject_path_with_fragment); diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index e4ffd9d7c9b5..d4be452377f3 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -280,20 +280,6 @@ TEST_F(ConnectionManagerUtilityTest, RemoveRefererIfFragmentIsFound) { EXPECT_TRUE(headers.get(Http::CustomHeaders::get().Referer).empty()); } -TEST_F(ConnectionManagerUtilityTest, AllowRefererIfFragmentIsFoundWithoutGuard) { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues( - {{"envoy.reloadable_features.http_allow_partial_urls_in_referer", "false"}}); - connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( - std::make_shared("10.0.0.1")); - ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true)); - TestRequestHeaderMapImpl headers{{"referer", "https://example.com/foo/bar/#fragment"}}; - EXPECT_EQ((MutateRequestRet{"10.0.0.1:0", true, Tracing::Reason::NotTraceable}), - callMutateRequestHeaders(headers, Protocol::Http2)); - EXPECT_EQ("https://example.com/foo/bar/#fragment", - headers.get(Http::CustomHeaders::get().Referer)[0]->value().getStringView()); -} - TEST_F(ConnectionManagerUtilityTest, RemoveRefererIfMalformedPath) { connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( std::make_shared("10.0.0.1")); @@ -314,20 +300,6 @@ TEST_F(ConnectionManagerUtilityTest, RemoveRefererIfUserinfoIncluded) { EXPECT_TRUE(headers.get(Http::CustomHeaders::get().Referer).empty()); } -TEST_F(ConnectionManagerUtilityTest, AllowRefererIfUserinfoIncludedWithoutGuard) { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues( - {{"envoy.reloadable_features.http_allow_partial_urls_in_referer", "false"}}); - connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( - std::make_shared("10.0.0.1")); - ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true)); - TestRequestHeaderMapImpl headers{{"referer", "https://user:info@example.com"}}; - EXPECT_EQ((MutateRequestRet{"10.0.0.1:0", true, Tracing::Reason::NotTraceable}), - callMutateRequestHeaders(headers, Protocol::Http2)); - EXPECT_EQ("https://user:info@example.com", - headers.get(Http::CustomHeaders::get().Referer)[0]->value().getStringView()); -} - TEST_F(ConnectionManagerUtilityTest, ValidRefererPassesSanitization) { connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( std::make_shared("10.0.0.1")); @@ -339,20 +311,6 @@ TEST_F(ConnectionManagerUtilityTest, ValidRefererPassesSanitization) { headers.get(Http::CustomHeaders::get().Referer)[0]->value().getStringView()); } -TEST_F(ConnectionManagerUtilityTest, ValidRefererPassesSanitizationWithoutGuard) { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues( - {{"envoy.reloadable_features.http_allow_partial_urls_in_referer", "false"}}); - connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( - std::make_shared("10.0.0.1")); - ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true)); - TestRequestHeaderMapImpl headers{{"referer", "https://example.com/"}}; - EXPECT_EQ((MutateRequestRet{"10.0.0.1:0", true, Tracing::Reason::NotTraceable}), - callMutateRequestHeaders(headers, Protocol::Http2)); - EXPECT_EQ("https://example.com/", - headers.get(Http::CustomHeaders::get().Referer)[0]->value().getStringView()); -} - TEST_F(ConnectionManagerUtilityTest, AlphaNumCharRefererPassesSanitization) { connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( std::make_shared("10.0.0.1")); @@ -375,19 +333,6 @@ TEST_F(ConnectionManagerUtilityTest, ValidPathOnlyRefererPassesSanitization) { headers.get(Http::CustomHeaders::get().Referer)[0]->value().getStringView()); } -TEST_F(ConnectionManagerUtilityTest, RemovePathOnlyRefererWithoutGuard) { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues( - {{"envoy.reloadable_features.http_allow_partial_urls_in_referer", "false"}}); - connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( - std::make_shared("10.0.0.1")); - ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true)); - TestRequestHeaderMapImpl headers{{"referer", "/foo/bar/"}}; - EXPECT_EQ((MutateRequestRet{"10.0.0.1:0", true, Tracing::Reason::NotTraceable}), - callMutateRequestHeaders(headers, Protocol::Http2)); - EXPECT_TRUE(headers.get(Http::CustomHeaders::get().Referer).empty()); -} - TEST_F(ConnectionManagerUtilityTest, ValidFileOnlyRefererPassesSanitization) { connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( std::make_shared("10.0.0.1"));