fix core: storing all cookies without possibly wrong selection, keeping session cookies semantics

core/include/userver/clients/http/cookie_jar.hpp

@@ -0,0 +1,82 @@
+#pragma once
+
+/// @file userver/clients/http/cookie_jar.hpp
+/// @brief @copybrief clients::http::CookieJar
+
+#include <cstddef>
+#include <string>
+#include <vector>
+
+#include <userver/server/http/http_response_cookie.hpp>
+#include <userver/utils/fast_pimpl.hpp>
+
+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(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_; 
+        }
+
+        inline const std::string& Value() const { 
+            return value_; 
+        }
+
+    private:
+        friend class CookieJar;
+
+        std::string name_;
+        std::string value_;
+        std::chrono::system_clock::time_point creation_time_;
+        size_t path_length_;
+    };
+
+    /// @brief List of cookies
+    /// @warning Be aware that list can contain cookies with the same name
+    using Cookies = std::vector<Cookie>;
+
+    CookieJar();
+    ~CookieJar();
+    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
+    void Merge(CookieJar&& cookie_jar);
+
+    /// @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<std::string> GetAnyCookieValue(std::string_view name);
+
+
+    /// @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, 96, 8> impl_;
+};
+
+}  // namespace clients::http
+
+USERVER_NAMESPACE_END

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<std::string, std::string>& 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

core/include/userver/clients/http/response.hpp

@@ -5,6 +5,7 @@
 
 #include <string>
 
+#include <userver/clients/http/cookie_jar.hpp>
 #include <userver/clients/http/error.hpp>
 #include <userver/clients/http/local_stats.hpp>
 #include <userver/http/header_map.hpp>
@@ -25,6 +26,7 @@ using Headers = USERVER_NAMESPACE::http::headers::HeaderMap;
 class Response final {
 public:
     using CookiesMap = server::http::Cookie::CookiesMap;
+    using CookiesEngine = std::variant<CookiesMap, CookieJar>;
 
     Response() = default;
 
@@ -41,8 +43,36 @@ 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_; }
+
+    /// @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 response holds specified storage
+    /// @return Check result
+    template <typename Value>
+    bool is_cookie_storage() const {
+        return cookies_engine_ && std::get_if<Value>(cookies_engine_.get()) != nullptr;
+    }
+
+    /// @brief Sets cookie engine
+    void set_cookie_engine(const std::shared_ptr<CookiesEngine>& cookies_engine);
 
     /// status_code
     Status status_code() const;
@@ -71,7 +101,7 @@ class Response final {
 
 private:
     Headers headers_;
-    CookiesMap cookies_;
+    std::shared_ptr<CookiesEngine> cookies_engine_;
     std::string response_;
     Status status_code_{Status::kInvalid};
     LocalStats stats_;

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<std::chrono::system_clock::time_point> 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<std::chrono::seconds> MaxAge() const noexcept;
     Cookie& SetMaxAge(std::chrono::seconds value) noexcept;
 
     std::string SameSite() const;

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<HttpResponse> Process100(const HttpRequest& request) {
     return std::nullopt;
 }
 
+std::vector<std::string> Pairs(const clients::http::CookieJar::Cookies& cookies) {
+    std::vector<std::string> out;
+    out.reserve(cookies.size());
+    for (const auto& cookie : cookies) {
+        out.push_back(cookie.Name() + '=' + cookie.Value());
+    }
+    return out;
+}
+
+std::vector<std::string> Pairs(const clients::http::Response::CookiesMap& cookies) {
+    std::vector<std::string> 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<std::size_t> responses_200 = std::make_shared<std::size_t>(0);
 
@@ -442,6 +466,26 @@ struct CheckCookie {
     }
 };
 
+struct ReturnCookies {
+    const std::vector<std::string> 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 +1170,53 @@ UTEST(HttpClient, Cookies) {
     test({{"a", "B"}, {"A", "b"}}, {"a=B", "A=b"});
 }
 
+UTEST(HttpClient, CookiesFromServerMapAPI) {
+    // Without compliant CookieJar with rfc 6265, we will just check raw cookies without deduplication etc
+    const auto test = [](std::vector<std::string> response_cookies, std::vector<std::string> 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 =
+                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());
+            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<std::string> response_cookies, std::vector<std::string> 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({"A=1", "A=2", "B=1", "B=2; Max-Age=0"}, {"A=2"});
+}
+
 UTEST(HttpClient, HeadersAndWhitespaces) {
     auto http_client_ptr = utest::CreateHttpClient();