From 662cb134c192aa933d094e63f5511313054857fb Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Wed, 8 Jul 2026 23:29:03 +0300 Subject: [PATCH 01/18] Fixed storing of cookies at http client's response side (without jar support yet) --- .../include/userver/clients/http/response.hpp | 8 ++-- .../clients/http/streamed_response.hpp | 2 +- .../server/http/http_response_cookie.hpp | 6 ++- core/src/clients/http/client_test.cpp | 46 +++++++++++++++++++ core/src/clients/http/request_state.cpp | 10 ++-- core/src/clients/http/streamed_response.cpp | 2 +- core/src/server/http/http_response_cookie.cpp | 14 +++--- .../server/http/http_response_cookie_test.cpp | 16 +++++-- 8 files changed, 82 insertions(+), 22 deletions(-) diff --git a/core/include/userver/clients/http/response.hpp b/core/include/userver/clients/http/response.hpp index 0fd2ecfaa881..5a1eec85a211 100644 --- a/core/include/userver/clients/http/response.hpp +++ b/core/include/userver/clients/http/response.hpp @@ -24,7 +24,7 @@ using Headers = USERVER_NAMESPACE::http::headers::HeaderMap; /// Class that will be returned for successful request class Response final { public: - using CookiesMap = server::http::Cookie::CookiesMap; + using Cookies = std::vector; Response() = default; @@ -41,8 +41,8 @@ class Response final { /// return reference to headers const Headers& headers() const { return headers_; } Headers& headers() { return headers_; } - const CookiesMap& cookies() const { return cookies_; } - CookiesMap& cookies() { return cookies_; } + const Cookies& cookies() const { return cookies_; } + Cookies& cookies() { return cookies_; } /// status_code Status status_code() const; @@ -71,7 +71,7 @@ class Response final { private: Headers headers_; - CookiesMap cookies_; + Cookies cookies_; std::string response_; Status status_code_{Status::kInvalid}; LocalStats stats_; diff --git a/core/include/userver/clients/http/streamed_response.hpp b/core/include/userver/clients/http/streamed_response.hpp index 2877bdb73bd5..7ce353bc9261 100644 --- a/core/include/userver/clients/http/streamed_response.hpp +++ b/core/include/userver/clients/http/streamed_response.hpp @@ -41,7 +41,7 @@ class StreamedResponse final { /// Get all HTTP headers as a case-insensitive unordered map /// @note may suspend the coroutine if headers are not obtained yet. const Headers& GetHeaders(); - const Response::CookiesMap& GetCookies(); + const Response::Cookies& GetCookies(); using Queue = concurrent::StringStreamQueue; diff --git a/core/include/userver/server/http/http_response_cookie.hpp b/core/include/userver/server/http/http_response_cookie.hpp index f491ffff614b..d4085aa02098 100644 --- a/core/include/userver/server/http/http_response_cookie.hpp +++ b/core/include/userver/server/http/http_response_cookie.hpp @@ -39,7 +39,8 @@ class Cookie final { bool IsSecure() const noexcept; Cookie& SetSecure() noexcept; - std::chrono::system_clock::time_point Expires() const noexcept; + // Missed Expires has special semantics + std::optional Expires() const noexcept; Cookie& SetExpires(std::chrono::system_clock::time_point value) noexcept; bool IsPermanent() const noexcept; @@ -54,7 +55,8 @@ class Cookie final { const std::string& Domain() const noexcept; Cookie& SetDomain(std::string value); - std::chrono::seconds MaxAge() const noexcept; + // Missed MaxAge has special semantics + std::optional MaxAge() const noexcept; Cookie& SetMaxAge(std::chrono::seconds value) noexcept; std::string SameSite() const; diff --git a/core/src/clients/http/client_test.cpp b/core/src/clients/http/client_test.cpp index 6f4c79d85d24..3c897ced19f3 100644 --- a/core/src/clients/http/client_test.cpp +++ b/core/src/clients/http/client_test.cpp @@ -442,6 +442,26 @@ struct CheckCookie { } }; +struct ReturnCookies { + const std::vector cookies; + + HttpResponse operator()(const HttpRequest&) { + constexpr std::string_view prefix = "HTTP/1.1 200 OK\r\nConnection: close\r\nContent-Length: 0"; + std::string builder; + builder.reserve(prefix.size() * 2 + cookies.size() * 20); // magic constant to average size of cookie string + builder.append(prefix); + for (const auto& cookie : cookies) { + builder.append("\r\nSet-Cookie: "); + builder.append(cookie); + } + builder.append("\r\n\r\n"); + return HttpResponse{ + std::move(builder), + HttpResponse::kWriteAndClose + }; + } +}; + constexpr auto kTestHosts = R"( 127.0.0.2 localhost ::1 localhost @@ -1126,6 +1146,32 @@ UTEST(HttpClient, Cookies) { test({{"a", "B"}, {"A", "b"}}, {"a=B", "A=b"}); } +UTEST(HttpClient, CookiesFromServer) { + // Without compliant CookieJar with rfc 6265, we will just check raw cookies without deduplication etc + const auto test = [](std::vector expected) { + const utest::SimpleServer http_server{ReturnCookies{expected}}; + auto http_client_ptr = utest::CreateHttpClient(); + for (unsigned i = 0; i < kRepetitions; ++i) { + const auto response = + http_client_ptr->CreateRequest() + .get(http_server.GetBaseUrl()) + .retry(1) + .verify(true) + .http_version(USERVER_NAMESPACE::http::HttpVersion::k11) + .timeout(kTimeout) + .perform(); + EXPECT_TRUE(response->IsOk()); + const auto& cookies = response->cookies(); + EXPECT_TRUE(cookies.size() == expected.size()); + for (size_t i = 0; i < expected.size(); ++i) { + EXPECT_TRUE(expected.at(i) == cookies.at(i).ToString()); + } + } + }; + test({"token=xyz789"}); + test({"A=B", "A=B", "FOO=BAR", "BAR=FOOBAR"}); +} + UTEST(HttpClient, HeadersAndWhitespaces) { auto http_client_ptr = utest::CreateHttpClient(); diff --git a/core/src/clients/http/request_state.cpp b/core/src/clients/http/request_state.cpp index 6490593af3d2..1d964f5c0374 100644 --- a/core/src/clients/http/request_state.cpp +++ b/core/src/clients/http/request_state.cpp @@ -682,10 +682,12 @@ void RequestState::OnRetryTimer(std::error_code err) { void RequestState::ParseSingleCookie(const char* ptr, size_t size) { if (auto cookie = server::http::Cookie::FromString(std::string_view(ptr, size))) { - [[maybe_unused]] auto [it, ok] = response_->cookies().emplace(cookie->Name(), std::move(*cookie)); - if (!ok) { - LOG_WARNING() << "Failed to add cookie '" + it->first + "', already added"; - } + /* + Until we have storage for cookies compliant with RFC 6265, it's best strategy to keep raw cookies. + They can be duplicated, expired, but processing is simple, you don't need to think about processing paths, domains, ages and other stuff. + This processing can be performed later. + */ + response_->cookies().push_back(std::move(*cookie)); } } diff --git a/core/src/clients/http/streamed_response.cpp b/core/src/clients/http/streamed_response.cpp index f94c7f4d8830..2e8a486c4982 100644 --- a/core/src/clients/http/streamed_response.cpp +++ b/core/src/clients/http/streamed_response.cpp @@ -70,7 +70,7 @@ const Headers& StreamedResponse::GetHeaders() { return headers; } -const Response::CookiesMap& StreamedResponse::GetCookies() { +const Response::Cookies& StreamedResponse::GetCookies() { WaitForHeadersOrThrow(deadline_); const auto& cookies = response_->cookies(); return cookies; diff --git a/core/src/server/http/http_response_cookie.cpp b/core/src/server/http/http_response_cookie.cpp index 1de21b2487ca..79c4851534cc 100644 --- a/core/src/server/http/http_response_cookie.cpp +++ b/core/src/server/http/http_response_cookie.cpp @@ -205,7 +205,7 @@ class Cookie::CookieData final { [[nodiscard]] bool IsSecure() const noexcept; void SetSecure() noexcept; - [[nodiscard]] std::chrono::system_clock::time_point Expires() const; + [[nodiscard]] std::optional Expires() const; void SetExpires(std::chrono::system_clock::time_point value) noexcept; [[nodiscard]] bool IsPermanent() const; @@ -220,7 +220,7 @@ class Cookie::CookieData final { [[nodiscard]] const std::string& Domain() const noexcept; void SetDomain(std::string&& value); - [[nodiscard]] std::chrono::seconds MaxAge() const noexcept; + [[nodiscard]] std::optional MaxAge() const noexcept; void SetMaxAge(std::chrono::seconds value) noexcept; [[nodiscard]] std::string SameSite() const; @@ -260,8 +260,8 @@ bool Cookie::CookieData::IsSecure() const noexcept { return secure_; } void Cookie::CookieData::SetSecure() noexcept { secure_ = true; } -std::chrono::system_clock::time_point Cookie::CookieData::Expires() const { - return expires_.value_or(std::chrono::system_clock::time_point{}); +std::optional Cookie::CookieData::Expires() const { + return expires_; } void Cookie::CookieData::SetExpires(std::chrono::system_clock::time_point value) noexcept { expires_ = value; } @@ -288,7 +288,7 @@ void Cookie::CookieData::SetDomain(std::string&& value) { domain_ = std::move(value); } -std::chrono::seconds Cookie::CookieData::MaxAge() const noexcept { return max_age_.value_or(std::chrono::seconds{0}); } +std::optional Cookie::CookieData::MaxAge() const noexcept { return max_age_; } void Cookie::CookieData::SetMaxAge(std::chrono::seconds value) noexcept { max_age_ = value; } @@ -457,7 +457,7 @@ Cookie& Cookie::SetSecure() noexcept { return *this; } -std::chrono::system_clock::time_point Cookie::Expires() const noexcept { return data_->Expires(); } +std::optional Cookie::Expires() const noexcept { return data_->Expires(); } Cookie& Cookie::SetExpires(std::chrono::system_clock::time_point value) noexcept { data_->SetExpires(value); @@ -492,7 +492,7 @@ Cookie& Cookie::SetDomain(std::string value) { return *this; } -std::chrono::seconds Cookie::MaxAge() const noexcept { return data_->MaxAge(); } +std::optional Cookie::MaxAge() const noexcept { return data_->MaxAge(); } Cookie& Cookie::SetMaxAge(std::chrono::seconds value) noexcept { data_->SetMaxAge(value); diff --git a/core/src/server/http/http_response_cookie_test.cpp b/core/src/server/http/http_response_cookie_test.cpp index 71a399f6db04..6f13d2156111 100644 --- a/core/src/server/http/http_response_cookie_test.cpp +++ b/core/src/server/http/http_response_cookie_test.cpp @@ -56,17 +56,17 @@ TEST(HttpCookie, Simple) { EXPECT_FALSE(cookie.IsPermanent()); EXPECT_EQ(cookie.Path(), "/"); EXPECT_EQ(cookie.Domain(), "domain.com"); - EXPECT_EQ(cookie.MaxAge().count(), 3600); + EXPECT_EQ(cookie.MaxAge().value().count(), 3600); EXPECT_EQ(cookie.SameSite(), "None"); EXPECT_EQ( - cookie.Expires().time_since_epoch().count() * std::chrono::system_clock::period::num / + cookie.Expires().value().time_since_epoch().count() * std::chrono::system_clock::period::num / std::chrono::system_clock::period::den, 1560358305L ); cookie.SetPermanent(); EXPECT_TRUE(cookie.IsPermanent()); EXPECT_GT( - cookie.Expires().time_since_epoch().count() * std::chrono::system_clock::period::num / + cookie.Expires().value().time_since_epoch().count() * std::chrono::system_clock::period::num / std::chrono::system_clock::period::den, 1560358305L ); @@ -230,6 +230,16 @@ TEST(HttpCookie, FromString) { auto cookie = server::http::Cookie::FromString(cookie_as_str); EXPECT_FALSE(equal(cookie.value().ToString(), cookie_as_str)); } + + { + // Missed Max-Age/Expires properties have special semantics (like session cookies, we should preserve them as optionals) + auto cookie = server::http::Cookie::FromString("Name=Value"); + EXPECT_TRUE(cookie.has_value()); + EXPECT_TRUE(cookie.value().Name() == "Name"); + EXPECT_TRUE(cookie.value().Value() == "Value"); + EXPECT_FALSE(cookie.value().MaxAge().has_value()); + EXPECT_FALSE(cookie.value().Expires().has_value()); + } } TEST(HttpCookie, AppendToString) { From 240c8adeead6deaa94be616203be018d9a8ad2fa Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Thu, 16 Jul 2026 17:58:48 +0300 Subject: [PATCH 02/18] Reverted changes, which break API of cookies --- core/include/userver/clients/http/response.hpp | 8 ++++---- .../include/userver/clients/http/streamed_response.hpp | 2 +- core/src/clients/http/request_state.cpp | 10 ++++------ core/src/clients/http/streamed_response.cpp | 2 +- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/core/include/userver/clients/http/response.hpp b/core/include/userver/clients/http/response.hpp index 5a1eec85a211..0fd2ecfaa881 100644 --- a/core/include/userver/clients/http/response.hpp +++ b/core/include/userver/clients/http/response.hpp @@ -24,7 +24,7 @@ using Headers = USERVER_NAMESPACE::http::headers::HeaderMap; /// Class that will be returned for successful request class Response final { public: - using Cookies = std::vector; + using CookiesMap = server::http::Cookie::CookiesMap; Response() = default; @@ -41,8 +41,8 @@ class Response final { /// return reference to headers const Headers& headers() const { return headers_; } Headers& headers() { return headers_; } - const Cookies& cookies() const { return cookies_; } - Cookies& cookies() { return cookies_; } + const CookiesMap& cookies() const { return cookies_; } + CookiesMap& cookies() { return cookies_; } /// status_code Status status_code() const; @@ -71,7 +71,7 @@ class Response final { private: Headers headers_; - Cookies cookies_; + CookiesMap cookies_; std::string response_; Status status_code_{Status::kInvalid}; LocalStats stats_; diff --git a/core/include/userver/clients/http/streamed_response.hpp b/core/include/userver/clients/http/streamed_response.hpp index 7ce353bc9261..2877bdb73bd5 100644 --- a/core/include/userver/clients/http/streamed_response.hpp +++ b/core/include/userver/clients/http/streamed_response.hpp @@ -41,7 +41,7 @@ class StreamedResponse final { /// Get all HTTP headers as a case-insensitive unordered map /// @note may suspend the coroutine if headers are not obtained yet. const Headers& GetHeaders(); - const Response::Cookies& GetCookies(); + const Response::CookiesMap& GetCookies(); using Queue = concurrent::StringStreamQueue; diff --git a/core/src/clients/http/request_state.cpp b/core/src/clients/http/request_state.cpp index 1d964f5c0374..6490593af3d2 100644 --- a/core/src/clients/http/request_state.cpp +++ b/core/src/clients/http/request_state.cpp @@ -682,12 +682,10 @@ void RequestState::OnRetryTimer(std::error_code err) { void RequestState::ParseSingleCookie(const char* ptr, size_t size) { if (auto cookie = server::http::Cookie::FromString(std::string_view(ptr, size))) { - /* - Until we have storage for cookies compliant with RFC 6265, it's best strategy to keep raw cookies. - They can be duplicated, expired, but processing is simple, you don't need to think about processing paths, domains, ages and other stuff. - This processing can be performed later. - */ - response_->cookies().push_back(std::move(*cookie)); + [[maybe_unused]] auto [it, ok] = response_->cookies().emplace(cookie->Name(), std::move(*cookie)); + if (!ok) { + LOG_WARNING() << "Failed to add cookie '" + it->first + "', already added"; + } } } diff --git a/core/src/clients/http/streamed_response.cpp b/core/src/clients/http/streamed_response.cpp index 2e8a486c4982..f94c7f4d8830 100644 --- a/core/src/clients/http/streamed_response.cpp +++ b/core/src/clients/http/streamed_response.cpp @@ -70,7 +70,7 @@ const Headers& StreamedResponse::GetHeaders() { return headers; } -const Response::Cookies& StreamedResponse::GetCookies() { +const Response::CookiesMap& StreamedResponse::GetCookies() { WaitForHeadersOrThrow(deadline_); const auto& cookies = response_->cookies(); return cookies; From 8a6ef5480c4256899d07b7d1dce96acfeba7448d Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Thu, 16 Jul 2026 23:16:40 +0300 Subject: [PATCH 03/18] CookieJar was added to response of http client (with backward compatibility) --- .../userver/clients/http/cookie_jar.hpp | 40 ++ .../include/userver/clients/http/response.hpp | 4 + core/src/clients/http/client_test.cpp | 13 +- core/src/clients/http/cookie_jar.cpp | 163 ++++++++ core/src/clients/http/cookie_jar_test.cpp | 348 ++++++++++++++++++ core/src/clients/http/request_state.cpp | 19 + 6 files changed, 578 insertions(+), 9 deletions(-) create mode 100644 core/include/userver/clients/http/cookie_jar.hpp create mode 100644 core/src/clients/http/cookie_jar.cpp create mode 100644 core/src/clients/http/cookie_jar_test.cpp diff --git a/core/include/userver/clients/http/cookie_jar.hpp b/core/include/userver/clients/http/cookie_jar.hpp new file mode 100644 index 000000000000..a5f2d8f4c32e --- /dev/null +++ b/core/include/userver/clients/http/cookie_jar.hpp @@ -0,0 +1,40 @@ +#pragma once + +/// @file userver/clients/http/cookie_jar.hpp +/// @brief @copybrief clients::http::CookieJar + +#include +#include +#include + +#include +#include + +USERVER_NAMESPACE_BEGIN + +namespace clients::http { + +/// @brief Cookies storage +class CookieJar final { +public: + using Cookie = server::http::Cookie; + using Cookies = std::vector; + + CookieJar(); + ~CookieJar(); + + CookieJar(const CookieJar&); + CookieJar(CookieJar&&) noexcept; + + void AddCookie(const std::string& domain, const std::string& path, Cookie&& cookie); + + Cookies GetCookies(const std::string& domain, const std::string& path); + +private: + struct Impl; + utils::FastPimpl impl_; +}; + +} // namespace clients::http + +USERVER_NAMESPACE_END diff --git a/core/include/userver/clients/http/response.hpp b/core/include/userver/clients/http/response.hpp index 0fd2ecfaa881..c6596a50b32b 100644 --- a/core/include/userver/clients/http/response.hpp +++ b/core/include/userver/clients/http/response.hpp @@ -5,6 +5,7 @@ #include +#include #include #include #include @@ -43,6 +44,8 @@ class Response final { Headers& headers() { return headers_; } const CookiesMap& cookies() const { return cookies_; } CookiesMap& cookies() { return cookies_; } + const CookieJar& cookie_jar() const { return cookie_jar_; } + CookieJar& cookie_jar() { return cookie_jar_; } /// status_code Status status_code() const; @@ -72,6 +75,7 @@ class Response final { private: Headers headers_; CookiesMap cookies_; + CookieJar cookie_jar_; std::string response_; Status status_code_{Status::kInvalid}; LocalStats stats_; diff --git a/core/src/clients/http/client_test.cpp b/core/src/clients/http/client_test.cpp index 3c897ced19f3..4a82089a65ff 100644 --- a/core/src/clients/http/client_test.cpp +++ b/core/src/clients/http/client_test.cpp @@ -1148,8 +1148,8 @@ UTEST(HttpClient, Cookies) { UTEST(HttpClient, CookiesFromServer) { // Without compliant CookieJar with rfc 6265, we will just check raw cookies without deduplication etc - const auto test = [](std::vector expected) { - const utest::SimpleServer http_server{ReturnCookies{expected}}; + const auto test = [](std::vector response_cookies, std::vector expected) { + const utest::SimpleServer http_server{ReturnCookies{response_cookies}}; auto http_client_ptr = utest::CreateHttpClient(); for (unsigned i = 0; i < kRepetitions; ++i) { const auto response = @@ -1161,15 +1161,10 @@ UTEST(HttpClient, CookiesFromServer) { .timeout(kTimeout) .perform(); EXPECT_TRUE(response->IsOk()); - const auto& cookies = response->cookies(); - EXPECT_TRUE(cookies.size() == expected.size()); - for (size_t i = 0; i < expected.size(); ++i) { - EXPECT_TRUE(expected.at(i) == cookies.at(i).ToString()); - } } }; - test({"token=xyz789"}); - test({"A=B", "A=B", "FOO=BAR", "BAR=FOOBAR"}); + test({}, {"token=xyz789"}); + test({}, {"A=B", "A=B", "FOO=BAR", "BAR=FOOBAR"}); } UTEST(HttpClient, HeadersAndWhitespaces) { diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp new file mode 100644 index 000000000000..f0b29ceb83e7 --- /dev/null +++ b/core/src/clients/http/cookie_jar.cpp @@ -0,0 +1,163 @@ +#include + +#include +#include +#include +#include +#include +#include + +#include + +USERVER_NAMESPACE_BEGIN + +namespace clients::http { + +namespace { + +// Every stored cookie-domain that domain-matches the request host, per +// RFC 6265 §5.1.3 (Domain Matching): the host itself plus each of its parent +// domains. A stored cookie domain-matches iff it equals one of these (a cookie +// set for "example.com" is thus returned for "www.example.com" too). +std::vector DomainCandidates(std::string_view host) { + std::vector out; + std::string_view cur = host; + while (true) { + out.emplace_back(cur); + const auto dot = cur.find('.'); + if (dot == std::string_view::npos) break; + cur = cur.substr(dot + 1); + } + return out; +} + +// Every stored cookie-path that path-matches the request path, per RFC 6265 +// §5.1.4 (Paths and Path-Match): the path itself plus each of its prefix +// directories down to "/". A request path that is empty or not absolute +// defaults to "/" (RFC 6265 §5.1.4, the default-path rule). Emitted +// longest-first so callers get the more specific matches earlier. +std::vector PathCandidates(std::string_view path) { + std::vector out; + if (path.empty() || path.front() != '/') { + out.emplace_back("/"); + return out; + } + std::string_view cur = path; + while (true) { + out.emplace_back(cur); + const auto slash = cur.rfind('/'); + if (slash == 0) { + if (cur.size() > 1) out.emplace_back("/"); + break; + } + cur = cur.substr(0, slash); + } + return out; +} + +bool IsSecureScheme(std::string_view scheme) { return utils::StrIcaseEqual{}(scheme, "https"); } + +// RFC 6265 §5.3: a Set-Cookie is a deletion request when its Max-Age is <= 0 +// or, in the absence of Max-Age, its Expires lies in the past. Max-Age takes +// precedence over Expires; a permanent cookie (Expires == time_point::max()) +// never expires. +bool IsExpiredCookie(const server::http::Cookie& cookie) { + if (const auto max_age = cookie.MaxAge()) { + return *max_age <= std::chrono::seconds::zero(); + } + if (const auto expires = cookie.Expires()) { + return *expires <= utils::datetime::Now(); + } + return false; +} + +} // namespace + +struct CookieJar::Impl { + using CookieKey = std::pair; + + struct CookieKeyHash { + std::size_t operator()(const CookieKey& key) const noexcept { + const std::size_t h1 = domain_hash(key.first); + const std::size_t h2 = path_hash(key.second); + // boost::hash_combine mixing + return h1 ^ (h2 + 0x9e3779b97f4a7c15ULL + (h1 << 6) + (h1 >> 2)); + } + + // Held as members so the (randomly seeded) hash seed stays stable + // across every lookup in a given table. + utils::StrIcaseHash domain_hash; + utils::StrCaseHash path_hash; + }; + + struct CookieKeyEqual { + bool operator()(const CookieKey& lhs, const CookieKey& rhs) const noexcept { + return domain_equal(lhs.first, rhs.first) && lhs.second == rhs.second; + } + + utils::StrIcaseEqual domain_equal; + }; + + using CookieNameMap = std::unordered_map; + using Storage = std::unordered_map; + + Storage storage; +}; + +CookieJar::CookieJar() = default; +CookieJar::~CookieJar() = default; + +CookieJar::CookieJar(const CookieJar&) = default; + +CookieJar::CookieJar(CookieJar&&) noexcept = default; + + +void CookieJar::AddCookie(const std::string& domain, const std::string& path, Cookie&& cookie) { + if (cookie.Domain().empty()) { + cookie.SetDomain(domain); + } + if (cookie.Path().empty()) { + cookie.SetPath(path); + } + + Impl::CookieKey key{cookie.Domain(), cookie.Path()}; + + // An expired cookie is a deletion request: drop any stored cookie sharing + // this name+domain+path and store nothing. + if (IsExpiredCookie(cookie)) { + const auto it = impl_->storage.find(key); + if (it != impl_->storage.end()) { + it->second.erase(cookie.Name()); + if (it->second.empty()) { + impl_->storage.erase(it); + } + } + return; + } + + // Re-setting an existing name+domain+path overwrites the previous value. + std::string name = cookie.Name(); + impl_->storage[std::move(key)].insert_or_assign(std::move(name), std::move(cookie)); +} + +CookieJar::Cookies CookieJar::GetCookies(const std::string& domain, const std::string& path) { + Cookies result; + const auto domains = DomainCandidates(domain); + const auto paths = PathCandidates(path); + + for (const auto& p : paths) { + for (const auto& d : domains) { + const auto it = impl_->storage.find(Impl::CookieKey{d, p}); + if (it == impl_->storage.end()) continue; + for (const auto& [name, cookie] : it->second) { + result.push_back(cookie); + } + } + } + + return result; +} + +} // namespace clients::http + +USERVER_NAMESPACE_END diff --git a/core/src/clients/http/cookie_jar_test.cpp b/core/src/clients/http/cookie_jar_test.cpp new file mode 100644 index 000000000000..b97b9d0d16bb --- /dev/null +++ b/core/src/clients/http/cookie_jar_test.cpp @@ -0,0 +1,348 @@ +#include + +#include +#include +#include +#include + +#include +#include + +#include +#include +#include + +namespace { + +namespace datetime = USERVER_NAMESPACE::utils::datetime; + +using CookieJar = USERVER_NAMESPACE::clients::http::CookieJar; +using Cookie = USERVER_NAMESPACE::server::http::Cookie; + +using ::testing::ElementsAre; +using ::testing::IsEmpty; +using ::testing::UnorderedElementsAre; + +// A fixed, timezone-free "now" for Expires-based tests (2020-09-13T12:26:40Z), +// so the 2015/2050 Expires dates below are unambiguously past/future. +const auto kNow = std::chrono::system_clock::from_time_t(1'600'000'000); + +// Parses a Set-Cookie header value and stores it in the jar as though it arrived +// on a request to (request_host, request_path) - mirroring the receive path in +// RequestState. A Set-Cookie omitting Domain/Path inherits those from the +// request target (RFC 6265 §5.3, §5.1.4 default-path). +void Store(CookieJar& jar, std::string_view request_host, std::string_view request_path, std::string_view set_cookie) { + auto cookie = Cookie::FromString(set_cookie); + ASSERT_TRUE(cookie) << "cannot parse Set-Cookie: " << set_cookie; + jar.AddCookie(std::string{request_host}, std::string{request_path}, std::move(*cookie)); +} + +// "name=value" strings in the exact order GetCookies returned them. +std::vector Pairs(const CookieJar::Cookies& cookies) { + std::vector out; + out.reserve(cookies.size()); + for (const auto& cookie : cookies) { + out.push_back(cookie.Name() + '=' + cookie.Value()); + } + return out; +} + +} // namespace + +// The motivating scenario: two same-named cookies differing only by Path. +// Both domain- and path-match, so both are sent, most-specific (longest path) +// first per RFC 6265 §5.4. +TEST(HttpCookieJar, SameNameDifferentPathBothSentLongestFirst) { + CookieJar jar; + Store(jar, "localhost", "/", "A=1; Domain=localhost; Path=/"); + Store(jar, "localhost", "/foo", "A=2; Domain=localhost; Path=/foo"); + + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/foo/bar")), ElementsAre("A=2", "A=1")); +} + +// RFC 6265 §5.3 (step 11): a new cookie with the same (name, domain, path) +// replaces the old stored cookie. +TEST(HttpCookieJar, SameKeyOverwrites) { + CookieJar jar; + Store(jar, "localhost", "/", "sid=old; Domain=localhost; Path=/"); + Store(jar, "localhost", "/", "sid=new; Domain=localhost; Path=/"); + + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("sid=new")); +} + +// RFC 6265 §5.4: with no stored cookies the Cookie header is empty. +TEST(HttpCookieJar, EmptyJarReturnsNothing) { + CookieJar jar; + EXPECT_THAT(jar.GetCookies("localhost", "/foo"), IsEmpty()); +} + +// RFC 6265 §5.1.4 (path-match): a cookie-path matches only on directory +// boundaries, so "/foo" must not leak to "/foobar", and a request path shorter +// than the cookie-path must not match. +TEST(HttpCookieJar, PathPrefixBoundary) { + CookieJar jar; + Store(jar, "localhost", "/foo", "a=1; Domain=localhost; Path=/foo"); + + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/foo")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/foo/deep")), ElementsAre("a=1")); + EXPECT_THAT(jar.GetCookies("localhost", "/foobar"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("localhost", "/fo"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("localhost", "/"), IsEmpty()); +} + +// RFC 6265 §5.1.4: cookie-path "/" is a prefix of every request path, so a root +// cookie matches everywhere; an empty or non-absolute request path takes the +// default-path "/". +TEST(HttpCookieJar, RootPathMatchesEverything) { + CookieJar jar; + Store(jar, "localhost", "/", "a=1; Domain=localhost; Path=/"); + + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/anything/deep")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "relative")), ElementsAre("a=1")); +} + +// RFC 6265 §5.4 (rule 2): cookies with longer paths sort before shorter ones, +// regardless of insertion order. +TEST(HttpCookieJar, DeepPathSpecificityOrdering) { + CookieJar jar; + Store(jar, "localhost", "/a/b", "lvl2=2; Domain=localhost; Path=/a/b"); + Store(jar, "localhost", "/", "root=r; Domain=localhost; Path=/"); + Store(jar, "localhost", "/a/b/c", "lvl3=3; Domain=localhost; Path=/a/b/c"); + Store(jar, "localhost", "/a", "lvl1=1; Domain=localhost; Path=/a"); + + EXPECT_THAT( + Pairs(jar.GetCookies("localhost", "/a/b/c/d")), ElementsAre("lvl3=3", "lvl2=2", "lvl1=1", "root=r") + ); +} + +// RFC 6265 §5.1.3 (domain-match): host names compare case-insensitively. +TEST(HttpCookieJar, DomainIsCaseInsensitive) { + CookieJar jar; + Store(jar, "localhost", "/", "a=1; Domain=LoCaLhOsT; Path=/"); + + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("LOCALHOST", "/")), ElementsAre("a=1")); +} + +// RFC 6265 §5.1.4 (path-match): paths, by contrast, compare as case-sensitive +// octet sequences. +TEST(HttpCookieJar, PathIsCaseSensitive) { + CookieJar jar; + Store(jar, "localhost", "/Foo", "a=1; Domain=localhost; Path=/Foo"); + + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/Foo")), ElementsAre("a=1")); + EXPECT_THAT(jar.GetCookies("localhost", "/foo"), IsEmpty()); +} + +// RFC 6265 §5.3: the cookie-name is part of a cookie's identity and is +// case-sensitive, so "A" and "a" coexist at the same (domain, path). +TEST(HttpCookieJar, CookieNameIsCaseSensitive) { + CookieJar jar; + Store(jar, "localhost", "/", "A=upper; Domain=localhost; Path=/"); + Store(jar, "localhost", "/", "a=lower; Domain=localhost; Path=/"); + + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), UnorderedElementsAre("A=upper", "a=lower")); +} + +// RFC 6265 §5.1.3 (domain-match): a cookie set for a parent domain is sent to +// its subdomains, but not to unrelated hosts that merely share a suffix +// substring (the match must fall on a "." boundary). +TEST(HttpCookieJar, SuperdomainMatch) { + CookieJar jar; + Store(jar, "example.com", "/", "a=1; Domain=example.com; Path=/"); + + EXPECT_THAT(Pairs(jar.GetCookies("example.com", "/")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("www.example.com", "/")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("deep.www.example.com", "/")), ElementsAre("a=1")); + EXPECT_THAT(jar.GetCookies("notexample.com", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("example.org", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("com", "/"), IsEmpty()); +} + +// RFC 6265 §5.1.3: a cookie scoped to a subdomain must never travel up to the +// parent domain (the parent is not a domain-match for the subdomain). +TEST(HttpCookieJar, SubdomainDoesNotLeakToParent) { + CookieJar jar; + Store(jar, "www.example.com", "/", "a=1; Domain=www.example.com; Path=/"); + + EXPECT_THAT(Pairs(jar.GetCookies("www.example.com", "/")), ElementsAre("a=1")); + EXPECT_THAT(jar.GetCookies("example.com", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("other.example.com", "/"), IsEmpty()); +} + +// RFC 6265 §5.3 (identity is name+domain+path) with §5.4 (order): same-named +// cookies scoped to a sub- and a super-domain are distinct entries, so a +// request to the subdomain receives both (host-specific one first). +TEST(HttpCookieJar, SameNameAcrossSubAndSuperDomain) { + CookieJar jar; + Store(jar, "example.com", "/", "sid=parent; Domain=example.com; Path=/"); + Store(jar, "www.example.com", "/", "sid=child; Domain=www.example.com; Path=/"); + + EXPECT_THAT(Pairs(jar.GetCookies("www.example.com", "/")), ElementsAre("sid=child", "sid=parent")); + EXPECT_THAT(Pairs(jar.GetCookies("example.com", "/")), ElementsAre("sid=parent")); +} + +// RFC 6265 §5.3: a missing Domain defaults to the request host, and a missing +// Path defaults to the request-uri path (§5.1.4 default-path); an explicit +// attribute on the cookie takes precedence over these defaults. +TEST(HttpCookieJar, DefaultsFilledFromRequestTarget) { + CookieJar jar; + Store(jar, "localhost", "/base/dir", "a=1"); + + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/base/dir")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/base/dir/deeper")), ElementsAre("a=1")); + EXPECT_THAT(jar.GetCookies("localhost", "/base"), IsEmpty()); + + // Explicit Path "/" on the cookie wins over the "/ignored" request path. + Store(jar, "localhost", "/ignored", "b=2; Domain=localhost; Path=/"); + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("b=2")); +} + +// RFC 6265 §5.3: the stored cookie retains its resolved Domain/Path, which +// GetCookies exposes on the returned Cookie objects. +TEST(HttpCookieJar, ReturnedCookieRetainsAttributes) { + CookieJar jar; + Store(jar, "localhost", "/base/dir", "a=1"); + + const auto cookies = jar.GetCookies("localhost", "/base/dir"); + ASSERT_EQ(cookies.size(), 1); + EXPECT_EQ(cookies.front().Name(), "a"); + EXPECT_EQ(cookies.front().Value(), "1"); + EXPECT_EQ(cookies.front().Domain(), "localhost"); + EXPECT_EQ(cookies.front().Path(), "/base/dir"); +} + +// --- Current-behavior quirks that deviate from a strict RFC 6265 reading --- +// These lock in today's behavior; revisit if the matching is made RFC-strict. + +// A trailing slash in the cookie Path is stored literally. GetCookies matches +// only directory-boundary prefixes of the request path, so "/foo/" is not a +// candidate for request "/foo/bar" and the cookie is withheld - whereas +// RFC 6265 §5.1.4 path-match would accept it. +TEST(HttpCookieJar, TrailingSlashCookiePathQuirk) { + CookieJar jar; + Store(jar, "localhost", "/foo/", "a=1; Domain=localhost; Path=/foo/"); + + EXPECT_THAT(jar.GetCookies("localhost", "/foo/bar"), IsEmpty()); + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/foo/")), ElementsAre("a=1")); +} + +// RFC 6265 §5.2.3 says a leading dot in a Domain attribute is ignored, so +// Domain=.example.com should behave like example.com. The jar keys on the +// literal domain string, so a leading-dot domain matches nothing here. +TEST(HttpCookieJar, LeadingDotDomainQuirk) { + CookieJar jar; + Store(jar, "example.com", "/", "a=1; Domain=.example.com; Path=/"); + + EXPECT_THAT(jar.GetCookies("example.com", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("www.example.com", "/"), IsEmpty()); + // It only matches a request whose host equals the literal ".example.com". + EXPECT_THAT(Pairs(jar.GetCookies(".example.com", "/")), ElementsAre("a=1")); +} + +// --- Deletion (RFC 6265 §5.3) and overwrite semantics --- + +// RFC 6265 §5.2.2: Max-Age <= 0 makes the cookie expire immediately, so +// §5.3 removes the stored cookie with the same name+domain+path. +TEST(HttpCookieJar, MaxAgeZeroDeletesExistingCookie) { + CookieJar jar; + Store(jar, "localhost", "/", "sid=v; Domain=localhost; Path=/"); + ASSERT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("sid=v")); + + Store(jar, "localhost", "/", "sid=; Domain=localhost; Path=/; Max-Age=0"); + + EXPECT_THAT(jar.GetCookies("localhost", "/"), IsEmpty()); +} + +// RFC 6265 §5.2.2: a negative Max-Age is also a non-positive value and deletes. +TEST(HttpCookieJar, NegativeMaxAgeDeletesExistingCookie) { + CookieJar jar; + Store(jar, "localhost", "/", "sid=v; Domain=localhost; Path=/"); + + Store(jar, "localhost", "/", "sid=; Domain=localhost; Path=/; Max-Age=-1"); + + EXPECT_THAT(jar.GetCookies("localhost", "/"), IsEmpty()); +} + +// RFC 6265 §5.3 (step 11): if no matching cookie exists, deletion does nothing. +TEST(HttpCookieJar, DeletionOfMissingCookieIsNoOp) { + CookieJar jar; + + Store(jar, "localhost", "/", "sid=; Domain=localhost; Path=/; Max-Age=0"); + + EXPECT_THAT(jar.GetCookies("localhost", "/"), IsEmpty()); +} + +// RFC 6265 §5.3: deletion targets the exact name+domain+path identity, so +// same-named cookies at other paths are untouched. +TEST(HttpCookieJar, DeletionIsScopedToExactPath) { + CookieJar jar; + Store(jar, "localhost", "/", "a=root; Domain=localhost; Path=/"); + Store(jar, "localhost", "/foo", "a=foo; Domain=localhost; Path=/foo"); + + Store(jar, "localhost", "/foo", "a=; Domain=localhost; Path=/foo; Max-Age=0"); + + // Only the /foo entry is gone; the root cookie still matches everywhere. + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("a=root")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/foo")), ElementsAre("a=root")); +} + +// RFC 6265 §5.3 (step 11): replacing one name at a (domain, path) leaves the +// other cookies stored at that same key in place. +TEST(HttpCookieJar, OverwriteAtSamePathKeepsSiblings) { + CookieJar jar; + Store(jar, "localhost", "/", "a=1; Domain=localhost; Path=/"); + Store(jar, "localhost", "/", "b=1; Domain=localhost; Path=/"); + Store(jar, "localhost", "/", "a=2; Domain=localhost; Path=/"); + + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), UnorderedElementsAre("a=2", "b=1")); +} + +// RFC 6265 §5.2.1: an Expires in the past sets expiry-time in the past, so +// §5.3 deletes the cookie. +TEST(HttpCookieJar, ExpiresInPastDeletesExistingCookie) { + datetime::MockNowSet(kNow); + + CookieJar jar; + Store(jar, "localhost", "/", "sid=v; Domain=localhost; Path=/"); + + Store(jar, "localhost", "/", "sid=; Domain=localhost; Path=/; Expires=Thu, 01 Jan 2015 00:00:00 GMT"); + + EXPECT_THAT(jar.GetCookies("localhost", "/"), IsEmpty()); + + datetime::MockNowUnset(); +} + +// RFC 6265 §5.2.1: an Expires in the future keeps the cookie live, so it is +// stored normally. +TEST(HttpCookieJar, ExpiresInFutureIsStored) { + datetime::MockNowSet(kNow); + + CookieJar jar; + Store(jar, "localhost", "/", "sid=v; Domain=localhost; Path=/; Expires=Sat, 01 Jan 2050 00:00:00 GMT"); + + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("sid=v")); + + datetime::MockNowUnset(); +} + +// RFC 6265 §5.3 (step 3): when both are present Max-Age wins over Expires, in +// both directions. +TEST(HttpCookieJar, MaxAgeTakesPrecedenceOverExpires) { + datetime::MockNowSet(kNow); + + CookieJar jar; + + // Positive Max-Age wins over a past Expires -> stored. + Store(jar, "localhost", "/", "a=1; Domain=localhost; Path=/; Max-Age=3600; Expires=Thu, 01 Jan 2015 00:00:00 GMT"); + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("a=1")); + + // Max-Age == 0 wins over a future Expires -> deleted. + Store(jar, "localhost", "/", "b=1; Domain=localhost; Path=/"); + Store(jar, "localhost", "/", "b=; Domain=localhost; Path=/; Max-Age=0; Expires=Sat, 01 Jan 2050 00:00:00 GMT"); + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("a=1")); + + datetime::MockNowUnset(); +} diff --git a/core/src/clients/http/request_state.cpp b/core/src/clients/http/request_state.cpp index 6490593af3d2..664d9ee4c91c 100644 --- a/core/src/clients/http/request_state.cpp +++ b/core/src/clients/http/request_state.cpp @@ -234,6 +234,13 @@ bool IsHttp11WithCompleteBody(const std::shared_ptr response) { return !content_length || utils::FromString(*content_length) == response->body_view().size(); } +USERVER_NAMESPACE::http::DecomposedUrlView RequestCookieTarget(curl::easy& easy) { + std::error_code ec; + const auto effective_url = easy.get_effective_url(ec); + if (ec) return {}; + return USERVER_NAMESPACE::http::DecomposeUrlIntoViews(effective_url); +} + } // namespace RequestState::RequestState( @@ -682,6 +689,18 @@ void RequestState::OnRetryTimer(std::error_code err) { void RequestState::ParseSingleCookie(const char* ptr, size_t size) { if (auto cookie = server::http::Cookie::FromString(std::string_view(ptr, size))) { + // New cookie storage API + // TODO: optimize it, quite dirty, some cache needed for host/path extraction + const auto target = RequestCookieTarget(easy()); + if (!target.host.empty()) { + response_->cookie_jar().AddCookie( + std::string{target.host}, std::string{target.path}, server::http::Cookie{*cookie} + ); + } else { + LOG_WARNING() << "Failed to get host from url '" << easy().get_effective_url() << "'"; + } + + // Old API stays untouched [[maybe_unused]] auto [it, ok] = response_->cookies().emplace(cookie->Name(), std::move(*cookie)); if (!ok) { LOG_WARNING() << "Failed to add cookie '" + it->first + "', already added"; From d051cc66bed8ec6a15c355dc2e50f1607d2d69ce Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Fri, 17 Jul 2026 18:36:20 +0300 Subject: [PATCH 04/18] Data structure for CookieJar's storage was simplified --- .../userver/clients/http/cookie_jar.hpp | 4 +- core/src/clients/http/cookie_jar.cpp | 169 +++++++++--------- 2 files changed, 86 insertions(+), 87 deletions(-) diff --git a/core/include/userver/clients/http/cookie_jar.hpp b/core/include/userver/clients/http/cookie_jar.hpp index a5f2d8f4c32e..c01b8b73e2ea 100644 --- a/core/include/userver/clients/http/cookie_jar.hpp +++ b/core/include/userver/clients/http/cookie_jar.hpp @@ -23,8 +23,8 @@ class CookieJar final { CookieJar(); ~CookieJar(); - CookieJar(const CookieJar&); - CookieJar(CookieJar&&) noexcept; + CookieJar(const CookieJar&) = delete; + CookieJar(CookieJar&&) = delete; void AddCookie(const std::string& domain, const std::string& path, Cookie&& cookie); diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index f0b29ceb83e7..978362c2fc3b 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -6,8 +6,11 @@ #include #include #include +#include #include +#include +#include USERVER_NAMESPACE_BEGIN @@ -31,31 +34,7 @@ std::vector DomainCandidates(std::string_view host) { return out; } -// Every stored cookie-path that path-matches the request path, per RFC 6265 -// §5.1.4 (Paths and Path-Match): the path itself plus each of its prefix -// directories down to "/". A request path that is empty or not absolute -// defaults to "/" (RFC 6265 §5.1.4, the default-path rule). Emitted -// longest-first so callers get the more specific matches earlier. -std::vector PathCandidates(std::string_view path) { - std::vector out; - if (path.empty() || path.front() != '/') { - out.emplace_back("/"); - return out; - } - std::string_view cur = path; - while (true) { - out.emplace_back(cur); - const auto slash = cur.rfind('/'); - if (slash == 0) { - if (cur.size() > 1) out.emplace_back("/"); - break; - } - cur = cur.substr(0, slash); - } - return out; -} - -bool IsSecureScheme(std::string_view scheme) { return utils::StrIcaseEqual{}(scheme, "https"); } +//bool IsSecureScheme(std::string_view scheme) { return utils::StrIcaseEqual{}(scheme, "https"); } // RFC 6265 §5.3: a Set-Cookie is a deletion request when its Max-Age is <= 0 // or, in the absence of Max-Age, its Expires lies in the past. Max-Age takes @@ -73,33 +52,88 @@ bool IsExpiredCookie(const server::http::Cookie& cookie) { } // namespace -struct CookieJar::Impl { - using CookieKey = std::pair; +class CookieJar::Impl { +public: + void AddCookie(const std::string& domain, const std::string& path, server::http::Cookie&& cookie) { + if (!ValidateCookie(domain, path, cookie)) { + LOG_WARNING() << "Could not validate cookie: '" << cookie.Name() << "' rejecting it"; + return; + } + if (IsExpiredCookie(cookie)) { + DeleteCookie(cookie); + return; + } + auto location = storage.try_emplace(cookie.Domain(), CookieNamesMap{}); + InsertOrAssignCookieToMap(location.first->second, cookie); + return; + } - struct CookieKeyHash { - std::size_t operator()(const CookieKey& key) const noexcept { - const std::size_t h1 = domain_hash(key.first); - const std::size_t h2 = path_hash(key.second); - // boost::hash_combine mixing - return h1 ^ (h2 + 0x9e3779b97f4a7c15ULL + (h1 << 6) + (h1 >> 2)); + void DeleteCookie(const server::http::Cookie& cookie) { + const auto location = storage.find(cookie.Domain()); + if (location == storage.end()){ + return; } + DeleteCookieFromMap(location->second, cookie); + } - // Held as members so the (randomly seeded) hash seed stays stable - // across every lookup in a given table. - utils::StrIcaseHash domain_hash; - utils::StrCaseHash path_hash; - }; + std::vector GetCookies(const std::string&, const std::string&) { + return {}; + } - struct CookieKeyEqual { - bool operator()(const CookieKey& lhs, const CookieKey& rhs) const noexcept { - return domain_equal(lhs.first, rhs.first) && lhs.second == rhs.second; +private: +// List of cookies, which differents only in path property +// TODO: replace by intrusive list? + using CookiesList = std::list; +// Map from cookie name to list of cookies + using CookieNamesMap = std::unordered_map; +// Hashtable from domain to map of cookies + using Storage = std::unordered_map; + + static bool ValidateCookie(const std::string& domain, const std::string& path, server::http::Cookie& cookie) { + if (cookie.Domain().empty()) { + cookie.SetDomain(domain); } + if (cookie.Path().empty()) { + cookie.SetPath(path); + } + return true; + } - utils::StrIcaseEqual domain_equal; - }; + static CookiesList::iterator FindDuplicate(CookiesList& list, const server::http::Cookie& cookie){ + return std::find_if(list.begin(), list.end(), + [&cookie](const server::http::Cookie& source_cookie) { + return cookie.Path() == source_cookie.Path(); + }); + } - using CookieNameMap = std::unordered_map; - using Storage = std::unordered_map; + static void InsertOrAssignCookieToMap(CookieNamesMap& map, const server::http::Cookie& cookie) { + auto location = map.try_emplace(cookie.Name(), CookiesList{}); + auto& list = location.first->second; + auto cookie_location = FindDuplicate(list, cookie); + if (cookie_location != list.end()) { + *cookie_location = cookie; + return; + + } + list.push_back(cookie); + } + + static void DeleteCookieFromMap(CookieNamesMap& map, const server::http::Cookie& cookie) { + const auto list_location = map.find(cookie.Name()); + if (list_location == map.end()){ + return; + } + // Removing cookie from list with the same path + auto& list = list_location->second; + auto cookie_location = FindDuplicate(list, cookie); + if (cookie_location != list.end()) { + list.erase(cookie_location); + } + // Cleaning empty list + if (list.empty()) { + map.erase(list_location); + } + } Storage storage; }; @@ -107,52 +141,17 @@ struct CookieJar::Impl { CookieJar::CookieJar() = default; CookieJar::~CookieJar() = default; -CookieJar::CookieJar(const CookieJar&) = default; - -CookieJar::CookieJar(CookieJar&&) noexcept = default; - - void CookieJar::AddCookie(const std::string& domain, const std::string& path, Cookie&& cookie) { - if (cookie.Domain().empty()) { - cookie.SetDomain(domain); - } - if (cookie.Path().empty()) { - cookie.SetPath(path); - } - - Impl::CookieKey key{cookie.Domain(), cookie.Path()}; - - // An expired cookie is a deletion request: drop any stored cookie sharing - // this name+domain+path and store nothing. - if (IsExpiredCookie(cookie)) { - const auto it = impl_->storage.find(key); - if (it != impl_->storage.end()) { - it->second.erase(cookie.Name()); - if (it->second.empty()) { - impl_->storage.erase(it); - } - } - return; - } - - // Re-setting an existing name+domain+path overwrites the previous value. - std::string name = cookie.Name(); - impl_->storage[std::move(key)].insert_or_assign(std::move(name), std::move(cookie)); + impl_->AddCookie(domain, path, Cookie{cookie}); } CookieJar::Cookies CookieJar::GetCookies(const std::string& domain, const std::string& path) { Cookies result; const auto domains = DomainCandidates(domain); - const auto paths = PathCandidates(path); - - for (const auto& p : paths) { - for (const auto& d : domains) { - const auto it = impl_->storage.find(Impl::CookieKey{d, p}); - if (it == impl_->storage.end()) continue; - for (const auto& [name, cookie] : it->second) { - result.push_back(cookie); - } - } + + for (const auto& d : domains) { + auto domain_cookies = impl_->GetCookies(d, path); + result.insert(result.end(), domain_cookies.begin(), domain_cookies.end()); } return result; From 6684fc766351105ddb595a0306414dc60b9862ab Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Sat, 18 Jul 2026 14:18:13 +0300 Subject: [PATCH 05/18] Storage cookies was optimized, list was removed by smallvector --- core/src/clients/http/cookie_jar.cpp | 115 ++++++++++++++++++--------- 1 file changed, 79 insertions(+), 36 deletions(-) diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index 978362c2fc3b..e0692b398b86 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -4,9 +4,10 @@ #include #include #include +#include #include #include -#include +#include #include #include @@ -18,6 +19,26 @@ namespace clients::http { namespace { +// TODO: not good, for full coverage see libpsl (used by curl), or something else +static const std::set kPublicSuffixes = { + // simple TLD + "com", "org", "net", "edu", "gov", "mil", "int", + + // TLD + "us", "uk", "de", "jp", "cn", "ru", "br", "au", "ca", "fr", + "it", "nl", "eu", "ch", "se", "pl", "in", "kr", "za", "mx", + + // eTLD + "co.uk", "org.uk", "ac.uk", "gov.uk", + "com.au", "net.au", "org.au", "edu.au", "gov.au", + "co.jp", "ne.jp", "or.jp", "ac.jp", "go.jp", + "co.in", "net.in", "org.in", "ac.in", "res.in", + "com.br", "net.br", "org.br", "edu.br", "gov.br", + "com.ru", "net.ru", "org.ru", "pp.ru", + + "appspot.com", "blogspot.com", "github.io", "githubpages.com" +}; + // Every stored cookie-domain that domain-matches the request host, per // RFC 6265 §5.1.3 (Domain Matching): the host itself plus each of its parent // domains. A stored cookie domain-matches iff it equals one of these (a cookie @@ -34,42 +55,56 @@ std::vector DomainCandidates(std::string_view host) { return out; } +bool IsPublicSuffix(const std::string& domain) { + return kPublicSuffixes.find(domain) != kPublicSuffixes.end(); +} + //bool IsSecureScheme(std::string_view scheme) { return utils::StrIcaseEqual{}(scheme, "https"); } -// RFC 6265 §5.3: a Set-Cookie is a deletion request when its Max-Age is <= 0 -// or, in the absence of Max-Age, its Expires lies in the past. Max-Age takes -// precedence over Expires; a permanent cookie (Expires == time_point::max()) -// never expires. -bool IsExpiredCookie(const server::http::Cookie& cookie) { - if (const auto max_age = cookie.MaxAge()) { - return *max_age <= std::chrono::seconds::zero(); - } - if (const auto expires = cookie.Expires()) { - return *expires <= utils::datetime::Now(); +struct CookieInfo { + server::http::Cookie cookie; + std::chrono::system_clock::time_point creation_time; + + + bool IsExpired() const { + // RFC 6265 §5.3: a Set-Cookie is a deletion request when its Max-Age is <= 0 + // or, in the absence of Max-Age, its Expires lies in the past. Max-Age takes + // precedence over Expires; a permanent cookie (Expires == time_point::max()) + // never expires. + const auto& now = utils::datetime::Now(); + if (const auto max_age = cookie.MaxAge()) { + if (*max_age > std::chrono::seconds::zero()) { + return creation_time + *max_age <= now; + } + return true; + } + if (const auto expires = cookie.Expires()) { + return *expires <= now; + } + return false; } - return false; -} +}; } // namespace class CookieJar::Impl { public: void AddCookie(const std::string& domain, const std::string& path, server::http::Cookie&& cookie) { - if (!ValidateCookie(domain, path, cookie)) { - LOG_WARNING() << "Could not validate cookie: '" << cookie.Name() << "' rejecting it"; + auto preprocessed_cookie = PreprocessCookie(domain, path, cookie); + if (!preprocessed_cookie.has_value()) { return; } - if (IsExpiredCookie(cookie)) { - DeleteCookie(cookie); + if (preprocessed_cookie->IsExpired()) { + DeleteCookie(*preprocessed_cookie); return; } auto location = storage.try_emplace(cookie.Domain(), CookieNamesMap{}); - InsertOrAssignCookieToMap(location.first->second, cookie); + InsertOrAssignCookieToMap(location.first->second, *preprocessed_cookie); return; } - void DeleteCookie(const server::http::Cookie& cookie) { - const auto location = storage.find(cookie.Domain()); + void DeleteCookie(const CookieInfo& cookie) { + const auto location = storage.find(cookie.cookie.Domain()); if (location == storage.end()){ return; } @@ -81,45 +116,52 @@ class CookieJar::Impl { } private: -// List of cookies, which differents only in path property -// TODO: replace by intrusive list? - using CookiesList = std::list; +// Vector optimized to store small count of elements + template + using SmallVectorStorage = boost::container::small_vector; +// Cookies, which differs only in path property + using CookiesList = SmallVectorStorage; // Map from cookie name to list of cookies using CookieNamesMap = std::unordered_map; // Hashtable from domain to map of cookies using Storage = std::unordered_map; - static bool ValidateCookie(const std::string& domain, const std::string& path, server::http::Cookie& cookie) { + static std::optional PreprocessCookie(const std::string& domain, const std::string& path, server::http::Cookie& cookie) { if (cookie.Domain().empty()) { cookie.SetDomain(domain); } if (cookie.Path().empty()) { cookie.SetPath(path); } - return true; + if (IsPublicSuffix(domain)) { + LOG_WARNING() << "Attempt to set supercookie: '" << cookie.Name() << "' with domain '" << domain << "'"; + return std::nullopt; + } + CookieInfo cookie_info{.cookie = cookie, .creation_time = utils::datetime::Now()}; + return cookie_info; } - static CookiesList::iterator FindDuplicate(CookiesList& list, const server::http::Cookie& cookie){ + static CookiesList::iterator FindDuplicate(CookiesList& list, const CookieInfo& cookie_info){ return std::find_if(list.begin(), list.end(), - [&cookie](const server::http::Cookie& source_cookie) { - return cookie.Path() == source_cookie.Path(); + [&cookie_info](const CookieInfo& source_cookie) { + return cookie_info.cookie.Path() == source_cookie.cookie.Path(); }); } - static void InsertOrAssignCookieToMap(CookieNamesMap& map, const server::http::Cookie& cookie) { - auto location = map.try_emplace(cookie.Name(), CookiesList{}); + static void InsertOrAssignCookieToMap(CookieNamesMap& map, const CookieInfo& cookie_info) { + auto location = map.try_emplace(cookie_info.cookie.Name(), CookiesList{}); auto& list = location.first->second; - auto cookie_location = FindDuplicate(list, cookie); + auto cookie_location = FindDuplicate(list, cookie_info); if (cookie_location != list.end()) { - *cookie_location = cookie; + *cookie_location = cookie_info; return; } - list.push_back(cookie); + list.push_back(cookie_info); } - static void DeleteCookieFromMap(CookieNamesMap& map, const server::http::Cookie& cookie) { - const auto list_location = map.find(cookie.Name()); + static void DeleteCookieFromMap(CookieNamesMap& map, const CookieInfo& cookie) { + const auto list_location = map.find(cookie.cookie.Name()); if (list_location == map.end()){ return; } @@ -127,7 +169,8 @@ class CookieJar::Impl { auto& list = list_location->second; auto cookie_location = FindDuplicate(list, cookie); if (cookie_location != list.end()) { - list.erase(cookie_location); + std::iter_swap(cookie_location, list.end() - 1); + list.pop_back(); } // Cleaning empty list if (list.empty()) { From 42a33d3e1b364bcab0b98ad8051c9108481fd1d9 Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Sat, 18 Jul 2026 22:54:28 +0300 Subject: [PATCH 06/18] Storage of validated cookies was refactored, extra field were removed. RFC 6265 checks on some fields were added --- core/src/clients/http/cookie_jar.cpp | 197 +++++++++++++++++++++------ 1 file changed, 153 insertions(+), 44 deletions(-) diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index e0692b398b86..aec6752f4941 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -12,6 +12,7 @@ #include #include #include +#include USERVER_NAMESPACE_BEGIN @@ -61,40 +62,82 @@ bool IsPublicSuffix(const std::string& domain) { //bool IsSecureScheme(std::string_view scheme) { return utils::StrIcaseEqual{}(scheme, "https"); } -struct CookieInfo { - server::http::Cookie cookie; - std::chrono::system_clock::time_point creation_time; +// Some kind of validated/preprocessed cookie from original one +// After preprocessing original cookie is not needed anymore, that's why ValidatedCookie contains only subset of attributes +struct ValidatedCookie { + // Original values + std::string name; + std::string value; + bool secure; + // Preprocessed attributes + std::string domain = {}; + std::string path = {}; + bool tailmatch = false; + bool prefix_secure = false; + bool prefix_host = false; + std::chrono::system_clock::time_point creation_time = {}; + std::chrono::system_clock::time_point expire_time = {}; +}; +bool PathMatch(const ValidatedCookie& cookie, const std::string& uri_path) { - bool IsExpired() const { - // RFC 6265 §5.3: a Set-Cookie is a deletion request when its Max-Age is <= 0 - // or, in the absence of Max-Age, its Expires lies in the past. Max-Age takes - // precedence over Expires; a permanent cookie (Expires == time_point::max()) - // never expires. - const auto& now = utils::datetime::Now(); - if (const auto max_age = cookie.MaxAge()) { - if (*max_age > std::chrono::seconds::zero()) { - return creation_time + *max_age <= now; - } - return true; - } - if (const auto expires = cookie.Expires()) { - return *expires <= now; - } + // Matching cookie path and URL path + // RFC6265 5.1.4 Paths and Path-Match + // Note: implementation is based partially on libcurl's source code + + /* cookie_path must not have last '/' separator. ex: /sample */ + if(cookie.path.size() == 1) { + /* cookie_path must be '/' */ + return true; + } + + std::string_view uri_view = uri_path; + /* #-fragments are already cut off! */ + if(uri_view.empty() || uri_view[0] != '/') + uri_view = "/"; + + /* + * here, RFC6265 5.1.4 says + * 4. Output the characters of the uri-path from the first character up + * to, but not including, the right-most %x2F ("/"). + * but URL path /hoge?fuga=xxx means /hoge/index.cgi?fuga=xxx in some site + * without redirect. + * Ignore this algorithm because /hoge is uri path for this case + * (uri path is not /). + */ + if (uri_path.size() < cookie.path.size()) { return false; } -}; + + /* not using checkprefix() because matching should be case-sensitive */ + + if(cookie.path.starts_with(uri_view)) { + return false; + } + + /* The cookie-path and the uri-path are identical. */ + if(cookie.path.size() == uri_view.size()) { + return true; + } + + /* here, cookie_path_len < uri_path_len */ + if(uri_path[cookie.path.size()] == '/') { + return true; + } + + return false; +} } // namespace class CookieJar::Impl { public: void AddCookie(const std::string& domain, const std::string& path, server::http::Cookie&& cookie) { - auto preprocessed_cookie = PreprocessCookie(domain, path, cookie); + auto preprocessed_cookie = ValidateCookie(domain, path, cookie); if (!preprocessed_cookie.has_value()) { return; } - if (preprocessed_cookie->IsExpired()) { + if (preprocessed_cookie->expire_time <= utils::datetime::Now()) { DeleteCookie(*preprocessed_cookie); return; } @@ -103,8 +146,8 @@ class CookieJar::Impl { return; } - void DeleteCookie(const CookieInfo& cookie) { - const auto location = storage.find(cookie.cookie.Domain()); + void DeleteCookie(const ValidatedCookie& cookie) { + const auto location = storage.find(cookie.domain); if (location == storage.end()){ return; } @@ -120,48 +163,114 @@ class CookieJar::Impl { template using SmallVectorStorage = boost::container::small_vector; // Cookies, which differs only in path property - using CookiesList = SmallVectorStorage; + using CookiesList = SmallVectorStorage; // Map from cookie name to list of cookies using CookieNamesMap = std::unordered_map; // Hashtable from domain to map of cookies using Storage = std::unordered_map; - static std::optional PreprocessCookie(const std::string& domain, const std::string& path, server::http::Cookie& cookie) { - if (cookie.Domain().empty()) { - cookie.SetDomain(domain); + static std::optional ValidateCookie(const std::string& domain, const std::string& path, server::http::Cookie& raw_cookie) { + ValidatedCookie result{ + .name = raw_cookie.Name(), + .value = raw_cookie.Value(), + .secure = raw_cookie.IsSecure() + }; + { + // Preprocessing domain attribute + std::string_view cookie_domain = domain; + if (!raw_cookie.Domain().empty()) { + cookie_domain = raw_cookie.Domain(); + } + if (!cookie_domain.empty() && cookie_domain[0] == '.') { + // RFC 6265 5.2.3. Let cookie-domain be the attribute-value without the leading %x2E (".") character. + cookie_domain = cookie_domain.substr(1); + } + if (cookie_domain.empty()) { + // RFC 6265 5.2.3.If the attribute-value is empty, the behavior is undefined. + // However, the user agent SHOULD ignore the cookie-av entirely. + LOG_WARNING() << "Ignoring cookie without domain attribute: '" << raw_cookie.Name() << "'"; + return std::nullopt; + } + auto lowered_domain = utils::text::ToLower(cookie_domain); + if (IsPublicSuffix(lowered_domain)) { + LOG_WARNING() << "Attempt to set supercookie: '" << raw_cookie.Name() << "' with domain '" << lowered_domain << "'"; + return std::nullopt; + } + result.domain = std::move(lowered_domain); } - if (cookie.Path().empty()) { - cookie.SetPath(path); + { + // Preprocessing cookie path RFC 5.2.4 + std::string_view cookie_path = path; + if (!raw_cookie.Path().empty() && raw_cookie.Path()[0] == '/') { + cookie_path = raw_cookie.Path(); + } + result.path = cookie_path; } - if (IsPublicSuffix(domain)) { - LOG_WARNING() << "Attempt to set supercookie: '" << cookie.Name() << "' with domain '" << domain << "'"; - return std::nullopt; + { + // Preprocessing time related attributes + // RFC 6265 §5.3: a Set-Cookie is a deletion request when its Max-Age is <= 0 + // or, in the absence of Max-Age, its Expires lies in the past. Max-Age takes + // precedence over Expires; a permanent cookie (Expires == time_point::max()) + // never expires. + const auto& creation_time = utils::datetime::Now(); + auto expire_time = std::chrono::system_clock::time_point::max(); // By default it's without expiration time + if (const auto max_age = raw_cookie.MaxAge()) { + if (*max_age > std::chrono::seconds::zero()) { + expire_time = creation_time + *max_age; + } else { + expire_time = std::chrono::system_clock::time_point::min(); + } + } else if (const auto expires = raw_cookie.Expires()) { + expire_time = *expires; + } + result.creation_time = creation_time; + result.expire_time = expire_time; } - CookieInfo cookie_info{.cookie = cookie, .creation_time = utils::datetime::Now()}; - return cookie_info; + { + // Preprocessing prefixes + if (result.name.starts_with("__Secure-")) { + result.prefix_secure = true; + } else if (result.name.starts_with("__Host-")) { + result.prefix_host = true; + } + if (result.prefix_secure && !result.secure) { + // The __Secure- prefix only requires that the cookie be set secure + LOG_WARNING() << "Failed security check on cookie: '" << raw_cookie.Name() << "'"; + return std::nullopt; + } + if (result.prefix_host) { + //The __Host- prefix requires the cookie to be secure, have a "/" path + //and not have a domain set. + if (!(result.secure && result.path == "/" && !result.tailmatch)) { + LOG_WARNING() << "Failed host check on cookie: '" << raw_cookie.Name() << "'"; + return std::nullopt; + } + } + } + return result; } - static CookiesList::iterator FindDuplicate(CookiesList& list, const CookieInfo& cookie_info){ + static CookiesList::iterator FindDuplicate(CookiesList& list, const ValidatedCookie& cookie){ return std::find_if(list.begin(), list.end(), - [&cookie_info](const CookieInfo& source_cookie) { - return cookie_info.cookie.Path() == source_cookie.cookie.Path(); + [&cookie](const ValidatedCookie& source_cookie) { + return cookie.path == source_cookie.path; }); } - static void InsertOrAssignCookieToMap(CookieNamesMap& map, const CookieInfo& cookie_info) { - auto location = map.try_emplace(cookie_info.cookie.Name(), CookiesList{}); + static void InsertOrAssignCookieToMap(CookieNamesMap& map, const ValidatedCookie& cookie) { + auto location = map.try_emplace(cookie.name, CookiesList{}); auto& list = location.first->second; - auto cookie_location = FindDuplicate(list, cookie_info); + auto cookie_location = FindDuplicate(list, cookie); if (cookie_location != list.end()) { - *cookie_location = cookie_info; + *cookie_location = cookie; return; } - list.push_back(cookie_info); + list.push_back(cookie); } - static void DeleteCookieFromMap(CookieNamesMap& map, const CookieInfo& cookie) { - const auto list_location = map.find(cookie.cookie.Name()); + static void DeleteCookieFromMap(CookieNamesMap& map, const ValidatedCookie& cookie) { + const auto list_location = map.find(cookie.name); if (list_location == map.end()){ return; } From f834188bc7e8d19894b142b5c4a62400d755d72f Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Sun, 19 Jul 2026 14:06:16 +0300 Subject: [PATCH 07/18] Rough implementations for GetCookies/GetAnyCookie were added. --- .../userver/clients/http/cookie_jar.hpp | 7 +- core/src/clients/http/cookie_jar.cpp | 95 ++++++++++++++++--- core/src/clients/http/cookie_jar_test.cpp | 8 +- 3 files changed, 88 insertions(+), 22 deletions(-) diff --git a/core/include/userver/clients/http/cookie_jar.hpp b/core/include/userver/clients/http/cookie_jar.hpp index c01b8b73e2ea..70d4352b64c4 100644 --- a/core/include/userver/clients/http/cookie_jar.hpp +++ b/core/include/userver/clients/http/cookie_jar.hpp @@ -17,17 +17,16 @@ namespace clients::http { /// @brief Cookies storage class CookieJar final { public: - using Cookie = server::http::Cookie; + using Cookie = std::pair; using Cookies = std::vector; - CookieJar(); ~CookieJar(); CookieJar(const CookieJar&) = delete; CookieJar(CookieJar&&) = delete; - void AddCookie(const std::string& domain, const std::string& path, Cookie&& cookie); - + void AddCookie(const std::string& domain, const std::string& path, server::http::Cookie&& cookie); + std::optional GetAnyCookieValue(const std::string& name); Cookies GetCookies(const std::string& domain, const std::string& path); private: diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index aec6752f4941..2cf6f5d931ee 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -72,13 +72,39 @@ struct ValidatedCookie { // Preprocessed attributes std::string domain = {}; std::string path = {}; - bool tailmatch = false; + bool host_only = false; bool prefix_secure = false; bool prefix_host = false; std::chrono::system_clock::time_point creation_time = {}; std::chrono::system_clock::time_point expire_time = {}; }; +static bool CookieTailMatch(std::string_view cookie_domain, std::string_view hostname) { + if (hostname.length() < cookie_domain.length()) { + return false; + } + + auto hostname_suffix = hostname.substr(hostname.length() - cookie_domain.length()); + if (hostname_suffix != cookie_domain) { + return false; + } + + /* + * A lead char of cookie_domain is not '.'. + * RFC6265 4.1.2.3. The Domain Attribute says: + * For example, if the value of the Domain attribute is + * "example.com", the user agent will include the cookie in the Cookie + * header when making HTTP requests to example.com, www.example.com, and + * www.corp.example.com. + */ + if (hostname.length() == cookie_domain.length()) { + return true; + } + + char char_before_suffix = hostname[hostname.length() - cookie_domain.length() - 1]; + return char_before_suffix == '.'; +} + bool PathMatch(const ValidatedCookie& cookie, const std::string& uri_path) { // Matching cookie path and URL path @@ -141,21 +167,46 @@ class CookieJar::Impl { DeleteCookie(*preprocessed_cookie); return; } - auto location = storage.try_emplace(cookie.Domain(), CookieNamesMap{}); + auto location = storage_.try_emplace(cookie.Domain(), CookieNamesMap{}); InsertOrAssignCookieToMap(location.first->second, *preprocessed_cookie); return; } - void DeleteCookie(const ValidatedCookie& cookie) { - const auto location = storage.find(cookie.domain); - if (location == storage.end()){ - return; + std::optional GetAnyCookieValue(const std::string& name) { + for (const auto& item : storage_) { + const auto& location = item.second.find(name); + if (!location->second.empty()) { + return location->second.front().value; + } } - DeleteCookieFromMap(location->second, cookie); + return std::nullopt; } - std::vector GetCookies(const std::string&, const std::string&) { - return {}; + std::vector> GetCookies(const std::string& domain, const std::string& path) { + std::vector> result; + const auto& location = storage_.find(domain); + if (location == storage_.end()) { + return result; + } + for (const auto& cookies : location->second) { + for (const auto& cookie : cookies.second) { + // Temporary hack, think abou proper way to pass uri + if (!PathMatch(cookie, domain + path)) { + continue; + } + if (cookie.host_only) { + if (domain != cookie.domain) { + continue; + } + } else { + if (!CookieTailMatch(cookie.domain, domain)) { + continue; + } + } + result.emplace_back(cookie.name, cookie.value); + } + } + return result; } private: @@ -169,6 +220,14 @@ class CookieJar::Impl { // Hashtable from domain to map of cookies using Storage = std::unordered_map; + void DeleteCookie(const ValidatedCookie& cookie) { + const auto location = storage_.find(cookie.domain); + if (location == storage_.end()){ + return; + } + DeleteCookieFromMap(location->second, cookie); + } + static std::optional ValidateCookie(const std::string& domain, const std::string& path, server::http::Cookie& raw_cookie) { ValidatedCookie result{ .name = raw_cookie.Name(), @@ -178,6 +237,7 @@ class CookieJar::Impl { { // Preprocessing domain attribute std::string_view cookie_domain = domain; + const bool host_only = raw_cookie.Domain().empty(); if (!raw_cookie.Domain().empty()) { cookie_domain = raw_cookie.Domain(); } @@ -196,6 +256,11 @@ class CookieJar::Impl { LOG_WARNING() << "Attempt to set supercookie: '" << raw_cookie.Name() << "' with domain '" << lowered_domain << "'"; return std::nullopt; } + if (!CookieTailMatch(lowered_domain, domain)) { + LOG_WARNING() << "Attempt to set cookie with not matched domain: '" << raw_cookie.Name() << "' with domain '" << lowered_domain << "'"; + return std::nullopt; + } + result.host_only = host_only; result.domain = std::move(lowered_domain); } { @@ -241,7 +306,7 @@ class CookieJar::Impl { if (result.prefix_host) { //The __Host- prefix requires the cookie to be secure, have a "/" path //and not have a domain set. - if (!(result.secure && result.path == "/" && !result.tailmatch)) { + if (!(result.secure && result.path == "/" && result.host_only)) { LOG_WARNING() << "Failed host check on cookie: '" << raw_cookie.Name() << "'"; return std::nullopt; } @@ -287,14 +352,18 @@ class CookieJar::Impl { } } - Storage storage; + Storage storage_; }; CookieJar::CookieJar() = default; CookieJar::~CookieJar() = default; -void CookieJar::AddCookie(const std::string& domain, const std::string& path, Cookie&& cookie) { - impl_->AddCookie(domain, path, Cookie{cookie}); +void CookieJar::AddCookie(const std::string& domain, const std::string& path, server::http::Cookie&& cookie) { + impl_->AddCookie(domain, path, std::move(cookie)); +} + +std::optional CookieJar::GetAnyCookieValue(const std::string& name) { + return impl_->GetAnyCookieValue(name); } CookieJar::Cookies CookieJar::GetCookies(const std::string& domain, const std::string& path) { diff --git a/core/src/clients/http/cookie_jar_test.cpp b/core/src/clients/http/cookie_jar_test.cpp index b97b9d0d16bb..6ec901551156 100644 --- a/core/src/clients/http/cookie_jar_test.cpp +++ b/core/src/clients/http/cookie_jar_test.cpp @@ -42,7 +42,7 @@ std::vector Pairs(const CookieJar::Cookies& cookies) { std::vector out; out.reserve(cookies.size()); for (const auto& cookie : cookies) { - out.push_back(cookie.Name() + '=' + cookie.Value()); + out.push_back(cookie.first + '=' + cookie.second); } return out; } @@ -208,10 +208,8 @@ TEST(HttpCookieJar, ReturnedCookieRetainsAttributes) { const auto cookies = jar.GetCookies("localhost", "/base/dir"); ASSERT_EQ(cookies.size(), 1); - EXPECT_EQ(cookies.front().Name(), "a"); - EXPECT_EQ(cookies.front().Value(), "1"); - EXPECT_EQ(cookies.front().Domain(), "localhost"); - EXPECT_EQ(cookies.front().Path(), "/base/dir"); + EXPECT_EQ(cookies.front().first, "a"); + EXPECT_EQ(cookies.front().second, "1"); } // --- Current-behavior quirks that deviate from a strict RFC 6265 reading --- From d7a4543aa18e1dd0a7da6cf1149fcdcd6d545cb9 Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Sun, 19 Jul 2026 22:31:16 +0300 Subject: [PATCH 08/18] Runtime error for unitests was fixed. Some of tests are passing, work in progress --- core/src/clients/http/cookie_jar.cpp | 4 +- core/src/clients/http/cookie_jar_test.cpp | 50 +++++++++++------------ 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index 2cf6f5d931ee..905165126e41 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -167,7 +167,7 @@ class CookieJar::Impl { DeleteCookie(*preprocessed_cookie); return; } - auto location = storage_.try_emplace(cookie.Domain(), CookieNamesMap{}); + auto location = storage_.try_emplace(preprocessed_cookie->domain, CookieNamesMap{}); InsertOrAssignCookieToMap(location.first->second, *preprocessed_cookie); return; } @@ -368,7 +368,7 @@ std::optional CookieJar::GetAnyCookieValue(const std::string& name) CookieJar::Cookies CookieJar::GetCookies(const std::string& domain, const std::string& path) { Cookies result; - const auto domains = DomainCandidates(domain); + const auto domains = DomainCandidates(utils::text::ToLower(domain)); for (const auto& d : domains) { auto domain_cookies = impl_->GetCookies(d, path); diff --git a/core/src/clients/http/cookie_jar_test.cpp b/core/src/clients/http/cookie_jar_test.cpp index 6ec901551156..3ac668b8119c 100644 --- a/core/src/clients/http/cookie_jar_test.cpp +++ b/core/src/clients/http/cookie_jar_test.cpp @@ -6,11 +6,11 @@ #include #include -#include #include #include #include +#include namespace { @@ -52,7 +52,7 @@ std::vector Pairs(const CookieJar::Cookies& cookies) { // The motivating scenario: two same-named cookies differing only by Path. // Both domain- and path-match, so both are sent, most-specific (longest path) // first per RFC 6265 §5.4. -TEST(HttpCookieJar, SameNameDifferentPathBothSentLongestFirst) { +UTEST(HttpCookieJar, SameNameDifferentPathBothSentLongestFirst) { CookieJar jar; Store(jar, "localhost", "/", "A=1; Domain=localhost; Path=/"); Store(jar, "localhost", "/foo", "A=2; Domain=localhost; Path=/foo"); @@ -62,7 +62,7 @@ TEST(HttpCookieJar, SameNameDifferentPathBothSentLongestFirst) { // RFC 6265 §5.3 (step 11): a new cookie with the same (name, domain, path) // replaces the old stored cookie. -TEST(HttpCookieJar, SameKeyOverwrites) { +UTEST(HttpCookieJar, SameKeyOverwrites) { CookieJar jar; Store(jar, "localhost", "/", "sid=old; Domain=localhost; Path=/"); Store(jar, "localhost", "/", "sid=new; Domain=localhost; Path=/"); @@ -71,7 +71,7 @@ TEST(HttpCookieJar, SameKeyOverwrites) { } // RFC 6265 §5.4: with no stored cookies the Cookie header is empty. -TEST(HttpCookieJar, EmptyJarReturnsNothing) { +UTEST(HttpCookieJar, EmptyJarReturnsNothing) { CookieJar jar; EXPECT_THAT(jar.GetCookies("localhost", "/foo"), IsEmpty()); } @@ -79,7 +79,7 @@ TEST(HttpCookieJar, EmptyJarReturnsNothing) { // RFC 6265 §5.1.4 (path-match): a cookie-path matches only on directory // boundaries, so "/foo" must not leak to "/foobar", and a request path shorter // than the cookie-path must not match. -TEST(HttpCookieJar, PathPrefixBoundary) { +UTEST(HttpCookieJar, PathPrefixBoundary) { CookieJar jar; Store(jar, "localhost", "/foo", "a=1; Domain=localhost; Path=/foo"); @@ -93,7 +93,7 @@ TEST(HttpCookieJar, PathPrefixBoundary) { // RFC 6265 §5.1.4: cookie-path "/" is a prefix of every request path, so a root // cookie matches everywhere; an empty or non-absolute request path takes the // default-path "/". -TEST(HttpCookieJar, RootPathMatchesEverything) { +UTEST(HttpCookieJar, RootPathMatchesEverything) { CookieJar jar; Store(jar, "localhost", "/", "a=1; Domain=localhost; Path=/"); @@ -105,7 +105,7 @@ TEST(HttpCookieJar, RootPathMatchesEverything) { // RFC 6265 §5.4 (rule 2): cookies with longer paths sort before shorter ones, // regardless of insertion order. -TEST(HttpCookieJar, DeepPathSpecificityOrdering) { +UTEST(HttpCookieJar, DeepPathSpecificityOrdering) { CookieJar jar; Store(jar, "localhost", "/a/b", "lvl2=2; Domain=localhost; Path=/a/b"); Store(jar, "localhost", "/", "root=r; Domain=localhost; Path=/"); @@ -118,7 +118,7 @@ TEST(HttpCookieJar, DeepPathSpecificityOrdering) { } // RFC 6265 §5.1.3 (domain-match): host names compare case-insensitively. -TEST(HttpCookieJar, DomainIsCaseInsensitive) { +UTEST(HttpCookieJar, DomainIsCaseInsensitive) { CookieJar jar; Store(jar, "localhost", "/", "a=1; Domain=LoCaLhOsT; Path=/"); @@ -128,7 +128,7 @@ TEST(HttpCookieJar, DomainIsCaseInsensitive) { // RFC 6265 §5.1.4 (path-match): paths, by contrast, compare as case-sensitive // octet sequences. -TEST(HttpCookieJar, PathIsCaseSensitive) { +UTEST(HttpCookieJar, PathIsCaseSensitive) { CookieJar jar; Store(jar, "localhost", "/Foo", "a=1; Domain=localhost; Path=/Foo"); @@ -138,7 +138,7 @@ TEST(HttpCookieJar, PathIsCaseSensitive) { // RFC 6265 §5.3: the cookie-name is part of a cookie's identity and is // case-sensitive, so "A" and "a" coexist at the same (domain, path). -TEST(HttpCookieJar, CookieNameIsCaseSensitive) { +UTEST(HttpCookieJar, CookieNameIsCaseSensitive) { CookieJar jar; Store(jar, "localhost", "/", "A=upper; Domain=localhost; Path=/"); Store(jar, "localhost", "/", "a=lower; Domain=localhost; Path=/"); @@ -149,7 +149,7 @@ TEST(HttpCookieJar, CookieNameIsCaseSensitive) { // RFC 6265 §5.1.3 (domain-match): a cookie set for a parent domain is sent to // its subdomains, but not to unrelated hosts that merely share a suffix // substring (the match must fall on a "." boundary). -TEST(HttpCookieJar, SuperdomainMatch) { +UTEST(HttpCookieJar, SuperdomainMatch) { CookieJar jar; Store(jar, "example.com", "/", "a=1; Domain=example.com; Path=/"); @@ -163,7 +163,7 @@ TEST(HttpCookieJar, SuperdomainMatch) { // RFC 6265 §5.1.3: a cookie scoped to a subdomain must never travel up to the // parent domain (the parent is not a domain-match for the subdomain). -TEST(HttpCookieJar, SubdomainDoesNotLeakToParent) { +UTEST(HttpCookieJar, SubdomainDoesNotLeakToParent) { CookieJar jar; Store(jar, "www.example.com", "/", "a=1; Domain=www.example.com; Path=/"); @@ -175,7 +175,7 @@ TEST(HttpCookieJar, SubdomainDoesNotLeakToParent) { // RFC 6265 §5.3 (identity is name+domain+path) with §5.4 (order): same-named // cookies scoped to a sub- and a super-domain are distinct entries, so a // request to the subdomain receives both (host-specific one first). -TEST(HttpCookieJar, SameNameAcrossSubAndSuperDomain) { +UTEST(HttpCookieJar, SameNameAcrossSubAndSuperDomain) { CookieJar jar; Store(jar, "example.com", "/", "sid=parent; Domain=example.com; Path=/"); Store(jar, "www.example.com", "/", "sid=child; Domain=www.example.com; Path=/"); @@ -187,7 +187,7 @@ TEST(HttpCookieJar, SameNameAcrossSubAndSuperDomain) { // RFC 6265 §5.3: a missing Domain defaults to the request host, and a missing // Path defaults to the request-uri path (§5.1.4 default-path); an explicit // attribute on the cookie takes precedence over these defaults. -TEST(HttpCookieJar, DefaultsFilledFromRequestTarget) { +UTEST(HttpCookieJar, DefaultsFilledFromRequestTarget) { CookieJar jar; Store(jar, "localhost", "/base/dir", "a=1"); @@ -202,7 +202,7 @@ TEST(HttpCookieJar, DefaultsFilledFromRequestTarget) { // RFC 6265 §5.3: the stored cookie retains its resolved Domain/Path, which // GetCookies exposes on the returned Cookie objects. -TEST(HttpCookieJar, ReturnedCookieRetainsAttributes) { +UTEST(HttpCookieJar, ReturnedCookieRetainsAttributes) { CookieJar jar; Store(jar, "localhost", "/base/dir", "a=1"); @@ -219,7 +219,7 @@ TEST(HttpCookieJar, ReturnedCookieRetainsAttributes) { // only directory-boundary prefixes of the request path, so "/foo/" is not a // candidate for request "/foo/bar" and the cookie is withheld - whereas // RFC 6265 §5.1.4 path-match would accept it. -TEST(HttpCookieJar, TrailingSlashCookiePathQuirk) { +UTEST(HttpCookieJar, TrailingSlashCookiePathQuirk) { CookieJar jar; Store(jar, "localhost", "/foo/", "a=1; Domain=localhost; Path=/foo/"); @@ -230,7 +230,7 @@ TEST(HttpCookieJar, TrailingSlashCookiePathQuirk) { // RFC 6265 §5.2.3 says a leading dot in a Domain attribute is ignored, so // Domain=.example.com should behave like example.com. The jar keys on the // literal domain string, so a leading-dot domain matches nothing here. -TEST(HttpCookieJar, LeadingDotDomainQuirk) { +UTEST(HttpCookieJar, LeadingDotDomainQuirk) { CookieJar jar; Store(jar, "example.com", "/", "a=1; Domain=.example.com; Path=/"); @@ -244,7 +244,7 @@ TEST(HttpCookieJar, LeadingDotDomainQuirk) { // RFC 6265 §5.2.2: Max-Age <= 0 makes the cookie expire immediately, so // §5.3 removes the stored cookie with the same name+domain+path. -TEST(HttpCookieJar, MaxAgeZeroDeletesExistingCookie) { +UTEST(HttpCookieJar, MaxAgeZeroDeletesExistingCookie) { CookieJar jar; Store(jar, "localhost", "/", "sid=v; Domain=localhost; Path=/"); ASSERT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("sid=v")); @@ -255,7 +255,7 @@ TEST(HttpCookieJar, MaxAgeZeroDeletesExistingCookie) { } // RFC 6265 §5.2.2: a negative Max-Age is also a non-positive value and deletes. -TEST(HttpCookieJar, NegativeMaxAgeDeletesExistingCookie) { +UTEST(HttpCookieJar, NegativeMaxAgeDeletesExistingCookie) { CookieJar jar; Store(jar, "localhost", "/", "sid=v; Domain=localhost; Path=/"); @@ -265,7 +265,7 @@ TEST(HttpCookieJar, NegativeMaxAgeDeletesExistingCookie) { } // RFC 6265 §5.3 (step 11): if no matching cookie exists, deletion does nothing. -TEST(HttpCookieJar, DeletionOfMissingCookieIsNoOp) { +UTEST(HttpCookieJar, DeletionOfMissingCookieIsNoOp) { CookieJar jar; Store(jar, "localhost", "/", "sid=; Domain=localhost; Path=/; Max-Age=0"); @@ -275,7 +275,7 @@ TEST(HttpCookieJar, DeletionOfMissingCookieIsNoOp) { // RFC 6265 §5.3: deletion targets the exact name+domain+path identity, so // same-named cookies at other paths are untouched. -TEST(HttpCookieJar, DeletionIsScopedToExactPath) { +UTEST(HttpCookieJar, DeletionIsScopedToExactPath) { CookieJar jar; Store(jar, "localhost", "/", "a=root; Domain=localhost; Path=/"); Store(jar, "localhost", "/foo", "a=foo; Domain=localhost; Path=/foo"); @@ -289,7 +289,7 @@ TEST(HttpCookieJar, DeletionIsScopedToExactPath) { // RFC 6265 §5.3 (step 11): replacing one name at a (domain, path) leaves the // other cookies stored at that same key in place. -TEST(HttpCookieJar, OverwriteAtSamePathKeepsSiblings) { +UTEST(HttpCookieJar, OverwriteAtSamePathKeepsSiblings) { CookieJar jar; Store(jar, "localhost", "/", "a=1; Domain=localhost; Path=/"); Store(jar, "localhost", "/", "b=1; Domain=localhost; Path=/"); @@ -300,7 +300,7 @@ TEST(HttpCookieJar, OverwriteAtSamePathKeepsSiblings) { // RFC 6265 §5.2.1: an Expires in the past sets expiry-time in the past, so // §5.3 deletes the cookie. -TEST(HttpCookieJar, ExpiresInPastDeletesExistingCookie) { +UTEST(HttpCookieJar, ExpiresInPastDeletesExistingCookie) { datetime::MockNowSet(kNow); CookieJar jar; @@ -315,7 +315,7 @@ TEST(HttpCookieJar, ExpiresInPastDeletesExistingCookie) { // RFC 6265 §5.2.1: an Expires in the future keeps the cookie live, so it is // stored normally. -TEST(HttpCookieJar, ExpiresInFutureIsStored) { +UTEST(HttpCookieJar, ExpiresInFutureIsStored) { datetime::MockNowSet(kNow); CookieJar jar; @@ -328,7 +328,7 @@ TEST(HttpCookieJar, ExpiresInFutureIsStored) { // RFC 6265 §5.3 (step 3): when both are present Max-Age wins over Expires, in // both directions. -TEST(HttpCookieJar, MaxAgeTakesPrecedenceOverExpires) { +UTEST(HttpCookieJar, MaxAgeTakesPrecedenceOverExpires) { datetime::MockNowSet(kNow); CookieJar jar; From 28e828c9ad4b6fafc680612801bb996c18a4c8f2 Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Mon, 20 Jul 2026 14:03:43 +0300 Subject: [PATCH 09/18] Tests and bugs related with domain checks were fixed --- core/src/clients/http/cookie_jar.cpp | 44 ++++++++++++----------- core/src/clients/http/cookie_jar_test.cpp | 17 +++++---- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index 905165126e41..b58dd6bdc2e5 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -131,13 +131,13 @@ bool PathMatch(const ValidatedCookie& cookie, const std::string& uri_path) { * Ignore this algorithm because /hoge is uri path for this case * (uri path is not /). */ - if (uri_path.size() < cookie.path.size()) { + if (uri_view.size() < cookie.path.size()) { return false; } /* not using checkprefix() because matching should be case-sensitive */ - if(cookie.path.starts_with(uri_view)) { + if (!cookie.path.starts_with(uri_view)) { return false; } @@ -147,13 +147,21 @@ bool PathMatch(const ValidatedCookie& cookie, const std::string& uri_path) { } /* here, cookie_path_len < uri_path_len */ - if(uri_path[cookie.path.size()] == '/') { + if(uri_view[cookie.path.size()] == '/') { return true; } return false; } +static std::string LowerDomainWithoutLeadingDot(std::string_view domain) { + if (!domain.empty() && domain[0] == '.') { + // RFC 6265 5.2.3. Let cookie-domain be the attribute-value without the leading %x2E (".") character. + domain = domain.substr(1); + } + return utils::text::ToLower(domain); +} + } // namespace class CookieJar::Impl { @@ -167,8 +175,7 @@ class CookieJar::Impl { DeleteCookie(*preprocessed_cookie); return; } - auto location = storage_.try_emplace(preprocessed_cookie->domain, CookieNamesMap{}); - InsertOrAssignCookieToMap(location.first->second, *preprocessed_cookie); + InsertOrAssignCookieToMap(std::move(preprocessed_cookie).value()); return; } @@ -190,8 +197,7 @@ class CookieJar::Impl { } for (const auto& cookies : location->second) { for (const auto& cookie : cookies.second) { - // Temporary hack, think abou proper way to pass uri - if (!PathMatch(cookie, domain + path)) { + if (!PathMatch(cookie, path)) { continue; } if (cookie.host_only) { @@ -238,20 +244,13 @@ class CookieJar::Impl { // Preprocessing domain attribute std::string_view cookie_domain = domain; const bool host_only = raw_cookie.Domain().empty(); - if (!raw_cookie.Domain().empty()) { - cookie_domain = raw_cookie.Domain(); - } - if (!cookie_domain.empty() && cookie_domain[0] == '.') { - // RFC 6265 5.2.3. Let cookie-domain be the attribute-value without the leading %x2E (".") character. - cookie_domain = cookie_domain.substr(1); - } - if (cookie_domain.empty()) { + auto lowered_domain = LowerDomainWithoutLeadingDot(host_only ? domain : raw_cookie.Domain()); + if (lowered_domain.empty()) { // RFC 6265 5.2.3.If the attribute-value is empty, the behavior is undefined. // However, the user agent SHOULD ignore the cookie-av entirely. LOG_WARNING() << "Ignoring cookie without domain attribute: '" << raw_cookie.Name() << "'"; return std::nullopt; } - auto lowered_domain = utils::text::ToLower(cookie_domain); if (IsPublicSuffix(lowered_domain)) { LOG_WARNING() << "Attempt to set supercookie: '" << raw_cookie.Name() << "' with domain '" << lowered_domain << "'"; return std::nullopt; @@ -322,16 +321,19 @@ class CookieJar::Impl { }); } - static void InsertOrAssignCookieToMap(CookieNamesMap& map, const ValidatedCookie& cookie) { - auto location = map.try_emplace(cookie.name, CookiesList{}); + void InsertOrAssignCookieToMap(ValidatedCookie&& cookie) { + const auto& map_location = storage_.try_emplace(cookie.domain, CookieNamesMap{}); + auto location = map_location.first->second.try_emplace(cookie.name, CookiesList{}); auto& list = location.first->second; auto cookie_location = FindDuplicate(list, cookie); if (cookie_location != list.end()) { - *cookie_location = cookie; + // Preserving old creation time, needed for sorting output cookies + cookie.creation_time = cookie_location->creation_time; + *cookie_location = std::move(cookie); return; } - list.push_back(cookie); + list.push_back(std::move(cookie)); } static void DeleteCookieFromMap(CookieNamesMap& map, const ValidatedCookie& cookie) { @@ -368,7 +370,7 @@ std::optional CookieJar::GetAnyCookieValue(const std::string& name) CookieJar::Cookies CookieJar::GetCookies(const std::string& domain, const std::string& path) { Cookies result; - const auto domains = DomainCandidates(utils::text::ToLower(domain)); + const auto domains = DomainCandidates(LowerDomainWithoutLeadingDot(domain)); for (const auto& d : domains) { auto domain_cookies = impl_->GetCookies(d, path); diff --git a/core/src/clients/http/cookie_jar_test.cpp b/core/src/clients/http/cookie_jar_test.cpp index 3ac668b8119c..7ec46a192863 100644 --- a/core/src/clients/http/cookie_jar_test.cpp +++ b/core/src/clients/http/cookie_jar_test.cpp @@ -232,12 +232,17 @@ UTEST(HttpCookieJar, TrailingSlashCookiePathQuirk) { // literal domain string, so a leading-dot domain matches nothing here. UTEST(HttpCookieJar, LeadingDotDomainQuirk) { CookieJar jar; - Store(jar, "example.com", "/", "a=1; Domain=.example.com; Path=/"); - - EXPECT_THAT(jar.GetCookies("example.com", "/"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("www.example.com", "/"), IsEmpty()); - // It only matches a request whose host equals the literal ".example.com". - EXPECT_THAT(Pairs(jar.GetCookies(".example.com", "/")), ElementsAre("a=1")); + Store(jar, "v1.myapi.com", "/", "a=1; Domain=.v1.myapi.com; Path=/"); + + EXPECT_THAT(Pairs(jar.GetCookies("v1.myapi.com", "/")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies(".v1.myapi.com", "/")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("api.v1.myapi.com", "/")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("test.api.v1.myapi.com", "/")), ElementsAre("a=1")); + + EXPECT_THAT(jar.GetCookies("api.v2.myapi.com", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("myapi.com", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies(".myapi.com", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("amyapi.com", "/"), IsEmpty()); } // --- Deletion (RFC 6265 §5.3) and overwrite semantics --- From 3ada078834a9c67cfaa34f3adb65a3f478c73f26 Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Mon, 20 Jul 2026 14:37:25 +0300 Subject: [PATCH 10/18] Bug in path's prefix matching was fixed. All tests related with cookie jar were fixed --- .../userver/clients/http/cookie_jar.hpp | 25 ++++++++++++++++-- core/src/clients/http/cookie_jar.cpp | 26 +++++++++++++++---- core/src/clients/http/cookie_jar_test.cpp | 6 ++--- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/core/include/userver/clients/http/cookie_jar.hpp b/core/include/userver/clients/http/cookie_jar.hpp index 70d4352b64c4..471e27341b4f 100644 --- a/core/include/userver/clients/http/cookie_jar.hpp +++ b/core/include/userver/clients/http/cookie_jar.hpp @@ -16,8 +16,30 @@ namespace clients::http { /// @brief Cookies storage class CookieJar final { + struct Impl; public: - using Cookie = std::pair; + class Cookie { + public: + Cookie(const std::string& name, const std::string& value, const std::chrono::system_clock::time_point& creation_time, const size_t path_length); + + inline const std::string& Name() const { + return name_; + } + + inline const std::string& Value() const { + return value_; + } + + private: + friend class CookieJar::Impl; + + + std::string name_; + std::string value_; + std::chrono::system_clock::time_point creation_time_; + size_t path_length_; + }; + using Cookies = std::vector; CookieJar(); ~CookieJar(); @@ -30,7 +52,6 @@ class CookieJar final { Cookies GetCookies(const std::string& domain, const std::string& path); private: - struct Impl; utils::FastPimpl impl_; }; diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index b58dd6bdc2e5..d7a1ea563c77 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -137,7 +137,7 @@ bool PathMatch(const ValidatedCookie& cookie, const std::string& uri_path) { /* not using checkprefix() because matching should be case-sensitive */ - if (!cookie.path.starts_with(uri_view)) { + if (!uri_view.starts_with(cookie.path)) { return false; } @@ -164,6 +164,11 @@ static std::string LowerDomainWithoutLeadingDot(std::string_view domain) { } // namespace +CookieJar::Cookie::Cookie(const std::string& name, const std::string& value, + const std::chrono::system_clock::time_point& creation_time, const size_t path_length) : + name_(name), value_(value), creation_time_(creation_time), path_length_(path_length){ +} + class CookieJar::Impl { public: void AddCookie(const std::string& domain, const std::string& path, server::http::Cookie&& cookie) { @@ -189,8 +194,8 @@ class CookieJar::Impl { return std::nullopt; } - std::vector> GetCookies(const std::string& domain, const std::string& path) { - std::vector> result; + CookieJar::Cookies GetCookies(const std::string& domain, const std::string& path) { + CookieJar::Cookies result; const auto& location = storage_.find(domain); if (location == storage_.end()) { return result; @@ -209,9 +214,21 @@ class CookieJar::Impl { continue; } } - result.emplace_back(cookie.name, cookie.value); + result.emplace_back(cookie.name, cookie.value, cookie.creation_time, cookie.path.size()); } } + // In fact this optional but recommended step. Maybe add flag? + // RFC 6265 5.4.1 - specifies order with remark: + // `Not all user agents sort the cookie-list in this order, but + // this order reflects common practice when this document was + // written, and, historically, there have been servers that + // (erroneously) depended on this order.` + std::sort(result.begin(), result.end(), [](const CookieJar::Cookie& lhs, const CookieJar::Cookie& rhs) { + if (lhs.path_length_ != rhs.path_length_) { + return lhs.path_length_ > rhs.path_length_; + } + return lhs.creation_time_ < rhs.creation_time_; + }); return result; } @@ -242,7 +259,6 @@ class CookieJar::Impl { }; { // Preprocessing domain attribute - std::string_view cookie_domain = domain; const bool host_only = raw_cookie.Domain().empty(); auto lowered_domain = LowerDomainWithoutLeadingDot(host_only ? domain : raw_cookie.Domain()); if (lowered_domain.empty()) { diff --git a/core/src/clients/http/cookie_jar_test.cpp b/core/src/clients/http/cookie_jar_test.cpp index 7ec46a192863..c34d15c62956 100644 --- a/core/src/clients/http/cookie_jar_test.cpp +++ b/core/src/clients/http/cookie_jar_test.cpp @@ -42,7 +42,7 @@ std::vector Pairs(const CookieJar::Cookies& cookies) { std::vector out; out.reserve(cookies.size()); for (const auto& cookie : cookies) { - out.push_back(cookie.first + '=' + cookie.second); + out.push_back(cookie.Name() + '=' + cookie.Value()); } return out; } @@ -208,8 +208,8 @@ UTEST(HttpCookieJar, ReturnedCookieRetainsAttributes) { const auto cookies = jar.GetCookies("localhost", "/base/dir"); ASSERT_EQ(cookies.size(), 1); - EXPECT_EQ(cookies.front().first, "a"); - EXPECT_EQ(cookies.front().second, "1"); + EXPECT_EQ(cookies.front().Name(), "a"); + EXPECT_EQ(cookies.front().Value(), "1"); } // --- Current-behavior quirks that deviate from a strict RFC 6265 reading --- From a77df2612f3ee76ccbf14682bf372ddc6f62b7b6 Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Mon, 20 Jul 2026 14:49:50 +0300 Subject: [PATCH 11/18] Support for merging two cookie jars was added --- .../userver/clients/http/cookie_jar.hpp | 3 ++- core/src/clients/http/cookie_jar.cpp | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/core/include/userver/clients/http/cookie_jar.hpp b/core/include/userver/clients/http/cookie_jar.hpp index 471e27341b4f..eabb9bbff091 100644 --- a/core/include/userver/clients/http/cookie_jar.hpp +++ b/core/include/userver/clients/http/cookie_jar.hpp @@ -21,7 +21,7 @@ class CookieJar final { class Cookie { public: Cookie(const std::string& name, const std::string& value, const std::chrono::system_clock::time_point& creation_time, const size_t path_length); - + inline const std::string& Name() const { return name_; } @@ -47,6 +47,7 @@ class CookieJar final { CookieJar(const CookieJar&) = delete; CookieJar(CookieJar&&) = delete; + void Merge(CookieJar&& cookie_jar); void AddCookie(const std::string& domain, const std::string& path, server::http::Cookie&& cookie); std::optional GetAnyCookieValue(const std::string& name); Cookies GetCookies(const std::string& domain, const std::string& path); diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index d7a1ea563c77..f8db20fd21af 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -171,6 +171,22 @@ CookieJar::Cookie::Cookie(const std::string& name, const std::string& value, class CookieJar::Impl { public: + + void Merge(Impl& other) { + // TODO: not so optimal way to merge in the case of many paths + for (auto& item : other.storage_) { + auto location = storage_.find(item.first); + if (location == storage_.end()) { + storage_.emplace(location->first, std::move(location->second)); + continue; + } + for (auto& cookie_map : location->second) { + for (auto& validated_cookie : cookie_map.second) { + InsertOrAssignCookieToMap(std::move(validated_cookie)); + } + } + } + } void AddCookie(const std::string& domain, const std::string& path, server::http::Cookie&& cookie) { auto preprocessed_cookie = ValidateCookie(domain, path, cookie); if (!preprocessed_cookie.has_value()) { @@ -376,6 +392,10 @@ class CookieJar::Impl { CookieJar::CookieJar() = default; CookieJar::~CookieJar() = default; +void CookieJar::Merge(CookieJar&& cookie_jar) { + impl_->Merge(*cookie_jar.impl_); +} + void CookieJar::AddCookie(const std::string& domain, const std::string& path, server::http::Cookie&& cookie) { impl_->AddCookie(domain, path, std::move(cookie)); } From 496bd618ca8a9fb46d9d046847bccb5aab973a0e Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Mon, 20 Jul 2026 21:43:32 +0300 Subject: [PATCH 12/18] Computation default-path was fixed with test. Supercookie test was added --- core/src/clients/http/cookie_jar.cpp | 14 +++++++++++--- core/src/clients/http/cookie_jar_test.cpp | 13 +++++++++++-- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index f8db20fd21af..1475246bf39c 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -296,9 +296,17 @@ class CookieJar::Impl { } { // Preprocessing cookie path RFC 5.2.4 - std::string_view cookie_path = path; - if (!raw_cookie.Path().empty() && raw_cookie.Path()[0] == '/') { - cookie_path = raw_cookie.Path(); + std::string_view cookie_path = raw_cookie.Path(); + if (cookie_path.empty() || cookie_path[0] != '/') { + // Computing default-path + cookie_path = path; + if (cookie_path.empty() || cookie_path[0] != '/') { + cookie_path = "/"; + } + const auto last_index = cookie_path.rfind('/'); + if (last_index != 0) { + cookie_path = cookie_path.substr(0, last_index); + } } result.path = cookie_path; } diff --git a/core/src/clients/http/cookie_jar_test.cpp b/core/src/clients/http/cookie_jar_test.cpp index c34d15c62956..5dfd21a20704 100644 --- a/core/src/clients/http/cookie_jar_test.cpp +++ b/core/src/clients/http/cookie_jar_test.cpp @@ -76,6 +76,15 @@ UTEST(HttpCookieJar, EmptyJarReturnsNothing) { EXPECT_THAT(jar.GetCookies("localhost", "/foo"), IsEmpty()); } +// Security issue, don't allow supercookie +UTEST(HttpCookieJar, EmptyJarForSuperCookies) { + CookieJar jar; + Store(jar, "a.com", "/", "session=cracked; Domain=.com;"); + EXPECT_THAT(jar.GetCookies("a.com", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("hacked.com", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("hacked.a.com", "/"), IsEmpty()); +} + // RFC 6265 §5.1.4 (path-match): a cookie-path matches only on directory // boundaries, so "/foo" must not leak to "/foobar", and a request path shorter // than the cookie-path must not match. @@ -185,7 +194,7 @@ UTEST(HttpCookieJar, SameNameAcrossSubAndSuperDomain) { } // RFC 6265 §5.3: a missing Domain defaults to the request host, and a missing -// Path defaults to the request-uri path (§5.1.4 default-path); an explicit +// Path defaults to the default-path (§5.1.4); an explicit // attribute on the cookie takes precedence over these defaults. UTEST(HttpCookieJar, DefaultsFilledFromRequestTarget) { CookieJar jar; @@ -193,7 +202,7 @@ UTEST(HttpCookieJar, DefaultsFilledFromRequestTarget) { EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/base/dir")), ElementsAre("a=1")); EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/base/dir/deeper")), ElementsAre("a=1")); - EXPECT_THAT(jar.GetCookies("localhost", "/base"), IsEmpty()); + EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/base")), ElementsAre("a=1")); // Explicit Path "/" on the cookie wins over the "/ignored" request path. Store(jar, "localhost", "/ignored", "b=2; Domain=localhost; Path=/"); From 0d958ff51c75363d5c98a51524485b509622bc29 Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Mon, 20 Jul 2026 23:54:53 +0300 Subject: [PATCH 13/18] Security tests were added. API was simplified. Fixed bugs for sorting cookies for different domains. Minor comments were added --- .../userver/clients/http/cookie_jar.hpp | 26 +- core/src/clients/http/cookie_jar.cpp | 64 +++-- core/src/clients/http/cookie_jar_test.cpp | 227 ++++++++++-------- core/src/clients/http/request_state.cpp | 16 +- 4 files changed, 186 insertions(+), 147 deletions(-) diff --git a/core/include/userver/clients/http/cookie_jar.hpp b/core/include/userver/clients/http/cookie_jar.hpp index eabb9bbff091..722026409b53 100644 --- a/core/include/userver/clients/http/cookie_jar.hpp +++ b/core/include/userver/clients/http/cookie_jar.hpp @@ -14,10 +14,11 @@ USERVER_NAMESPACE_BEGIN namespace clients::http { -/// @brief Cookies storage +/// @brief Storage for cookies, compliable with RFC 6265. Can be used for sending and receiving cookies on agent side. class CookieJar final { struct Impl; public: + /// @brief Extracted cookie from storage, holds auxiliary internal values like creation time/path length class Cookie { public: Cookie(const std::string& name, const std::string& value, const std::chrono::system_clock::time_point& creation_time, const size_t path_length); @@ -31,8 +32,7 @@ class CookieJar final { } private: - friend class CookieJar::Impl; - + friend class CookieJar; std::string name_; std::string value_; @@ -47,10 +47,26 @@ class CookieJar final { CookieJar(const CookieJar&) = delete; CookieJar(CookieJar&&) = delete; + /// @brief Merges cookie jar into current one. Can be useful for merging cookies from other requests/domains + /// @param cookie_jar Cookie jar to merge void Merge(CookieJar&& cookie_jar); - void AddCookie(const std::string& domain, const std::string& path, server::http::Cookie&& cookie); + + /// @brief Adds cookie with associated url to storage. In general case, url is needed to compute missing fields + /// @param url Request URI cookie came from + /// @param cookie Cookie to store + // TODO: not effective due url parsing, but simple api to use, optimize? + void AddCookie(std::string_view url, server::http::Cookie&& cookie); + + /// @brief Gets ANY cookie value, associated with name. In general case, multiple cookies can be stored with the same name, order is not specified + /// @param name Name of cookie + /// @return Cookie's value std::optional GetAnyCookieValue(const std::string& name); - Cookies GetCookies(const std::string& domain, const std::string& path); + + + /// @brief Gets cookies, associated with current url. In general case, domain/path properties is taken into account + /// @param url Url to be matched with cookies + /// @return Ordered list of cookies + Cookies GetCookies(std::string_view url); private: utils::FastPimpl impl_; diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index 1475246bf39c..49ec4556b19a 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include USERVER_NAMESPACE_BEGIN @@ -105,7 +106,7 @@ static bool CookieTailMatch(std::string_view cookie_domain, std::string_view hos return char_before_suffix == '.'; } -bool PathMatch(const ValidatedCookie& cookie, const std::string& uri_path) { +bool PathMatch(const ValidatedCookie& cookie, std::string_view uri_path) { // Matching cookie path and URL path // RFC6265 5.1.4 Paths and Path-Match @@ -187,8 +188,9 @@ class CookieJar::Impl { } } } - void AddCookie(const std::string& domain, const std::string& path, server::http::Cookie&& cookie) { - auto preprocessed_cookie = ValidateCookie(domain, path, cookie); + void AddCookie(std::string_view url, server::http::Cookie&& cookie) { + const auto& parsed_url = userver::http::DecomposeUrlIntoViews(url); + auto preprocessed_cookie = ValidateCookie(parsed_url.scheme, parsed_url.host, parsed_url.path, cookie); if (!preprocessed_cookie.has_value()) { return; } @@ -210,7 +212,7 @@ class CookieJar::Impl { return std::nullopt; } - CookieJar::Cookies GetCookies(const std::string& domain, const std::string& path) { + CookieJar::Cookies GetCookies(const std::string& domain, std::string_view path) { CookieJar::Cookies result; const auto& location = storage_.find(domain); if (location == storage_.end()) { @@ -233,18 +235,6 @@ class CookieJar::Impl { result.emplace_back(cookie.name, cookie.value, cookie.creation_time, cookie.path.size()); } } - // In fact this optional but recommended step. Maybe add flag? - // RFC 6265 5.4.1 - specifies order with remark: - // `Not all user agents sort the cookie-list in this order, but - // this order reflects common practice when this document was - // written, and, historically, there have been servers that - // (erroneously) depended on this order.` - std::sort(result.begin(), result.end(), [](const CookieJar::Cookie& lhs, const CookieJar::Cookie& rhs) { - if (lhs.path_length_ != rhs.path_length_) { - return lhs.path_length_ > rhs.path_length_; - } - return lhs.creation_time_ < rhs.creation_time_; - }); return result; } @@ -267,7 +257,8 @@ class CookieJar::Impl { DeleteCookieFromMap(location->second, cookie); } - static std::optional ValidateCookie(const std::string& domain, const std::string& path, server::http::Cookie& raw_cookie) { + static std::optional ValidateCookie(std::string_view scheme, std::string_view domain, + std::string_view path, server::http::Cookie& raw_cookie) { ValidatedCookie result{ .name = raw_cookie.Name(), .value = raw_cookie.Value(), @@ -331,13 +322,15 @@ class CookieJar::Impl { result.expire_time = expire_time; } { - // Preprocessing prefixes + // Preprocessing + const auto& lowered_scheme = utils::text::ToLower(scheme); + const bool secure_scheme = lowered_scheme == "https"; if (result.name.starts_with("__Secure-")) { result.prefix_secure = true; } else if (result.name.starts_with("__Host-")) { result.prefix_host = true; } - if (result.prefix_secure && !result.secure) { + if ((secure_scheme || result.prefix_secure) && !result.secure) { // The __Secure- prefix only requires that the cookie be set secure LOG_WARNING() << "Failed security check on cookie: '" << raw_cookie.Name() << "'"; return std::nullopt; @@ -345,11 +338,15 @@ class CookieJar::Impl { if (result.prefix_host) { //The __Host- prefix requires the cookie to be secure, have a "/" path //and not have a domain set. - if (!(result.secure && result.path == "/" && result.host_only)) { + if (!(secure_scheme && result.secure && result.path == "/" && result.host_only)) { LOG_WARNING() << "Failed host check on cookie: '" << raw_cookie.Name() << "'"; return std::nullopt; } } + if (raw_cookie.IsHttpOnly() && !lowered_scheme.empty() && !lowered_scheme.starts_with("http")) { + LOG_WARNING() << "Cookie was received not from http: '" << raw_cookie.Name() << "'"; + return std::nullopt; + } } return result; } @@ -404,23 +401,38 @@ void CookieJar::Merge(CookieJar&& cookie_jar) { impl_->Merge(*cookie_jar.impl_); } -void CookieJar::AddCookie(const std::string& domain, const std::string& path, server::http::Cookie&& cookie) { - impl_->AddCookie(domain, path, std::move(cookie)); +void CookieJar::AddCookie(std::string_view url, server::http::Cookie&& cookie) { + impl_->AddCookie(url, std::move(cookie)); } std::optional CookieJar::GetAnyCookieValue(const std::string& name) { return impl_->GetAnyCookieValue(name); } -CookieJar::Cookies CookieJar::GetCookies(const std::string& domain, const std::string& path) { +CookieJar::Cookies CookieJar::GetCookies(std::string_view url) { Cookies result; - const auto domains = DomainCandidates(LowerDomainWithoutLeadingDot(domain)); + const auto& parsed_url = userver::http::DecomposeUrlIntoViews(url); + const auto domains = DomainCandidates(LowerDomainWithoutLeadingDot(parsed_url.host)); for (const auto& d : domains) { - auto domain_cookies = impl_->GetCookies(d, path); + auto domain_cookies = impl_->GetCookies(d, parsed_url.path); + if (domain_cookies.empty()) { + continue; + } result.insert(result.end(), domain_cookies.begin(), domain_cookies.end()); } - + // In fact this optional but recommended step. Maybe add flag? + // RFC 6265 5.4.1 - specifies order with remark: + // `Not all user agents sort the cookie-list in this order, but + // this order reflects common practice when this document was + // written, and, historically, there have been servers that + // (erroneously) depended on this order.` + std::sort(result.begin(), result.end(), [](const CookieJar::Cookie& lhs, const CookieJar::Cookie& rhs) { + if (lhs.path_length_ != rhs.path_length_) { + return lhs.path_length_ > rhs.path_length_; + } + return lhs.creation_time_ < rhs.creation_time_; + }); return result; } diff --git a/core/src/clients/http/cookie_jar_test.cpp b/core/src/clients/http/cookie_jar_test.cpp index 5dfd21a20704..c30c2890abed 100644 --- a/core/src/clients/http/cookie_jar_test.cpp +++ b/core/src/clients/http/cookie_jar_test.cpp @@ -31,10 +31,10 @@ const auto kNow = std::chrono::system_clock::from_time_t(1'600'000'000); // on a request to (request_host, request_path) - mirroring the receive path in // RequestState. A Set-Cookie omitting Domain/Path inherits those from the // request target (RFC 6265 §5.3, §5.1.4 default-path). -void Store(CookieJar& jar, std::string_view request_host, std::string_view request_path, std::string_view set_cookie) { +void Store(CookieJar& jar, std::string_view url, std::string_view set_cookie) { auto cookie = Cookie::FromString(set_cookie); ASSERT_TRUE(cookie) << "cannot parse Set-Cookie: " << set_cookie; - jar.AddCookie(std::string{request_host}, std::string{request_path}, std::move(*cookie)); + jar.AddCookie(url, std::move(*cookie)); } // "name=value" strings in the exact order GetCookies returned them. @@ -54,35 +54,55 @@ std::vector Pairs(const CookieJar::Cookies& cookies) { // first per RFC 6265 §5.4. UTEST(HttpCookieJar, SameNameDifferentPathBothSentLongestFirst) { CookieJar jar; - Store(jar, "localhost", "/", "A=1; Domain=localhost; Path=/"); - Store(jar, "localhost", "/foo", "A=2; Domain=localhost; Path=/foo"); - - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/foo/bar")), ElementsAre("A=2", "A=1")); + + Store(jar, "localhost", "Garbage=3; Domain=localhost; Path=/garbage"); + Store(jar, "localhost", "A=3; Domain=localhost"); + Store(jar, "localhost/foo/boo", "A=2; Domain=localhost;"); + Store(jar, "localhost/foo/boo", "A=1; Domain=localhost; Path=/foo/bar"); + + EXPECT_THAT(Pairs(jar.GetCookies("localhost/foo/bar")), ElementsAre("A=1", "A=2", "A=3")); } // RFC 6265 §5.3 (step 11): a new cookie with the same (name, domain, path) // replaces the old stored cookie. UTEST(HttpCookieJar, SameKeyOverwrites) { CookieJar jar; - Store(jar, "localhost", "/", "sid=old; Domain=localhost; Path=/"); - Store(jar, "localhost", "/", "sid=new; Domain=localhost; Path=/"); + Store(jar, "localhost", "sid=old; Domain=localhost; Path=/"); + Store(jar, "localhost", "sid=new; Domain=localhost; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("sid=new")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("sid=new")); } // RFC 6265 §5.4: with no stored cookies the Cookie header is empty. UTEST(HttpCookieJar, EmptyJarReturnsNothing) { CookieJar jar; - EXPECT_THAT(jar.GetCookies("localhost", "/foo"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("localhost/foo"), IsEmpty()); } // Security issue, don't allow supercookie UTEST(HttpCookieJar, EmptyJarForSuperCookies) { CookieJar jar; - Store(jar, "a.com", "/", "session=cracked; Domain=.com;"); - EXPECT_THAT(jar.GetCookies("a.com", "/"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("hacked.com", "/"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("hacked.a.com", "/"), IsEmpty()); + Store(jar, "a.com", "session=cracked; Domain=.com;"); + EXPECT_THAT(jar.GetCookies("a.com"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("hacked.com"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("hacked.a.com"), IsEmpty()); +} + +UTEST(HttpCookieJar, NonHttpCookie) { + CookieJar jar; + Store(jar, "myscheme://mytest.com", "a=test; Domain=mytest.com; HttpOnly"); + Store(jar, "http://mytest.com", "a=ok; Domain=mytest.com; HttpOnly"); + EXPECT_THAT(Pairs(jar.GetCookies("mytest.com")), ElementsAre("a=ok")); +} + +UTEST(HttpCookieJar, SecurityCookies) { + CookieJar jar; + Store(jar, "http://mytest.com", "a=test; Secure"); + Store(jar, "http://mytest.com", "__Secure-b=test"); + Store(jar, "https://mytest.com", "a=ok; Secure"); + Store(jar, "https://mytest.com", "__Secure-b=ok; Secure"); + Store(jar, "https://mytest.com", "__Secure-b=test"); + EXPECT_THAT(Pairs(jar.GetCookies("mytest.com")), ElementsAre("a=ok", "__Secure-b=ok")); } // RFC 6265 §5.1.4 (path-match): a cookie-path matches only on directory @@ -90,13 +110,13 @@ UTEST(HttpCookieJar, EmptyJarForSuperCookies) { // than the cookie-path must not match. UTEST(HttpCookieJar, PathPrefixBoundary) { CookieJar jar; - Store(jar, "localhost", "/foo", "a=1; Domain=localhost; Path=/foo"); + Store(jar, "localhost/foo", "a=1; Domain=localhost; Path=/foo"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/foo")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/foo/deep")), ElementsAre("a=1")); - EXPECT_THAT(jar.GetCookies("localhost", "/foobar"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("localhost", "/fo"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("localhost", "/"), IsEmpty()); + EXPECT_THAT(Pairs(jar.GetCookies("localhost/foo")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost/foo/deep")), ElementsAre("a=1")); + EXPECT_THAT(jar.GetCookies("localhost/foobar"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("localhost/fo"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("localhost/"), IsEmpty()); } // RFC 6265 §5.1.4: cookie-path "/" is a prefix of every request path, so a root @@ -104,55 +124,55 @@ UTEST(HttpCookieJar, PathPrefixBoundary) { // default-path "/". UTEST(HttpCookieJar, RootPathMatchesEverything) { CookieJar jar; - Store(jar, "localhost", "/", "a=1; Domain=localhost; Path=/"); + Store(jar, "localhost", "a=1; Domain=localhost; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/anything/deep")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "relative")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost/anything/deep")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost/relative")), ElementsAre("a=1")); } // RFC 6265 §5.4 (rule 2): cookies with longer paths sort before shorter ones, // regardless of insertion order. UTEST(HttpCookieJar, DeepPathSpecificityOrdering) { CookieJar jar; - Store(jar, "localhost", "/a/b", "lvl2=2; Domain=localhost; Path=/a/b"); - Store(jar, "localhost", "/", "root=r; Domain=localhost; Path=/"); - Store(jar, "localhost", "/a/b/c", "lvl3=3; Domain=localhost; Path=/a/b/c"); - Store(jar, "localhost", "/a", "lvl1=1; Domain=localhost; Path=/a"); + Store(jar, "localhost/a/b", "lvl2=2; Domain=localhost; Path=/a/b"); + Store(jar, "localhost/", "root=r; Domain=localhost; Path=/"); + Store(jar, "localhost/a/b/c", "lvl3=3; Domain=localhost; Path=/a/b/c"); + Store(jar, "localhost/a", "lvl1=1; Domain=localhost; Path=/a"); EXPECT_THAT( - Pairs(jar.GetCookies("localhost", "/a/b/c/d")), ElementsAre("lvl3=3", "lvl2=2", "lvl1=1", "root=r") + Pairs(jar.GetCookies("localhost/a/b/c/d")), ElementsAre("lvl3=3", "lvl2=2", "lvl1=1", "root=r") ); } // RFC 6265 §5.1.3 (domain-match): host names compare case-insensitively. UTEST(HttpCookieJar, DomainIsCaseInsensitive) { CookieJar jar; - Store(jar, "localhost", "/", "a=1; Domain=LoCaLhOsT; Path=/"); + Store(jar, "localhost", "a=1; Domain=LoCaLhOsT; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("LOCALHOST", "/")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("LOCALHOST")), ElementsAre("a=1")); } // RFC 6265 §5.1.4 (path-match): paths, by contrast, compare as case-sensitive // octet sequences. UTEST(HttpCookieJar, PathIsCaseSensitive) { CookieJar jar; - Store(jar, "localhost", "/Foo", "a=1; Domain=localhost; Path=/Foo"); + Store(jar, "localhost/Foo", "a=1; Domain=localhost; Path=/Foo"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/Foo")), ElementsAre("a=1")); - EXPECT_THAT(jar.GetCookies("localhost", "/foo"), IsEmpty()); + EXPECT_THAT(Pairs(jar.GetCookies("localhost/Foo")), ElementsAre("a=1")); + EXPECT_THAT(jar.GetCookies("localhost/foo"), IsEmpty()); } // RFC 6265 §5.3: the cookie-name is part of a cookie's identity and is // case-sensitive, so "A" and "a" coexist at the same (domain, path). UTEST(HttpCookieJar, CookieNameIsCaseSensitive) { CookieJar jar; - Store(jar, "localhost", "/", "A=upper; Domain=localhost; Path=/"); - Store(jar, "localhost", "/", "a=lower; Domain=localhost; Path=/"); + Store(jar, "localhost", "A=upper; Domain=localhost; Path=/"); + Store(jar, "localhost", "a=lower; Domain=localhost; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), UnorderedElementsAre("A=upper", "a=lower")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost/")), UnorderedElementsAre("A=upper", "a=lower")); } // RFC 6265 §5.1.3 (domain-match): a cookie set for a parent domain is sent to @@ -160,25 +180,25 @@ UTEST(HttpCookieJar, CookieNameIsCaseSensitive) { // substring (the match must fall on a "." boundary). UTEST(HttpCookieJar, SuperdomainMatch) { CookieJar jar; - Store(jar, "example.com", "/", "a=1; Domain=example.com; Path=/"); - - EXPECT_THAT(Pairs(jar.GetCookies("example.com", "/")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("www.example.com", "/")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("deep.www.example.com", "/")), ElementsAre("a=1")); - EXPECT_THAT(jar.GetCookies("notexample.com", "/"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("example.org", "/"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("com", "/"), IsEmpty()); + Store(jar, "example.com", "a=1; Domain=example.com; Path=/"); + + EXPECT_THAT(Pairs(jar.GetCookies("example.com")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("www.example.com")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("deep.www.example.com")), ElementsAre("a=1")); + EXPECT_THAT(jar.GetCookies("notexample.com"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("example.org"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("com"), IsEmpty()); } // RFC 6265 §5.1.3: a cookie scoped to a subdomain must never travel up to the // parent domain (the parent is not a domain-match for the subdomain). UTEST(HttpCookieJar, SubdomainDoesNotLeakToParent) { CookieJar jar; - Store(jar, "www.example.com", "/", "a=1; Domain=www.example.com; Path=/"); + Store(jar, "www.example.com", "a=1; Domain=www.example.com; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("www.example.com", "/")), ElementsAre("a=1")); - EXPECT_THAT(jar.GetCookies("example.com", "/"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("other.example.com", "/"), IsEmpty()); + EXPECT_THAT(Pairs(jar.GetCookies("www.example.com")), ElementsAre("a=1")); + EXPECT_THAT(jar.GetCookies("example.com"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("other.example.com"), IsEmpty()); } // RFC 6265 §5.3 (identity is name+domain+path) with §5.4 (order): same-named @@ -186,11 +206,11 @@ UTEST(HttpCookieJar, SubdomainDoesNotLeakToParent) { // request to the subdomain receives both (host-specific one first). UTEST(HttpCookieJar, SameNameAcrossSubAndSuperDomain) { CookieJar jar; - Store(jar, "example.com", "/", "sid=parent; Domain=example.com; Path=/"); - Store(jar, "www.example.com", "/", "sid=child; Domain=www.example.com; Path=/"); + Store(jar, "example.com", "sid=parent; Domain=example.com; Path=/"); + Store(jar, "www.example.com", "sid=child; Domain=www.example.com; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("www.example.com", "/")), ElementsAre("sid=child", "sid=parent")); - EXPECT_THAT(Pairs(jar.GetCookies("example.com", "/")), ElementsAre("sid=parent")); + EXPECT_THAT(Pairs(jar.GetCookies("www.example.com")), ElementsAre("sid=parent", "sid=child")); + EXPECT_THAT(Pairs(jar.GetCookies("example.com")), ElementsAre("sid=parent")); } // RFC 6265 §5.3: a missing Domain defaults to the request host, and a missing @@ -198,24 +218,24 @@ UTEST(HttpCookieJar, SameNameAcrossSubAndSuperDomain) { // attribute on the cookie takes precedence over these defaults. UTEST(HttpCookieJar, DefaultsFilledFromRequestTarget) { CookieJar jar; - Store(jar, "localhost", "/base/dir", "a=1"); + Store(jar, "localhost/base/dir", "a=1"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/base/dir")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/base/dir/deeper")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/base")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost/base/dir")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost/base/dir/deeper")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost/base")), ElementsAre("a=1")); // Explicit Path "/" on the cookie wins over the "/ignored" request path. - Store(jar, "localhost", "/ignored", "b=2; Domain=localhost; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("b=2")); + Store(jar, "localhost/ignored", "b=2; Domain=localhost; Path=/"); + EXPECT_THAT(Pairs(jar.GetCookies("localhost/")), ElementsAre("b=2")); } // RFC 6265 §5.3: the stored cookie retains its resolved Domain/Path, which // GetCookies exposes on the returned Cookie objects. UTEST(HttpCookieJar, ReturnedCookieRetainsAttributes) { CookieJar jar; - Store(jar, "localhost", "/base/dir", "a=1"); + Store(jar, "localhost/base/dir", "a=1"); - const auto cookies = jar.GetCookies("localhost", "/base/dir"); + const auto cookies = jar.GetCookies("localhost/base/dir"); ASSERT_EQ(cookies.size(), 1); EXPECT_EQ(cookies.front().Name(), "a"); EXPECT_EQ(cookies.front().Value(), "1"); @@ -230,10 +250,10 @@ UTEST(HttpCookieJar, ReturnedCookieRetainsAttributes) { // RFC 6265 §5.1.4 path-match would accept it. UTEST(HttpCookieJar, TrailingSlashCookiePathQuirk) { CookieJar jar; - Store(jar, "localhost", "/foo/", "a=1; Domain=localhost; Path=/foo/"); + Store(jar, "localhost/foo/", "a=1; Domain=localhost; Path=/foo/"); - EXPECT_THAT(jar.GetCookies("localhost", "/foo/bar"), IsEmpty()); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/foo/")), ElementsAre("a=1")); + EXPECT_THAT(jar.GetCookies("localhost/foo/bar"), IsEmpty()); + EXPECT_THAT(Pairs(jar.GetCookies("localhost/foo/")), ElementsAre("a=1")); } // RFC 6265 §5.2.3 says a leading dot in a Domain attribute is ignored, so @@ -241,17 +261,17 @@ UTEST(HttpCookieJar, TrailingSlashCookiePathQuirk) { // literal domain string, so a leading-dot domain matches nothing here. UTEST(HttpCookieJar, LeadingDotDomainQuirk) { CookieJar jar; - Store(jar, "v1.myapi.com", "/", "a=1; Domain=.v1.myapi.com; Path=/"); + Store(jar, "v1.myapi.com", "a=1; Domain=.v1.myapi.com; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("v1.myapi.com", "/")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies(".v1.myapi.com", "/")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("api.v1.myapi.com", "/")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("test.api.v1.myapi.com", "/")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("v1.myapi.com")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies(".v1.myapi.com")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("api.v1.myapi.com")), ElementsAre("a=1")); + EXPECT_THAT(Pairs(jar.GetCookies("test.api.v1.myapi.com")), ElementsAre("a=1")); - EXPECT_THAT(jar.GetCookies("api.v2.myapi.com", "/"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("myapi.com", "/"), IsEmpty()); - EXPECT_THAT(jar.GetCookies(".myapi.com", "/"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("amyapi.com", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("api.v2.myapi.com"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("myapi.com"), IsEmpty()); + EXPECT_THAT(jar.GetCookies(".myapi.com"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("amyapi.com"), IsEmpty()); } // --- Deletion (RFC 6265 §5.3) and overwrite semantics --- @@ -260,56 +280,56 @@ UTEST(HttpCookieJar, LeadingDotDomainQuirk) { // §5.3 removes the stored cookie with the same name+domain+path. UTEST(HttpCookieJar, MaxAgeZeroDeletesExistingCookie) { CookieJar jar; - Store(jar, "localhost", "/", "sid=v; Domain=localhost; Path=/"); - ASSERT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("sid=v")); + Store(jar, "localhost", "sid=v; Domain=localhost; Path=/"); + ASSERT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("sid=v")); - Store(jar, "localhost", "/", "sid=; Domain=localhost; Path=/; Max-Age=0"); + Store(jar, "localhost", "sid=; Domain=localhost; Path=/; Max-Age=0"); - EXPECT_THAT(jar.GetCookies("localhost", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("localhost"), IsEmpty()); } // RFC 6265 §5.2.2: a negative Max-Age is also a non-positive value and deletes. UTEST(HttpCookieJar, NegativeMaxAgeDeletesExistingCookie) { CookieJar jar; - Store(jar, "localhost", "/", "sid=v; Domain=localhost; Path=/"); + Store(jar, "localhost", "sid=v; Domain=localhost; Path=/"); - Store(jar, "localhost", "/", "sid=; Domain=localhost; Path=/; Max-Age=-1"); + Store(jar, "localhost", "sid=; Domain=localhost; Path=/; Max-Age=-1"); - EXPECT_THAT(jar.GetCookies("localhost", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("localhost"), IsEmpty()); } // RFC 6265 §5.3 (step 11): if no matching cookie exists, deletion does nothing. UTEST(HttpCookieJar, DeletionOfMissingCookieIsNoOp) { CookieJar jar; - Store(jar, "localhost", "/", "sid=; Domain=localhost; Path=/; Max-Age=0"); + Store(jar, "localhost", "sid=; Domain=localhost; Path=/; Max-Age=0"); - EXPECT_THAT(jar.GetCookies("localhost", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("localhost"), IsEmpty()); } // RFC 6265 §5.3: deletion targets the exact name+domain+path identity, so // same-named cookies at other paths are untouched. UTEST(HttpCookieJar, DeletionIsScopedToExactPath) { CookieJar jar; - Store(jar, "localhost", "/", "a=root; Domain=localhost; Path=/"); - Store(jar, "localhost", "/foo", "a=foo; Domain=localhost; Path=/foo"); + Store(jar, "localhost", "a=root; Domain=localhost; Path=/"); + Store(jar, "localhost/foo", "a=foo; Domain=localhost; Path=/foo"); - Store(jar, "localhost", "/foo", "a=; Domain=localhost; Path=/foo; Max-Age=0"); + Store(jar, "localhost/foo", "a=; Domain=localhost; Path=/foo; Max-Age=0"); // Only the /foo entry is gone; the root cookie still matches everywhere. - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("a=root")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/foo")), ElementsAre("a=root")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("a=root")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost/foo")), ElementsAre("a=root")); } // RFC 6265 §5.3 (step 11): replacing one name at a (domain, path) leaves the // other cookies stored at that same key in place. UTEST(HttpCookieJar, OverwriteAtSamePathKeepsSiblings) { CookieJar jar; - Store(jar, "localhost", "/", "a=1; Domain=localhost; Path=/"); - Store(jar, "localhost", "/", "b=1; Domain=localhost; Path=/"); - Store(jar, "localhost", "/", "a=2; Domain=localhost; Path=/"); + Store(jar, "localhost", "a=1; Domain=localhost; Path=/"); + Store(jar, "localhost", "b=1; Domain=localhost; Path=/"); + Store(jar, "localhost", "a=2; Domain=localhost; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), UnorderedElementsAre("a=2", "b=1")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost")), UnorderedElementsAre("a=2", "b=1")); } // RFC 6265 §5.2.1: an Expires in the past sets expiry-time in the past, so @@ -318,11 +338,10 @@ UTEST(HttpCookieJar, ExpiresInPastDeletesExistingCookie) { datetime::MockNowSet(kNow); CookieJar jar; - Store(jar, "localhost", "/", "sid=v; Domain=localhost; Path=/"); - - Store(jar, "localhost", "/", "sid=; Domain=localhost; Path=/; Expires=Thu, 01 Jan 2015 00:00:00 GMT"); + Store(jar, "localhost", "sid=v; Domain=localhost; Path=/"); + Store(jar, "localhost", "sid=; Domain=localhost; Path=/; Expires=Thu, 01 Jan 2015 00:00:00 GMT"); - EXPECT_THAT(jar.GetCookies("localhost", "/"), IsEmpty()); + EXPECT_THAT(jar.GetCookies("localhost"), IsEmpty()); datetime::MockNowUnset(); } @@ -333,9 +352,9 @@ UTEST(HttpCookieJar, ExpiresInFutureIsStored) { datetime::MockNowSet(kNow); CookieJar jar; - Store(jar, "localhost", "/", "sid=v; Domain=localhost; Path=/; Expires=Sat, 01 Jan 2050 00:00:00 GMT"); + Store(jar, "localhost", "sid=v; Domain=localhost; Path=/; Expires=Sat, 01 Jan 2050 00:00:00 GMT"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("sid=v")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("sid=v")); datetime::MockNowUnset(); } @@ -348,13 +367,13 @@ UTEST(HttpCookieJar, MaxAgeTakesPrecedenceOverExpires) { CookieJar jar; // Positive Max-Age wins over a past Expires -> stored. - Store(jar, "localhost", "/", "a=1; Domain=localhost; Path=/; Max-Age=3600; Expires=Thu, 01 Jan 2015 00:00:00 GMT"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("a=1")); + Store(jar, "localhost", "a=1; Domain=localhost; Path=/; Max-Age=3600; Expires=Thu, 01 Jan 2015 00:00:00 GMT"); + EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("a=1")); // Max-Age == 0 wins over a future Expires -> deleted. - Store(jar, "localhost", "/", "b=1; Domain=localhost; Path=/"); - Store(jar, "localhost", "/", "b=; Domain=localhost; Path=/; Max-Age=0; Expires=Sat, 01 Jan 2050 00:00:00 GMT"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost", "/")), ElementsAre("a=1")); + Store(jar, "localhost", "b=1; Domain=localhost; Path=/"); + Store(jar, "localhost", "b=; Domain=localhost; Path=/; Max-Age=0; Expires=Sat, 01 Jan 2050 00:00:00 GMT"); + EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("a=1")); datetime::MockNowUnset(); } diff --git a/core/src/clients/http/request_state.cpp b/core/src/clients/http/request_state.cpp index 664d9ee4c91c..7f0f7d8c46eb 100644 --- a/core/src/clients/http/request_state.cpp +++ b/core/src/clients/http/request_state.cpp @@ -234,13 +234,6 @@ bool IsHttp11WithCompleteBody(const std::shared_ptr response) { return !content_length || utils::FromString(*content_length) == response->body_view().size(); } -USERVER_NAMESPACE::http::DecomposedUrlView RequestCookieTarget(curl::easy& easy) { - std::error_code ec; - const auto effective_url = easy.get_effective_url(ec); - if (ec) return {}; - return USERVER_NAMESPACE::http::DecomposeUrlIntoViews(effective_url); -} - } // namespace RequestState::RequestState( @@ -691,11 +684,10 @@ void RequestState::ParseSingleCookie(const char* ptr, size_t size) { if (auto cookie = server::http::Cookie::FromString(std::string_view(ptr, size))) { // New cookie storage API // TODO: optimize it, quite dirty, some cache needed for host/path extraction - const auto target = RequestCookieTarget(easy()); - if (!target.host.empty()) { - response_->cookie_jar().AddCookie( - std::string{target.host}, std::string{target.path}, server::http::Cookie{*cookie} - ); + std::error_code ec; + const auto effective_url = easy().get_effective_url(ec); + if (!ec && !effective_url.empty()) { + response_->cookie_jar().AddCookie(effective_url, server::http::Cookie{*cookie}); } else { LOG_WARNING() << "Failed to get host from url '" << easy().get_effective_url() << "'"; } From d44acd51a741d0e19b7da4af096ec64e8a4682af Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Tue, 21 Jul 2026 23:10:27 +0300 Subject: [PATCH 14/18] Setter of cookie jar for request was added --- .../userver/clients/http/cookie_jar.hpp | 10 +++- core/include/userver/clients/http/request.hpp | 5 ++ .../include/userver/clients/http/response.hpp | 34 +++++++++-- core/src/clients/http/client_test.cpp | 56 ++++++++++++++++++- core/src/clients/http/cookie_jar.cpp | 4 ++ core/src/clients/http/request.cpp | 15 +++++ core/src/clients/http/request_state.cpp | 26 ++++----- core/src/clients/http/request_state.hpp | 2 + core/src/clients/http/response.cpp | 24 ++++++++ 9 files changed, 151 insertions(+), 25 deletions(-) diff --git a/core/include/userver/clients/http/cookie_jar.hpp b/core/include/userver/clients/http/cookie_jar.hpp index 722026409b53..a609939ac0f0 100644 --- a/core/include/userver/clients/http/cookie_jar.hpp +++ b/core/include/userver/clients/http/cookie_jar.hpp @@ -40,12 +40,16 @@ class CookieJar final { size_t path_length_; }; + /// @brief List of cookies + /// @warning Be aware that list can contain cookies with the same name using Cookies = std::vector; + CookieJar(); ~CookieJar(); - - CookieJar(const CookieJar&) = delete; - CookieJar(CookieJar&&) = delete; + CookieJar(const CookieJar&); + CookieJar(CookieJar&&); + CookieJar& operator=(const CookieJar&); + CookieJar& operator=(CookieJar&&); /// @brief Merges cookie jar into current one. Can be useful for merging cookies from other requests/domains /// @param cookie_jar Cookie jar to merge diff --git a/core/include/userver/clients/http/request.hpp b/core/include/userver/clients/http/request.hpp index a5bc2d341fe8..4b796354be68 100644 --- a/core/include/userver/clients/http/request.hpp +++ b/core/include/userver/clients/http/request.hpp @@ -250,6 +250,11 @@ class Request final { /// Cookies for request as map Request cookies(const std::unordered_map& cookies) &&; + /// Sets cookie jar + Request& cookies(CookieJar&& cookie_jar) &; + /// Sets cookie jar + Request cookies(CookieJar&& cookie_jar) &&; + /// Follow redirects or not. Default: follow Request& follow_redirects(bool follow = true) &; /// @overload diff --git a/core/include/userver/clients/http/response.hpp b/core/include/userver/clients/http/response.hpp index c6596a50b32b..3176201d1a80 100644 --- a/core/include/userver/clients/http/response.hpp +++ b/core/include/userver/clients/http/response.hpp @@ -42,10 +42,33 @@ class Response final { /// return reference to headers const Headers& headers() const { return headers_; } Headers& headers() { return headers_; } - const CookiesMap& cookies() const { return cookies_; } - CookiesMap& cookies() { return cookies_; } - const CookieJar& cookie_jar() const { return cookie_jar_; } - CookieJar& cookie_jar() { return cookie_jar_; } + + /// @brief Gets received cookies + /// @return Reference to map of cookies + /// @warning The cookie map is accessible by default, if cookie jar was enabled exception will be thrown + const CookiesMap& cookies() const; + + /// @brief Gets received cookies + /// @return Reference to map of cookies + /// @warning The cookie map is accessible by default, if cookie jar was enabled exception will be thrown + CookiesMap& cookies(); + + /// @brief Gets cookie jar + /// @return Reference to cookie jar + /// @warning It works only when cookie jar was enabled, otherwise exception will be thrown + const CookieJar& cookie_jar() const; + + /// @brief Gets cookie jar + /// @return Reference to cookie jar + /// @warning It works only when cookie jar was enabled, otherwise exception will be thrown + CookieJar& cookie_jar(); + + /// @brief Checks, whether cookie jar was enabled + /// @return Check result + bool is_cookie_jar() const; + + /// @brief Sets cookie jar to store cookie + void set_cookie_jar(CookieJar&& cookie_jar); /// status_code Status status_code() const; @@ -74,8 +97,7 @@ class Response final { private: Headers headers_; - CookiesMap cookies_; - CookieJar cookie_jar_; + std::variant cookies_engine_; std::string response_; Status status_code_{Status::kInvalid}; LocalStats stats_; diff --git a/core/src/clients/http/client_test.cpp b/core/src/clients/http/client_test.cpp index 4a82089a65ff..1a2bcb0d39df 100644 --- a/core/src/clients/http/client_test.cpp +++ b/core/src/clients/http/client_test.cpp @@ -96,6 +96,12 @@ g3n5Bom64kOrAWOk2xcpd0Pm00o= constexpr char kResponse301WithHeaderPattern[] = "HTTP/1.1 301 OK\r\nConnection: close\r\nContent-Length: 0\r\n{}\r\n\r\n"; + +using ::testing::IsEmpty; +using ::testing::ElementsAreArray; +using ::testing::UnorderedElementsAreArray; + + class RequestMethodTestData final { public: using Request = clients::http::Request; @@ -155,6 +161,24 @@ std::optional Process100(const HttpRequest& request) { return std::nullopt; } +std::vector Pairs(const clients::http::CookieJar::Cookies& cookies) { + std::vector out; + out.reserve(cookies.size()); + for (const auto& cookie : cookies) { + out.push_back(cookie.Name() + '=' + cookie.Value()); + } + return out; +} + +std::vector Pairs(const clients::http::Response::CookiesMap& cookies) { + std::vector out; + out.reserve(cookies.size()); + for (const auto& cookie : cookies) { + out.push_back(cookie.first + '=' + cookie.second.Value()); + } + return out; +} + struct EchoCallback { std::shared_ptr responses_200 = std::make_shared(0); @@ -1146,7 +1170,7 @@ UTEST(HttpClient, Cookies) { test({{"a", "B"}, {"A", "b"}}, {"a=B", "A=b"}); } -UTEST(HttpClient, CookiesFromServer) { +UTEST(HttpClient, CookiesFromServerMapAPI) { // Without compliant CookieJar with rfc 6265, we will just check raw cookies without deduplication etc const auto test = [](std::vector response_cookies, std::vector expected) { const utest::SimpleServer http_server{ReturnCookies{response_cookies}}; @@ -1161,10 +1185,36 @@ UTEST(HttpClient, CookiesFromServer) { .timeout(kTimeout) .perform(); EXPECT_TRUE(response->IsOk()); + EXPECT_THAT(Pairs(response->cookies()), UnorderedElementsAreArray(expected)); + EXPECT_ANY_THROW(response->cookie_jar()); + } + }; + test({"token=xyz789"}, {"token=xyz789"}); + test({"A=1", "A=2", "FOO=BAR"}, {"A=1", "FOO=BAR"}); +} + +UTEST(HttpClient, CookiesFromServerCookieJar) { + const auto test = [](std::vector response_cookies, std::vector expected) { + clients::http::CookieJar cookie_jar; + const utest::SimpleServer http_server{ReturnCookies{response_cookies}}; + auto http_client_ptr = utest::CreateHttpClient(); + for (unsigned i = 0; i < kRepetitions; ++i) { + const auto response = + http_client_ptr->CreateRequest() + .get(http_server.GetBaseUrl()) + .retry(1) + .verify(true) + .http_version(USERVER_NAMESPACE::http::HttpVersion::k11) + .cookies(std::move(cookie_jar)) + .timeout(kTimeout) + .perform(); + EXPECT_TRUE(response->IsOk()); + cookie_jar = response->cookie_jar(); + EXPECT_THAT(Pairs(cookie_jar.GetCookies(http_server.GetBaseUrl())), ElementsAreArray(expected)); + EXPECT_ANY_THROW(response->cookies()); } }; - test({}, {"token=xyz789"}); - test({}, {"A=B", "A=B", "FOO=BAR", "BAR=FOOBAR"}); + test({"A=1", "A=2", "B=1", "B=2; Max-Age=0"}, {"A=2"}); } UTEST(HttpClient, HeadersAndWhitespaces) { diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index 49ec4556b19a..f1d8138d4f9c 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -396,6 +396,10 @@ class CookieJar::Impl { CookieJar::CookieJar() = default; CookieJar::~CookieJar() = default; +CookieJar::CookieJar(const CookieJar&) = default; +CookieJar::CookieJar(CookieJar&&) = default; +CookieJar& CookieJar::operator=(const CookieJar&) = default; +CookieJar& CookieJar::operator=(CookieJar&&) = default; void CookieJar::Merge(CookieJar&& cookie_jar) { impl_->Merge(*cookie_jar.impl_); diff --git a/core/src/clients/http/request.cpp b/core/src/clients/http/request.cpp index f9f02d4cb868..c4d69f4b61ba 100644 --- a/core/src/clients/http/request.cpp +++ b/core/src/clients/http/request.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -441,6 +442,20 @@ Request Request::cookies(const std::unordered_map& coo return std::move(this->cookies(cookies)); } +Request& Request::cookies(CookieJar&& cookie_jar) & { + const auto& cookies_list = cookie_jar.GetCookies(pimpl_->easy().get_effective_url()); + SetCookies(pimpl_->easy(), + cookies_list | std::views::transform([](const CookieJar::Cookie& s) { + return std::pair(s.Name(), s.Value()); + })); + pimpl_->set_cookie_jar(std::move(cookie_jar)); + return *this; +} + +Request Request::cookies(CookieJar&& cookie_jar) && { + return std::move(this->cookies(std::move(cookie_jar))); +} + Request& Request::method(HttpMethod method) & { pimpl_->SetMethod(method); return *this; diff --git a/core/src/clients/http/request_state.cpp b/core/src/clients/http/request_state.cpp index 7f0f7d8c46eb..0d42dcc4a750 100644 --- a/core/src/clients/http/request_state.cpp +++ b/core/src/clients/http/request_state.cpp @@ -408,6 +408,10 @@ void RequestState::http_auth_type( easy().set_password(password.c_str()); } +void RequestState::set_cookie_jar(CookieJar&& cookie_jar) { + response_->set_cookie_jar(std::move(cookie_jar)); +} + void RequestState::Cancel() { // We can not call `retry_.timer.reset();` here because of data race is_cancelled_ = true; @@ -682,20 +686,16 @@ void RequestState::OnRetryTimer(std::error_code err) { void RequestState::ParseSingleCookie(const char* ptr, size_t size) { if (auto cookie = server::http::Cookie::FromString(std::string_view(ptr, size))) { - // New cookie storage API - // TODO: optimize it, quite dirty, some cache needed for host/path extraction - std::error_code ec; - const auto effective_url = easy().get_effective_url(ec); - if (!ec && !effective_url.empty()) { - response_->cookie_jar().AddCookie(effective_url, server::http::Cookie{*cookie}); + if (response_->is_cookie_jar()) { + // CookieJar storage was enabled + // TODO: optimize it, quite dirty, some cache needed for url parsing + response_->cookie_jar().AddCookie(easy().get_effective_url(), server::http::Cookie{*cookie}); } else { - LOG_WARNING() << "Failed to get host from url '" << easy().get_effective_url() << "'"; - } - - // Old API stays untouched - [[maybe_unused]] auto [it, ok] = response_->cookies().emplace(cookie->Name(), std::move(*cookie)); - if (!ok) { - LOG_WARNING() << "Failed to add cookie '" + it->first + "', already added"; + // For API compatibiliy we preserve old API + [[maybe_unused]] auto [it, ok] = response_->cookies().emplace(cookie->Name(), std::move(*cookie)); + if (!ok) { + LOG_WARNING() << "Failed to add cookie '" + it->first + "', already added"; + } } } } diff --git a/core/src/clients/http/request_state.hpp b/core/src/clients/http/request_state.hpp index c2f3e41929f7..96fbed3e5309 100644 --- a/core/src/clients/http/request_state.hpp +++ b/core/src/clients/http/request_state.hpp @@ -108,6 +108,8 @@ class RequestState : public std::enable_shared_from_this { utils::zstring_view user, utils::zstring_view password ); + /// sets cookijar engine for sending/receiving cookies + void set_cookie_jar(CookieJar&& cookie_jar); /// get timeout value in milliseconds long timeout() const { return original_timeout_.count(); } diff --git a/core/src/clients/http/response.cpp b/core/src/clients/http/response.cpp index 35f0dfe9e03b..7e1d03f000d6 100644 --- a/core/src/clients/http/response.cpp +++ b/core/src/clients/http/response.cpp @@ -9,6 +9,30 @@ USERVER_NAMESPACE_BEGIN namespace clients::http { +const Response::CookiesMap& Response::cookies() const { + return std::get(cookies_engine_); +} + +Response::CookiesMap& Response::cookies() { + return std::get(cookies_engine_); +} + +const CookieJar& Response::cookie_jar() const { + return std::get(cookies_engine_); +} + +CookieJar& Response::cookie_jar() { + return std::get(cookies_engine_); +} + +bool Response::is_cookie_jar() const { + return cookies_engine_.index() == 1; +} + +void Response::set_cookie_jar(CookieJar&& cookie_jar) { + cookies_engine_.emplace(std::move(cookie_jar)); +} + Status Response::status_code() const { return status_code_; } void Response::RaiseForStatus(int code, const LocalStats& stats) { From edc67571634350a7c8924944d9dafbc9460a94f5 Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Wed, 22 Jul 2026 13:04:41 +0300 Subject: [PATCH 15/18] Fixed error for usage empty response for cookie jar. Storage was refactored, fixed client_test.cpp. Additional test for ip addresses was added --- core/include/userver/clients/http/response.hpp | 14 +++++++++----- core/src/clients/http/cookie_jar.cpp | 14 +++++++++++--- core/src/clients/http/cookie_jar_test.cpp | 11 +++++++++++ core/src/clients/http/request.cpp | 4 +++- core/src/clients/http/request_state.cpp | 17 ++++++++++++----- core/src/clients/http/request_state.hpp | 6 ++++-- core/src/clients/http/response.cpp | 16 ++++++---------- 7 files changed, 56 insertions(+), 26 deletions(-) diff --git a/core/include/userver/clients/http/response.hpp b/core/include/userver/clients/http/response.hpp index 3176201d1a80..b618d55c9bba 100644 --- a/core/include/userver/clients/http/response.hpp +++ b/core/include/userver/clients/http/response.hpp @@ -26,6 +26,7 @@ using Headers = USERVER_NAMESPACE::http::headers::HeaderMap; class Response final { public: using CookiesMap = server::http::Cookie::CookiesMap; + using CookiesEngine = std::variant; Response() = default; @@ -63,12 +64,15 @@ class Response final { /// @warning It works only when cookie jar was enabled, otherwise exception will be thrown CookieJar& cookie_jar(); - /// @brief Checks, whether cookie jar was enabled + /// @brief Checks, whether response holds specified storage /// @return Check result - bool is_cookie_jar() const; + template + bool is_cookie_storage() const { + return cookies_engine_ && std::get_if(cookies_engine_.get()) != nullptr; + } - /// @brief Sets cookie jar to store cookie - void set_cookie_jar(CookieJar&& cookie_jar); + /// @brief Sets cookie engine + void set_cookie_engine(const std::shared_ptr& cookies_engine); /// status_code Status status_code() const; @@ -97,7 +101,7 @@ class Response final { private: Headers headers_; - std::variant cookies_engine_; + std::shared_ptr cookies_engine_; std::string response_; Status status_code_{Status::kInvalid}; LocalStats stats_; diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index f1d8138d4f9c..71cc4ca62ada 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -7,13 +7,13 @@ #include #include #include +#include #include #include #include #include #include -#include USERVER_NAMESPACE_BEGIN @@ -57,6 +57,14 @@ std::vector DomainCandidates(std::string_view host) { return out; } +// There is issue with calling userver's version from non coroutines. We temporary using this one +std::string ToLower(std::string_view str) { + std::string result; + result.resize(str.size()); + std::ranges::transform(str, result.begin(), [](unsigned char c) { return std::tolower(c); }); + return result; +} + bool IsPublicSuffix(const std::string& domain) { return kPublicSuffixes.find(domain) != kPublicSuffixes.end(); } @@ -160,7 +168,7 @@ static std::string LowerDomainWithoutLeadingDot(std::string_view domain) { // RFC 6265 5.2.3. Let cookie-domain be the attribute-value without the leading %x2E (".") character. domain = domain.substr(1); } - return utils::text::ToLower(domain); + return ToLower(domain); } } // namespace @@ -323,7 +331,7 @@ class CookieJar::Impl { } { // Preprocessing - const auto& lowered_scheme = utils::text::ToLower(scheme); + const auto& lowered_scheme = ToLower(scheme); const bool secure_scheme = lowered_scheme == "https"; if (result.name.starts_with("__Secure-")) { result.prefix_secure = true; diff --git a/core/src/clients/http/cookie_jar_test.cpp b/core/src/clients/http/cookie_jar_test.cpp index c30c2890abed..37bf3d89e7aa 100644 --- a/core/src/clients/http/cookie_jar_test.cpp +++ b/core/src/clients/http/cookie_jar_test.cpp @@ -332,6 +332,17 @@ UTEST(HttpCookieJar, OverwriteAtSamePathKeepsSiblings) { EXPECT_THAT(Pairs(jar.GetCookies("localhost")), UnorderedElementsAre("a=2", "b=1")); } +UTEST(HttpCookieJar, CookiesWithIpAndPort) { + CookieJar jar; + Store(jar, "127.0.0.1", "ip=true"); + Store(jar, "localhost:8081", "port=true"); + Store(jar, "127.0.0.2:8081", "ip_with_port=true"); + + EXPECT_THAT(Pairs(jar.GetCookies("127.0.0.1")), ElementsAre("ip=true")); + EXPECT_THAT(Pairs(jar.GetCookies("localhost:8081")), ElementsAre("port=true")); + EXPECT_THAT(Pairs(jar.GetCookies("127.0.0.2:8081")), ElementsAre("ip_with_port=true")); +} + // RFC 6265 §5.2.1: an Expires in the past sets expiry-time in the past, so // §5.3 deletes the cookie. UTEST(HttpCookieJar, ExpiresInPastDeletesExistingCookie) { diff --git a/core/src/clients/http/request.cpp b/core/src/clients/http/request.cpp index c4d69f4b61ba..471cc976e5d3 100644 --- a/core/src/clients/http/request.cpp +++ b/core/src/clients/http/request.cpp @@ -430,12 +430,14 @@ Request Request::proxy_auth_type(ProxyAuthType value) && { return std::move(this Request& Request::cookies(const Cookies& cookies) & { SetCookies(pimpl_->easy(), cookies); + pimpl_->set_cookie_engine(Response::CookiesMap()); return *this; } Request Request::cookies(const Cookies& cookies) && { return std::move(this->cookies(cookies)); } Request& Request::cookies(const std::unordered_map& cookies) & { SetCookies(pimpl_->easy(), cookies); + pimpl_->set_cookie_engine(Response::CookiesMap()); return *this; } Request Request::cookies(const std::unordered_map& cookies) && { @@ -448,7 +450,7 @@ Request& Request::cookies(CookieJar&& cookie_jar) & { cookies_list | std::views::transform([](const CookieJar::Cookie& s) { return std::pair(s.Name(), s.Value()); })); - pimpl_->set_cookie_jar(std::move(cookie_jar)); + pimpl_->set_cookie_engine(std::move(cookie_jar)); return *this; } diff --git a/core/src/clients/http/request_state.cpp b/core/src/clients/http/request_state.cpp index 0d42dcc4a750..2c610bea271d 100644 --- a/core/src/clients/http/request_state.cpp +++ b/core/src/clients/http/request_state.cpp @@ -280,6 +280,8 @@ RequestState::RequestState( // for CURL disables the use of *_proxy env variables. easy().set_proxy(""); + cookies_engine_ = std::make_shared(); + RequestCompleted(); } @@ -408,8 +410,8 @@ void RequestState::http_auth_type( easy().set_password(password.c_str()); } -void RequestState::set_cookie_jar(CookieJar&& cookie_jar) { - response_->set_cookie_jar(std::move(cookie_jar)); +void RequestState::set_cookie_engine(Response::CookiesEngine&& engine) { + *cookies_engine_ = std::move(engine); } void RequestState::Cancel() { @@ -686,17 +688,21 @@ void RequestState::OnRetryTimer(std::error_code err) { void RequestState::ParseSingleCookie(const char* ptr, size_t size) { if (auto cookie = server::http::Cookie::FromString(std::string_view(ptr, size))) { - if (response_->is_cookie_jar()) { + if (response_->is_cookie_storage()) { // CookieJar storage was enabled // TODO: optimize it, quite dirty, some cache needed for url parsing - response_->cookie_jar().AddCookie(easy().get_effective_url(), server::http::Cookie{*cookie}); - } else { + response_->cookie_jar().AddCookie(easy().get_effective_url(), std::move(*cookie)); + return; + } + if (response_->is_cookie_storage()) { // For API compatibiliy we preserve old API [[maybe_unused]] auto [it, ok] = response_->cookies().emplace(cookie->Name(), std::move(*cookie)); if (!ok) { LOG_WARNING() << "Failed to add cookie '" + it->first + "', already added"; } + return; } + LOG_WARNING() << "Cookies engine was disabled. Ignoring cookie '" + cookie->Name() + "'"; } } @@ -1119,6 +1125,7 @@ void RequestState::ResetDataForNewRequest() { SetBaggageHeader(easy()); response_ = std::make_shared(); + response_->set_cookie_engine(cookies_engine_); response_->SetStatusCode(Status::kInvalid); is_cancelled_ = false; diff --git a/core/src/clients/http/request_state.hpp b/core/src/clients/http/request_state.hpp index 96fbed3e5309..57d7e97beece 100644 --- a/core/src/clients/http/request_state.hpp +++ b/core/src/clients/http/request_state.hpp @@ -108,8 +108,8 @@ class RequestState : public std::enable_shared_from_this { utils::zstring_view user, utils::zstring_view password ); - /// sets cookijar engine for sending/receiving cookies - void set_cookie_jar(CookieJar&& cookie_jar); + /// sets cookie engine + void set_cookie_engine(Response::CookiesEngine&& engine); /// get timeout value in milliseconds long timeout() const { return original_timeout_.count(); } @@ -230,6 +230,8 @@ class RequestState : public std::enable_shared_from_this { crypto::Certificate cert_; crypto::Certificate ca_; + /// cookies engine + std::shared_ptr cookies_engine_; /// response std::shared_ptr response_; diff --git a/core/src/clients/http/response.cpp b/core/src/clients/http/response.cpp index 7e1d03f000d6..375c648b7e4c 100644 --- a/core/src/clients/http/response.cpp +++ b/core/src/clients/http/response.cpp @@ -10,27 +10,23 @@ USERVER_NAMESPACE_BEGIN namespace clients::http { const Response::CookiesMap& Response::cookies() const { - return std::get(cookies_engine_); + return std::get(*cookies_engine_); } Response::CookiesMap& Response::cookies() { - return std::get(cookies_engine_); + return std::get(*cookies_engine_); } const CookieJar& Response::cookie_jar() const { - return std::get(cookies_engine_); + return std::get(*cookies_engine_); } CookieJar& Response::cookie_jar() { - return std::get(cookies_engine_); + return std::get(*cookies_engine_); } -bool Response::is_cookie_jar() const { - return cookies_engine_.index() == 1; -} - -void Response::set_cookie_jar(CookieJar&& cookie_jar) { - cookies_engine_.emplace(std::move(cookie_jar)); +void Response::set_cookie_engine(const std::shared_ptr& cookie_engine) { + cookies_engine_ = cookie_engine; } Status Response::status_code() const { return status_code_; } From 701abfffc49dbf3bf7c7ec4fd7b77b80b3612e99 Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Wed, 22 Jul 2026 23:50:21 +0300 Subject: [PATCH 16/18] Additional tests for covering unicode and security attributes were added. Related bugs were fixed --- core/src/clients/http/cookie_jar.cpp | 18 +++++++++++----- core/src/clients/http/cookie_jar_test.cpp | 26 +++++++++++++++++------ 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index 71cc4ca62ada..baea890c829d 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -69,7 +69,9 @@ bool IsPublicSuffix(const std::string& domain) { return kPublicSuffixes.find(domain) != kPublicSuffixes.end(); } -//bool IsSecureScheme(std::string_view scheme) { return utils::StrIcaseEqual{}(scheme, "https"); } +bool IsSecureScheme(std::string_view scheme) { + return utils::StrIcaseEqual{}(scheme, "https"); +} // Some kind of validated/preprocessed cookie from original one // After preprocessing original cookie is not needed anymore, that's why ValidatedCookie contains only subset of attributes @@ -220,14 +222,18 @@ class CookieJar::Impl { return std::nullopt; } - CookieJar::Cookies GetCookies(const std::string& domain, std::string_view path) { + CookieJar::Cookies GetCookies(std::string_view scheme, const std::string& domain, std::string_view path) { CookieJar::Cookies result; const auto& location = storage_.find(domain); if (location == storage_.end()) { return result; } + const bool secure_scheme = IsSecureScheme(scheme); for (const auto& cookies : location->second) { for (const auto& cookie : cookies.second) { + if (cookie.secure && !secure_scheme) { + continue; + } if (!PathMatch(cookie, path)) { continue; } @@ -305,6 +311,8 @@ class CookieJar::Impl { const auto last_index = cookie_path.rfind('/'); if (last_index != 0) { cookie_path = cookie_path.substr(0, last_index); + } else { + cookie_path = "/"; } } result.path = cookie_path; @@ -332,13 +340,13 @@ class CookieJar::Impl { { // Preprocessing const auto& lowered_scheme = ToLower(scheme); - const bool secure_scheme = lowered_scheme == "https"; + const bool secure_scheme = IsSecureScheme(lowered_scheme); if (result.name.starts_with("__Secure-")) { result.prefix_secure = true; } else if (result.name.starts_with("__Host-")) { result.prefix_host = true; } - if ((secure_scheme || result.prefix_secure) && !result.secure) { + if ((result.prefix_secure && !result.secure) || (!secure_scheme && result.secure)) { // The __Secure- prefix only requires that the cookie be set secure LOG_WARNING() << "Failed security check on cookie: '" << raw_cookie.Name() << "'"; return std::nullopt; @@ -427,7 +435,7 @@ CookieJar::Cookies CookieJar::GetCookies(std::string_view url) { const auto domains = DomainCandidates(LowerDomainWithoutLeadingDot(parsed_url.host)); for (const auto& d : domains) { - auto domain_cookies = impl_->GetCookies(d, parsed_url.path); + auto domain_cookies = impl_->GetCookies(parsed_url.scheme, d, parsed_url.path); if (domain_cookies.empty()) { continue; } diff --git a/core/src/clients/http/cookie_jar_test.cpp b/core/src/clients/http/cookie_jar_test.cpp index 37bf3d89e7aa..067ea3bd31e4 100644 --- a/core/src/clients/http/cookie_jar_test.cpp +++ b/core/src/clients/http/cookie_jar_test.cpp @@ -95,14 +95,21 @@ UTEST(HttpCookieJar, NonHttpCookie) { EXPECT_THAT(Pairs(jar.GetCookies("mytest.com")), ElementsAre("a=ok")); } +// Checking combination implicit/explicit security attributes UTEST(HttpCookieJar, SecurityCookies) { CookieJar jar; - Store(jar, "http://mytest.com", "a=test; Secure"); - Store(jar, "http://mytest.com", "__Secure-b=test"); - Store(jar, "https://mytest.com", "a=ok; Secure"); - Store(jar, "https://mytest.com", "__Secure-b=ok; Secure"); - Store(jar, "https://mytest.com", "__Secure-b=test"); - EXPECT_THAT(Pairs(jar.GetCookies("mytest.com")), ElementsAre("a=ok", "__Secure-b=ok")); + Store(jar, "http://mytest.com", "foo=test"); + Store(jar, "http://mytest.com", "secureFoo=test; Secure"); + Store(jar, "http://mytest.com", "__Secure-foo=test"); + Store(jar, "http://mytest.com", "__Host-foo=test"); + Store(jar, "https://mytest.com", "bar=ok; Secure"); + Store(jar, "https://mytest.com", "__Secure-bar=ok; Secure"); + Store(jar, "https://mytest.com", "__Secure-bar=test"); + Store(jar, "https://mytest.com", "__Host-bar=test; Path=/; Secure; "); + Store(jar, "https://mytest.com", "__Host-barbar=test; Domain=mytest.com; Path=/; Secure; "); + EXPECT_THAT(Pairs(jar.GetCookies("mytest.com")), ElementsAre("foo=test")); + EXPECT_THAT(Pairs(jar.GetCookies("http://mytest.com")), ElementsAre("foo=test")); + EXPECT_THAT(Pairs(jar.GetCookies("https://mytest.com")), ElementsAre("foo=test", "bar=ok", "__Secure-bar=ok", "__Host-bar=test")); } // RFC 6265 §5.1.4 (path-match): a cookie-path matches only on directory @@ -119,6 +126,13 @@ UTEST(HttpCookieJar, PathPrefixBoundary) { EXPECT_THAT(jar.GetCookies("localhost/"), IsEmpty()); } +UTEST(HttpCookieJar, UnicodeUrls) { + CookieJar jar; + Store(jar, "тест.рф/test", "a=1"); + + EXPECT_THAT(Pairs(jar.GetCookies("тест.рф")), ElementsAre("a=1")); +} + // RFC 6265 §5.1.4: cookie-path "/" is a prefix of every request path, so a root // cookie matches everywhere; an empty or non-absolute request path takes the // default-path "/". From 4be2566f79cee232a5a3cd52fce947508a4b70a6 Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Thu, 23 Jul 2026 10:52:36 +0300 Subject: [PATCH 17/18] Verbosity of tests were reduced via additional fixture --- core/src/clients/http/cookie_jar_test.cpp | 400 ++++++++++------------ 1 file changed, 181 insertions(+), 219 deletions(-) diff --git a/core/src/clients/http/cookie_jar_test.cpp b/core/src/clients/http/cookie_jar_test.cpp index 067ea3bd31e4..d2ee6fc37dbe 100644 --- a/core/src/clients/http/cookie_jar_test.cpp +++ b/core/src/clients/http/cookie_jar_test.cpp @@ -23,236 +23,217 @@ using ::testing::ElementsAre; using ::testing::IsEmpty; using ::testing::UnorderedElementsAre; +#define EXPECT_COOKIES(url, expected) EXPECT_THAT(GetCookies(url), expected) + // A fixed, timezone-free "now" for Expires-based tests (2020-09-13T12:26:40Z), // so the 2015/2050 Expires dates below are unambiguously past/future. const auto kNow = std::chrono::system_clock::from_time_t(1'600'000'000); -// Parses a Set-Cookie header value and stores it in the jar as though it arrived -// on a request to (request_host, request_path) - mirroring the receive path in -// RequestState. A Set-Cookie omitting Domain/Path inherits those from the -// request target (RFC 6265 §5.3, §5.1.4 default-path). -void Store(CookieJar& jar, std::string_view url, std::string_view set_cookie) { - auto cookie = Cookie::FromString(set_cookie); - ASSERT_TRUE(cookie) << "cannot parse Set-Cookie: " << set_cookie; - jar.AddCookie(url, std::move(*cookie)); -} +class HttpCookieJar : public ::testing::Test { +protected: + // Helper method to store cookie at url. Uses subsequent parsing + void Store(std::string_view url, std::string_view set_cookie) { + auto cookie = Cookie::FromString(set_cookie); + ASSERT_TRUE(cookie) << "cannot parse Set-Cookie: " << set_cookie; + cookie_jar_.AddCookie(url, std::move(*cookie)); + } -// "name=value" strings in the exact order GetCookies returned them. -std::vector Pairs(const CookieJar::Cookies& cookies) { - std::vector out; - out.reserve(cookies.size()); - for (const auto& cookie : cookies) { - out.push_back(cookie.Name() + '=' + cookie.Value()); + // Helper method for easy comparison extracted cookies. Exctracted cookies in "name=value" format + std::vector GetCookies(std::string_view url) { + const auto& cookies = cookie_jar_.GetCookies(url); + std::vector out; + out.reserve(cookies.size()); + for (const auto& cookie : cookies) { + out.push_back(cookie.Name() + '=' + cookie.Value()); + } + return out; } - return out; -} +private: + CookieJar cookie_jar_ = CookieJar{}; +}; } // namespace // The motivating scenario: two same-named cookies differing only by Path. // Both domain- and path-match, so both are sent, most-specific (longest path) // first per RFC 6265 §5.4. -UTEST(HttpCookieJar, SameNameDifferentPathBothSentLongestFirst) { - CookieJar jar; - - Store(jar, "localhost", "Garbage=3; Domain=localhost; Path=/garbage"); - Store(jar, "localhost", "A=3; Domain=localhost"); - Store(jar, "localhost/foo/boo", "A=2; Domain=localhost;"); - Store(jar, "localhost/foo/boo", "A=1; Domain=localhost; Path=/foo/bar"); +UTEST_F(HttpCookieJar, SameNameDifferentPathBothSentLongestFirst) { + Store("localhost", "Garbage=3; Domain=localhost; Path=/garbage"); + Store("localhost", "A=3; Domain=localhost"); + Store("localhost/foo/boo", "A=2; Domain=localhost;"); + Store("localhost/foo/boo", "A=1; Domain=localhost; Path=/foo/bar"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost/foo/bar")), ElementsAre("A=1", "A=2", "A=3")); + EXPECT_COOKIES("localhost/foo/bar", ElementsAre("A=1", "A=2", "A=3")); } // RFC 6265 §5.3 (step 11): a new cookie with the same (name, domain, path) // replaces the old stored cookie. -UTEST(HttpCookieJar, SameKeyOverwrites) { - CookieJar jar; - Store(jar, "localhost", "sid=old; Domain=localhost; Path=/"); - Store(jar, "localhost", "sid=new; Domain=localhost; Path=/"); +UTEST_F(HttpCookieJar, SameKeyOverwrites) { + Store("localhost", "sid=old; Domain=localhost; Path=/"); + Store("localhost", "sid=new; Domain=localhost; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("sid=new")); + EXPECT_COOKIES("localhost", ElementsAre("sid=new")); } // RFC 6265 §5.4: with no stored cookies the Cookie header is empty. -UTEST(HttpCookieJar, EmptyJarReturnsNothing) { - CookieJar jar; - EXPECT_THAT(jar.GetCookies("localhost/foo"), IsEmpty()); +UTEST_F(HttpCookieJar, EmptyJarReturnsNothing) { + EXPECT_COOKIES("localhost/foo", IsEmpty()); } // Security issue, don't allow supercookie -UTEST(HttpCookieJar, EmptyJarForSuperCookies) { - CookieJar jar; - Store(jar, "a.com", "session=cracked; Domain=.com;"); - EXPECT_THAT(jar.GetCookies("a.com"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("hacked.com"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("hacked.a.com"), IsEmpty()); +UTEST_F(HttpCookieJar, EmptyJarForSuperCookies) { + Store("a.com", "session=cracked; Domain=.com;"); + + EXPECT_COOKIES("a.com", IsEmpty()); + EXPECT_COOKIES("hacked.com", IsEmpty()); + EXPECT_COOKIES("hacked.a.com", IsEmpty()); } -UTEST(HttpCookieJar, NonHttpCookie) { - CookieJar jar; - Store(jar, "myscheme://mytest.com", "a=test; Domain=mytest.com; HttpOnly"); - Store(jar, "http://mytest.com", "a=ok; Domain=mytest.com; HttpOnly"); - EXPECT_THAT(Pairs(jar.GetCookies("mytest.com")), ElementsAre("a=ok")); +UTEST_F(HttpCookieJar, NonHttpCookie) { + Store("myscheme://mytest.com", "a=test; Domain=mytest.com; HttpOnly"); + Store("http://mytest.com", "a=ok; Domain=mytest.com; HttpOnly"); + EXPECT_COOKIES("mytest.com", ElementsAre("a=ok")); } // Checking combination implicit/explicit security attributes -UTEST(HttpCookieJar, SecurityCookies) { - CookieJar jar; - Store(jar, "http://mytest.com", "foo=test"); - Store(jar, "http://mytest.com", "secureFoo=test; Secure"); - Store(jar, "http://mytest.com", "__Secure-foo=test"); - Store(jar, "http://mytest.com", "__Host-foo=test"); - Store(jar, "https://mytest.com", "bar=ok; Secure"); - Store(jar, "https://mytest.com", "__Secure-bar=ok; Secure"); - Store(jar, "https://mytest.com", "__Secure-bar=test"); - Store(jar, "https://mytest.com", "__Host-bar=test; Path=/; Secure; "); - Store(jar, "https://mytest.com", "__Host-barbar=test; Domain=mytest.com; Path=/; Secure; "); - EXPECT_THAT(Pairs(jar.GetCookies("mytest.com")), ElementsAre("foo=test")); - EXPECT_THAT(Pairs(jar.GetCookies("http://mytest.com")), ElementsAre("foo=test")); - EXPECT_THAT(Pairs(jar.GetCookies("https://mytest.com")), ElementsAre("foo=test", "bar=ok", "__Secure-bar=ok", "__Host-bar=test")); +UTEST_F(HttpCookieJar, SecurityCookies) { + Store("http://mytest.com", "foo=test"); + Store("http://mytest.com", "secureFoo=test; Secure"); + Store("http://mytest.com", "__Secure-foo=test"); + Store("http://mytest.com", "__Host-foo=test"); + Store("https://mytest.com", "bar=ok; Secure"); + Store("https://mytest.com", "__Secure-bar=ok; Secure"); + Store("https://mytest.com", "__Secure-bar=test"); + Store("https://mytest.com", "__Host-bar=test; Path=/; Secure; "); + Store("https://mytest.com", "__Host-barbar=test; Domain=mytest.com; Path=/; Secure; "); + EXPECT_COOKIES("mytest.com", ElementsAre("foo=test")); + EXPECT_COOKIES("http://mytest.com", ElementsAre("foo=test")); + EXPECT_COOKIES("https://mytest.com", ElementsAre("foo=test", "bar=ok", "__Secure-bar=ok", "__Host-bar=test")); } // RFC 6265 §5.1.4 (path-match): a cookie-path matches only on directory // boundaries, so "/foo" must not leak to "/foobar", and a request path shorter // than the cookie-path must not match. -UTEST(HttpCookieJar, PathPrefixBoundary) { - CookieJar jar; - Store(jar, "localhost/foo", "a=1; Domain=localhost; Path=/foo"); - - EXPECT_THAT(Pairs(jar.GetCookies("localhost/foo")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost/foo/deep")), ElementsAre("a=1")); - EXPECT_THAT(jar.GetCookies("localhost/foobar"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("localhost/fo"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("localhost/"), IsEmpty()); +UTEST_F(HttpCookieJar, PathPrefixBoundary) { + Store("localhost/foo", "a=1; Domain=localhost; Path=/foo"); + + EXPECT_COOKIES("localhost/foo", ElementsAre("a=1")); + EXPECT_COOKIES("localhost/foo/deep", ElementsAre("a=1")); + EXPECT_COOKIES("localhost/foobar", IsEmpty()); + EXPECT_COOKIES("localhost/fo", IsEmpty()); + EXPECT_COOKIES("localhost/", IsEmpty()); } -UTEST(HttpCookieJar, UnicodeUrls) { - CookieJar jar; - Store(jar, "тест.рф/test", "a=1"); +UTEST_F(HttpCookieJar, UnicodeUrls) { + Store("тест.рф/test", "a=1"); - EXPECT_THAT(Pairs(jar.GetCookies("тест.рф")), ElementsAre("a=1")); + EXPECT_COOKIES("тест.рф", ElementsAre("a=1")); } // RFC 6265 §5.1.4: cookie-path "/" is a prefix of every request path, so a root // cookie matches everywhere; an empty or non-absolute request path takes the // default-path "/". -UTEST(HttpCookieJar, RootPathMatchesEverything) { - CookieJar jar; - Store(jar, "localhost", "a=1; Domain=localhost; Path=/"); - - EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost/anything/deep")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost/relative")), ElementsAre("a=1")); +UTEST_F(HttpCookieJar, RootPathMatchesEverything) { + Store("localhost", "a=1; Domain=localhost; Path=/"); + + EXPECT_COOKIES("localhost", ElementsAre("a=1")); + EXPECT_COOKIES("localhost/anything/deep", ElementsAre("a=1")); + EXPECT_COOKIES("localhost", ElementsAre("a=1")); + EXPECT_COOKIES("localhost/relative", ElementsAre("a=1")); } // RFC 6265 §5.4 (rule 2): cookies with longer paths sort before shorter ones, // regardless of insertion order. -UTEST(HttpCookieJar, DeepPathSpecificityOrdering) { - CookieJar jar; - Store(jar, "localhost/a/b", "lvl2=2; Domain=localhost; Path=/a/b"); - Store(jar, "localhost/", "root=r; Domain=localhost; Path=/"); - Store(jar, "localhost/a/b/c", "lvl3=3; Domain=localhost; Path=/a/b/c"); - Store(jar, "localhost/a", "lvl1=1; Domain=localhost; Path=/a"); - - EXPECT_THAT( - Pairs(jar.GetCookies("localhost/a/b/c/d")), ElementsAre("lvl3=3", "lvl2=2", "lvl1=1", "root=r") - ); +UTEST_F(HttpCookieJar, DeepPathSpecificityOrdering) { + Store("localhost/a/b", "lvl2=2; Domain=localhost; Path=/a/b"); + Store("localhost/", "root=r; Domain=localhost; Path=/"); + Store("localhost/a/b/c", "lvl3=3; Domain=localhost; Path=/a/b/c"); + Store("localhost/a", "lvl1=1; Domain=localhost; Path=/a"); + + EXPECT_COOKIES("localhost/a/b/c/d", ElementsAre("lvl3=3", "lvl2=2", "lvl1=1", "root=r")); } // RFC 6265 §5.1.3 (domain-match): host names compare case-insensitively. -UTEST(HttpCookieJar, DomainIsCaseInsensitive) { - CookieJar jar; - Store(jar, "localhost", "a=1; Domain=LoCaLhOsT; Path=/"); +UTEST_F(HttpCookieJar, DomainIsCaseInsensitive) { + Store("localhost", "a=1; Domain=LoCaLhOsT; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("LOCALHOST")), ElementsAre("a=1")); + EXPECT_COOKIES("localhost", ElementsAre("a=1")); + EXPECT_COOKIES("LOCALHOST", ElementsAre("a=1")); } // RFC 6265 §5.1.4 (path-match): paths, by contrast, compare as case-sensitive // octet sequences. -UTEST(HttpCookieJar, PathIsCaseSensitive) { - CookieJar jar; - Store(jar, "localhost/Foo", "a=1; Domain=localhost; Path=/Foo"); +UTEST_F(HttpCookieJar, PathIsCaseSensitive) { + Store("localhost/Foo", "a=1; Domain=localhost; Path=/Foo"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost/Foo")), ElementsAre("a=1")); - EXPECT_THAT(jar.GetCookies("localhost/foo"), IsEmpty()); + EXPECT_COOKIES("localhost/Foo", ElementsAre("a=1")); + EXPECT_COOKIES("localhost/foo", IsEmpty()); } // RFC 6265 §5.3: the cookie-name is part of a cookie's identity and is // case-sensitive, so "A" and "a" coexist at the same (domain, path). -UTEST(HttpCookieJar, CookieNameIsCaseSensitive) { - CookieJar jar; - Store(jar, "localhost", "A=upper; Domain=localhost; Path=/"); - Store(jar, "localhost", "a=lower; Domain=localhost; Path=/"); +UTEST_F(HttpCookieJar, CookieNameIsCaseSensitive) { + Store("localhost", "A=upper; Domain=localhost; Path=/"); + Store("localhost", "a=lower; Domain=localhost; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost/")), UnorderedElementsAre("A=upper", "a=lower")); + EXPECT_COOKIES("localhost/", UnorderedElementsAre("A=upper", "a=lower")); } // RFC 6265 §5.1.3 (domain-match): a cookie set for a parent domain is sent to // its subdomains, but not to unrelated hosts that merely share a suffix // substring (the match must fall on a "." boundary). -UTEST(HttpCookieJar, SuperdomainMatch) { - CookieJar jar; - Store(jar, "example.com", "a=1; Domain=example.com; Path=/"); - - EXPECT_THAT(Pairs(jar.GetCookies("example.com")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("www.example.com")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("deep.www.example.com")), ElementsAre("a=1")); - EXPECT_THAT(jar.GetCookies("notexample.com"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("example.org"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("com"), IsEmpty()); +UTEST_F(HttpCookieJar, SuperdomainMatch) { + Store("example.com", "a=1; Domain=example.com; Path=/"); + + EXPECT_COOKIES("example.com", ElementsAre("a=1")); + EXPECT_COOKIES("www.example.com", ElementsAre("a=1")); + EXPECT_COOKIES("deep.www.example.com", ElementsAre("a=1")); + EXPECT_COOKIES("notexample.com", IsEmpty()); + EXPECT_COOKIES("example.org", IsEmpty()); + EXPECT_COOKIES("com", IsEmpty()); } // RFC 6265 §5.1.3: a cookie scoped to a subdomain must never travel up to the // parent domain (the parent is not a domain-match for the subdomain). -UTEST(HttpCookieJar, SubdomainDoesNotLeakToParent) { - CookieJar jar; - Store(jar, "www.example.com", "a=1; Domain=www.example.com; Path=/"); +UTEST_F(HttpCookieJar, SubdomainDoesNotLeakToParent) { + Store("www.example.com", "a=1; Domain=www.example.com; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("www.example.com")), ElementsAre("a=1")); - EXPECT_THAT(jar.GetCookies("example.com"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("other.example.com"), IsEmpty()); + EXPECT_COOKIES("www.example.com", ElementsAre("a=1")); + EXPECT_COOKIES("example.com", IsEmpty()); + EXPECT_COOKIES("other.example.com", IsEmpty()); } // RFC 6265 §5.3 (identity is name+domain+path) with §5.4 (order): same-named // cookies scoped to a sub- and a super-domain are distinct entries, so a // request to the subdomain receives both (host-specific one first). -UTEST(HttpCookieJar, SameNameAcrossSubAndSuperDomain) { - CookieJar jar; - Store(jar, "example.com", "sid=parent; Domain=example.com; Path=/"); - Store(jar, "www.example.com", "sid=child; Domain=www.example.com; Path=/"); +UTEST_F(HttpCookieJar, SameNameAcrossSubAndSuperDomain) { + Store("example.com", "sid=parent; Domain=example.com; Path=/"); + Store("www.example.com", "sid=child; Domain=www.example.com; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("www.example.com")), ElementsAre("sid=parent", "sid=child")); - EXPECT_THAT(Pairs(jar.GetCookies("example.com")), ElementsAre("sid=parent")); + EXPECT_COOKIES("www.example.com", ElementsAre("sid=parent", "sid=child")); + EXPECT_COOKIES("example.com", ElementsAre("sid=parent")); } // RFC 6265 §5.3: a missing Domain defaults to the request host, and a missing // Path defaults to the default-path (§5.1.4); an explicit // attribute on the cookie takes precedence over these defaults. -UTEST(HttpCookieJar, DefaultsFilledFromRequestTarget) { - CookieJar jar; - Store(jar, "localhost/base/dir", "a=1"); +UTEST_F(HttpCookieJar, DefaultsFilledFromRequestTarget) { + Store("localhost/base/dir", "a=1"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost/base/dir")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost/base/dir/deeper")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost/base")), ElementsAre("a=1")); + EXPECT_COOKIES("localhost/base/dir", ElementsAre("a=1")); + EXPECT_COOKIES("localhost/base/dir/deeper", ElementsAre("a=1")); + EXPECT_COOKIES("localhost/base", ElementsAre("a=1")); // Explicit Path "/" on the cookie wins over the "/ignored" request path. - Store(jar, "localhost/ignored", "b=2; Domain=localhost; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost/")), ElementsAre("b=2")); + Store("localhost/ignored", "b=2; Domain=localhost; Path=/"); + EXPECT_COOKIES("localhost/", ElementsAre("b=2")); } // RFC 6265 §5.3: the stored cookie retains its resolved Domain/Path, which // GetCookies exposes on the returned Cookie objects. -UTEST(HttpCookieJar, ReturnedCookieRetainsAttributes) { - CookieJar jar; - Store(jar, "localhost/base/dir", "a=1"); - - const auto cookies = jar.GetCookies("localhost/base/dir"); - ASSERT_EQ(cookies.size(), 1); - EXPECT_EQ(cookies.front().Name(), "a"); - EXPECT_EQ(cookies.front().Value(), "1"); +UTEST_F(HttpCookieJar, ReturnedCookieRetainsAttributes) { + Store("localhost/base/dir", "a=1"); + EXPECT_COOKIES("localhost/base/dir", ElementsAre("a=1")); } // --- Current-behavior quirks that deviate from a strict RFC 6265 reading --- @@ -262,143 +243,124 @@ UTEST(HttpCookieJar, ReturnedCookieRetainsAttributes) { // only directory-boundary prefixes of the request path, so "/foo/" is not a // candidate for request "/foo/bar" and the cookie is withheld - whereas // RFC 6265 §5.1.4 path-match would accept it. -UTEST(HttpCookieJar, TrailingSlashCookiePathQuirk) { - CookieJar jar; - Store(jar, "localhost/foo/", "a=1; Domain=localhost; Path=/foo/"); +UTEST_F(HttpCookieJar, TrailingSlashCookiePathQuirk) { + Store("localhost/foo/", "a=1; Domain=localhost; Path=/foo/"); - EXPECT_THAT(jar.GetCookies("localhost/foo/bar"), IsEmpty()); - EXPECT_THAT(Pairs(jar.GetCookies("localhost/foo/")), ElementsAre("a=1")); + EXPECT_COOKIES("localhost/foo/bar", IsEmpty()); + EXPECT_COOKIES("localhost/foo/", ElementsAre("a=1")); } // RFC 6265 §5.2.3 says a leading dot in a Domain attribute is ignored, so // Domain=.example.com should behave like example.com. The jar keys on the // literal domain string, so a leading-dot domain matches nothing here. -UTEST(HttpCookieJar, LeadingDotDomainQuirk) { - CookieJar jar; - Store(jar, "v1.myapi.com", "a=1; Domain=.v1.myapi.com; Path=/"); - - EXPECT_THAT(Pairs(jar.GetCookies("v1.myapi.com")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies(".v1.myapi.com")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("api.v1.myapi.com")), ElementsAre("a=1")); - EXPECT_THAT(Pairs(jar.GetCookies("test.api.v1.myapi.com")), ElementsAre("a=1")); +UTEST_F(HttpCookieJar, LeadingDotDomainQuirk) { + Store("v1.myapi.com", "a=1; Domain=.v1.myapi.com; Path=/"); + + EXPECT_COOKIES("v1.myapi.com", ElementsAre("a=1")); + EXPECT_COOKIES(".v1.myapi.com", ElementsAre("a=1")); + EXPECT_COOKIES("api.v1.myapi.com", ElementsAre("a=1")); + EXPECT_COOKIES("test.api.v1.myapi.com", ElementsAre("a=1")); - EXPECT_THAT(jar.GetCookies("api.v2.myapi.com"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("myapi.com"), IsEmpty()); - EXPECT_THAT(jar.GetCookies(".myapi.com"), IsEmpty()); - EXPECT_THAT(jar.GetCookies("amyapi.com"), IsEmpty()); + EXPECT_COOKIES("api.v2.myapi.com", IsEmpty()); + EXPECT_COOKIES("myapi.com", IsEmpty()); + EXPECT_COOKIES(".myapi.com", IsEmpty()); + EXPECT_COOKIES("amyapi.com", IsEmpty()); } // --- Deletion (RFC 6265 §5.3) and overwrite semantics --- // RFC 6265 §5.2.2: Max-Age <= 0 makes the cookie expire immediately, so // §5.3 removes the stored cookie with the same name+domain+path. -UTEST(HttpCookieJar, MaxAgeZeroDeletesExistingCookie) { - CookieJar jar; - Store(jar, "localhost", "sid=v; Domain=localhost; Path=/"); - ASSERT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("sid=v")); +UTEST_F(HttpCookieJar, MaxAgeZeroDeletesExistingCookie) { + Store("localhost", "sid=v; Domain=localhost; Path=/"); + EXPECT_COOKIES("localhost", ElementsAre("sid=v")); - Store(jar, "localhost", "sid=; Domain=localhost; Path=/; Max-Age=0"); - - EXPECT_THAT(jar.GetCookies("localhost"), IsEmpty()); + Store("localhost", "sid=; Domain=localhost; Path=/; Max-Age=0"); + EXPECT_COOKIES("localhost", IsEmpty()); } // RFC 6265 §5.2.2: a negative Max-Age is also a non-positive value and deletes. -UTEST(HttpCookieJar, NegativeMaxAgeDeletesExistingCookie) { - CookieJar jar; - Store(jar, "localhost", "sid=v; Domain=localhost; Path=/"); - - Store(jar, "localhost", "sid=; Domain=localhost; Path=/; Max-Age=-1"); +UTEST_F(HttpCookieJar, NegativeMaxAgeDeletesExistingCookie) { + Store("localhost", "sid=v; Domain=localhost; Path=/"); + Store("localhost", "sid=; Domain=localhost; Path=/; Max-Age=-1"); - EXPECT_THAT(jar.GetCookies("localhost"), IsEmpty()); + EXPECT_COOKIES("localhost", IsEmpty()); } // RFC 6265 §5.3 (step 11): if no matching cookie exists, deletion does nothing. -UTEST(HttpCookieJar, DeletionOfMissingCookieIsNoOp) { - CookieJar jar; +UTEST_F(HttpCookieJar, DeletionOfMissingCookieIsNoOp) { + Store("localhost", "sid=; Domain=localhost; Path=/; Max-Age=0"); - Store(jar, "localhost", "sid=; Domain=localhost; Path=/; Max-Age=0"); - - EXPECT_THAT(jar.GetCookies("localhost"), IsEmpty()); + EXPECT_COOKIES("localhost", IsEmpty()); } // RFC 6265 §5.3: deletion targets the exact name+domain+path identity, so // same-named cookies at other paths are untouched. -UTEST(HttpCookieJar, DeletionIsScopedToExactPath) { - CookieJar jar; - Store(jar, "localhost", "a=root; Domain=localhost; Path=/"); - Store(jar, "localhost/foo", "a=foo; Domain=localhost; Path=/foo"); +UTEST_F(HttpCookieJar, DeletionIsScopedToExactPath) { + Store("localhost", "a=root; Domain=localhost; Path=/"); + Store("localhost/foo", "a=foo; Domain=localhost; Path=/foo"); - Store(jar, "localhost/foo", "a=; Domain=localhost; Path=/foo; Max-Age=0"); + Store("localhost/foo", "a=; Domain=localhost; Path=/foo; Max-Age=0"); // Only the /foo entry is gone; the root cookie still matches everywhere. - EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("a=root")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost/foo")), ElementsAre("a=root")); + EXPECT_COOKIES("localhost", ElementsAre("a=root")); + EXPECT_COOKIES("localhost/foo", ElementsAre("a=root")); } // RFC 6265 §5.3 (step 11): replacing one name at a (domain, path) leaves the // other cookies stored at that same key in place. -UTEST(HttpCookieJar, OverwriteAtSamePathKeepsSiblings) { - CookieJar jar; - Store(jar, "localhost", "a=1; Domain=localhost; Path=/"); - Store(jar, "localhost", "b=1; Domain=localhost; Path=/"); - Store(jar, "localhost", "a=2; Domain=localhost; Path=/"); +UTEST_F(HttpCookieJar, OverwriteAtSamePathKeepsSiblings) { + Store("localhost", "a=1; Domain=localhost; Path=/"); + Store("localhost", "b=1; Domain=localhost; Path=/"); + Store("localhost", "a=2; Domain=localhost; Path=/"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost")), UnorderedElementsAre("a=2", "b=1")); + EXPECT_COOKIES("localhost", UnorderedElementsAre("a=2", "b=1")); } -UTEST(HttpCookieJar, CookiesWithIpAndPort) { - CookieJar jar; - Store(jar, "127.0.0.1", "ip=true"); - Store(jar, "localhost:8081", "port=true"); - Store(jar, "127.0.0.2:8081", "ip_with_port=true"); +UTEST_F(HttpCookieJar, CookiesWithIpAndPort) { + Store("127.0.0.1", "ip=true"); + Store("localhost:8081", "port=true"); + Store("127.0.0.2:8081", "ip_with_port=true"); - EXPECT_THAT(Pairs(jar.GetCookies("127.0.0.1")), ElementsAre("ip=true")); - EXPECT_THAT(Pairs(jar.GetCookies("localhost:8081")), ElementsAre("port=true")); - EXPECT_THAT(Pairs(jar.GetCookies("127.0.0.2:8081")), ElementsAre("ip_with_port=true")); + EXPECT_COOKIES("127.0.0.1", ElementsAre("ip=true")); + EXPECT_COOKIES("localhost:8081", ElementsAre("port=true")); + EXPECT_COOKIES("127.0.0.2:8081", ElementsAre("ip_with_port=true")); } // RFC 6265 §5.2.1: an Expires in the past sets expiry-time in the past, so // §5.3 deletes the cookie. -UTEST(HttpCookieJar, ExpiresInPastDeletesExistingCookie) { +UTEST_F(HttpCookieJar, ExpiresInPastDeletesExistingCookie) { datetime::MockNowSet(kNow); + Store("localhost", "sid=v; Domain=localhost; Path=/"); + Store("localhost", "sid=; Domain=localhost; Path=/; Expires=Thu, 01 Jan 2015 00:00:00 GMT"); - CookieJar jar; - Store(jar, "localhost", "sid=v; Domain=localhost; Path=/"); - Store(jar, "localhost", "sid=; Domain=localhost; Path=/; Expires=Thu, 01 Jan 2015 00:00:00 GMT"); - - EXPECT_THAT(jar.GetCookies("localhost"), IsEmpty()); - + EXPECT_COOKIES("localhost", IsEmpty()); datetime::MockNowUnset(); } // RFC 6265 §5.2.1: an Expires in the future keeps the cookie live, so it is // stored normally. -UTEST(HttpCookieJar, ExpiresInFutureIsStored) { +UTEST_F(HttpCookieJar, ExpiresInFutureIsStored) { datetime::MockNowSet(kNow); + Store("localhost", "sid=v; Domain=localhost; Path=/; Expires=Sat, 01 Jan 2050 00:00:00 GMT"); - CookieJar jar; - Store(jar, "localhost", "sid=v; Domain=localhost; Path=/; Expires=Sat, 01 Jan 2050 00:00:00 GMT"); - - EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("sid=v")); - + EXPECT_COOKIES("localhost", ElementsAre("sid=v")); datetime::MockNowUnset(); } // RFC 6265 §5.3 (step 3): when both are present Max-Age wins over Expires, in // both directions. -UTEST(HttpCookieJar, MaxAgeTakesPrecedenceOverExpires) { +UTEST_F(HttpCookieJar, MaxAgeTakesPrecedenceOverExpires) { datetime::MockNowSet(kNow); - CookieJar jar; - // Positive Max-Age wins over a past Expires -> stored. - Store(jar, "localhost", "a=1; Domain=localhost; Path=/; Max-Age=3600; Expires=Thu, 01 Jan 2015 00:00:00 GMT"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("a=1")); + Store("localhost", "a=1; Domain=localhost; Path=/; Max-Age=3600; Expires=Thu, 01 Jan 2015 00:00:00 GMT"); + EXPECT_COOKIES("localhost", ElementsAre("a=1")); // Max-Age == 0 wins over a future Expires -> deleted. - Store(jar, "localhost", "b=1; Domain=localhost; Path=/"); - Store(jar, "localhost", "b=; Domain=localhost; Path=/; Max-Age=0; Expires=Sat, 01 Jan 2050 00:00:00 GMT"); - EXPECT_THAT(Pairs(jar.GetCookies("localhost")), ElementsAre("a=1")); + Store("localhost", "b=1; Domain=localhost; Path=/"); + Store("localhost", "b=; Domain=localhost; Path=/; Max-Age=0; Expires=Sat, 01 Jan 2050 00:00:00 GMT"); + EXPECT_COOKIES("localhost", ElementsAre("a=1")); datetime::MockNowUnset(); } From 794cedae8a7d95e9b78098eed747cd1c24c47fba Mon Sep 17 00:00:00 2001 From: Aleksander Lysenko Date: Fri, 24 Jul 2026 10:45:49 +0300 Subject: [PATCH 18/18] Extra allocations were reduced within implementation of CookieJar --- .../userver/clients/http/cookie_jar.hpp | 5 +- core/src/clients/http/cookie_jar.cpp | 104 +++++++++--------- 2 files changed, 56 insertions(+), 53 deletions(-) diff --git a/core/include/userver/clients/http/cookie_jar.hpp b/core/include/userver/clients/http/cookie_jar.hpp index a609939ac0f0..88b904e9f3cb 100644 --- a/core/include/userver/clients/http/cookie_jar.hpp +++ b/core/include/userver/clients/http/cookie_jar.hpp @@ -15,13 +15,14 @@ USERVER_NAMESPACE_BEGIN namespace clients::http { /// @brief Storage for cookies, compliable with RFC 6265. Can be used for sending and receiving cookies on agent side. +/// @warning Current implementation is not safe for concurrent use by multiple coroutines class CookieJar final { struct Impl; public: /// @brief Extracted cookie from storage, holds auxiliary internal values like creation time/path length class Cookie { public: - Cookie(const std::string& name, const std::string& value, const std::chrono::system_clock::time_point& creation_time, const size_t path_length); + Cookie(std::string_view name, std::string_view value, const std::chrono::system_clock::time_point& creation_time, const size_t path_length); inline const std::string& Name() const { return name_; @@ -64,7 +65,7 @@ class CookieJar final { /// @brief Gets ANY cookie value, associated with name. In general case, multiple cookies can be stored with the same name, order is not specified /// @param name Name of cookie /// @return Cookie's value - std::optional GetAnyCookieValue(const std::string& name); + std::optional GetAnyCookieValue(std::string_view name); /// @brief Gets cookies, associated with current url. In general case, domain/path properties is taken into account diff --git a/core/src/clients/http/cookie_jar.cpp b/core/src/clients/http/cookie_jar.cpp index baea890c829d..c3a5107ceeb5 100644 --- a/core/src/clients/http/cookie_jar.cpp +++ b/core/src/clients/http/cookie_jar.cpp @@ -22,7 +22,7 @@ namespace clients::http { namespace { // TODO: not good, for full coverage see libpsl (used by curl), or something else -static const std::set kPublicSuffixes = { +static const std::set> kPublicSuffixes = { // simple TLD "com", "org", "net", "edu", "gov", "mil", "int", @@ -41,22 +41,6 @@ static const std::set kPublicSuffixes = { "appspot.com", "blogspot.com", "github.io", "githubpages.com" }; -// Every stored cookie-domain that domain-matches the request host, per -// RFC 6265 §5.1.3 (Domain Matching): the host itself plus each of its parent -// domains. A stored cookie domain-matches iff it equals one of these (a cookie -// set for "example.com" is thus returned for "www.example.com" too). -std::vector DomainCandidates(std::string_view host) { - std::vector out; - std::string_view cur = host; - while (true) { - out.emplace_back(cur); - const auto dot = cur.find('.'); - if (dot == std::string_view::npos) break; - cur = cur.substr(dot + 1); - } - return out; -} - // There is issue with calling userver's version from non coroutines. We temporary using this one std::string ToLower(std::string_view str) { std::string result; @@ -65,7 +49,7 @@ std::string ToLower(std::string_view str) { return result; } -bool IsPublicSuffix(const std::string& domain) { +bool IsPublicSuffix(std::string_view domain) { return kPublicSuffixes.find(domain) != kPublicSuffixes.end(); } @@ -175,7 +159,7 @@ static std::string LowerDomainWithoutLeadingDot(std::string_view domain) { } // namespace -CookieJar::Cookie::Cookie(const std::string& name, const std::string& value, +CookieJar::Cookie::Cookie(std::string_view name, std::string_view value, const std::chrono::system_clock::time_point& creation_time, const size_t path_length) : name_(name), value_(value), creation_time_(creation_time), path_length_(path_length){ } @@ -212,7 +196,7 @@ class CookieJar::Impl { return; } - std::optional GetAnyCookieValue(const std::string& name) { + std::optional GetAnyCookieValue(std::string_view name) { for (const auto& item : storage_) { const auto& location = item.second.find(name); if (!location->second.empty()) { @@ -222,11 +206,10 @@ class CookieJar::Impl { return std::nullopt; } - CookieJar::Cookies GetCookies(std::string_view scheme, const std::string& domain, std::string_view path) { - CookieJar::Cookies result; + void GetCookies(CookieJar::Cookies& out, std::string_view scheme, std::string_view domain, std::string_view path) { const auto& location = storage_.find(domain); if (location == storage_.end()) { - return result; + return; } const bool secure_scheme = IsSecureScheme(scheme); for (const auto& cookies : location->second) { @@ -246,10 +229,9 @@ class CookieJar::Impl { continue; } } - result.emplace_back(cookie.name, cookie.value, cookie.creation_time, cookie.path.size()); + out.emplace_back(cookie.name, cookie.value, cookie.creation_time, cookie.path.size()); } } - return result; } private: @@ -259,9 +241,9 @@ class CookieJar::Impl { // Cookies, which differs only in path property using CookiesList = SmallVectorStorage; // Map from cookie name to list of cookies - using CookieNamesMap = std::unordered_map; + using CookieNamesMap = std::unordered_map>; // Hashtable from domain to map of cookies - using Storage = std::unordered_map; + using Storage = std::unordered_map>; void DeleteCookie(const ValidatedCookie& cookie) { const auto location = storage_.find(cookie.domain); @@ -375,10 +357,22 @@ class CookieJar::Impl { } void InsertOrAssignCookieToMap(ValidatedCookie&& cookie) { - const auto& map_location = storage_.try_emplace(cookie.domain, CookieNamesMap{}); - auto location = map_location.first->second.try_emplace(cookie.name, CookiesList{}); - auto& list = location.first->second; - auto cookie_location = FindDuplicate(list, cookie); + // A little optimization to exclude extra allocation instead of try_emplace way + const auto& map_location = storage_.find(cookie.domain); + if (map_location == storage_.end()) { + std::string domain = cookie.domain; + std::string cookie_name = cookie.name; + storage_.emplace(std::move(domain), CookieNamesMap{{std::move(cookie_name), CookiesList{std::move(cookie)}}}); + return; + } + const auto& cookie_list_location = map_location->second.find(cookie.name); + if (cookie_list_location == map_location->second.end()) { + std::string cookie_name = cookie.name; + map_location->second.emplace(std::move(cookie_name), CookiesList{std::move(cookie)}); + return; + } + auto& list = cookie_list_location->second; + const auto& cookie_location = FindDuplicate(list, cookie); if (cookie_location != list.end()) { // Preserving old creation time, needed for sorting output cookies cookie.creation_time = cookie_location->creation_time; @@ -425,34 +419,42 @@ void CookieJar::AddCookie(std::string_view url, server::http::Cookie&& cookie) { impl_->AddCookie(url, std::move(cookie)); } -std::optional CookieJar::GetAnyCookieValue(const std::string& name) { +std::optional CookieJar::GetAnyCookieValue(std::string_view name) { return impl_->GetAnyCookieValue(name); } CookieJar::Cookies CookieJar::GetCookies(std::string_view url) { Cookies result; const auto& parsed_url = userver::http::DecomposeUrlIntoViews(url); - const auto domains = DomainCandidates(LowerDomainWithoutLeadingDot(parsed_url.host)); - - for (const auto& d : domains) { - auto domain_cookies = impl_->GetCookies(parsed_url.scheme, d, parsed_url.path); - if (domain_cookies.empty()) { - continue; + const auto& lowered_domain = LowerDomainWithoutLeadingDot(parsed_url.host); + // Iteration without additional allocations + std::string_view domain = lowered_domain; + auto iterations = std::ranges::count(domain, '.'); + for (;;) { + if (domain.empty()) { + break; } - result.insert(result.end(), domain_cookies.begin(), domain_cookies.end()); - } - // In fact this optional but recommended step. Maybe add flag? - // RFC 6265 5.4.1 - specifies order with remark: - // `Not all user agents sort the cookie-list in this order, but - // this order reflects common practice when this document was - // written, and, historically, there have been servers that - // (erroneously) depended on this order.` - std::sort(result.begin(), result.end(), [](const CookieJar::Cookie& lhs, const CookieJar::Cookie& rhs) { - if (lhs.path_length_ != rhs.path_length_) { - return lhs.path_length_ > rhs.path_length_; + impl_->GetCookies(result, parsed_url.scheme, domain, parsed_url.path); + if (iterations <= 1) { + break; } - return lhs.creation_time_ < rhs.creation_time_; - }); + --iterations; + domain = domain.substr(domain.find('.') + 1); + } + if (result.size() > 1) { + // In fact this optional but recommended step. Maybe add flag? + // RFC 6265 5.4.1 - specifies order with remark: + // `Not all user agents sort the cookie-list in this order, but + // this order reflects common practice when this document was + // written, and, historically, there have been servers that + // (erroneously) depended on this order.` + std::sort(result.begin(), result.end(), [](const CookieJar::Cookie& lhs, const CookieJar::Cookie& rhs) { + if (lhs.path_length_ != rhs.path_length_) { + return lhs.path_length_ > rhs.path_length_; + } + return lhs.creation_time_ < rhs.creation_time_; + }); + } return result; }