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

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

@@ -0,0 +1,38 @@
+#pragma once
+
+/// @file userver/clients/http/cookie_jar.hpp
+/// @brief @copybrief clients::http::CookieJar
+#include <string>
+#include <vector>
+#include <optional>
+
+USERVER_NAMESPACE_BEGIN
+
+namespace clients::http {
+
+class Request;
+
+/// @brief Storage for cookies, compliable with RFC 6265. Can be used for sending and receiving cookies on agent side.
+class CookieJar final {
+public:
+    CookieJar();
+    CookieJar(std::vector<std::string>&& cookies);
+    ~CookieJar();
+    CookieJar(const CookieJar&);
+    CookieJar(CookieJar&&);
+    CookieJar& operator=(const CookieJar&);
+    CookieJar& operator=(CookieJar&&);
+
+    /// @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
+    /// @warning This method has lineral complexity
+    std::optional<std::string> FindCookieValue(std::string_view name);
+private:
+    friend class Request;
+    std::vector<std::string> cookies_;
+};
+
+}  // namespace clients::http
+
+USERVER_NAMESPACE_END

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

@@ -10,6 +10,7 @@
 #include <userver/clients/dns/resolver_fwd.hpp>
 #include <userver/clients/http/error.hpp>
 #include <userver/clients/http/response.hpp>
+#include <userver/clients/http/cookie_jar.hpp>
 #include <userver/clients/http/response_future.hpp>
 #include <userver/concurrent/queue.hpp>
 #include <userver/crypto/certificate.hpp>
@@ -250,6 +251,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(const CookieJar& cookie_jar) &;
+    /// Sets cookie jar
+    Request cookies(const CookieJar& cookie_jar) &&;
+
     /// Follow redirects or not. Default: follow
     Request& follow_redirects(bool follow = true) &;
     /// @overload
@@ -447,6 +453,8 @@ class Request final {
     /// Returns HTTP body of a request, leaving it empty
     std::string ExtractData();
 
+    CookieJar GetCookieJar();
+
 private:
     std::shared_ptr<RequestState> pimpl_;
 };

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

@@ -79,4 +79,4 @@ class Response final {
 
 }  // namespace clients::http
 
-USERVER_NAMESPACE_END
+USERVER_NAMESPACE_END

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,15 @@ std::optional<HttpResponse> Process100(const HttpRequest& request) {
     return std::nullopt;
 }
 
+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 +457,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 +1161,28 @@ 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));
+        }
+    };
+    test({"token=xyz789"}, {"token=xyz789"});
+    test({"A=1", "A=2", "FOO=BAR"}, {"A=1", "FOO=BAR"});
+}
+
 UTEST(HttpClient, DuplicateSetCookieOverwrites) {
     const utest::SimpleServer http_server{[](const utest::SimpleServer::Request&) {
         static const utest::SimpleServer::Response kResponse{

core/src/clients/http/cookie_jar.cpp

@@ -0,0 +1,60 @@
+#include <userver/clients/http/cookie_jar.hpp>
+
+#include <chrono>
+#include <cstddef>
+#include <string>
+#include <string_view>
+#include <set>
+#include <utility>
+#include <vector>
+#include <algorithm>
+#include <boost/container/small_vector.hpp>
+
+#include <userver/utils/datetime.hpp>
+#include <userver/utils/str_icase.hpp>
+#include <userver/logging/log.hpp>
+#include <userver/http/url.hpp>
+
+USERVER_NAMESPACE_BEGIN
+
+namespace clients::http {
+
+CookieJar::CookieJar() = default;
+CookieJar::~CookieJar() = default;
+CookieJar::CookieJar(std::vector<std::string>&& cookies) : 
+    cookies_(std::move(cookies)) {
+
+}
+CookieJar::CookieJar(const CookieJar&) = default;
+CookieJar::CookieJar(CookieJar&&) = default;
+CookieJar& CookieJar::operator=(const CookieJar&) = default;
+CookieJar& CookieJar::operator=(CookieJar&&) = default;
+
+std::optional<std::string> CookieJar::FindCookieValue(std::string_view name) {
+    //  TODO: not optimal way to search
+    for (std::string_view line : cookies_) {
+        while (!line.empty() && (line.back() == '\n' || line.back() == '\r')) {
+            line.remove_suffix(1);
+        }
+
+        const auto value_pos = line.rfind('\t');
+        if (value_pos == std::string_view::npos || value_pos == 0) {
+            // A comment or a malformed line, both have nothing to look at.
+            continue;
+        }
+        const auto name_pos = line.rfind('\t', value_pos - 1);
+        if (name_pos == std::string_view::npos) {
+            continue;
+        }
+
+        // Cookie names are case-sensitive, RFC 6265, section 5.3.
+        if (line.substr(name_pos + 1, value_pos - name_pos - 1) == name) {
+            return std::string{line.substr(value_pos + 1)};
+        }
+    }
+    return std::nullopt;
+}
+
+}  // namespace clients::http
+
+USERVER_NAMESPACE_END