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

core/CMakeLists.txt

@@ -53,6 +53,35 @@ add_library(${PROJECT_NAME} STATIC ${SOURCES})
 # thread_local usage in Moodycamel is incompatible with userver scheduler that migrates coroutines between threads.
 target_compile_definitions(${PROJECT_NAME} PUBLIC MOODYCAMEL_NO_THREAD_LOCAL)
 
+# A statically linked libcurl leaves no dynamic symbols to interpose, so the Public Suffix List workaround in
+# src/clients/http/cookie_jar.cpp cannot work and is compiled out
+
+get_target_property(_curl_target CURL::libcurl ALIASED_TARGET)
+if(NOT _curl_target)
+    set(_curl_target CURL::libcurl)
+endif()
+
+get_target_property(_curl_config ${_curl_target} IMPORTED_CONFIGURATIONS)
+if(_curl_config)
+    list(GET _curl_config 0 _curl_config)
+    get_target_property(_curl_path ${_curl_target} IMPORTED_LOCATION_${_curl_config})
+else()
+    get_target_property(_curl_path ${_curl_target} IMPORTED_LOCATION)
+endif()
+
+get_target_property(_curl_type ${_curl_target} TYPE)
+get_target_property(_curl_defs ${_curl_target} INTERFACE_COMPILE_DEFINITIONS)
+
+if(_curl_type STREQUAL "STATIC_LIBRARY"
+   OR "CURL_STATICLIB" IN_LIST _curl_defs
+   OR _curl_path MATCHES "\\${CMAKE_STATIC_LIBRARY_SUFFIX}$"
+)
+    set(USERVER_IMPL_STATIC_CURL ON)
+    target_compile_definitions(${PROJECT_NAME} PRIVATE USERVER_IMPL_STATIC_CURL)
+else()
+    set(USERVER_IMPL_STATIC_CURL OFF)
+endif()
+
 include(GenGdbPrinters)
 gen_gdb_cmd(${PROJECT_NAME} "cmd/utask")
 
@@ -280,6 +309,9 @@ if(USERVER_BUILD_TESTS)
     add_executable(${PROJECT_NAME}-unittest ${UNIT_TEST_SOURCES})
     target_include_directories(${PROJECT_NAME}-unittest PRIVATE $<TARGET_PROPERTY:${PROJECT_NAME},INCLUDE_DIRECTORIES>)
     target_link_libraries(${PROJECT_NAME}-unittest PRIVATE userver-utest)
+    if(USERVER_IMPL_STATIC_CURL)
+        target_compile_definitions(${PROJECT_NAME}-unittest PRIVATE USERVER_IMPL_STATIC_CURL)
+    endif()
 
     add_google_tests(${PROJECT_NAME}-unittest)
     add_subdirectory(functional_tests)

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

@@ -0,0 +1,39 @@
+#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() = default;
+
+    /// @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 linear complexity
+    std::optional<std::string> FindCookieValue(std::string_view name) const;
+private:
+    // Constructs CookieJar by list of cookies in netscape file format
+    explicit CookieJar(std::vector<std::string>&& cookies);
+
+    // To allow request to construct/extract cookies in netscape format
+    friend class Request;
+
+    //  List of cookies in netscape file format, directly exposed for libcurl engine
+    //  Other format like Set-Cookie can have side effects, for more information see libcurl docs
+    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=2", "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,84 @@
+#include <userver/clients/http/cookie_jar.hpp>
+
+#include <atomic>
+
+USERVER_NAMESPACE_BEGIN
+
+namespace clients::http {
+
+CookieJar::CookieJar(std::vector<std::string>&& cookies) : 
+    cookies_(std::move(cookies)) {
+}
+
+std::optional<std::string> CookieJar::FindCookieValue(std::string_view name) const {
+    //  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
+
+#if defined(__linux__) && !defined(USERVER_IMPL_STATIC_CURL)
+/*
+    Dynamically linked curl and libpsl can use filesystem in order to get fresh prefix graph for PUBLIC SUFFIX LIST.
+    This check is performed at dangerous place, at receiving of cookie on libev thread.
+    Internally psl graph is cached per easy handle for up to 72 hours. 
+    Algorithm to temporary cope (until is new curl's api is provided) is following:
+        1) For statically linked curl OR no linux based api we are supposed that psl is turned off.
+        2) For dynamically linked curl and dynamically linked psl we are trying to skip fs operating by injecting `psl_latest` method
+        3) For dynamically linked curl with statically linked psl we cannot do anything until new curl's api. Test should catch this tricky curl
+*/
+
+#include <dlfcn.h>
+
+struct psl_ctx_st;
+
+namespace {
+
+//  Counter of invocation of injected method. Useful for testing whether injection is working
+std::atomic<std::size_t> psl_latest_calls{0};
+
+}  // namespace
+
+extern "C" {
+
+#ifndef __clang__
+[[gnu::visibility("default")]] [[gnu::externally_visible]]
+#endif
+psl_ctx_st* psl_latest(const char*) {
+    psl_latest_calls.fetch_add(1, std::memory_order_relaxed);
+    //  Internally `psl_latest` goes through list of predefined locations to load dataset from fs with builtin fallback
+    //  Here are we skip fs lookup to builtin directly
+    static auto func = reinterpret_cast<const psl_ctx_st* (*)()>(dlsym(RTLD_DEFAULT, "psl_builtin"));
+    if (func == nullptr) return nullptr;
+    return const_cast<psl_ctx_st*>(func());
+}
+
+//  Note: overriding free is not needed. By default it does not release builtin context
+
+//  Gets counter of invocations of injected `psl_latest`, useful for tests
+std::size_t userver_impl_psl_latest_calls() { return psl_latest_calls.load(std::memory_order_relaxed); }
+}
+
+#endif