Skip to content

Commit

Permalink
Everywhere: Remove some use of the URL constructors
Browse files Browse the repository at this point in the history
These make it too easy to construct an invalid URL, which makes it
difficult to remove the valid state of URL - which this API relies
on.
  • Loading branch information
shannonbooth authored and trflynn89 committed Feb 19, 2025
1 parent 2823ac9 commit d62cf0a
Show file tree
Hide file tree
Showing 11 changed files with 341 additions and 329 deletions.
4 changes: 3 additions & 1 deletion Libraries/LibCore/MimeData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <AK/StringBuilder.h>
#include <LibCore/File.h>
#include <LibCore/MimeData.h>
#include <LibURL/Parser.h>

namespace Core {

Expand All @@ -18,7 +19,8 @@ Vector<URL::URL> MimeData::urls() const
return {};
Vector<URL::URL> urls;
for (auto& line : StringView(it->value).split_view('\n')) {
urls.append(URL::URL(line));
if (auto maybe_url = URL::Parser::basic_parse(line); maybe_url.has_value())
urls.append(maybe_url.release_value());
}
return urls;
}
Expand Down
6 changes: 3 additions & 3 deletions Libraries/LibURL/URL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,9 @@ URL create_with_file_scheme(ByteString const& path, ByteString const& fragment,

URL create_with_url_or_path(ByteString const& url_or_path)
{
URL url = url_or_path;
if (url.is_valid())
return url;
auto url = Parser::basic_parse(url_or_path);
if (url.has_value())
return url.release_value();

ByteString path = LexicalPath::canonicalized_path(url_or_path);
return create_with_file_scheme(path);
Expand Down
5 changes: 3 additions & 2 deletions Libraries/LibWeb/CSS/CSSStyleSheet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* SPDX-License-Identifier: BSD-2-Clause
*/

#include <LibURL/Parser.h>
#include <LibWeb/Bindings/CSSStyleSheetPrototype.h>
#include <LibWeb/Bindings/Intrinsics.h>
#include <LibWeb/CSS/CSSImportRule.h>
Expand Down Expand Up @@ -42,10 +43,10 @@ WebIDL::ExceptionOr<GC::Ref<CSSStyleSheet>> CSSStyleSheet::construct_impl(JS::Re
if (options.has_value() && options->base_url.has_value()) {
Optional<URL::URL> sheet_location_url;
if (sheet->location().has_value())
sheet_location_url = sheet->location().release_value();
sheet_location_url = URL::Parser::basic_parse(sheet->location().release_value());

// AD-HOC: This isn't explicitly mentioned in the specification, but multiple modern browsers do this.
Optional<URL::URL> url = sheet->location().has_value() ? sheet_location_url->complete_url(options->base_url.value()) : options->base_url.value();
Optional<URL::URL> url = sheet->location().has_value() ? sheet_location_url->complete_url(options->base_url.value()) : URL::Parser::basic_parse(options->base_url.value());
if (!url.has_value())
return WebIDL::NotAllowedError::create(realm, "Constructed style sheets must have a valid base URL"_string);

Expand Down
6 changes: 2 additions & 4 deletions Libraries/LibWeb/HTML/Scripting/ImportMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,8 @@ WebIDL::ExceptionOr<HashMap<URL::URL, ModuleSpecifierMap>> sort_and_normalise_sc
}

// 4. Let normalizedScopePrefix be the serialization of scopePrefixURL.
auto normalised_scope_prefix = scope_prefix_url->serialize();

// 5. Set normalized[normalizedScopePrefix] to the result of sorting and normalizing a module specifier map given potentialSpecifierMap and baseURL.
normalised.set(normalised_scope_prefix, TRY(sort_and_normalise_module_specifier_map(realm, potential_specifier_map.as_object(), base_url)));
normalised.set(scope_prefix_url.value(), TRY(sort_and_normalise_module_specifier_map(realm, potential_specifier_map.as_object(), base_url)));
}

// 3. Return the result of sorting in descending order normalized, with an entry a being less than an entry b if a's key is code unit less than b's key.
Expand Down Expand Up @@ -316,7 +314,7 @@ void merge_existing_and_new_import_maps(Window& global, ImportMap& new_import_ma
// 1. For each record of global's resolved module set:
for (auto const& record : global.resolved_module_set()) {
// 1. If scopePrefix is record's serialized base URL, or if scopePrefix ends with U+002F (/) and scopePrefix is a code unit prefix of record's serialized base URL, then:
if (scope_prefix == record.serialized_base_url || (scope_prefix.to_string().ends_with('/') && record.serialized_base_url.has_value() && Infra::is_code_unit_prefix(scope_prefix.to_string(), *record.serialized_base_url))) {
if (scope_prefix.to_string() == record.serialized_base_url || (scope_prefix.to_string().ends_with('/') && record.serialized_base_url.has_value() && Infra::is_code_unit_prefix(scope_prefix.to_string(), *record.serialized_base_url))) {
// 1. For each specifierKey → resolutionResult of scopeImports:
scope_imports.remove_all_matching([&](ByteString const& specifier_key, Optional<URL::URL> const&) {
// 1. If specifierKey is record's specifier, or if all of the following conditions are true:
Expand Down
8 changes: 7 additions & 1 deletion Libraries/LibWeb/Loader/ProxyMappings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

#include "ProxyMappings.h"
#include <LibURL/Parser.h>

Web::ProxyMappings& Web::ProxyMappings::the()
{
Expand All @@ -17,7 +18,12 @@ Core::ProxyData Web::ProxyMappings::proxy_for_url(URL::URL const& url) const
auto url_string = url.to_byte_string();
for (auto& it : m_mappings) {
if (url_string.matches(it.key)) {
auto result = Core::ProxyData::parse_url(m_proxies[it.value]);
auto maybe_url = URL::Parser::basic_parse(m_proxies[it.value]);
if (!maybe_url.has_value()) {
dbgln("Failed to parse proxy URL: {}", m_proxies[it.value]);
continue;
}
auto result = Core::ProxyData::parse_url(maybe_url.value());
if (result.is_error()) {
dbgln("Failed to parse proxy URL: {}", m_proxies[it.value]);
continue;
Expand Down
11 changes: 6 additions & 5 deletions Libraries/LibWeb/PermissionsPolicy/AutoplayAllowlist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <AK/String.h>
#include <LibURL/Origin.h>
#include <LibURL/Parser.h>
#include <LibURL/URL.h>
#include <LibWeb/DOM/Document.h>
#include <LibWeb/DOMURL/DOMURL.h>
Expand Down Expand Up @@ -73,16 +74,16 @@ ErrorOr<void> AutoplayAllowlist::enable_for_origins(ReadonlySpan<String> origins
TRY(allowlist.try_ensure_capacity(origins.size()));

for (auto const& origin : origins) {
URL::URL url { origin };
auto url = URL::Parser::basic_parse(origin);

if (!url.is_valid())
url = TRY(String::formatted("https://{}", origin));
if (!url.is_valid()) {
if (!url.has_value())
url = URL::Parser::basic_parse(TRY(String::formatted("https://{}", origin)));
if (!url.has_value()) {
dbgln("Invalid origin for autoplay allowlist: {}", origin);
continue;
}

TRY(allowlist.try_append(url.origin()));
TRY(allowlist.try_append(url->origin()));
}

return {};
Expand Down
11 changes: 6 additions & 5 deletions Libraries/LibWeb/WebDriver/Proxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

#include <AK/JsonValue.h>
#include <LibURL/Parser.h>
#include <LibURL/URL.h>
#include <LibWeb/WebDriver/Proxy.h>

Expand Down Expand Up @@ -45,23 +46,23 @@ static ErrorOr<void, Error> validate_proxy_item(StringView key, JsonValue const&
if (key == "proxyAutoconfigUrl"sv) {
if (!value.is_string())
return Error::from_code(ErrorCode::InvalidArgument, "Proxy configuration item 'proxyAutoconfigUrl' must be a string"sv);
if (URL::URL url { value.as_string() }; !url.is_valid())
if (auto url = URL::Parser::basic_parse(value.as_string()); !url.has_value())
return Error::from_code(ErrorCode::InvalidArgument, "Proxy configuration item 'proxyAutoconfigUrl' must be a valid URL"sv);
return {};
}

if (key == "ftpProxy"sv) {
if (!value.is_string())
return Error::from_code(ErrorCode::InvalidArgument, "Proxy configuration item 'ftpProxy' must be a string"sv);
if (URL::URL url { value.as_string() }; !url.is_valid() || url.scheme() != "ftp"sv)
if (auto url = URL::Parser::basic_parse(value.as_string()); !url.has_value() || url->scheme() != "ftp"sv)
return Error::from_code(ErrorCode::InvalidArgument, "Proxy configuration item 'ftpProxy' must be a valid FTP URL"sv);
return {};
}

if (key == "httpProxy"sv) {
if (!value.is_string())
return Error::from_code(ErrorCode::InvalidArgument, "Proxy configuration item 'httpProxy' must be a string"sv);
if (URL::URL url { value.as_string() }; !url.is_valid() || url.scheme() != "http"sv)
if (auto url = URL::Parser::basic_parse(value.as_string()); !url.has_value() || url->scheme() != "http"sv)
return Error::from_code(ErrorCode::InvalidArgument, "Proxy configuration item 'httpProxy' must be a valid HTTP URL"sv);
return {};
}
Expand All @@ -82,15 +83,15 @@ static ErrorOr<void, Error> validate_proxy_item(StringView key, JsonValue const&
if (key == "sslProxy"sv) {
if (!value.is_string())
return Error::from_code(ErrorCode::InvalidArgument, "Proxy configuration item 'sslProxy' must be a string"sv);
if (URL::URL url { value.as_string() }; !url.is_valid() || url.scheme() != "https"sv)
if (auto url = URL::Parser::basic_parse(value.as_string()); !url.has_value() || url->scheme() != "https"sv)
return Error::from_code(ErrorCode::InvalidArgument, "Proxy configuration item 'sslProxy' must be a valid HTTPS URL"sv);
return {};
}

if (key == "socksProxy"sv) {
if (!value.is_string())
return Error::from_code(ErrorCode::InvalidArgument, "Proxy configuration item 'proxyAutoconfigUrl' must be a string"sv);
if (URL::URL url { value.as_string() }; !url.is_valid())
if (auto url = URL::Parser::basic_parse(value.as_string()); !url.has_value())
return Error::from_code(ErrorCode::InvalidArgument, "Proxy configuration item 'proxyAutoconfigUrl' must be a valid URL"sv);
return {};
}
Expand Down
3 changes: 2 additions & 1 deletion Libraries/LibWebView/URL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <AK/String.h>
#include <LibFileSystem/FileSystem.h>
#include <LibURL/Parser.h>
#include <LibWebView/URL.h>

namespace WebView {
Expand All @@ -25,7 +26,7 @@ Optional<URL::URL> sanitize_url(StringView url, Optional<StringView> search_engi
if (!search_engine.has_value())
return {};

return MUST(String::formatted(*search_engine, URL::percent_decode(url)));
return URL::Parser::basic_parse(MUST(String::formatted(*search_engine, URL::percent_decode(url))));
};

ByteString url_with_scheme = url;
Expand Down
7 changes: 4 additions & 3 deletions Services/WebContent/WebDriverConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <AK/Vector.h>
#include <LibCore/File.h>
#include <LibJS/Runtime/Value.h>
#include <LibURL/Parser.h>
#include <LibWeb/CSS/CSSStyleValue.h>
#include <LibWeb/CSS/ComputedProperties.h>
#include <LibWeb/CSS/PropertyID.h>
Expand Down Expand Up @@ -289,7 +290,7 @@ Messages::WebDriverClient::NavigateToResponse WebDriverConnection::navigate_to(J
// 2. Let url be the result of getting the property url from the parameters argument.
if (!payload.is_object() || !payload.as_object().has_string("url"sv))
return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, "Payload doesn't have a string `url`"sv);
URL::URL url(payload.as_object().get_byte_string("url"sv).value());
auto url = URL::Parser::basic_parse(payload.as_object().get_byte_string("url"sv).value());

// FIXME: 3. If url is not an absolute URL or is not an absolute URL with fragment or not a local scheme, return error with error code invalid argument.

Expand All @@ -302,7 +303,7 @@ Messages::WebDriverClient::NavigateToResponse WebDriverConnection::navigate_to(J
// FIXME: a. If timer has not been started, start a timer. If this algorithm has not completed before timer reaches the session’s session page load timeout in milliseconds, return an error with error code timeout.

// 7. Navigate the current top-level browsing context to url.
current_top_level_browsing_context()->page().load(url);
current_top_level_browsing_context()->page().load(url.value());

auto navigation_complete = GC::create_function(current_top_level_browsing_context()->heap(), [this](Web::WebDriver::Response result) {
// 9. Set the current browsing context with the current top-level browsing context.
Expand All @@ -317,7 +318,7 @@ Messages::WebDriverClient::NavigateToResponse WebDriverConnection::navigate_to(J
// AD-HOC: We wait for the navigation to complete regardless of whether the current URL differs from the provided
// URL. Even if they're the same, the navigation queues a tasks that we must await, otherwise subsequent
// endpoint invocations will attempt to operate on the wrong page.
if (url.is_special() && url.scheme() != "file"sv) {
if (url->is_special() && url->scheme() != "file"sv) {
// a. Try to wait for navigation to complete.
wait_for_navigation_to_complete(navigation_complete);

Expand Down
Loading

0 comments on commit d62cf0a

Please sign in to comment.