From 959ed55b305a41f28cda13f683737f58fa3b04d5 Mon Sep 17 00:00:00 2001 From: wbpcode Date: Tue, 16 Nov 2021 08:39:35 +0000 Subject: [PATCH] header map: removed runtime envoy.reloadable_features.header_map_correctly_coalesce_cookies Signed-off-by: wbpcode --- docs/root/version_history/current.rst | 1 + source/common/http/header_map_impl.cc | 9 ++++--- source/common/http/header_map_impl.h | 2 -- source/common/runtime/runtime_features.cc | 1 - test/common/http/header_map_impl_test.cc | 4 ---- test/common/http/inline_cookie_test.cc | 24 ------------------- .../quic/envoy_quic_server_stream_test.cc | 11 ++++----- tools/code_format/check_format.py | 2 +- 8 files changed, 10 insertions(+), 44 deletions(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 0d9dcd8fa1d65..8e2f47a1b2bb0 100644 --- 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 index bfd77c8dcfa3a..1f7d51e357eb8 100644 --- 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 index 5f64bc311c5ca..441571b923f59 100644 --- 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 index b62c35bfc2496..f5f4388f92199 100644 --- 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 index ce4bb8a7ccd1e..1b95f4b49ad14 100644 --- 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 index 5d8269f81975e..b2d08a06682ab 100644 --- 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::LowerCaseString("header_for_compare")); - auto mock_snapshot = std::make_shared>(); - testing::NiceMock 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 index ee01d965e3fd1..d34d569bdb927 100644 --- 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 index b1c9853ec8091..09744dc0e2831 100755 --- 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