From 87c4080d009d8f3d714a2f9ad4774696d933902d Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Fri, 24 Feb 2023 11:51:56 -0500 Subject: [PATCH] Browser+LibWeb+WebContent: Store cookie expiry times in UTC We are currently converting parsed expiry times to local time, whereas the RFC dictates we parse them as UTC. When expiring cookies, we must also use the current UTC time to compare against the cookies' expiry times. --- Userland/Applications/Browser/CookieJar.cpp | 40 ++++++++++--------- .../Applications/Browser/CookiesModel.cpp | 2 +- .../Applications/Browser/StorageWidget.cpp | 2 +- Userland/Libraries/LibWeb/Cookie/Cookie.cpp | 29 ++++++++++++-- Userland/Libraries/LibWeb/Cookie/Cookie.h | 14 ++++--- .../Libraries/LibWeb/Cookie/ParsedCookie.cpp | 26 +++++++----- .../Libraries/LibWeb/Cookie/ParsedCookie.h | 8 ++-- .../WebContent/WebDriverConnection.cpp | 9 +++-- 8 files changed, 83 insertions(+), 47 deletions(-) diff --git a/Userland/Applications/Browser/CookieJar.cpp b/Userland/Applications/Browser/CookieJar.cpp index 2e39fce90d5..76d987f8f8b 100644 --- a/Userland/Applications/Browser/CookieJar.cpp +++ b/Userland/Applications/Browser/CookieJar.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021-2022, Tim Flynn + * Copyright (c) 2021-2023, Tim Flynn * Copyright (c) 2022, the SerenityOS developers. * Copyright (c) 2022, Tobias Christiansen * @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -140,9 +141,9 @@ void CookieJar::dump_cookies() builder.appendff("{}{}{}\n", key_color, cookie.path, no_color); builder.appendff("\t{}Value{} = {}\n", attribute_color, no_color, cookie.value); - builder.appendff("\t{}CreationTime{} = {}\n", attribute_color, no_color, cookie.creation_time.to_deprecated_string()); - builder.appendff("\t{}LastAccessTime{} = {}\n", attribute_color, no_color, cookie.last_access_time.to_deprecated_string()); - builder.appendff("\t{}ExpiryTime{} = {}\n", attribute_color, no_color, cookie.expiry_time.to_deprecated_string()); + builder.appendff("\t{}CreationTime{} = {}\n", attribute_color, no_color, cookie.creation_time_to_string()); + builder.appendff("\t{}LastAccessTime{} = {}\n", attribute_color, no_color, cookie.last_access_time_to_string()); + builder.appendff("\t{}ExpiryTime{} = {}\n", attribute_color, no_color, cookie.expiry_time_to_string()); builder.appendff("\t{}Secure{} = {:s}\n", attribute_color, no_color, cookie.secure); builder.appendff("\t{}HttpOnly{} = {:s}\n", attribute_color, no_color, cookie.http_only); builder.appendff("\t{}HostOnly{} = {:s}\n", attribute_color, no_color, cookie.host_only); @@ -275,7 +276,7 @@ void CookieJar::store_cookie(Web::Cookie::ParsedCookie const& parsed_cookie, con // 2. Create a new cookie with name cookie-name, value cookie-value. Set the creation-time and the last-access-time to the current date and time. Web::Cookie::Cookie cookie { parsed_cookie.name, parsed_cookie.value, parsed_cookie.same_site_attribute }; - cookie.creation_time = Core::DateTime::now(); + cookie.creation_time = Time::now_realtime(); cookie.last_access_time = cookie.creation_time; if (parsed_cookie.expiry_time_from_max_age_attribute.has_value()) { @@ -289,9 +290,9 @@ void CookieJar::store_cookie(Web::Cookie::ParsedCookie const& parsed_cookie, con cookie.persistent = true; cookie.expiry_time = parsed_cookie.expiry_time_from_expires_attribute.value(); } else { - // Set the cookie's persistent-flag to false. Set the cookie's expiry-time to the latest representable gddate. + // Set the cookie's persistent-flag to false. Set the cookie's expiry-time to the latest representable date. cookie.persistent = false; - cookie.expiry_time = Core::DateTime::create(9999, 12, 31, 23, 59, 59); + cookie.expiry_time = Time::max(); } // 4. If the cookie-attribute-list contains an attribute with an attribute-name of "Domain": @@ -396,13 +397,13 @@ Vector CookieJar::get_matching_cookies(const URL& url, Depr // - Cookies with longer paths are listed before cookies with shorter paths. // - Among cookies that have equal-length path fields, cookies with earlier creation-times are listed before cookies with later creation-times. auto cookie_path_length = cookie.path.length(); - auto cookie_creation_time = cookie.creation_time.timestamp(); + auto cookie_creation_time = cookie.creation_time; cookie_list.insert_before_matching(move(cookie), [cookie_path_length, cookie_creation_time](auto const& entry) { if (cookie_path_length > entry.path.length()) { return true; } else if (cookie_path_length == entry.path.length()) { - if (cookie_creation_time < entry.creation_time.timestamp()) + if (cookie_creation_time < entry.creation_time) return true; } return false; @@ -410,7 +411,7 @@ Vector CookieJar::get_matching_cookies(const URL& url, Depr }); // 3. Update the last-access-time of each cookie in the cookie-list to the current date and time. - auto now = Core::DateTime::now(); + auto now = Time::now_realtime(); for (auto& cookie : cookie_list) { cookie.last_access_time = now; @@ -450,8 +451,8 @@ static ErrorOr parse_cookie(ReadonlySpan row) if (value.type() != SQL::SQLType::Integer) return Error::from_string_view(name); - auto time = value.to_int().value(); - field = Core::DateTime::from_timestamp(time); + auto time = value.to_int().value(); + field = Time::from_seconds(time); return {}; }; @@ -492,9 +493,9 @@ void CookieJar::insert_cookie_into_database(Web::Cookie::Cookie const& cookie) cookie.name, cookie.value, to_underlying(cookie.same_site), - cookie.creation_time.timestamp(), - cookie.last_access_time.timestamp(), - cookie.expiry_time.timestamp(), + cookie.creation_time.to_seconds(), + cookie.last_access_time.to_seconds(), + cookie.expiry_time.to_seconds(), cookie.domain, cookie.path, cookie.secure, @@ -509,9 +510,9 @@ void CookieJar::update_cookie_in_database(Web::Cookie::Cookie const& cookie) m_statements.update_cookie, {}, [this]() { purge_expired_cookies(); }, {}, cookie.value, to_underlying(cookie.same_site), - cookie.creation_time.timestamp(), - cookie.last_access_time.timestamp(), - cookie.expiry_time.timestamp(), + cookie.creation_time.to_seconds(), + cookie.last_access_time.to_seconds(), + cookie.expiry_time.to_seconds(), cookie.secure, cookie.http_only, cookie.host_only, @@ -581,7 +582,8 @@ void CookieJar::select_all_cookies_from_database(OnSelectAllCookiesResult on_res void CookieJar::purge_expired_cookies() { - auto now = Core::DateTime::now().timestamp(); + auto now = Time::now_realtime().to_seconds(); m_database.execute_statement(m_statements.expire_cookie, {}, {}, {}, now); } + } diff --git a/Userland/Applications/Browser/CookiesModel.cpp b/Userland/Applications/Browser/CookiesModel.cpp index 50df78a2439..2d16901bd56 100644 --- a/Userland/Applications/Browser/CookiesModel.cpp +++ b/Userland/Applications/Browser/CookiesModel.cpp @@ -80,7 +80,7 @@ GUI::Variant CookiesModel::data(GUI::ModelIndex const& index, GUI::ModelRole rol case Column::Value: return cookie.value; case Column::ExpiryTime: - return cookie.expiry_time.to_deprecated_string(); + return cookie.expiry_time_to_string(); case Column::SameSite: return Web::Cookie::same_site_to_string(cookie.same_site); } diff --git a/Userland/Applications/Browser/StorageWidget.cpp b/Userland/Applications/Browser/StorageWidget.cpp index 4c7314fd87c..9837b879cd4 100644 --- a/Userland/Applications/Browser/StorageWidget.cpp +++ b/Userland/Applications/Browser/StorageWidget.cpp @@ -130,7 +130,7 @@ void StorageWidget::clear_session_storage_entries() void StorageWidget::delete_cookie(Web::Cookie::Cookie cookie) { // Delete cookie by making its expiry time in the past. - cookie.expiry_time = Core::DateTime::from_timestamp(0); + cookie.expiry_time = Time::from_seconds(0); if (on_update_cookie) on_update_cookie(move(cookie)); } diff --git a/Userland/Libraries/LibWeb/Cookie/Cookie.cpp b/Userland/Libraries/LibWeb/Cookie/Cookie.cpp index 229ca380904..bbef4fb5c68 100644 --- a/Userland/Libraries/LibWeb/Cookie/Cookie.cpp +++ b/Userland/Libraries/LibWeb/Cookie/Cookie.cpp @@ -1,15 +1,38 @@ /* * Copyright (c) 2022, Tobias Christiansen + * Copyright (c) 2023, Tim Flynn * * SPDX-License-Identifier: BSD-2-Clause */ #include "Cookie.h" +#include #include #include namespace Web::Cookie { +static DeprecatedString time_to_string(Time const& time) +{ + auto local_time = Core::DateTime::from_timestamp(time.to_seconds()); + return local_time.to_deprecated_string("%Y-%m-%d %H:%M:%S %Z"sv); +} + +DeprecatedString Cookie::creation_time_to_string() const +{ + return time_to_string(creation_time); +} + +DeprecatedString Cookie::last_access_time_to_string() const +{ + return time_to_string(last_access_time); +} + +DeprecatedString Cookie::expiry_time_to_string() const +{ + return time_to_string(expiry_time); +} + StringView same_site_to_string(SameSite same_site) { switch (same_site) { @@ -64,11 +87,11 @@ ErrorOr IPC::decode(Decoder& decoder) auto value = TRY(decoder.decode()); auto domain = TRY(decoder.decode()); auto path = TRY(decoder.decode()); - auto creation_time = TRY(decoder.decode()); - auto expiry_time = TRY(decoder.decode()); + auto creation_time = TRY(decoder.decode