diff --git a/source/common/http/header_map_impl.cc b/source/common/http/header_map_impl.cc index ee6956d049d9b..3e2bd3b599adc 100644 --- a/source/common/http/header_map_impl.cc +++ b/source/common/http/header_map_impl.cc @@ -22,6 +22,9 @@ namespace { // This includes the NULL (StringUtil::itoa technically only needs 21). constexpr size_t MaxIntegerLength{32}; +constexpr absl::string_view DelimiterForInlineHeaders{","}; +constexpr absl::string_view DelimiterForInlineCookies{"; "}; + void validateCapacity(uint64_t new_capacity) { // If the resizing will cause buffer overflow due to hitting uint32_t::max, an OOM is likely // imminent. Fast-fail rather than allow a buffer overflow attack (issue #1421) @@ -46,6 +49,13 @@ 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) { + return DelimiterForInlineCookies; + } + return DelimiterForInlineHeaders; +} + } // namespace // Initialize as a Type::Inline @@ -368,8 +378,10 @@ 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 uint64_t added_size = - appendToHeader((*lookup.value().entry_)->value(), value.getStringView()); + appendToHeader((*lookup.value().entry_)->value(), value.getStringView(), delimiter); addSize(added_size); value.clear(); } @@ -434,10 +446,8 @@ 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 std::string delimiter = (key == Http::Headers::get().Cookie ? "; " : ","); - const uint64_t added_size = header_map_correctly_coalesce_cookies_ - ? appendToHeader(entry[0]->value(), value, delimiter) - : appendToHeader(entry[0]->value(), value); + const auto delimiter = delimiterByHeader(key, header_map_correctly_coalesce_cookies_); + const uint64_t added_size = appendToHeader(entry[0]->value(), value, delimiter); addSize(added_size); } else { addCopy(key, value); diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 4409f8378ca7e..9fa5f12098952 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -254,44 +254,46 @@ bool maybeAdjustForIpv6(absl::string_view absolute_url, uint64_t& offset, uint64 return true; } -std::string parseCookie(const HeaderMap& headers, const std::string& key, - const std::string& cookie) { - - std::string ret; - - headers.iterateReverse([&key, &ret, &cookie](const HeaderEntry& header) -> HeaderMap::Iterate { - // Find the cookie headers in the request (typically, there's only one). - if (header.key() == cookie) { - - // Split the cookie header into individual cookies. - for (const auto& s : StringUtil::splitToken(header.value().getStringView(), ";")) { - // Find the key part of the cookie (i.e. the name of the cookie). - size_t first_non_space = s.find_first_not_of(' '); - size_t equals_index = s.find('='); - if (equals_index == absl::string_view::npos) { - // The cookie is malformed if it does not have an `=`. Continue - // checking other cookies in this header. - continue; - } - const absl::string_view k = s.substr(first_non_space, equals_index - first_non_space); - // If the key matches, parse the value from the rest of the cookie string. - if (k == key) { - absl::string_view v = s.substr(equals_index + 1, s.size() - 1); - - // Cookie values may be wrapped in double quotes. - // https://tools.ietf.org/html/rfc6265#section-4.1.1 - if (v.size() >= 2 && v.back() == '"' && v[0] == '"') { - v = v.substr(1, v.size() - 2); - } - ret = std::string{v}; - return HeaderMap::Iterate::Break; - } +absl::string_view parseCookie(absl::string_view cookie_value, absl::string_view key) { + // Split the cookie header into individual cookies. + for (const auto& s : StringUtil::splitToken(cookie_value, ";")) { + // Find the key part of the cookie (i.e. the name of the cookie). + size_t first_non_space = s.find_first_not_of(' '); + size_t equals_index = s.find('='); + if (equals_index == absl::string_view::npos) { + // The cookie is malformed if it does not have an `=`. Continue + // checking other cookies in this header. + continue; + } + absl::string_view k = s.substr(first_non_space, equals_index - first_non_space); + // If the key matches, parse the value from the rest of the cookie string. + if (k == key) { + absl::string_view v = s.substr(equals_index + 1, s.size() - 1); + + // Cookie values may be wrapped in double quotes. + // https://tools.ietf.org/html/rfc6265#section-4.1.1 + if (v.size() >= 2 && v.back() == '"' && v[0] == '"') { + v = v.substr(1, v.size() - 2); } + return v; } - return HeaderMap::Iterate::Continue; - }); + } + return EMPTY_STRING; +} - return ret; +std::string parseCookie(const HeaderMap& headers, const std::string& key, + const LowerCaseString& cookie) { + const Http::HeaderMap::GetResult cookie_headers = headers.get(cookie); + + for (size_t index = 0; index < cookie_headers.size(); index++) { + auto cookie_header_value = cookie_headers[index]->value().getStringView(); + absl::string_view result = parseCookie(cookie_header_value, key); + if (!result.empty()) { + return std::string{result}; + } + } + + return EMPTY_STRING; } bool Utility::Url::initialize(absl::string_view absolute_url, bool is_connect) { @@ -429,11 +431,12 @@ std::string Utility::stripQueryString(const HeaderString& path) { } std::string Utility::parseCookieValue(const HeaderMap& headers, const std::string& key) { - return parseCookie(headers, key, Http::Headers::get().Cookie.get()); + // TODO(wbpcode): Modify the headers parameter type to 'RequestHeaderMap'. + return parseCookie(headers, key, Http::Headers::get().Cookie); } std::string Utility::parseSetCookieValue(const Http::HeaderMap& headers, const std::string& key) { - return parseCookie(headers, key, Http::Headers::get().SetCookie.get()); + return parseCookie(headers, key, Http::Headers::get().SetCookie); } std::string Utility::makeSetCookieValue(const std::string& key, const std::string& value, diff --git a/test/common/http/BUILD b/test/common/http/BUILD index df4b5763d2490..edd7e75664edf 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -319,6 +319,15 @@ envoy_cc_fuzz_test( ], ) +envoy_cc_test( + name = "inline_cookie_test", + srcs = ["inline_cookie_test.cc"], + deps = [ + "//source/common/http:header_map_lib", + "//test/mocks/runtime:runtime_mocks", + ], +) + envoy_cc_test( name = "header_utility_test", srcs = ["header_utility_test.cc"], diff --git a/test/common/http/inline_cookie_test.cc b/test/common/http/inline_cookie_test.cc new file mode 100644 index 0000000000000..5d8269f81975e --- /dev/null +++ b/test/common/http/inline_cookie_test.cc @@ -0,0 +1,61 @@ +#include "source/common/http/header_map_impl.h" +#include "source/common/http/header_utility.h" + +#include "test/mocks/runtime/mocks.h" +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Http { +namespace { + +// Test that the cookie header can work correctly after being registered as an inline header. The +// test will register the cookie as an inline header. In order to avoid affecting other tests, the +// test is placed in this separate source file. +TEST(InlineCookieTest, InlineCookieTest) { + Http::CustomInlineHeaderRegistry::registerInlineHeader( + Http::Headers::get().Cookie); + 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"}, + {"header_for_compare", "value2"}}; + + // Delimiter for inline 'cookie' header is specialized '; '. + EXPECT_EQ("key1:value1; key2:value2", headers.get_("cookie")); + // 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 +} // namespace Http +} // namespace Envoy