LadybirdBrowser/ladybird
1
1
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2024-10-26 06:49:12 +03:00
Code Issues Packages Projects Releases Wiki Activity

Browser: Synchronize database statements used for cookies

Browse Source 
Since SQLServer is inherently asynchronous, database statements can run
in parallel. Our `CookieJar` did not require synchronous actions on the
database for its cookies, resulting in cookies not being set
immediately. This resulted in a bug that could be exposed by setting
`document.cookie` and immediately querying its value, revealing that the
cookie was not yet persisted.

Solve this by requiring all database statements to be executed
synchronously. Ideally SQLServer has a mechanism to determine interquery
dependencies and blocks until dependent queries are fully executed, but
until we have that, this works around that limitation.
This commit is contained in:
Jelle Raaijmakers 2023-08-07 23:33:01 +02:00 committed by Tim Flynn
parent 568c486610
commit 14091f32c6
Notes: sideshowbarker 2024-07-17 03:45:48 +09:00 
Author: https://github.com/gmta
Commit: https://github.com/SerenityOS/serenity/commit/14091f32c6
Pull-request: https://github.com/SerenityOS/serenity/pull/20416
Reviewed-by: https://github.com/trflynn89
2 changed files with 26 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
Userland/Applications/Browser/CookieJar.cpp
View File

@ -2,6 +2,7 @@
* Copyright (c) 2021-2023, Tim Flynn <trflynn89@serenityos.org>
* Copyright (c) 2022, the SerenityOS developers.
* Copyright (c) 2022, Tobias Christiansen <tobyase@serenityos.org>
* Copyright (c) 2023, Jelle Raaijmakers <jelle@gmta.nl>
*
* SPDX-License-Identifier: BSD-2-Clause
*/
@ -349,11 +350,14 @@ void CookieJar::store_cookie(Web::Cookie::ParsedCookie const& parsed_cookie, con
if (source != Web::Cookie::Source::Http && cookie.http_only)
return;
// Synchronize persisting the cookie
auto sync_promise = Core::Promise<Empty>::construct();
select_cookie_from_database(
move(cookie),
// 11. If the cookie store contains a cookie with the same name, domain, and path as the newly created cookie:
[this, source](auto& cookie, auto old_cookie) {
[this, source, sync_promise](auto& cookie, auto old_cookie) {
// If the newly created cookie was received from a "non-HTTP" API and the old-cookie's http-only-flag is set, abort these
// steps and ignore the newly created cookie entirely.
if (source != Web::Cookie::Source::Http && old_cookie.http_only)
@ -365,12 +369,16 @@ void CookieJar::store_cookie(Web::Cookie::ParsedCookie const& parsed_cookie, con
// Remove the old-cookie from the cookie store.
// NOTE: Rather than deleting then re-inserting this cookie, we update it in-place.
update_cookie_in_database(cookie);
sync_promise->resolve({});
},
// 12. Insert the newly created cookie into the cookie store.
[this](auto cookie) {
[this, sync_promise](auto cookie) {
insert_cookie_into_database(cookie);
sync_promise->resolve({});
});
MUST(sync_promise->await());
}
Vector<Web::Cookie::Cookie> CookieJar::get_matching_cookies(const URL& url, DeprecatedString const& canonicalized_domain, Web::Cookie::Source source, MatchingCookiesSpecMode mode)
@ -600,8 +608,6 @@ void CookieJar::select_all_cookies_from_database(OnSelectAllCookiesResult on_res
// FIXME: Make surrounding APIs asynchronous.
m_storage.visit(
[&](PersistedStorage& storage) {
auto promise = Core::Promise<Empty>::construct();
storage.database.execute_statement(
storage.statements.select_all_cookies,
[on_result = move(on_result)](auto row) {
@ -610,14 +616,8 @@ void CookieJar::select_all_cookies_from_database(OnSelectAllCookiesResult on_res
else
on_result(cookie.release_value());
},
[&]() {
promise->resolve({});
},
[&](auto) {
promise->resolve({});
});
MUST(promise->await());
{},
{});
},
[&](TransientStorage& storage) {
for (auto const& cookie : storage)

16
Userland/Applications/Browser/Database.h
View File

@ -1,5 +1,6 @@
/*
* Copyright (c) 2022, Tim Flynn <trflynn89@serenityos.org>
* Copyright (c) 2023, Jelle Raaijmakers <jelle@gmta.nl>
*
* SPDX-License-Identifier: BSD-2-Clause
*/
@ -14,6 +15,7 @@
#include <AK/RefCounted.h>
#include <AK/StringView.h>
#include <AK/Vector.h>
#include <LibCore/Promise.h>
#include <LibSQL/SQLClient.h>
#include <LibSQL/Type.h>
#include <LibSQL/Value.h>
@ -34,14 +36,24 @@ public:
template<typename... PlaceholderValues>
void execute_statement(SQL::StatementID statement_id, OnResult on_result, OnComplete on_complete, OnError on_error, PlaceholderValues&&... placeholder_values)
{
auto sync_promise = Core::Promise<Empty>::construct();
PendingExecution pending_execution {
.on_result = move(on_result),
.on_complete = move(on_complete),
.on_error = move(on_error),
.on_complete = [sync_promise, on_complete = move(on_complete)] {
if (on_complete)
on_complete();
sync_promise->resolve({}); },
.on_error = [sync_promise, on_error = move(on_error)](auto message) {
if (on_error)
on_error(message);
sync_promise->resolve({}); },
};
Vector<SQL::Value> values { SQL::Value(forward<PlaceholderValues>(placeholder_values))... };
execute_statement(statement_id, move(values), move(pending_execution));
MUST(sync_promise->await());
}
private: