From 0ebdf146fc8b75f5d966b6fc81f2d7ce41fe2d07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=BClker?= Date: Thu, 6 Feb 2025 17:47:29 +0100 Subject: [PATCH] Cleanup blocking fetch operations with bad ports (#35324) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- components/net/fetch/methods.rs | 59 ++++++++++-------------------- components/net/websocket_loader.rs | 4 +- 2 files changed, 21 insertions(+), 42 deletions(-) diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index 99bdd48bd16..9ec79ec1bd4 100644 --- a/components/net/fetch/methods.rs +++ b/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. - if should_be_blocked_due_to_bad_port(&request.current_url()) { + // Step 7. If should request be blocked due to a bad port, should fetching request be blocked + // 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( } /// -pub fn should_be_blocked_due_to_bad_port(url: &ServoUrl) -> bool { - // Step 1 is not applicable, this function just takes the URL directly. +pub fn should_request_be_blocked_due_to_a_bad_port(url: &ServoUrl) -> bool { + // Step 1. Let url be request’s current URL. + // NOTE: We receive the request url as an argument - // Step 2. - let scheme = url.scheme(); - - // Step 3. - // 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) { + // Step 2. If url’s scheme is an HTTP(S) scheme and url’s port is a bad port, then return blocked. + let is_http_scheme = matches!(url.scheme(), "http" | "https"); + let is_bad_port = url.port().is_some_and(is_bad_port); + if is_http_scheme && is_bad_port { return true; } - // Step 6. + // Step 3. Return allowed. false } -/// -fn is_network_scheme(scheme: &str) -> bool { - scheme == "ftp" || scheme == "http" || scheme == "https" -} - /// fn is_bad_port(port: u16) -> bool { static BAD_PORTS: [u16; 78] = [ diff --git a/components/net/websocket_loader.rs b/components/net/websocket_loader.rs index 4a215aff56f..d601644a125 100644 --- a/components/net/websocket_loader.rs +++ b/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()); }