AK: Make URL::m_port an Optional<u16>, Expose raw port getter

Our current way of signalling a missing port with m_port == 0 was
lacking, as 0 is a valid port number in URLs.

15 files changed, 42 insertions, 41 deletions

diff --git a/AK/URL.cpp b/AK/URL.cpp
index 01f3fe3114..494ea18beb 100644
--- a/AK/URL.cpp
+++ b/AK/URL.cpp
@@ -71,13 +71,13 @@ void URL::set_host(String host)
     m_valid = compute_validity();
 }
 
-void URL::set_port(u16 port)
+void URL::set_port(Optional<u16> port)
 {
     if (port == default_port_for_scheme(m_scheme)) {
-        m_port = 0;
+        m_port = {};
         return;
     }
-    m_port = port;
+    m_port = move(port);
     m_valid = compute_validity();
 }
 
@@ -234,8 +234,8 @@ String URL::serialize(ExcludeFragment exclude_fragment) const
         }
 
         builder.append(m_host);
-        if (m_port != 0)
-            builder.appendff(":{}", m_port);
+        if (m_port.has_value())
+            builder.appendff(":{}", *m_port);
     }
 
     if (cannot_be_a_base_url()) {
@@ -278,8 +278,8 @@ String URL::serialize_for_display() const
     if (!m_host.is_null()) {
         builder.append("//");
         builder.append(m_host);
-        if (m_port != 0)
-            builder.appendff(":{}", m_port);
+        if (m_port.has_value())
+            builder.appendff(":{}", *m_port);
     }
 
     if (cannot_be_a_base_url()) {
@@ -329,8 +329,8 @@ String URL::serialize_origin() const
     builder.append(m_scheme);
     builder.append("://"sv);
     builder.append(m_host);
-    if (m_port != 0)
-        builder.append(":{}", m_port);
+    if (m_port.has_value())
+        builder.append(":{}", *m_port);
     return builder.build();
 }
 
diff --git a/AK/URL.h b/AK/URL.h
index ee25ef74fd..33130463f2 100644
--- a/AK/URL.h
+++ b/AK/URL.h
@@ -56,7 +56,8 @@ public:
     Vector<String> const& paths() const { return m_paths; }
     String const& query() const { return m_query; }
     String const& fragment() const { return m_fragment; }
-    u16 port() const { return m_port ? m_port : default_port_for_scheme(m_scheme); }
+    Optional<u16> port() const { return m_port; }
+    u16 port_or_default() const { return m_port.value_or(default_port_for_scheme(m_scheme)); }
     bool cannot_be_a_base_url() const { return m_cannot_be_a_base_url; }
     bool cannot_have_a_username_or_password_or_port() const { return m_host.is_null() || m_host.is_empty() || m_cannot_be_a_base_url || m_scheme == "file"sv; }
 
@@ -68,7 +69,7 @@ public:
     void set_username(String);
     void set_password(String);
     void set_host(String);
-    void set_port(u16);
+    void set_port(Optional<u16>);
     void set_paths(Vector<String>);
     void set_query(String);
     void set_fragment(String);
@@ -129,8 +130,8 @@ private:
     String m_username;
     String m_password;
     String m_host;
-    // NOTE: If the port is the default port for the scheme, m_port should be 0.
-    u16 m_port { 0 };
+    // NOTE: If the port is the default port for the scheme, m_port should be empty.
+    Optional<u16> m_port;
     String m_path;
     Vector<String> m_paths;
     String m_query;
diff --git a/AK/URLParser.cpp b/AK/URLParser.cpp
index fe7beaa9c8..63cf3c449e 100644
--- a/AK/URLParser.cpp
+++ b/AK/URLParser.cpp
@@ -472,7 +472,7 @@ URL URLParser::parse(StringView const& raw_input, URL const* base_url, Optional<
                         return {};
                     }
                     if (port.value() == URL::default_port_for_scheme(url->scheme()))
-                        url->m_port = 0;
+                        url->m_port = {};
                     else
                         url->m_port = port.value();
                     buffer.clear();
diff --git a/Tests/AK/TestURL.cpp b/Tests/AK/TestURL.cpp
index 0c45ce2f05..4910b2168a 100644
--- a/Tests/AK/TestURL.cpp
+++ b/Tests/AK/TestURL.cpp
@@ -21,7 +21,7 @@ TEST_CASE(basic)
         EXPECT_EQ(url.is_valid(), true);
         EXPECT_EQ(url.scheme(), "http");
         EXPECT_EQ(url.host(), "www.serenityos.org");
-        EXPECT_EQ(url.port(), 80);
+        EXPECT_EQ(url.port_or_default(), 80);
         EXPECT_EQ(url.path(), "/");
         EXPECT(url.query().is_null());
         EXPECT(url.fragment().is_null());
@@ -31,7 +31,7 @@ TEST_CASE(basic)
         EXPECT_EQ(url.is_valid(), true);
         EXPECT_EQ(url.scheme(), "https");
         EXPECT_EQ(url.host(), "www.serenityos.org");
-        EXPECT_EQ(url.port(), 443);
+        EXPECT_EQ(url.port_or_default(), 443);
         EXPECT_EQ(url.path(), "/index.html");
         EXPECT(url.query().is_null());
         EXPECT(url.fragment().is_null());
@@ -41,7 +41,7 @@ TEST_CASE(basic)
         EXPECT_EQ(url.is_valid(), true);
         EXPECT_EQ(url.scheme(), "https");
         EXPECT_EQ(url.host(), "localhost");
-        EXPECT_EQ(url.port(), 1234);
+        EXPECT_EQ(url.port_or_default(), 1234);
         EXPECT_EQ(url.path(), "/~anon/test/page.html");
         EXPECT(url.query().is_null());
         EXPECT(url.fragment().is_null());
@@ -51,7 +51,7 @@ TEST_CASE(basic)
         EXPECT_EQ(url.is_valid(), true);
         EXPECT_EQ(url.scheme(), "http");
         EXPECT_EQ(url.host(), "www.serenityos.org");
-        EXPECT_EQ(url.port(), 80);
+        EXPECT_EQ(url.port_or_default(), 80);
         EXPECT_EQ(url.path(), "/index.html");
         EXPECT_EQ(url.query(), "");
         EXPECT_EQ(url.fragment(), "");
@@ -61,7 +61,7 @@ TEST_CASE(basic)
         EXPECT_EQ(url.is_valid(), true);
         EXPECT_EQ(url.scheme(), "http");
         EXPECT_EQ(url.host(), "www.serenityos.org");
-        EXPECT_EQ(url.port(), 80);
+        EXPECT_EQ(url.port_or_default(), 80);
         EXPECT_EQ(url.path(), "/index.html");
         EXPECT_EQ(url.query(), "foo=1&bar=2");
         EXPECT(url.fragment().is_null());
@@ -71,7 +71,7 @@ TEST_CASE(basic)
         EXPECT_EQ(url.is_valid(), true);
         EXPECT_EQ(url.scheme(), "http");
         EXPECT_EQ(url.host(), "www.serenityos.org");
-        EXPECT_EQ(url.port(), 80);
+        EXPECT_EQ(url.port_or_default(), 80);
         EXPECT_EQ(url.path(), "/index.html");
         EXPECT(url.query().is_null());
         EXPECT_EQ(url.fragment(), "fragment");
@@ -81,7 +81,7 @@ TEST_CASE(basic)
         EXPECT_EQ(url.is_valid(), true);
         EXPECT_EQ(url.scheme(), "http");
         EXPECT_EQ(url.host(), "www.serenityos.org");
-        EXPECT_EQ(url.port(), 80);
+        EXPECT_EQ(url.port_or_default(), 80);
         EXPECT_EQ(url.path(), "/index.html");
         EXPECT_EQ(url.query(), "foo=1&bar=2&baz=/?");
         EXPECT_EQ(url.fragment(), "frag/ment?test#");
@@ -105,7 +105,7 @@ TEST_CASE(some_bad_urls)
 TEST_CASE(serialization)
 {
     EXPECT_EQ(URL("http://www.serenityos.org/").serialize(), "http://www.serenityos.org/");
-    EXPECT_EQ(URL("http://www.serenityos.org:0/").serialize(), "http://www.serenityos.org/");
+    EXPECT_EQ(URL("http://www.serenityos.org:0/").serialize(), "http://www.serenityos.org:0/");
     EXPECT_EQ(URL("http://www.serenityos.org:80/").serialize(), "http://www.serenityos.org/");
     EXPECT_EQ(URL("http://www.serenityos.org:81/").serialize(), "http://www.serenityos.org:81/");
     EXPECT_EQ(URL("https://www.serenityos.org:443/foo/bar.html?query#fragment").serialize(), "https://www.serenityos.org/foo/bar.html?query#fragment");
@@ -117,7 +117,7 @@ TEST_CASE(file_url_with_hostname)
     EXPECT(url.is_valid());
     EXPECT_EQ(url.scheme(), "file");
     EXPECT_EQ(url.host(), "courage");
-    EXPECT_EQ(url.port(), 0);
+    EXPECT_EQ(url.port_or_default(), 0);
     EXPECT_EQ(url.path(), "/my/file");
     EXPECT_EQ(url.serialize(), "file://courage/my/file");
     EXPECT(url.query().is_null());
@@ -200,7 +200,7 @@ TEST_CASE(mailto_url)
     EXPECT(url.is_valid());
     EXPECT_EQ(url.scheme(), "mailto");
     EXPECT(url.host().is_null());
-    EXPECT_EQ(url.port(), 0);
+    EXPECT_EQ(url.port_or_default(), 0);
     EXPECT_EQ(url.paths().size(), 1u);
     EXPECT_EQ(url.paths()[0], "mail@example.com");
     EXPECT(url.query().is_null());
@@ -291,7 +291,7 @@ TEST_CASE(trailing_slash_with_complete_url)
 TEST_CASE(trailing_port)
 {
     URL url("http://example.com:8086");
-    EXPECT_EQ(url.port(), 8086);
+    EXPECT_EQ(url.port_or_default(), 8086);
 }
 
 TEST_CASE(port_overflow)
@@ -312,7 +312,7 @@ TEST_CASE(create_with_file_scheme)
     auto url = URL::create_with_file_scheme("/home/anon/README.md");
     EXPECT(url.is_valid());
     EXPECT_EQ(url.scheme(), "file");
-    EXPECT_EQ(url.port(), 0);
+    EXPECT_EQ(url.port_or_default(), 0);
     EXPECT_EQ(url.paths().size(), 3u);
     EXPECT_EQ(url.paths()[0], "home");
     EXPECT_EQ(url.paths()[1], "anon");
diff --git a/Userland/Libraries/LibGemini/GeminiJob.cpp b/Userland/Libraries/LibGemini/GeminiJob.cpp
index dbc81ff344..6ab80ae571 100644
--- a/Userland/Libraries/LibGemini/GeminiJob.cpp
+++ b/Userland/Libraries/LibGemini/GeminiJob.cpp
@@ -45,7 +45,7 @@ void GeminiJob::start()
         if (on_certificate_requested)
             on_certificate_requested(*this);
     };
-    bool success = ((TLS::TLSv12&)*m_socket).connect(m_request.url().host(), m_request.url().port());
+    bool success = ((TLS::TLSv12&)*m_socket).connect(m_request.url().host(), m_request.url().port_or_default());
     if (!success) {
         deferred_invoke([this] {
             return did_fail(Core::NetworkJob::Error::ConnectionFailed);
diff --git a/Userland/Libraries/LibHTTP/HttpJob.cpp b/Userland/Libraries/LibHTTP/HttpJob.cpp
index 275911769c..98ffbace48 100644
--- a/Userland/Libraries/LibHTTP/HttpJob.cpp
+++ b/Userland/Libraries/LibHTTP/HttpJob.cpp
@@ -26,7 +26,7 @@ void HttpJob::start()
             did_fail(Core::NetworkJob::Error::ConnectionFailed);
         });
     };
-    bool success = m_socket->connect(m_request.url().host(), m_request.url().port());
+    bool success = m_socket->connect(m_request.url().host(), m_request.url().port_or_default());
     if (!success) {
         deferred_invoke([this] {
             return did_fail(Core::NetworkJob::Error::ConnectionFailed);
diff --git a/Userland/Libraries/LibHTTP/HttpsJob.cpp b/Userland/Libraries/LibHTTP/HttpsJob.cpp
index 1fdb066a61..37e4b7502e 100644
--- a/Userland/Libraries/LibHTTP/HttpsJob.cpp
+++ b/Userland/Libraries/LibHTTP/HttpsJob.cpp
@@ -46,7 +46,7 @@ void HttpsJob::start()
         if (on_certificate_requested)
             on_certificate_requested(*this);
     };
-    bool success = ((TLS::TLSv12&)*m_socket).connect(m_request.url().host(), m_request.url().port());
+    bool success = ((TLS::TLSv12&)*m_socket).connect(m_request.url().host(), m_request.url().port_or_default());
     if (!success) {
         deferred_invoke([this] {
             return did_fail(Core::NetworkJob::Error::ConnectionFailed);
diff --git a/Userland/Libraries/LibWeb/Bindings/LocationObject.cpp b/Userland/Libraries/LibWeb/Bindings/LocationObject.cpp
index e1d5e355f5..a31e1e56e2 100644
--- a/Userland/Libraries/LibWeb/Bindings/LocationObject.cpp
+++ b/Userland/Libraries/LibWeb/Bindings/LocationObject.cpp
@@ -74,7 +74,7 @@ JS_DEFINE_NATIVE_FUNCTION(LocationObject::host_getter)
 {
     auto& window = static_cast<WindowObject&>(global_object);
     auto url = window.impl().associated_document().url();
-    return JS::js_string(vm, String::formatted("{}:{}", url.host(), url.port()));
+    return JS::js_string(vm, String::formatted("{}:{}", url.host(), url.port_or_default()));
 }
 
 JS_DEFINE_NATIVE_FUNCTION(LocationObject::hash_getter)
diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp
index 0a26a2f214..2cfa9a55c2 100644
--- a/Userland/Libraries/LibWeb/DOM/Document.cpp
+++ b/Userland/Libraries/LibWeb/DOM/Document.cpp
@@ -134,7 +134,7 @@ Origin Document::origin() const
 {
     if (!m_url.is_valid())
         return {};
-    return { m_url.protocol(), m_url.host(), m_url.port() };
+    return { m_url.protocol(), m_url.host(), m_url.port_or_default() };
 }
 
 void Document::set_origin(const Origin& origin)
diff --git a/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp b/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp
index 2a984e157b..aaf33b3bac 100644
--- a/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp
+++ b/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp
@@ -165,7 +165,7 @@ bool FrameLoader::load(LoadRequest& request, Type type)
         AK::URL favicon_url;
         favicon_url.set_protocol(url.protocol());
         favicon_url.set_host(url.host());
-        favicon_url.set_port(url.port());
+        favicon_url.set_port(url.port_or_default());
         favicon_url.set_paths({ "favicon.ico" });
 
         ResourceLoader::the().load(
diff --git a/Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp b/Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp
index 45f33e9b8b..fa294bec9b 100644
--- a/Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp
+++ b/Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp
@@ -108,8 +108,8 @@ void ResourceLoader::load(LoadRequest& request, Function<void(ReadonlyBytes, con
         dbgln("ResourceLoader: Failed load of: \"{}\", \033[32;1mError: {}\033[0m, Duration: {}ms", url, error_message, load_time_ms);
     };
 
-    if (is_port_blocked(url.port())) {
-        log_failure(request, String::formatted("The port #{} is blocked", url.port()));
+    if (is_port_blocked(url.port_or_default())) {
+        log_failure(request, String::formatted("The port #{} is blocked", url.port_or_default()));
         return;
     }
 
diff --git a/Userland/Libraries/LibWeb/XHR/XMLHttpRequest.cpp b/Userland/Libraries/LibWeb/XHR/XMLHttpRequest.cpp
index 143fe1c9c6..339e11b0ad 100644
--- a/Userland/Libraries/LibWeb/XHR/XMLHttpRequest.cpp
+++ b/Userland/Libraries/LibWeb/XHR/XMLHttpRequest.cpp
@@ -171,7 +171,7 @@ DOM::ExceptionOr<void> XMLHttpRequest::send()
     dbgln("XHR send from {} to {}", m_window->associated_document().url(), request_url);
 
     // TODO: Add support for preflight requests to support CORS requests
-    Origin request_url_origin = Origin(request_url.protocol(), request_url.host(), request_url.port());
+    Origin request_url_origin = Origin(request_url.protocol(), request_url.host(), request_url.port_or_default());
 
     bool should_enforce_same_origin_policy = true;
     if (auto* page = m_window->page())
diff --git a/Userland/Libraries/LibWebSocket/Impl/TCPWebSocketConnectionImpl.cpp b/Userland/Libraries/LibWebSocket/Impl/TCPWebSocketConnectionImpl.cpp
index db25972710..709f28fc7f 100644
--- a/Userland/Libraries/LibWebSocket/Impl/TCPWebSocketConnectionImpl.cpp
+++ b/Userland/Libraries/LibWebSocket/Impl/TCPWebSocketConnectionImpl.cpp
@@ -34,7 +34,7 @@ void TCPWebSocketConnectionImpl::connect(ConnectionInfo const& connection)
     m_socket->on_connected = [this] {
         on_connected();
     };
-    bool success = m_socket->connect(connection.url().host(), connection.url().port());
+    bool success = m_socket->connect(connection.url().host(), connection.url().port_or_default());
     if (!success) {
         deferred_invoke([this] {
             on_connection_error();
diff --git a/Userland/Libraries/LibWebSocket/Impl/TLSv12WebSocketConnectionImpl.cpp b/Userland/Libraries/LibWebSocket/Impl/TLSv12WebSocketConnectionImpl.cpp
index eb3595ac9e..370f77c4eb 100644
--- a/Userland/Libraries/LibWebSocket/Impl/TLSv12WebSocketConnectionImpl.cpp
+++ b/Userland/Libraries/LibWebSocket/Impl/TLSv12WebSocketConnectionImpl.cpp
@@ -42,7 +42,7 @@ void TLSv12WebSocketConnectionImpl::connect(ConnectionInfo const& connection)
     m_socket->on_tls_certificate_request = [](auto&) {
         // FIXME : Once we handle TLS certificate requests, handle it here as well.
     };
-    bool success = m_socket->connect(connection.url().host(), connection.url().port());
+    bool success = m_socket->connect(connection.url().host(), connection.url().port_or_default());
     if (!success) {
         deferred_invoke([this] {
             on_connection_error();
diff --git a/Userland/Libraries/LibWebSocket/WebSocket.cpp b/Userland/Libraries/LibWebSocket/WebSocket.cpp
index ebc2060d57..e4a14f4f40 100644
--- a/Userland/Libraries/LibWebSocket/WebSocket.cpp
+++ b/Userland/Libraries/LibWebSocket/WebSocket.cpp
@@ -151,10 +151,10 @@ void WebSocket::send_client_handshake()
     // 4. Host
     auto url = m_connection.url();
     builder.appendff("Host: {}", url.host());
-    if (!m_connection.is_secure() && url.port() != 80)
-        builder.appendff(":{}", url.port());
-    else if (m_connection.is_secure() && url.port() != 443)
-        builder.appendff(":{}", url.port());
+    if (!m_connection.is_secure() && url.port_or_default() != 80)
+        builder.appendff(":{}", url.port_or_default());
+    else if (m_connection.is_secure() && url.port_or_default() != 443)
+        builder.appendff(":{}", url.port_or_default());
     builder.append("\r\n");
 
     // 5. and 6. Connection Upgrade