http: removing envoy.reloadable_features.http_set_copy_replace_all_he…

…aders (envoyproxy#16209)

Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: inline
Fixes envoyproxy#16018

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Gokul Nair <gnair@twitter.com>

docs/root/version_history/current.rst

@@ -32,6 +32,7 @@ Removed Config or Runtime
 
 * http: removed ``envoy.reloadable_features.allow_500_after_100`` runtime guard and the legacy code path.
 * http: removed ``envoy.reloadable_features.hcm_stream_error_on_invalid_message`` for disabling closing HTTP/1.1 connections on error. Connection-closing can still be disabled by setting the HTTP/1 configuration :ref:`override_stream_error_on_invalid_http_message <envoy_v3_api_field_config.core.v3.Http1ProtocolOptions.override_stream_error_on_invalid_http_message>`.
+* http: removed ``envoy.reloadable_features.http_set_copy_replace_all_headers`` runtime guard and legacy code paths.
 * http: removed ``envoy.reloadable_features.overload_manager_disable_keepalive_drain_http2``; Envoy will now always send GOAWAY to HTTP2 downstreams when the :ref:`disable_keepalive <config_overload_manager_overload_actions>` overload action is active.
 * http: removed ``envoy.reloadable_features.http_match_on_all_headers`` runtime guard and legacy code paths.
 * http: removed ``envoy.reloadable_features.unify_grpc_handling`` runtime guard and legacy code paths.

source/common/http/header_map_impl.cc

@@ -449,19 +449,8 @@ void HeaderMapImpl::setReferenceKey(const LowerCaseString& key, absl::string_vie
 }
 
 void HeaderMapImpl::setCopy(const LowerCaseString& key, absl::string_view value) {
-  if (!Runtime::runtimeFeatureEnabled(
-          "envoy.reloadable_features.http_set_copy_replace_all_headers")) {
-    auto entry = getExisting(key);
-    if (!entry.empty()) {
-      updateSize(entry[0]->value().size(), value.size());
-      entry[0]->value(value);
-    } else {
-      addCopy(key, value);
-    }
-  } else {
-    remove(key);
-    addCopy(key, value);
-  }
+  remove(key);
+  addCopy(key, value);
 }
 
 uint64_t HeaderMapImpl::byteSize() const { return cached_byte_size_; }

source/common/runtime/runtime_features.cc

@@ -71,7 +71,6 @@ constexpr const char* runtime_features[] = {
     "envoy.reloadable_features.hash_multiple_header_values",
     "envoy.reloadable_features.health_check.graceful_goaway_handling",
     "envoy.reloadable_features.health_check.immediate_failure_exclude_from_cluster",
-    "envoy.reloadable_features.http_set_copy_replace_all_headers",
     "envoy.reloadable_features.http_transport_failure_reason_in_body",
     "envoy.reloadable_features.http_upstream_wait_connect_response",
     "envoy.reloadable_features.http2_skip_encoding_empty_trailers",

test/common/http/header_map_impl_test.cc

@@ -794,55 +794,7 @@ TEST_P(HeaderMapImplTest, SetReferenceKey) {
   EXPECT_EQ("monde", headers.get(foo)[0]->value().getStringView());
 }
 
-TEST_P(HeaderMapImplTest, SetCopyOldBehavior) {
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.http_set_copy_replace_all_headers", "false"}});
-
-  TestRequestHeaderMapImpl headers;
-  LowerCaseString foo("hello");
-  headers.setCopy(foo, "world");
-  EXPECT_EQ("world", headers.get(foo)[0]->value().getStringView());
-
-  // Overwrite value.
-  headers.setCopy(foo, "monde");
-  EXPECT_EQ("monde", headers.get(foo)[0]->value().getStringView());
-
-  // Add another foo header.
-  headers.addCopy(foo, "monde2");
-  EXPECT_EQ(headers.size(), 2);
-
-  // Only the first foo header is overridden.
-  headers.setCopy(foo, "override-monde");
-  EXPECT_EQ(headers.size(), 2);
-
-  HeaderAndValueCb cb;
-
-  InSequence seq;
-  EXPECT_CALL(cb, Call("hello", "override-monde"));
-  EXPECT_CALL(cb, Call("hello", "monde2"));
-  headers.iterate(cb.asIterateCb());
-
-  // Test setting an empty string and then overriding.
-  EXPECT_EQ(2UL, headers.remove(foo));
-  EXPECT_EQ(headers.size(), 0);
-  const std::string empty;
-  headers.setCopy(foo, empty);
-  EXPECT_EQ(headers.size(), 1);
-  headers.setCopy(foo, "not-empty");
-  EXPECT_EQ(headers.get(foo)[0]->value().getStringView(), "not-empty");
-
-  // Use setCopy with inline headers both indirectly and directly.
-  headers.clear();
-  EXPECT_EQ(headers.size(), 0);
-  headers.setCopy(Headers::get().Path, "/");
-  EXPECT_EQ(headers.size(), 1);
-  EXPECT_EQ(headers.getPathValue(), "/");
-  headers.setPath("/foo");
-  EXPECT_EQ(headers.size(), 1);
-  EXPECT_EQ(headers.getPathValue(), "/foo");
-}
-
-TEST_P(HeaderMapImplTest, SetCopyNewBehavior) {
+TEST_P(HeaderMapImplTest, SetCopy) {
   TestRequestHeaderMapImpl headers;
   LowerCaseString foo("hello");
   headers.setCopy(foo, "world");