From 497dd5b35402e67eaac943e88def4d6973ce9ca2 Mon Sep 17 00:00:00 2001 From: Idan Horowitz Date: Sat, 12 Feb 2022 16:02:56 +0200 Subject: [PATCH] LibWeb: Set response header cookies on redirects Since we were previously relying on Document::set_cookie in order to set cookies received as a 'Set-Cookie' response header, we would ignore any response header cookies in redirect (status code 3xx) responses. While this behaviour is not strictly enforced in the specification, most major browsers do set cookies in redirect responses, and some sites (e.g. Cookie Clicker) rely on this behaviour. Since cookies are stored per-site and not per-document, this behaviour is achieved by simply decoupling the cookie set mechanism from it. --- .../Libraries/LibWeb/Loader/FrameLoader.cpp | 34 +++++++++++++------ .../Libraries/LibWeb/Loader/FrameLoader.h | 2 ++ 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp b/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp index e20fe2227b..123e2d31ca 100644 --- a/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp +++ b/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -253,10 +254,33 @@ void FrameLoader::load_favicon(RefPtr bitmap) } } +void FrameLoader::store_response_cookies(AK::URL const& url, String const& cookies) +{ + auto* page = browsing_context().page(); + if (!page) + return; + + auto set_cookie_json_value = MUST(JsonValue::from_string(cookies)); + VERIFY(set_cookie_json_value.type() == JsonValue::Type::Array); + + for (const auto& set_cookie_entry : set_cookie_json_value.as_array().values()) { + VERIFY(set_cookie_entry.type() == JsonValue::Type::String); + + auto cookie = Cookie::parse_cookie(set_cookie_entry.as_string()); + if (!cookie.has_value()) + continue; + + page->client().page_did_set_cookie(url, cookie.value(), Cookie::Source::Http); // FIXME: Determine cookie source correctly + } +} + void FrameLoader::resource_did_load() { auto url = resource()->url(); + if (auto set_cookie = resource()->response_headers().get("Set-Cookie"); set_cookie.has_value()) + store_response_cookies(url, *set_cookie); + // For 3xx (Redirection) responses, the Location value refers to the preferred target resource for automatically redirecting the request. auto status_code = resource()->status_code(); if (status_code.has_value() && *status_code >= 300 && *status_code <= 399) { @@ -297,16 +321,6 @@ void FrameLoader::resource_did_load() return; } - auto set_cookie = resource()->response_headers().get("Set-Cookie"); - if (set_cookie.has_value()) { - auto set_cookie_json_value = MUST(JsonValue::from_string(set_cookie.value())); - VERIFY(set_cookie_json_value.type() == JsonValue::Type::Array); - for (const auto& set_cookie_entry : set_cookie_json_value.as_array().values()) { - VERIFY(set_cookie_entry.type() == JsonValue::Type::String); - document->set_cookie(set_cookie_entry.as_string(), Cookie::Source::Http); - } - } - if (!url.fragment().is_empty()) browsing_context().scroll_to_anchor(url.fragment()); else diff --git a/Userland/Libraries/LibWeb/Loader/FrameLoader.h b/Userland/Libraries/LibWeb/Loader/FrameLoader.h index 1e80d6f72e..5fecbce26c 100644 --- a/Userland/Libraries/LibWeb/Loader/FrameLoader.h +++ b/Userland/Libraries/LibWeb/Loader/FrameLoader.h @@ -43,6 +43,8 @@ private: void load_favicon(RefPtr bitmap = nullptr); bool parse_document(DOM::Document&, const ByteBuffer& data); + void store_response_cookies(AK::URL const& url, String const& cookies); + HTML::BrowsingContext& m_browsing_context; size_t m_redirects_count { 0 }; };