From afbe0c38ee5c532bf65889d9f07dc09aff16a4a7 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 4 Dec 2024 20:42:58 +0000 Subject: [PATCH] runtime: deprecate sanitize_te Signed-off-by: Alyssa Wilk --- changelogs/current.yaml | 3 + source/common/http/conn_manager_utility.cc | 4 -- source/common/runtime/runtime_features.cc | 1 - test/common/http/conn_manager_utility_test.cc | 12 ---- test/integration/header_integration_test.cc | 72 ------------------- test/integration/protocol_integration_test.cc | 6 -- 6 files changed, 3 insertions(+), 95 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index f782b138a937..5d729bc1ff2d 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -162,6 +162,9 @@ removed_config_or_runtime: - area: dns change: | Removed runtime flag ``envoy.reloadable_features.dns_reresolve_on_eai_again`` and legacy code paths. +- area: http + change: | + Removed runtime flag ``envoy.restart_features.sanitize_te`` and legacy code paths. - area: quic change: | Removed runtime flag ``envoy.restart_features.quic_handle_certs_with_shared_tls_code`` and legacy code paths. diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index ed6dca370dab..154705d4306b 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -295,10 +295,6 @@ ConnectionManagerUtility::MutateRequestHeadersResult ConnectionManagerUtility::m } void ConnectionManagerUtility::sanitizeTEHeader(RequestHeaderMap& request_headers) { - if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.sanitize_te")) { - return; - } - absl::string_view te_header = request_headers.getTEValue(); if (te_header.empty()) { return; diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 7667f36e66d4..a953ddd79249 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -86,7 +86,6 @@ RUNTIME_GUARD(envoy_reloadable_features_reject_invalid_yaml); RUNTIME_GUARD(envoy_reloadable_features_report_stream_reset_error_code); RUNTIME_GUARD(envoy_reloadable_features_sanitize_http2_headers_without_nghttp2); RUNTIME_GUARD(envoy_reloadable_features_sanitize_sni_in_access_log); -RUNTIME_GUARD(envoy_reloadable_features_sanitize_te); RUNTIME_GUARD(envoy_reloadable_features_skip_dns_lookup_for_proxied_requests); RUNTIME_GUARD(envoy_reloadable_features_streaming_shadow); RUNTIME_GUARD(envoy_reloadable_features_strict_duration_validation); diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 10c60f4cc07c..e118cf188c07 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -2299,17 +2299,5 @@ TEST_F(ConnectionManagerUtilityTest, DiscardTEHeaderWithoutTrailers) { EXPECT_EQ("", headers.getTEValue()); } -// Verify when TE header is present, the value should be kept if the reloadable feature -// "sanitize_te" is enabled. -TEST_F(ConnectionManagerUtilityTest, KeepTrailersTEHeaderReloadableFeatureDisabled) { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues({{"envoy.reloadable_features.sanitize_te", "false"}}); - - TestRequestHeaderMapImpl headers{{"te", "gzip"}}; - callMutateRequestHeaders(headers, Protocol::Http2); - - EXPECT_EQ("gzip", headers.getTEValue()); -} - } // namespace Http } // namespace Envoy diff --git a/test/integration/header_integration_test.cc b/test/integration/header_integration_test.cc index b3f49e4b462e..f3b316444dda 100644 --- a/test/integration/header_integration_test.cc +++ b/test/integration/header_integration_test.cc @@ -1339,78 +1339,6 @@ TEST_P(HeaderIntegrationTest, PathWithEscapedSlashesRedirected) { }); } -// Validates legacy TE handling: TE header is forwarded if it contains a supported value -TEST_P(HeaderIntegrationTest, TestTeHeaderPassthrough) { - config_helper_.addRuntimeOverride("envoy.reloadable_features.sanitize_te", "false"); - initializeFilter(HeaderMode::Append, false); - performRequest( - Http::TestRequestHeaderMapImpl{ - {":method", "GET"}, - {":path", "/"}, - {":scheme", "http"}, - {":authority", "no-headers.com"}, - {"x-request-foo", "downstram"}, - {"connection", "te, close"}, - {"te", "trailers"}, - }, - Http::TestRequestHeaderMapImpl{ - {":authority", "no-headers.com"}, - {":path", "/"}, - {":method", "GET"}, - {"x-request-foo", "downstram"}, - {"te", "trailers"}, - }, - Http::TestResponseHeaderMapImpl{ - {"server", "envoy"}, - {"content-length", "0"}, - {":status", "200"}, - {"x-return-foo", "upstream"}, - }, - Http::TestResponseHeaderMapImpl{ - {"server", "envoy"}, - {"x-return-foo", "upstream"}, - {":status", "200"}, - {"connection", "close"}, - }); -} - -// Validates legacy TE handling: that TE header stripped if it contains an unsupported value. -TEST_P(HeaderIntegrationTest, TestTeHeaderSanitized) { - config_helper_.addRuntimeOverride("envoy.reloadable_features.sanitize_te", "false"); - initializeFilter(HeaderMode::Append, false); - performRequest( - Http::TestRequestHeaderMapImpl{ - {":method", "GET"}, - {":path", "/"}, - {":scheme", "http"}, - {":authority", "no-headers.com"}, - {"x-request-foo", "downstram"}, - {"connection", "te, mike, sam, will, close"}, - {"te", "gzip"}, - {"mike", "foo"}, - {"sam", "bar"}, - {"will", "baz"}, - }, - Http::TestRequestHeaderMapImpl{ - {":authority", "no-headers.com"}, - {":path", "/"}, - {":method", "GET"}, - {"x-request-foo", "downstram"}, - }, - Http::TestResponseHeaderMapImpl{ - {"server", "envoy"}, - {"content-length", "0"}, - {":status", "200"}, - {"x-return-foo", "upstream"}, - }, - Http::TestResponseHeaderMapImpl{ - {"server", "envoy"}, - {"x-return-foo", "upstream"}, - {":status", "200"}, - {"connection", "close"}, - }); -} - using EmptyHeaderIntegrationTest = HttpProtocolIntegrationTest; using HeaderValueOption = envoy::config::core::v3::HeaderValueOption; diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index a6119814f2be..481c78045178 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -792,8 +792,6 @@ TEST_P(DownstreamProtocolIntegrationTest, TeSanitization) { } autonomous_upstream_ = true; - config_helper_.addRuntimeOverride("envoy.reloadable_features.sanitize_te", "true"); - default_request_headers_.setTE("gzip"); initialize(); @@ -815,8 +813,6 @@ TEST_P(DownstreamProtocolIntegrationTest, TeSanitizationTrailers) { } autonomous_upstream_ = true; - config_helper_.addRuntimeOverride("envoy.reloadable_features.sanitize_te", "true"); - default_request_headers_.setTE("trailers"); initialize(); @@ -838,8 +834,6 @@ TEST_P(DownstreamProtocolIntegrationTest, TeSanitizationTrailersMultipleValuesAn } autonomous_upstream_ = true; - config_helper_.addRuntimeOverride("envoy.reloadable_features.sanitize_te", "true"); - default_request_headers_.setTE("chunked;q=0.8 , trailers ,deflate "); initialize();