runtime: deprecating consistent_header_validation (#38137)

Risk Level: low Testing: n/a Docs Changes: n/a Release Notes: inline fixes #38038 Signed-off-by: Alyssa Wilk <[email protected]>
envoyproxy · Jan 28, 2025 · dd2f3b9 · dd2f3b9
1 parent ecce678
commit dd2f3b9
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 35 deletions.
diff --git 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 <deprecated>`
+- 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
@@ -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
@@ -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
@@ -177,16 +177,9 @@ template <class T> 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
@@ -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<Http::RequestHeaderMap> headers(arena, &header_map);
-  auto header = headers[CelValue::CreateStringView("referer\n")];
-  EXPECT_FALSE(header.has_value());
-}
-
 TEST(Context, RequestAttributes) {
   NiceMock<StreamInfo::MockStreamInfo> info;
   NiceMock<StreamInfo::MockStreamInfo> empty_info;