From 088b659abd123e0462bca7a6a9b534c0d89c668a Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Sat, 5 Oct 2024 18:08:18 +1300 Subject: [PATCH] LibURL: Do not treat port of 0 as a null port in Origin It is not treated as the same thing in the specification, or by us in other places too. This fixes 8 more origin related URL tests on WPT. --- .../URL/url-origin-for-port-number-of-zero.txt | 1 + .../URL/url-origin-for-port-number-of-zero.html | 7 +++++++ Userland/Libraries/LibIPC/Decoder.cpp | 2 +- Userland/Libraries/LibURL/Origin.cpp | 17 +++++++++++------ Userland/Libraries/LibURL/Origin.h | 8 ++++---- Userland/Libraries/LibURL/URL.cpp | 4 ++-- 6 files changed, 26 insertions(+), 13 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/URL/url-origin-for-port-number-of-zero.txt create mode 100644 Tests/LibWeb/Text/input/URL/url-origin-for-port-number-of-zero.html diff --git a/Tests/LibWeb/Text/expected/URL/url-origin-for-port-number-of-zero.txt b/Tests/LibWeb/Text/expected/URL/url-origin-for-port-number-of-zero.txt new file mode 100644 index 00000000000..7a4020e88e3 --- /dev/null +++ b/Tests/LibWeb/Text/expected/URL/url-origin-for-port-number-of-zero.txt @@ -0,0 +1 @@ +http://f:0 diff --git a/Tests/LibWeb/Text/input/URL/url-origin-for-port-number-of-zero.html b/Tests/LibWeb/Text/input/URL/url-origin-for-port-number-of-zero.html new file mode 100644 index 00000000000..03857d3efc4 --- /dev/null +++ b/Tests/LibWeb/Text/input/URL/url-origin-for-port-number-of-zero.html @@ -0,0 +1,7 @@ + + + diff --git a/Userland/Libraries/LibIPC/Decoder.cpp b/Userland/Libraries/LibIPC/Decoder.cpp index 0d5c6bc0b94..1d17fcd87ae 100644 --- a/Userland/Libraries/LibIPC/Decoder.cpp +++ b/Userland/Libraries/LibIPC/Decoder.cpp @@ -102,7 +102,7 @@ ErrorOr decode(Decoder& decoder) { auto scheme = TRY(decoder.decode()); auto host = TRY(decoder.decode()); - u16 port = TRY(decoder.decode()); + auto port = TRY(decoder.decode>()); return URL::Origin { move(scheme), move(host), port }; } diff --git a/Userland/Libraries/LibURL/Origin.cpp b/Userland/Libraries/LibURL/Origin.cpp index 8608dad5073..e47e4e0d91f 100644 --- a/Userland/Libraries/LibURL/Origin.cpp +++ b/Userland/Libraries/LibURL/Origin.cpp @@ -27,9 +27,9 @@ ByteString Origin::serialize() const result.append(Parser::serialize_host(host()).release_value_but_fixme_should_propagate_errors().to_byte_string()); // 5. If origin's port is non-null, append a U+003A COLON character (:), and origin's port, serialized, to result. - if (port() != 0) { + if (port().has_value()) { result.append(':'); - result.append(ByteString::number(port())); + result.append(ByteString::number(*port())); } // 6. Return result return result.to_byte_string(); @@ -41,10 +41,15 @@ namespace AK { unsigned Traits::hash(URL::Origin const& origin) { - auto hash_without_host = pair_int_hash(origin.scheme().hash(), origin.port()); - if (origin.host().has()) - return hash_without_host; - return pair_int_hash(hash_without_host, URL::Parser::serialize_host(origin.host()).release_value_but_fixme_should_propagate_errors().hash()); + unsigned hash = origin.scheme().hash(); + + if (origin.port().has_value()) + hash = pair_int_hash(hash, *origin.port()); + + if (!origin.host().has()) + hash = pair_int_hash(hash, URL::Parser::serialize_host(origin.host()).release_value_but_fixme_should_propagate_errors().hash()); + + return hash; } } // namespace AK diff --git a/Userland/Libraries/LibURL/Origin.h b/Userland/Libraries/LibURL/Origin.h index 5811fbe9c77..9232fabf91b 100644 --- a/Userland/Libraries/LibURL/Origin.h +++ b/Userland/Libraries/LibURL/Origin.h @@ -15,7 +15,7 @@ namespace URL { class Origin { public: Origin() = default; - Origin(Optional const& scheme, Host const& host, u16 port) + Origin(Optional const& scheme, Host const& host, Optional port) : m_scheme(scheme) , m_host(host) , m_port(port) @@ -23,14 +23,14 @@ public: } // https://html.spec.whatwg.org/multipage/origin.html#concept-origin-opaque - bool is_opaque() const { return !m_scheme.has_value() && m_host.has() && m_port == 0; } + bool is_opaque() const { return !m_scheme.has_value() && m_host.has() && !m_port.has_value(); } StringView scheme() const { return m_scheme.map([](auto& str) { return str.view(); }).value_or(StringView {}); } Host const& host() const { return m_host; } - u16 port() const { return m_port; } + Optional port() const { return m_port; } // https://html.spec.whatwg.org/multipage/origin.html#same-origin bool is_same_origin(Origin const& other) const @@ -91,7 +91,7 @@ public: private: Optional m_scheme; Host m_host; - u16 m_port { 0 }; + Optional m_port; }; } diff --git a/Userland/Libraries/LibURL/URL.cpp b/Userland/Libraries/LibURL/URL.cpp index e8ebd421b2f..061009dcf8f 100644 --- a/Userland/Libraries/LibURL/URL.cpp +++ b/Userland/Libraries/LibURL/URL.cpp @@ -386,7 +386,7 @@ Origin URL::origin() const // -> "wss" if (scheme().is_one_of("ftp"sv, "http"sv, "https"sv, "ws"sv, "wss"sv)) { // Return the tuple origin (url’s scheme, url’s host, url’s port, null). - return Origin(scheme().to_byte_string(), host(), port().value_or(0)); + return Origin(scheme().to_byte_string(), host(), port()); } // -> "file" @@ -394,7 +394,7 @@ Origin URL::origin() const if (scheme() == "file"sv || scheme() == "resource"sv) { // Unfortunate as it is, this is left as an exercise to the reader. When in doubt, return a new opaque origin. // Note: We must return an origin with the `file://' protocol for `file://' iframes to work from `file://' pages. - return Origin(scheme().to_byte_string(), String {}, 0); + return Origin(scheme().to_byte_string(), String {}, {}); } // -> Otherwise