chore: remove append_x_forwarded_host_idempotent runtime flag (envo…

…yproxy#32102) chore: remove `append_x_forwarded_host` runtime flag Signed-off-by: River Phillips <[email protected]>
phlax · Feb 1, 2024 · 6762bf3 · 6762bf3
1 parent 4148fb7
commit 6762bf3
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 27 deletions.
diff --git a/api/envoy/config/route/v3/route_components.proto b/api/envoy/config/route/v3/route_components.proto
@@ -1211,7 +1211,6 @@ message RouteAction {
   // :ref:`host_rewrite_path_regex <envoy_v3_api_field_config.route.v3.RouteAction.host_rewrite_path_regex>`)
   // causes the original value of the host header, if any, to be appended to the
   // :ref:`config_http_conn_man_headers_x-forwarded-host` HTTP header if it is different to the last value appended.
-  // This can be disabled by setting the runtime guard ``envoy_reloadable_features_append_xfh_idempotent`` to false.
   bool append_x_forwarded_host = 38;
 
   // Specifies the upstream timeout for the route. If not specified, the default is 15s. This

diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -29,6 +29,9 @@ removed_config_or_runtime:
 - area: overload manager
   change: |
     removed ``envoy.reloadable_features.overload_manager_error_unknown_action`` and legacy code paths.
+- area: http
+  change: |
+    Removed ``envoy_reloadable_features_append_xfh_idempotent`` runtime flag and legacy code paths.
 
 new_features:
 - area: aws_request_signing

diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc
@@ -467,22 +467,16 @@ void Utility::updateAuthority(RequestHeaderMap& headers, absl::string_view hostn
                               const bool append_xfh) {
   const auto host = headers.getHostValue();
 
-  if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.append_xfh_idempotent")) {
-    // Only append to x-forwarded-host if the value was not the last value appended.
-    const auto xfh = headers.getForwardedHostValue();
-
-    if (append_xfh && !host.empty()) {
-      if (!xfh.empty()) {
-        const auto xfh_split = StringUtil::splitToken(xfh, ",");
-        if (!xfh_split.empty() && xfh_split.back() != host) {
-          headers.appendForwardedHost(host, ",");
-        }
-      } else {
+  // Only append to x-forwarded-host if the value was not the last value appended.
+  const auto xfh = headers.getForwardedHostValue();
+
+  if (append_xfh && !host.empty()) {
+    if (!xfh.empty()) {
+      const auto xfh_split = StringUtil::splitToken(xfh, ",");
+      if (!xfh_split.empty() && xfh_split.back() != host) {
         headers.appendForwardedHost(host, ",");
       }
-    }
-  } else {
-    if (append_xfh && !host.empty()) {
+    } else {
       headers.appendForwardedHost(host, ",");
     }
   }

diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc
@@ -30,7 +30,6 @@
 // ASAP by filing a bug on github. Overriding non-buggy code is strongly discouraged to avoid the
 // problem of the bugs being found after the old code path has been removed.
 RUNTIME_GUARD(envoy_reloadable_features_abort_filter_chain_on_stream_reset);
-RUNTIME_GUARD(envoy_reloadable_features_append_xfh_idempotent);
 RUNTIME_GUARD(envoy_reloadable_features_avoid_zombie_streams);
 RUNTIME_GUARD(envoy_reloadable_features_check_mep_on_first_eject);
 RUNTIME_GUARD(envoy_reloadable_features_conn_pool_delete_when_idle);

diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc
@@ -527,23 +527,12 @@ TEST(HttpUtility, updateAuthority) {
 
   // Test that we only append to x-forwarded-host if it is not already present.
   {
-    TestScopedRuntime scoped_runtime;
-    scoped_runtime.mergeValues({{"envoy.reloadable_features.append_xfh_idempotent", "true"}});
     TestRequestHeaderMapImpl headers{{":authority", "dns.name"},
                                      {"x-forwarded-host", "host.com,dns.name"}};
     Utility::updateAuthority(headers, "newhost.com", true);
     EXPECT_EQ("newhost.com", headers.get_(":authority"));
     EXPECT_EQ("host.com,dns.name", headers.get_("x-forwarded-host"));
   }
-  {
-    TestScopedRuntime scoped_runtime;
-    scoped_runtime.mergeValues({{"envoy.reloadable_features.append_xfh_idempotent", "false"}});
-    TestRequestHeaderMapImpl headers{{":authority", "dns.name"},
-                                     {"x-forwarded-host", "host.com,dns.name"}};
-    Utility::updateAuthority(headers, "newhost.com", true);
-    EXPECT_EQ("newhost.com", headers.get_(":authority"));
-    EXPECT_EQ("host.com,dns.name,dns.name", headers.get_("x-forwarded-host"));
-  }
 }
 
 TEST(HttpUtility, createSslRedirectPath) {