From 9d411648adc0285bf6ed2d0490e5af01d7cf3e6e Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Sat, 19 Apr 2025 16:59:36 +1200 Subject: [PATCH] LibWeb: Avoid URL validity check for 'Resource' Which was previously signally an invalid Resource by a default constructed invalid URL. Instead, switch this over to an Optional URL. --- Libraries/LibWeb/HTML/HTMLLinkElement.cpp | 2 +- Libraries/LibWeb/Loader/LoadRequest.h | 11 ++++----- Libraries/LibWeb/Loader/Resource.cpp | 2 +- Libraries/LibWeb/Loader/Resource.h | 2 +- Libraries/LibWeb/Loader/ResourceLoader.cpp | 26 +++++++++++----------- 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/Libraries/LibWeb/HTML/HTMLLinkElement.cpp b/Libraries/LibWeb/HTML/HTMLLinkElement.cpp index 2f52baba346..2f075169957 100644 --- a/Libraries/LibWeb/HTML/HTMLLinkElement.cpp +++ b/Libraries/LibWeb/HTML/HTMLLinkElement.cpp @@ -610,7 +610,7 @@ bool HTMLLinkElement::load_favicon_and_use_if_window_is_active() return false; // FIXME: Refactor the caller(s) to handle the async nature of image loading - auto promise = decode_favicon(resource()->encoded_data(), resource()->url(), document()); + auto promise = decode_favicon(resource()->encoded_data(), *resource()->url(), document()); auto result = promise->await(); return !result.is_error(); } diff --git a/Libraries/LibWeb/Loader/LoadRequest.h b/Libraries/LibWeb/Loader/LoadRequest.h index 14d57235908..58c2125e33b 100644 --- a/Libraries/LibWeb/Loader/LoadRequest.h +++ b/Libraries/LibWeb/Loader/LoadRequest.h @@ -27,12 +27,12 @@ public: bool is_main_resource() const { return m_main_resource; } void set_main_resource(bool b) { m_main_resource = b; } - bool is_valid() const { return m_url.is_valid(); } + bool is_valid() const { return m_url.has_value(); } int id() const { return m_id; } - const URL::URL& url() const { return m_url; } - void set_url(const URL::URL& url) { m_url = url; } + Optional const& url() const { return m_url; } + void set_url(Optional url) { m_url = move(url); } ByteString const& method() const { return m_method; } void set_method(ByteString const& method) { m_method = method; } @@ -50,7 +50,8 @@ public: { auto body_hash = string_hash((char const*)m_body.data(), m_body.size()); auto body_and_headers_hash = pair_int_hash(body_hash, m_headers.hash()); - auto url_and_method_hash = pair_int_hash(m_url.to_byte_string().hash(), m_method.hash()); + auto url_hash = m_url.has_value() ? m_url->to_byte_string().hash() : 0; + auto url_and_method_hash = pair_int_hash(url_hash, m_method.hash()); return pair_int_hash(body_and_headers_hash, url_and_method_hash); } @@ -75,7 +76,7 @@ public: private: int m_id { 0 }; - URL::URL m_url; + Optional m_url; ByteString m_method { "GET" }; HashMap m_headers; ByteBuffer m_body; diff --git a/Libraries/LibWeb/Loader/Resource.cpp b/Libraries/LibWeb/Loader/Resource.cpp index 3b2bf088e6f..a5270e37ed4 100644 --- a/Libraries/LibWeb/Loader/Resource.cpp +++ b/Libraries/LibWeb/Loader/Resource.cpp @@ -102,7 +102,7 @@ void Resource::did_load(Badge, ReadonlyBytes data, HTTP::HeaderM if (content_type_options.value_or("").equals_ignoring_ascii_case("nosniff"sv)) { m_mime_type = "text/plain"; } else { - m_mime_type = Core::guess_mime_type_based_on_filename(url().file_path()); + m_mime_type = Core::guess_mime_type_based_on_filename(url()->file_path()); } } diff --git a/Libraries/LibWeb/Loader/Resource.h b/Libraries/LibWeb/Loader/Resource.h index bd475594c5d..b60c6f363e6 100644 --- a/Libraries/LibWeb/Loader/Resource.h +++ b/Libraries/LibWeb/Loader/Resource.h @@ -50,7 +50,7 @@ public: bool has_encoded_data() const { return !m_encoded_data.is_empty(); } - const URL::URL& url() const { return m_request.url(); } + Optional const& url() const { return m_request.url(); } ByteBuffer const& encoded_data() const { return m_encoded_data; } [[nodiscard]] HTTP::HeaderMap const& response_headers() const { return m_response_headers; } diff --git a/Libraries/LibWeb/Loader/ResourceLoader.cpp b/Libraries/LibWeb/Loader/ResourceLoader.cpp index 6b417ea5262..0fd68692691 100644 --- a/Libraries/LibWeb/Loader/ResourceLoader.cpp +++ b/Libraries/LibWeb/Loader/ResourceLoader.cpp @@ -89,7 +89,7 @@ RefPtr ResourceLoader::load_resource(Resource::Type type, LoadRequest& if (!request.is_valid()) return nullptr; - bool use_cache = request.url().scheme() != "file"; + bool use_cache = request.url()->scheme() != "file"; if (use_cache) { auto it = s_resource_cache.find(request); @@ -154,14 +154,14 @@ static HTTP::HeaderMap response_headers_for_file(StringView path, Optional static void log_failure(LoadRequest const& request, ErrorType const& error) { - auto url_for_logging = sanitized_url_for_logging(request.url()); + auto url_for_logging = sanitized_url_for_logging(*request.url()); auto load_time_ms = request.load_time().to_milliseconds(); dbgln("ResourceLoader: Failed load of: \"{}\", \033[31;1mError: {}\033[0m, Duration: {}ms", url_for_logging, error, load_time_ms); @@ -178,13 +178,13 @@ static void log_failure(LoadRequest const& request, ErrorType const& error) static void log_filtered_request(LoadRequest const& request) { - auto url_for_logging = sanitized_url_for_logging(request.url()); + auto url_for_logging = sanitized_url_for_logging(*request.url()); dbgln("ResourceLoader: Filtered request to: \"{}\"", url_for_logging); } static bool should_block_request(LoadRequest const& request) { - auto const& url = request.url(); + auto const& url = request.url().value(); auto is_port_blocked = [](int port) { static constexpr auto ports = to_array({ 1, 7, 9, 11, 13, 15, 17, 19, 20, 21, 22, 23, 25, 37, 42, @@ -210,7 +210,7 @@ static bool should_block_request(LoadRequest const& request) void ResourceLoader::load(LoadRequest& request, GC::Root success_callback, GC::Root error_callback, Optional timeout, GC::Root timeout_callback) { - auto const& url = request.url(); + auto const& url = request.url().value(); log_request_start(request); request.start_timer(); @@ -356,7 +356,7 @@ void ResourceLoader::load(LoadRequest& request, GC::Root succes // When local file is a directory use file directory loader to generate response auto maybe_is_valid_directory = Core::Directory::is_valid_directory(fd); if (!maybe_is_valid_directory.is_error() && maybe_is_valid_directory.value()) { - respond_directory_page(request, request.url(), success_callback, error_callback); + respond_directory_page(request, request.url().value(), success_callback, error_callback); return; } @@ -387,7 +387,7 @@ void ResourceLoader::load(LoadRequest& request, GC::Root succes } auto data = maybe_data.release_value(); - auto response_headers = response_headers_for_file(request.url().file_path(), st_or_error.value().st_mtime); + auto response_headers = response_headers_for_file(request.url()->file_path(), st_or_error.value().st_mtime); // FIXME: Implement timing info for file requests. Requests::RequestTimingInfo fixme_implement_timing_info {}; @@ -468,7 +468,7 @@ void ResourceLoader::load(LoadRequest& request, GC::Root succes void ResourceLoader::load_unbuffered(LoadRequest& request, GC::Root on_headers_received, GC::Root on_data_received, GC::Root on_complete) { - auto const& url = request.url(); + auto const& url = request.url().value(); log_request_start(request); request.start_timer(); @@ -516,7 +516,7 @@ void ResourceLoader::load_unbuffered(LoadRequest& request, GC::Root ResourceLoader::start_network_request(LoadRequest const& request) { - auto proxy = ProxyMappings::the().proxy_for_url(request.url()); + auto proxy = ProxyMappings::the().proxy_for_url(request.url().value()); HTTP::HeaderMap headers; @@ -527,7 +527,7 @@ RefPtr ResourceLoader::start_network_request(LoadRequest cons if (!headers.contains("User-Agent")) headers.set("User-Agent", m_user_agent.to_byte_string()); - auto protocol_request = m_request_client->start_request(request.method(), request.url(), headers, request.body(), proxy); + auto protocol_request = m_request_client->start_request(request.method(), request.url().value(), headers, request.body(), proxy); if (!protocol_request) { log_failure(request, "Failed to initiate load"sv); return nullptr; @@ -552,7 +552,7 @@ void ResourceLoader::handle_network_response_headers(LoadRequest const& request, 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); + store_response_cookies(*request.page(), request.url().value(), value); } }