From 5ac093885922246529a467054888e598f8832450 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 9 Jun 2024 13:27:58 +0200 Subject: [PATCH] LibHTTP+LibWeb: Stop bundling "Set-Cookie" headers as JSON Before we had HTTP::HeaderMap (which preserves multiple headers with the same name), we collected multiple "Set-Cookie" headers and bundled them together as a JSON array. This was a huge hack, and now we can stop doing that, since LibWeb gets access to the full set of headers now. --- Ladybird/Qt/RequestManagerQt.cpp | 7 +---- Userland/Libraries/LibHTTP/Job.cpp | 22 +--------------- Userland/Libraries/LibHTTP/Job.h | 1 - .../LibWeb/Loader/ResourceLoader.cpp | 26 +++++++------------ 4 files changed, 12 insertions(+), 44 deletions(-) diff --git a/Ladybird/Qt/RequestManagerQt.cpp b/Ladybird/Qt/RequestManagerQt.cpp index 0c295a2bb24..8d55af47101 100644 --- a/Ladybird/Qt/RequestManagerQt.cpp +++ b/Ladybird/Qt/RequestManagerQt.cpp @@ -7,7 +7,6 @@ #include "RequestManagerQt.h" #include "WebSocketImplQt.h" #include "WebSocketQt.h" -#include #include namespace Ladybird { @@ -113,7 +112,6 @@ void RequestManagerQt::Request::did_finish() auto buffer = m_reply.readAll(); auto http_status_code = m_reply.attribute(QNetworkRequest::Attribute::HttpStatusCodeAttribute).toInt(); HTTP::HeaderMap response_headers; - Vector set_cookie_headers; for (auto& it : m_reply.rawHeaderPairs()) { auto name = ByteString(it.first.data(), it.first.length()); auto value = ByteString(it.second.data(), it.second.length()); @@ -122,15 +120,12 @@ void RequestManagerQt::Request::did_finish() // We have to extract the full list of cookies via QNetworkReply::header(). auto set_cookie_list = m_reply.header(QNetworkRequest::SetCookieHeader).value>(); for (auto const& cookie : set_cookie_list) { - set_cookie_headers.append(cookie.toRawForm().data()); + response_headers.set(name, cookie.toRawForm().data()); } } else { response_headers.set(name, value); } } - if (!set_cookie_headers.is_empty()) { - response_headers.set("set-cookie"sv, JsonArray { set_cookie_headers }.to_byte_string()); - } bool success = http_status_code != 0; on_buffered_request_finish(success, buffer.length(), response_headers, http_status_code, ReadonlyBytes { buffer.data(), (size_t)buffer.size() }); } diff --git a/Userland/Libraries/LibHTTP/Job.cpp b/Userland/Libraries/LibHTTP/Job.cpp index d038c38999c..2e1074189d2 100644 --- a/Userland/Libraries/LibHTTP/Job.cpp +++ b/Userland/Libraries/LibHTTP/Job.cpp @@ -7,7 +7,6 @@ #include #include -#include #include #include #include @@ -315,8 +314,6 @@ void Job::on_socket_connected() return finish_up(); } if (on_headers_received) { - if (!m_set_cookie_headers.is_empty()) - m_headers.set("Set-Cookie", JsonArray { m_set_cookie_headers }.to_byte_string()); on_headers_received(m_headers, m_code > 0 ? m_code : Optional {}); } m_state = State::InBody; @@ -362,24 +359,7 @@ void Job::on_socket_connected() return deferred_invoke([this] { did_fail(Core::NetworkJob::Error::ProtocolFailed); }); } auto value = line.substring(name.length() + 2, line.length() - name.length() - 2); - if (name.equals_ignoring_ascii_case("Set-Cookie"sv)) { - dbgln_if(JOB_DEBUG, "Job: Received Set-Cookie header: '{}'", value); - m_set_cookie_headers.append(move(value)); - - auto can_read_without_blocking = m_socket->can_read_without_blocking(); - if (can_read_without_blocking.is_error()) - return deferred_invoke([this] { did_fail(Core::NetworkJob::Error::TransmissionFailed); }); - if (!can_read_without_blocking.value()) - return; - } else if (auto existing_value = m_headers.get(name); existing_value.has_value()) { - StringBuilder builder; - builder.append(existing_value.value()); - builder.append(','); - builder.append(value); - m_headers.set(name, builder.to_byte_string()); - } else { - m_headers.set(name, value); - } + m_headers.set(name, value); if (name.equals_ignoring_ascii_case("Content-Encoding"sv)) { // Assume that any content-encoding means that we can't decode it as a stream :( dbgln_if(JOB_DEBUG, "Content-Encoding {} detected, cannot stream output :(", value); diff --git a/Userland/Libraries/LibHTTP/Job.h b/Userland/Libraries/LibHTTP/Job.h index 14d91152975..15a18d5dbd9 100644 --- a/Userland/Libraries/LibHTTP/Job.h +++ b/Userland/Libraries/LibHTTP/Job.h @@ -54,7 +54,6 @@ protected: bool m_legacy_connection { false }; int m_code { -1 }; HTTP::HeaderMap m_headers; - Vector m_set_cookie_headers; struct ReceivedBuffer { ReceivedBuffer(ByteBuffer d) diff --git a/Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp b/Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp index 548a8113371..4c521e98a9d 100644 --- a/Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp +++ b/Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp @@ -6,7 +6,6 @@ */ #include -#include #include #include #include @@ -150,20 +149,12 @@ static void emit_signpost(ByteString const& message, int id) #endif } -static void store_response_cookies(Page& page, URL::URL const& url, ByteString const& cookies) +static void store_response_cookies(Page& page, URL::URL const& url, ByteString const& set_cookie_entry) { - auto set_cookie_json_value = MUST(JsonValue::from_string(cookies)); - VERIFY(set_cookie_json_value.type() == JsonValue::Type::Array); - - for (auto const& 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 - } + auto cookie = Cookie::parse_cookie(set_cookie_entry); + if (!cookie.has_value()) + return; + page.client().page_did_set_cookie(url, cookie.value(), Cookie::Source::Http); // FIXME: Determine cookie source correctly } static HTTP::HeaderMap response_headers_for_file(StringView path, Optional const& modified_time) @@ -542,8 +533,11 @@ void ResourceLoader::handle_network_response_headers(LoadRequest const& request, if (!request.page()) return; - if (auto set_cookie = response_headers.get("Set-Cookie"); set_cookie.has_value()) - store_response_cookies(*request.page(), request.url(), *set_cookie); + for (auto const& [header, value] : response_headers.headers()) { + if (header.equals_ignoring_ascii_case("Set-Cookie"sv)) { + store_response_cookies(*request.page(), request.url(), value); + } + } if (auto cache_control = response_headers.get("Cache-Control"); cache_control.has_value()) { if (cache_control.value().contains("no-store"sv))