From dd2f3b96c800b27878d95e61635036ab947861c8 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Tue, 28 Jan 2025 00:33:54 -0500 Subject: [PATCH] runtime: deprecating consistent_header_validation (#38137) Risk Level: low Testing: n/a Docs Changes: n/a Release Notes: inline fixes https://github.com/envoyproxy/envoy/issues/38038 Signed-off-by: Alyssa Wilk --- changelogs/current.yaml | 3 +++ .../formatter/substitution_format_utility.cc | 17 ++++------------- source/common/runtime/runtime_features.cc | 1 - source/extensions/filters/common/expr/context.h | 13 +++---------- .../filters/common/expr/context_test.cc | 11 ----------- 5 files changed, 10 insertions(+), 35 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index c5eb24f4a9c5..ab7c7c00149e 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -34,6 +34,9 @@ bug_fixes: removed_config_or_runtime: # *Normally occurs at the end of the* :ref:`deprecation period ` +- area: http + change: | + Removed runtime guard ``envoy.reloadable_features.consistent_header_validation`` and legacy code paths. - area: http change: | Removed runtime guard ``envoy.reloadable_features.sanitize_http2_headers_without_nghttp2`` and legacy code paths. diff --git a/source/common/formatter/substitution_format_utility.cc b/source/common/formatter/substitution_format_utility.cc index 919783fa997c..a2190c0f0bc4 100644 --- a/source/common/formatter/substitution_format_utility.cc +++ b/source/common/formatter/substitution_format_utility.cc @@ -105,19 +105,10 @@ SubstitutionFormatUtils::parseSubcommandHeaders(absl::string_view subcommand) { absl::StrCat("More than 1 alternative header specified in token: ", subcommand)); } - if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.consistent_header_validation")) { - if (!Http::HeaderUtility::headerNameIsValid(absl::AsciiStrToLower(main_header)) || - !Http::HeaderUtility::headerNameIsValid(absl::AsciiStrToLower(alternative_header))) { - return absl::InvalidArgumentError( - "Invalid header configuration. Format string contains invalid characters."); - } - } else { - // The main and alternative header should not contain invalid characters {NUL, LR, CF}. - if (!Envoy::Http::validHeaderString(main_header) || - !Envoy::Http::validHeaderString(alternative_header)) { - return absl::InvalidArgumentError( - "Invalid header configuration. Format string contains null or newline."); - } + if (!Http::HeaderUtility::headerNameIsValid(absl::AsciiStrToLower(main_header)) || + !Http::HeaderUtility::headerNameIsValid(absl::AsciiStrToLower(alternative_header))) { + return absl::InvalidArgumentError( + "Invalid header configuration. Format string contains invalid characters."); } return {std::make_pair(main_header, alternative_header)}; } diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index a3e153393760..dbacee75de34 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -34,7 +34,6 @@ RUNTIME_GUARD(envoy_reloadable_features_async_host_selection); RUNTIME_GUARD(envoy_reloadable_features_avoid_dfp_cluster_removal_on_cds_update); RUNTIME_GUARD(envoy_reloadable_features_boolean_to_string_fix); RUNTIME_GUARD(envoy_reloadable_features_check_switch_protocol_websocket_handshake); -RUNTIME_GUARD(envoy_reloadable_features_consistent_header_validation); RUNTIME_GUARD(envoy_reloadable_features_dfp_fail_on_empty_host_header); RUNTIME_GUARD(envoy_reloadable_features_disallow_quic_client_udp_mmsg); RUNTIME_GUARD(envoy_reloadable_features_dns_nodata_noname_is_success); diff --git a/source/extensions/filters/common/expr/context.h b/source/extensions/filters/common/expr/context.h index 2f8b1c1e21ee..1e702a1b7d1a 100644 --- a/source/extensions/filters/common/expr/context.h +++ b/source/extensions/filters/common/expr/context.h @@ -177,16 +177,9 @@ template class HeadersWrapper : public google::api::expr::runtime::Cel return {}; } auto str = std::string(key.StringOrDie().value()); - if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.consistent_header_validation")) { - if (!Http::HeaderUtility::headerNameIsValid(str)) { - // Reject key if it is an invalid header string - return {}; - } - } else { - if (!::Envoy::Http::validHeaderString(str)) { - // Reject key if it is an invalid header string - return {}; - } + if (!Http::HeaderUtility::headerNameIsValid(str)) { + // Reject key if it is an invalid header string + return {}; } return convertHeaderEntry(arena_, ::Envoy::Http::HeaderUtility::getAllOfHeaderAsString( *value_, ::Envoy::Http::LowerCaseString(str))); diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc index 48464220cb2c..80893c755801 100644 --- a/test/extensions/filters/common/expr/context_test.cc +++ b/test/extensions/filters/common/expr/context_test.cc @@ -51,17 +51,6 @@ TEST(Context, InvalidRequest) { EXPECT_FALSE(header.has_value()); } -TEST(Context, InvalidRequestLegacy) { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues({{"envoy.reloadable_features.consistent_header_validation", "false"}}); - - Http::TestRequestHeaderMapImpl header_map{{"referer", "dogs.com"}}; - Protobuf::Arena arena; - HeadersWrapper headers(arena, &header_map); - auto header = headers[CelValue::CreateStringView("referer\n")]; - EXPECT_FALSE(header.has_value()); -} - TEST(Context, RequestAttributes) { NiceMock info; NiceMock empty_info;