From fe4e90d7dc644f8a738ac1dfd7f75cd74ec5e588 Mon Sep 17 00:00:00 2001
From: Uthman Yahaya Baba <uthmanyahayababa@gmail.com>
Date: Fri, 18 Apr 2025 13:46:14 +0100
Subject: [PATCH] Replace NetworkError::Internal with structured enum variants

Signed-off-by: Uthman Yahaya Baba <uthmanyahayababa@gmail.com>
---
 components/net/fetch/methods.rs           | 43 +++++-----------
 components/net/http_loader.rs             | 62 +++++++----------------
 components/net/local_directory_listing.rs |  8 +--
 components/net/protocols/blob.rs          |  4 +-
 components/net/protocols/data.rs          |  4 +-
 components/net/protocols/file.rs          | 14 ++---
 components/net/tests/data_loader.rs       |  7 +--
 components/net/tests/fetch.rs             |  5 +-
 components/net/tests/http_loader.rs       |  2 +-
 components/shared/net/lib.rs              | 12 +++++
 10 files changed, 55 insertions(+), 106 deletions(-)

diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs
index 697a46fedda..17f3ccb9f32 100644
--- a/components/net/fetch/methods.rs
+++ b/components/net/fetch/methods.rs
@@ -227,9 +227,7 @@ pub async fn main_fetch(
             "about" | "blob" | "data" | "filesystem"
         )
     {
-        response = Some(Response::network_error(NetworkError::Internal(
-            "Non-local scheme".into(),
-        )));
+        response = Some(Response::network_error(NetworkError::UnsupportedScheme));
     }
 
     // Step 2.2.
@@ -285,19 +283,13 @@ pub async fn main_fetch(
 
     if check_result == csp::CheckResult::Blocked {
         warn!("Request blocked by CSP");
-        response = Some(Response::network_error(NetworkError::Internal(
-            "Blocked by Content-Security-Policy".into(),
-        )))
+        response = Some(Response::network_error(NetworkError::SecurityBlock))
     }
     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(),
-        )));
+        response = Some(Response::network_error(NetworkError::InvalidPort));
     }
     if should_request_be_blocked_as_mixed_content(request) {
-        response = Some(Response::network_error(NetworkError::Internal(
-            "Blocked as mixed content".into(),
-        )));
+        response = Some(Response::network_error(NetworkError::MixedContent));
     }
 
     // Step 8: If request’s referrer policy is the empty string, then set request’s referrer policy
@@ -371,13 +363,11 @@ pub async fn main_fetch(
                 // Substep 2. Return the result of running scheme fetch given fetchParams.
                 scheme_fetch(fetch_params, cache, target, done_chan, context).await
             } else if request.mode == RequestMode::SameOrigin {
-                Response::network_error(NetworkError::Internal("Cross-origin response".into()))
+                Response::network_error(NetworkError::CorsViolation)
             } else if request.mode == RequestMode::NoCors {
                 // Substep 1. If request’s redirect mode is not "follow", then return a network error.
                 if request.redirect_mode != RedirectMode::Follow {
-                    Response::network_error(NetworkError::Internal(
-                        "NoCors requests must follow redirects".into(),
-                    ))
+                    Response::network_error(NetworkError::CorsViolation)
                 } else {
                     // Substep 2. Set request’s response tainting to "opaque".
                     request.response_tainting = ResponseTainting::Opaque;
@@ -386,7 +376,7 @@ pub async fn main_fetch(
                     scheme_fetch(fetch_params, cache, target, done_chan, context).await
                 }
             } else if !matches!(current_scheme, "http" | "https") {
-                Response::network_error(NetworkError::Internal("Non-http scheme".into()))
+                Response::network_error(NetworkError::UnsupportedScheme)
             } else if request.use_cors_preflight ||
                 (request.unsafe_request &&
                     (!is_cors_safelisted_method(&request.method) ||
@@ -521,17 +511,14 @@ pub async fn main_fetch(
 
         let internal_response = if should_replace_with_nosniff_error {
             // Defer rebinding result
-            blocked_error_response =
-                Response::network_error(NetworkError::Internal("Blocked by nosniff".into()));
+            blocked_error_response = Response::network_error(NetworkError::SecurityBlock);
             &blocked_error_response
         } else if should_replace_with_mime_type_error {
             // Defer rebinding result
-            blocked_error_response =
-                Response::network_error(NetworkError::Internal("Blocked by mime type".into()));
+            blocked_error_response = Response::network_error(NetworkError::SecurityBlock);
             &blocked_error_response
         } else if should_replace_with_mixed_content {
-            blocked_error_response =
-                Response::network_error(NetworkError::Internal("Blocked as mixed content".into()));
+            blocked_error_response = Response::network_error(NetworkError::MixedContent);
             &blocked_error_response
         } else {
             internal_response
@@ -549,9 +536,7 @@ pub async fn main_fetch(
             !request.headers.contains_key(RANGE)
         {
             // Defer rebinding result
-            blocked_error_response = Response::network_error(NetworkError::Internal(
-                PARTIAL_RESPONSE_TO_NON_RANGE_REQUEST_ERROR.into(),
-            ));
+            blocked_error_response = Response::network_error(NetworkError::CacheError);
             &blocked_error_response
         } else {
             internal_response
@@ -594,9 +579,7 @@ pub async fn main_fetch(
         if response.termination_reason.is_none() &&
             !is_response_integrity_valid(integrity_metadata, &response)
         {
-            Response::network_error(NetworkError::Internal(
-                "Subresource integrity validation failed".into(),
-            ))
+            Response::network_error(NetworkError::SecurityBlock)
         } else {
             response
         }
@@ -821,7 +804,7 @@ async fn scheme_fetch(
 
         _ => match context.protocols.get(scheme) {
             Some(handler) => handler.load(request, done_chan, context).await,
-            None => Response::network_error(NetworkError::Internal("Unexpected scheme".into())),
+            None => Response::network_error(NetworkError::UnsupportedScheme),
         },
     }
 }
diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs
index 35624bb8645..c23593f1baa 100644
--- a/components/net/http_loader.rs
+++ b/components/net/http_loader.rs
@@ -787,7 +787,7 @@ pub async fn http_fetch(
                 (res.url_list.len() > 1 && request.redirect_mode != RedirectMode::Follow) ||
                 res.is_network_error()
             {
-                return Response::network_error(NetworkError::Internal("Request failed".into()));
+                return Response::network_error(NetworkError::ConnectionFailure);
             }
 
             // Subsubstep 4
@@ -843,7 +843,7 @@ pub async fn http_fetch(
 
         // Substep 4
         if cors_flag && cors_check(&fetch_params.request, &fetch_result).is_err() {
-            return Response::network_error(NetworkError::Internal("CORS check failed".into()));
+            return Response::network_error(NetworkError::CorsViolation);
         }
 
         fetch_result.return_internal = false;
@@ -894,9 +894,7 @@ pub async fn http_fetch(
 
         // Substep 5.
         response = match request.redirect_mode {
-            RedirectMode::Error => {
-                Response::network_error(NetworkError::Internal("Redirect mode error".into()))
-            },
+            RedirectMode::Error => Response::network_error(NetworkError::RedirectError),
             RedirectMode::Manual => response.to_filtered(ResponseType::OpaqueRedirect),
             RedirectMode::Follow => {
                 // set back to default
@@ -982,9 +980,7 @@ pub async fn http_redirect_fetch(
         },
         // Step 4
         Some(Ok(ref url)) if !matches!(url.scheme(), "http" | "https") => {
-            return Response::network_error(NetworkError::Internal(
-                "Location URL not an HTTP(S) scheme".into(),
-            ));
+            return Response::network_error(NetworkError::UnsupportedScheme);
         },
         Some(Ok(url)) => url,
     };
@@ -1022,7 +1018,7 @@ pub async fn http_redirect_fetch(
 
     // Step 7: If request’s redirect count is 20, then return a network error.
     if request.redirect_count >= 20 {
-        return Response::network_error(NetworkError::Internal("Too many redirects".into()));
+        return Response::network_error(NetworkError::RedirectError);
     }
 
     // Step 8: Increase request’s redirect count by 1.
@@ -1040,9 +1036,7 @@ pub async fn http_redirect_fetch(
     let has_credentials = has_credentials(&location_url);
 
     if request.mode == RequestMode::CorsMode && !same_origin && has_credentials {
-        return Response::network_error(NetworkError::Internal(
-            "Cross-origin credentials check failed".into(),
-        ));
+        return Response::network_error(NetworkError::CorsViolation);
     }
 
     // Step 9
@@ -1052,7 +1046,7 @@ pub async fn http_redirect_fetch(
 
     // Step 10
     if cors_flag && has_credentials {
-        return Response::network_error(NetworkError::Internal("Credentials check failed".into()));
+        return Response::network_error(NetworkError::CorsViolation);
     }
 
     // Step 11: If internalResponse’s status is not 303, request’s body is non-null, and request’s
@@ -1060,7 +1054,7 @@ pub async fn http_redirect_fetch(
     if response.actual_response().status != StatusCode::SEE_OTHER &&
         request.body.as_ref().is_some_and(|b| b.source_is_null())
     {
-        return Response::network_error(NetworkError::Internal("Request body is not done".into()));
+        return Response::network_error(NetworkError::ConnectionFailure);
     }
 
     // Step 12
@@ -1549,9 +1543,7 @@ async fn http_network_or_cache_fetch(
             // The cache will not be updated,
             // set its state to ready to construct.
             update_http_cache_state(context, http_request);
-            return Response::network_error(NetworkError::Internal(
-                "Couldn't find response in cache".into(),
-            ));
+            return Response::network_error(NetworkError::CacheError);
         }
 
         // Step 10.2 Let forwardResponse be the result of running HTTP-network fetch given httpFetchParams,
@@ -1609,9 +1601,7 @@ async fn http_network_or_cache_fetch(
         cross_origin_resource_policy_check(http_request, &response) ==
             CrossOriginResourcePolicy::Blocked
     {
-        return Response::network_error(NetworkError::Internal(
-            "Cross-origin resource policy check failed".into(),
-        ));
+        return Response::network_error(NetworkError::CorsViolation);
     }
 
     // TODO(#33616): Step 11. Set response’s URL list to a clone of httpRequest’s URL list.
@@ -1682,9 +1672,7 @@ async fn http_network_or_cache_fetch(
         // Step 15.1 If request’s window is "no-window", then return a network error.
 
         if request_has_no_window {
-            return Response::network_error(NetworkError::Internal(
-                "Can't find Window object".into(),
-            ));
+            return Response::network_error(NetworkError::ResourceError);
         }
 
         // (Step 15.2 does not exist, requires testing on Proxy-Authenticate headers)
@@ -1913,9 +1901,7 @@ async fn http_network_fetch(
     //
     match fetch_terminated_receiver.recv().await {
         Some(true) => {
-            return Response::network_error(NetworkError::Internal(
-                "Request body streaming failed.".into(),
-            ));
+            return Response::network_error(NetworkError::ConnectionFailure);
         },
         Some(false) => {},
         _ => warn!("Failed to receive confirmation request was streamed without error."),
@@ -1984,7 +1970,7 @@ async fn http_network_fetch(
     let meta_headers = meta.headers;
     let cancellation_listener = context.cancellation_listener.clone();
     if cancellation_listener.cancelled() {
-        return Response::network_error(NetworkError::Internal("Fetch aborted".into()));
+        return Response::network_error(NetworkError::ConnectionFailure);
     }
 
     *res_body.lock().unwrap() = ResponseBody::Receiving(vec![]);
@@ -2183,9 +2169,7 @@ async fn cors_preflight_fetch(
                 Some(methods) => methods.iter().collect(),
                 // Substep 3
                 None => {
-                    return Response::network_error(NetworkError::Internal(
-                        "CORS ACAM check failed".into(),
-                    ));
+                    return Response::network_error(NetworkError::CorsViolation);
                 },
             }
         } else {
@@ -2201,9 +2185,7 @@ async fn cors_preflight_fetch(
                 Some(names) => names.iter().collect(),
                 // Substep 3
                 None => {
-                    return Response::network_error(NetworkError::Internal(
-                        "CORS ACAH check failed".into(),
-                    ));
+                    return Response::network_error(NetworkError::CorsViolation);
                 },
             }
         } else {
@@ -2228,9 +2210,7 @@ async fn cors_preflight_fetch(
             (request.credentials_mode == CredentialsMode::Include ||
                 methods.iter().all(|m| m.as_ref() != "*"))
         {
-            return Response::network_error(NetworkError::Internal(
-                "CORS method check failed".into(),
-            ));
+            return Response::network_error(NetworkError::CorsViolation);
         }
 
         debug!(
@@ -2243,9 +2223,7 @@ async fn cors_preflight_fetch(
             is_cors_non_wildcard_request_header_name(name) &&
                 header_names.iter().all(|hn| hn != name)
         }) {
-            return Response::network_error(NetworkError::Internal(
-                "CORS authorization check failed".into(),
-            ));
+            return Response::network_error(NetworkError::CorsViolation);
         }
 
         // Substep 7
@@ -2258,9 +2236,7 @@ async fn cors_preflight_fetch(
                 (request.credentials_mode == CredentialsMode::Include ||
                     !header_names_contains_star)
             {
-                return Response::network_error(NetworkError::Internal(
-                    "CORS headers check failed".into(),
-                ));
+                return Response::network_error(NetworkError::CorsViolation);
             }
         }
 
@@ -2290,7 +2266,7 @@ async fn cors_preflight_fetch(
     }
 
     // Step 8
-    Response::network_error(NetworkError::Internal("CORS check failed".into()))
+    Response::network_error(NetworkError::CorsViolation)
 }
 
 /// [CORS check](https://fetch.spec.whatwg.org#concept-cors-check)
diff --git a/components/net/local_directory_listing.rs b/components/net/local_directory_listing.rs
index 773ed929492..b05c33a645d 100644
--- a/components/net/local_directory_listing.rs
+++ b/components/net/local_directory_listing.rs
@@ -19,18 +19,14 @@ pub fn fetch(request: &mut Request, url: ServoUrl, path_buf: PathBuf) -> Respons
     if !pref!(network_local_directory_listing_enabled) {
         // If you want to be able to browse local directories, configure Servo prefs so that
         // "network.local_directory_listing.enabled" is set to true.
-        return Response::network_error(NetworkError::Internal(
-            "Local directory listing feature has not been enabled in preferences".into(),
-        ));
+        return Response::network_error(NetworkError::LocalDirectoryError);
     }
 
     if !request.origin.is_opaque() {
         // Checking for an opaque origin as a shorthand for user activation
         // as opposed to a request originating from a script.
         // TODO(32534): carefully consider security of this approach.
-        return Response::network_error(NetworkError::Internal(
-            "Cannot request local directory listing from non-local origin.".into(),
-        ));
+        return Response::network_error(NetworkError::LocalDirectoryError);
     }
 
     let directory_contents = match std::fs::read_dir(path_buf.clone()) {
diff --git a/components/net/protocols/blob.rs b/components/net/protocols/blob.rs
index 53c609c91a7..3845bcfb271 100644
--- a/components/net/protocols/blob.rs
+++ b/components/net/protocols/blob.rs
@@ -33,9 +33,7 @@ impl ProtocolHandler for BlobProtocolHander {
 
         // Step 2.
         if request.method != Method::GET {
-            return Box::pin(ready(Response::network_error(NetworkError::Internal(
-                "Unexpected method for blob".into(),
-            ))));
+            return Box::pin(ready(Response::network_error(NetworkError::InvalidMethod)));
         }
 
         let range_header = request.headers.typed_get::<Range>();
diff --git a/components/net/protocols/data.rs b/components/net/protocols/data.rs
index ab2baa83bff..e79c3925f8e 100644
--- a/components/net/protocols/data.rs
+++ b/components/net/protocols/data.rs
@@ -47,9 +47,7 @@ impl ProtocolHandler for DataProtocolHander {
             },
             Err(_) => None,
         }
-        .unwrap_or_else(|| {
-            Response::network_error(NetworkError::Internal("Decoding data URL failed".into()))
-        });
+        .unwrap_or_else(|| Response::network_error(NetworkError::ResourceError));
 
         Box::pin(std::future::ready(response))
     }
diff --git a/components/net/protocols/file.rs b/components/net/protocols/file.rs
index 403669179dd..bff7ad002e9 100644
--- a/components/net/protocols/file.rs
+++ b/components/net/protocols/file.rs
@@ -34,9 +34,7 @@ impl ProtocolHandler for FileProtocolHander {
         let url = request.current_url();
 
         if request.method != Method::GET {
-            return Box::pin(ready(Response::network_error(NetworkError::Internal(
-                "Unexpected method for file".into(),
-            ))));
+            return Box::pin(ready(Response::network_error(NetworkError::InvalidMethod)));
         }
         let response = if let Ok(file_path) = url.to_file_path() {
             if file_path.is_dir() {
@@ -66,9 +64,7 @@ impl ProtocolHandler for FileProtocolHander {
                 };
                 let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file);
                 if reader.seek(SeekFrom::Start(range.start as u64)).is_err() {
-                    return Box::pin(ready(Response::network_error(NetworkError::Internal(
-                        "Unexpected method for file".into(),
-                    ))));
+                    return Box::pin(ready(Response::network_error(NetworkError::InvalidMethod)));
                 }
 
                 // Set response status to 206 if Range header is present.
@@ -98,12 +94,10 @@ impl ProtocolHandler for FileProtocolHander {
 
                 response
             } else {
-                Response::network_error(NetworkError::Internal("Opening file failed".into()))
+                Response::network_error(NetworkError::ResourceError)
             }
         } else {
-            Response::network_error(NetworkError::Internal(
-                "Constructing file path failed".into(),
-            ))
+            Response::network_error(NetworkError::ResourceError)
         };
 
         Box::pin(ready(response))
diff --git a/components/net/tests/data_loader.rs b/components/net/tests/data_loader.rs
index b4082812229..24e556e2e61 100644
--- a/components/net/tests/data_loader.rs
+++ b/components/net/tests/data_loader.rs
@@ -59,12 +59,7 @@ fn assert_parse(
         },
         None => {
             assert!(response.is_network_error());
-            assert_eq!(
-                response.metadata().err(),
-                Some(NetworkError::Internal(
-                    "Decoding data URL failed".to_owned()
-                ))
-            );
+            assert_eq!(response.metadata().err(), Some(NetworkError::ResourceError));
         },
     }
 }
diff --git a/components/net/tests/fetch.rs b/components/net/tests/fetch.rs
index 7dbf1ca0047..2e1f5f7c8ff 100644
--- a/components/net/tests/fetch.rs
+++ b/components/net/tests/fetch.rs
@@ -86,10 +86,7 @@ fn test_fetch_on_bad_port_is_network_error() {
     let fetch_response = fetch(request, None);
     assert!(fetch_response.is_network_error());
     let fetch_error = fetch_response.get_network_error().unwrap();
-    assert_eq!(
-        fetch_error,
-        &NetworkError::Internal("Request attempted on bad port".into())
-    )
+    assert_eq!(fetch_error, &NetworkError::InvalidPort)
 }
 
 #[test]
diff --git a/components/net/tests/http_loader.rs b/components/net/tests/http_loader.rs
index 1fc2d1b662d..9950c3531e6 100644
--- a/components/net/tests/http_loader.rs
+++ b/components/net/tests/http_loader.rs
@@ -1141,7 +1141,7 @@ fn test_load_errors_when_there_a_redirect_loop() {
 
     assert_eq!(
         response.get_network_error(),
-        Some(&NetworkError::Internal("Too many redirects".to_owned()))
+        Some(&NetworkError::RedirectError)
     );
 }
 
diff --git a/components/shared/net/lib.rs b/components/shared/net/lib.rs
index 05ad102f42f..0da8867d4c8 100644
--- a/components/shared/net/lib.rs
+++ b/components/shared/net/lib.rs
@@ -933,6 +933,18 @@ pub enum NetworkError {
     SslValidation(String, Vec<u8>),
     /// Crash error, to be converted to Resource::Crash in the HTML parser.
     Crash(String),
+    UnsupportedScheme,
+    CorsViolation,
+    ConnectionFailure,
+    Timeout,
+    RedirectError,
+    InvalidMethod,
+    ResourceError,
+    SecurityBlock,
+    MixedContent,
+    CacheError,
+    InvalidPort,
+    LocalDirectoryError,
 }
 
 impl NetworkError {