envoyproxy · mattklein123 · Nov 16, 2021 · Nov 16, 2021
diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
@@ -34,6 +34,7 @@ Removed Config or Runtime
 
 * compression: removed ``envoy.reloadable_features.enable_compression_without_content_length_header`` runtime guard and legacy code paths.
 * grpc-web: removed ``envoy.reloadable_features.grpc_web_fix_non_proto_encoded_response_handling`` and legacy code paths.
+* header map: removed ``envoy.reloadable_features.header_map_correctly_coalesce_cookies`` and legacy code paths.
 * health check: removed ``envoy.reloadable_features.health_check.immediate_failure_exclude_from_cluster`` runtime guard and legacy code paths.
 * http: removed ``envoy.reloadable_features.add_and_validate_scheme_header`` and legacy code paths.
 * http: removed ``envoy.reloadable_features.check_unsupported_typed_per_filter_config``, Envoy will always check unsupported typed per filter config if the filter isn't optional.

diff --git a/source/common/http/header_map_impl.cc b/source/common/http/header_map_impl.cc
@@ -49,8 +49,8 @@ bool validatedLowerCaseString(absl::string_view str) {
   return lower_case_str == str;
 }
 
-absl::string_view delimiterByHeader(const LowerCaseString& key, bool correctly_coalesce_cookies) {
-  if (correctly_coalesce_cookies && key == Http::Headers::get().Cookie) {
+absl::string_view delimiterByHeader(const LowerCaseString& key) {
+  if (key == Http::Headers::get().Cookie) {
     return DelimiterForInlineCookies;
   }
   return DelimiterForInlineHeaders;
@@ -378,8 +378,7 @@ void HeaderMapImpl::insertByKey(HeaderString&& key, HeaderString&& value) {
     if (*lookup.value().entry_ == nullptr) {
       maybeCreateInline(lookup.value().entry_, *lookup.value().key_, std::move(value));
     } else {
-      const auto delimiter =
-          delimiterByHeader(*lookup.value().key_, header_map_correctly_coalesce_cookies_);
+      const auto delimiter = delimiterByHeader(*lookup.value().key_);
       const uint64_t added_size =
           appendToHeader((*lookup.value().entry_)->value(), value.getStringView(), delimiter);
       addSize(added_size);
@@ -446,7 +445,7 @@ void HeaderMapImpl::appendCopy(const LowerCaseString& key, absl::string_view val
   // TODO(#9221): converge on and document a policy for coalescing multiple headers.
   auto entry = getExisting(key);
   if (!entry.empty()) {
-    const auto delimiter = delimiterByHeader(key, header_map_correctly_coalesce_cookies_);
+    const auto delimiter = delimiterByHeader(key);
     const uint64_t added_size = appendToHeader(entry[0]->value(), value, delimiter);
     addSize(added_size);
   } else {

diff --git a/source/common/http/header_map_impl.h b/source/common/http/header_map_impl.h
@@ -342,8 +342,6 @@ class HeaderMapImpl : NonCopyable {
   StatefulHeaderKeyFormatterPtr formatter_;
   // This holds the internal byte size of the HeaderMap.
   uint64_t cached_byte_size_ = 0;
-  const bool header_map_correctly_coalesce_cookies_ = Runtime::runtimeFeatureEnabled(
-      "envoy.reloadable_features.header_map_correctly_coalesce_cookies");
 };
 
 /**

diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc
@@ -90,7 +90,6 @@ constexpr const char* runtime_features[] = {
     "envoy.restart_features.explicit_wildcard_resource",
     "envoy.restart_features.use_apple_api_for_dns_lookups",
     // Misplaced flags: please do not add flags to this section.
-    "envoy.reloadable_features.header_map_correctly_coalesce_cookies",
     "envoy.reloadable_features.sanitize_http_header_referer",
     "envoy.reloadable_features.skip_dispatching_frames_for_closed_connection",
     // End misplaced flags: please do not add flags in this section.

diff --git a/test/common/http/header_map_impl_test.cc b/test/common/http/header_map_impl_test.cc
@@ -751,10 +751,6 @@ TEST_P(HeaderMapImplTest, DoubleCookieAdd) {
 }
 
 TEST_P(HeaderMapImplTest, AppendCookieHeadersWithSemicolon) {
-  if (!Runtime::runtimeFeatureEnabled(
-          "envoy.reloadable_features.header_map_correctly_coalesce_cookies")) {
-    return;
-  }
   TestRequestHeaderMapImpl headers;
   const std::string foo("foo=1");
   const std::string bar("bar=2");

diff --git a/test/common/http/inline_cookie_test.cc b/test/common/http/inline_cookie_test.cc
@@ -19,15 +19,7 @@ TEST(InlineCookieTest, InlineCookieTest) {
   Http::CustomInlineHeaderRegistry::registerInlineHeader<Http::RequestHeaderMap::header_map_type>(
       Http::LowerCaseString("header_for_compare"));
 
-  auto mock_snapshot = std::make_shared<testing::NiceMock<Runtime::MockSnapshot>>();
-  testing::NiceMock<Runtime::MockLoader> mock_loader;
-  Runtime::LoaderSingleton::initialize(&mock_loader);
-
   {
-    // Enable 'envoy.reloadable_features.header_map_correctly_coalesce_cookies' feature.
-    ON_CALL(mock_loader, threadsafeSnapshot()).WillByDefault(testing::Return(mock_snapshot));
-    ON_CALL(*mock_snapshot, runtimeFeatureEnabled(_)).WillByDefault(testing::Return(true));
-
     Http::TestRequestHeaderMapImpl headers{{"cookie", "key1:value1"},
                                            {"cookie", "key2:value2"},
                                            {"header_for_compare", "value1"},
@@ -38,22 +30,6 @@ TEST(InlineCookieTest, InlineCookieTest) {
     // Delimiter for inline 'header_for_compare' header is default ','.
     EXPECT_EQ("value1,value2", headers.get_("header_for_compare"));
   }
-
-  {
-    // Disable 'envoy.reloadable_features.header_map_correctly_coalesce_cookies' feature.
-    ON_CALL(mock_loader, threadsafeSnapshot()).WillByDefault(testing::Return(mock_snapshot));
-    ON_CALL(*mock_snapshot, runtimeFeatureEnabled(_)).WillByDefault(testing::Return(false));
-
-    Http::TestRequestHeaderMapImpl headers{{"cookie", "key1:value1"},
-                                           {"cookie", "key2:value2"},
-                                           {"header_for_compare", "value1"},
-                                           {"header_for_compare", "value2"}};
-
-    // 'envoy.reloadable_features.header_map_correctly_coalesce_cookies' is disabled then default
-    // ',' will be used as delimiter.
-    EXPECT_EQ("key1:value1,key2:value2", headers.get_("cookie"));
-    EXPECT_EQ("value1,value2", headers.get_("header_for_compare"));
-  }
 }
 
 } // namespace

diff --git a/test/common/quic/envoy_quic_server_stream_test.cc b/test/common/quic/envoy_quic_server_stream_test.cc
@@ -172,13 +172,10 @@ TEST_F(EnvoyQuicServerStreamTest, GetRequestAndResponse) {
         EXPECT_EQ(host_, headers->getHostValue());
         EXPECT_EQ("/", headers->getPathValue());
         EXPECT_EQ(Http::Headers::get().MethodValues.Get, headers->getMethodValue());
-        if (Runtime::runtimeFeatureEnabled(
-                "envoy.reloadable_features.header_map_correctly_coalesce_cookies")) {
-          // Verify that the duplicated headers are handled correctly before passing to stream
-          // decoder.
-          EXPECT_EQ("a=b; c=d",
-                    headers->get(Http::Headers::get().Cookie)[0]->value().getStringView());
-        }
+        // Verify that the duplicated headers are handled correctly before passing to stream
+        // decoder.
+        EXPECT_EQ("a=b; c=d",
+                  headers->get(Http::Headers::get().Cookie)[0]->value().getStringView());
       }));
   EXPECT_CALL(stream_decoder_, decodeData(BufferStringEqual(""), /*end_stream=*/true));
   spdy::SpdyHeaderBlock spdy_headers;

diff --git a/tools/code_format/check_format.py b/tools/code_format/check_format.py
@@ -268,7 +268,7 @@
   "envoy.reloadable_features.activate_timers_next_event_loop",
   "envoy.reloadable_features.grpc_json_transcoder_adhere_to_buffer_limits",
   "envoy.reloadable_features.upstream_http2_flood_checks",
-  "envoy.reloadable_features.header_map_correctly_coalesce_cookies",
+  "envoy.reloadable_features.sanitize_http_header_referer",
 }
 # yapf: enable