Cleanup blocking fetch operations with bad ports (#35324)

Blocking a fetch due to a bad port should be grouped together
with CSP blocks as per the spec, but these steps were previously
seperated.

Additionally, remove handling of ftp in
should_request_be_blocked_due_to_a_bad_port, since it did nothing
anyways.

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

components/net/fetch/methods.rs

@@ -240,14 +240,6 @@ pub async fn main_fetch(
         RequestPolicyContainer::PolicyContainer(container) => container.to_owned(),
     };
 
-    // Step 2.4.
-    if should_request_be_blocked_by_csp(request, &policy_container) == csp::CheckResult::Blocked {
-        warn!("Request blocked by CSP");
-        response = Some(Response::network_error(NetworkError::Internal(
-            "Blocked by Content-Security-Policy".into(),
-        )))
-    }
-
     // Step 3.
     // TODO: handle request abort.
 
@@ -278,14 +270,21 @@ pub async fn main_fetch(
         );
     }
 
-    // Step 5.
+    // Step 7. If should request be blocked due to a bad port, should fetching request be blocked
-    if should_be_blocked_due_to_bad_port(&request.current_url()) {
+    // as mixed content, or should request be blocked by Content Security Policy returns blocked,
+    // then set response to a network error.
+    // TODO: check "should fetching request be blocked as mixed content"
+    if should_request_be_blocked_by_csp(request, &policy_container) == csp::CheckResult::Blocked {
+        warn!("Request blocked by CSP");
+        response = Some(Response::network_error(NetworkError::Internal(
+            "Blocked by Content-Security-Policy".into(),
+        )))
+    }
+    if should_request_be_blocked_due_to_a_bad_port(&request.current_url()) {
         response = Some(Response::network_error(NetworkError::Internal(
             "Request attempted on bad port".into(),
         )));
     }
-    // TODO: handle blocking as mixed content.
-    // TODO: handle blocking by content security policy.
 
     // Step 8: If request’s referrer policy is the empty string, then set request’s referrer policy
     // to request’s policy container’s referrer policy.
@@ -858,41 +857,21 @@ fn should_be_blocked_due_to_mime_type(
 }
 
 /// <https://fetch.spec.whatwg.org/#block-bad-port>
-pub fn should_be_blocked_due_to_bad_port(url: &ServoUrl) -> bool {
+pub fn should_request_be_blocked_due_to_a_bad_port(url: &ServoUrl) -> bool {
-    // Step 1 is not applicable, this function just takes the URL directly.
+    // Step 1. Let url be request’s current URL.
+    // NOTE: We receive the request url as an argument
 
-    // Step 2.
+    // Step 2. If url’s scheme is an HTTP(S) scheme and url’s port is a bad port, then return blocked.
-    let scheme = url.scheme();
+    let is_http_scheme = matches!(url.scheme(), "http" | "https");
-
+    let is_bad_port = url.port().is_some_and(is_bad_port);
-    // Step 3.
+    if is_http_scheme && is_bad_port {
-    // If there is no explicit port, this means the default one is used for
-    // the given scheme, and thus this means the request should not be blocked
-    // due to a bad port.
-    let port = if let Some(port) = url.port() {
-        port
-    } else {
-        return false;
-    };
-
-    // Step 4.
-    if scheme == "ftp" && (port == 20 || port == 21) {
-        return false;
-    }
-
-    // Step 5.
-    if is_network_scheme(scheme) && is_bad_port(port) {
         return true;
     }
 
-    // Step 6.
+    // Step 3. Return allowed.
     false
 }
 
-/// <https://fetch.spec.whatwg.org/#network-scheme>
-fn is_network_scheme(scheme: &str) -> bool {
-    scheme == "ftp" || scheme == "http" || scheme == "https"
-}
-
 /// <https://fetch.spec.whatwg.org/#bad-port>
 fn is_bad_port(port: u16) -> bool {
     static BAD_PORTS: [u16; 78] = [

components/net/websocket_loader.rs

@@ -39,7 +39,7 @@ use url::Url;
 use crate::async_runtime::HANDLE;
 use crate::connector::{create_tls_config, CACertificates, TlsConfig};
 use crate::cookie::ServoCookie;
-use crate::fetch::methods::should_be_blocked_due_to_bad_port;
+use crate::fetch::methods::should_request_be_blocked_due_to_a_bad_port;
 use crate::hosts::replace_host;
 use crate::http_loader::HttpState;
 /// Create a tungstenite Request object for the initial HTTP request.
@@ -371,7 +371,7 @@ fn connect(
 
     let req_url = req_builder.url.clone();
 
-    if should_be_blocked_due_to_bad_port(&req_url) {
+    if should_request_be_blocked_due_to_a_bad_port(&req_url) {
         return Err("Port blocked".to_string());
     }