Replace NetworkError::Internal with structured enum variants

Signed-off-by: Uthman Yahaya Baba <uthmanyahayababa@gmail.com>
This commit is contained in:
Uthman Yahaya Baba 2025-04-18 13:46:14 +01:00
parent 0eec22303b
commit fe4e90d7dc
10 changed files with 55 additions and 106 deletions

View file

@ -227,9 +227,7 @@ pub async fn main_fetch(
"about" | "blob" | "data" | "filesystem" "about" | "blob" | "data" | "filesystem"
) )
{ {
response = Some(Response::network_error(NetworkError::Internal( response = Some(Response::network_error(NetworkError::UnsupportedScheme));
"Non-local scheme".into(),
)));
} }
// Step 2.2. // Step 2.2.
@ -285,19 +283,13 @@ pub async fn main_fetch(
if check_result == csp::CheckResult::Blocked { if check_result == csp::CheckResult::Blocked {
warn!("Request blocked by CSP"); warn!("Request blocked by CSP");
response = Some(Response::network_error(NetworkError::Internal( response = Some(Response::network_error(NetworkError::SecurityBlock))
"Blocked by Content-Security-Policy".into(),
)))
} }
if should_request_be_blocked_due_to_a_bad_port(&request.current_url()) { if should_request_be_blocked_due_to_a_bad_port(&request.current_url()) {
response = Some(Response::network_error(NetworkError::Internal( response = Some(Response::network_error(NetworkError::InvalidPort));
"Request attempted on bad port".into(),
)));
} }
if should_request_be_blocked_as_mixed_content(request) { if should_request_be_blocked_as_mixed_content(request) {
response = Some(Response::network_error(NetworkError::Internal( response = Some(Response::network_error(NetworkError::MixedContent));
"Blocked as mixed content".into(),
)));
} }
// Step 8: If requests referrer policy is the empty string, then set requests referrer policy // Step 8: If requests referrer policy is the empty string, then set requests referrer policy
@ -371,13 +363,11 @@ pub async fn main_fetch(
// Substep 2. Return the result of running scheme fetch given fetchParams. // Substep 2. Return the result of running scheme fetch given fetchParams.
scheme_fetch(fetch_params, cache, target, done_chan, context).await scheme_fetch(fetch_params, cache, target, done_chan, context).await
} else if request.mode == RequestMode::SameOrigin { } 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 { } else if request.mode == RequestMode::NoCors {
// Substep 1. If requests redirect mode is not "follow", then return a network error. // Substep 1. If requests redirect mode is not "follow", then return a network error.
if request.redirect_mode != RedirectMode::Follow { if request.redirect_mode != RedirectMode::Follow {
Response::network_error(NetworkError::Internal( Response::network_error(NetworkError::CorsViolation)
"NoCors requests must follow redirects".into(),
))
} else { } else {
// Substep 2. Set requests response tainting to "opaque". // Substep 2. Set requests response tainting to "opaque".
request.response_tainting = ResponseTainting::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 scheme_fetch(fetch_params, cache, target, done_chan, context).await
} }
} else if !matches!(current_scheme, "http" | "https") { } 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 || } else if request.use_cors_preflight ||
(request.unsafe_request && (request.unsafe_request &&
(!is_cors_safelisted_method(&request.method) || (!is_cors_safelisted_method(&request.method) ||
@ -521,17 +511,14 @@ pub async fn main_fetch(
let internal_response = if should_replace_with_nosniff_error { let internal_response = if should_replace_with_nosniff_error {
// Defer rebinding result // Defer rebinding result
blocked_error_response = blocked_error_response = Response::network_error(NetworkError::SecurityBlock);
Response::network_error(NetworkError::Internal("Blocked by nosniff".into()));
&blocked_error_response &blocked_error_response
} else if should_replace_with_mime_type_error { } else if should_replace_with_mime_type_error {
// Defer rebinding result // Defer rebinding result
blocked_error_response = blocked_error_response = Response::network_error(NetworkError::SecurityBlock);
Response::network_error(NetworkError::Internal("Blocked by mime type".into()));
&blocked_error_response &blocked_error_response
} else if should_replace_with_mixed_content { } else if should_replace_with_mixed_content {
blocked_error_response = blocked_error_response = Response::network_error(NetworkError::MixedContent);
Response::network_error(NetworkError::Internal("Blocked as mixed content".into()));
&blocked_error_response &blocked_error_response
} else { } else {
internal_response internal_response
@ -549,9 +536,7 @@ pub async fn main_fetch(
!request.headers.contains_key(RANGE) !request.headers.contains_key(RANGE)
{ {
// Defer rebinding result // Defer rebinding result
blocked_error_response = Response::network_error(NetworkError::Internal( blocked_error_response = Response::network_error(NetworkError::CacheError);
PARTIAL_RESPONSE_TO_NON_RANGE_REQUEST_ERROR.into(),
));
&blocked_error_response &blocked_error_response
} else { } else {
internal_response internal_response
@ -594,9 +579,7 @@ pub async fn main_fetch(
if response.termination_reason.is_none() && if response.termination_reason.is_none() &&
!is_response_integrity_valid(integrity_metadata, &response) !is_response_integrity_valid(integrity_metadata, &response)
{ {
Response::network_error(NetworkError::Internal( Response::network_error(NetworkError::SecurityBlock)
"Subresource integrity validation failed".into(),
))
} else { } else {
response response
} }
@ -821,7 +804,7 @@ async fn scheme_fetch(
_ => match context.protocols.get(scheme) { _ => match context.protocols.get(scheme) {
Some(handler) => handler.load(request, done_chan, context).await, Some(handler) => handler.load(request, done_chan, context).await,
None => Response::network_error(NetworkError::Internal("Unexpected scheme".into())), None => Response::network_error(NetworkError::UnsupportedScheme),
}, },
} }
} }

View file

@ -787,7 +787,7 @@ pub async fn http_fetch(
(res.url_list.len() > 1 && request.redirect_mode != RedirectMode::Follow) || (res.url_list.len() > 1 && request.redirect_mode != RedirectMode::Follow) ||
res.is_network_error() res.is_network_error()
{ {
return Response::network_error(NetworkError::Internal("Request failed".into())); return Response::network_error(NetworkError::ConnectionFailure);
} }
// Subsubstep 4 // Subsubstep 4
@ -843,7 +843,7 @@ pub async fn http_fetch(
// Substep 4 // Substep 4
if cors_flag && cors_check(&fetch_params.request, &fetch_result).is_err() { 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; fetch_result.return_internal = false;
@ -894,9 +894,7 @@ pub async fn http_fetch(
// Substep 5. // Substep 5.
response = match request.redirect_mode { response = match request.redirect_mode {
RedirectMode::Error => { RedirectMode::Error => Response::network_error(NetworkError::RedirectError),
Response::network_error(NetworkError::Internal("Redirect mode error".into()))
},
RedirectMode::Manual => response.to_filtered(ResponseType::OpaqueRedirect), RedirectMode::Manual => response.to_filtered(ResponseType::OpaqueRedirect),
RedirectMode::Follow => { RedirectMode::Follow => {
// set back to default // set back to default
@ -982,9 +980,7 @@ pub async fn http_redirect_fetch(
}, },
// Step 4 // Step 4
Some(Ok(ref url)) if !matches!(url.scheme(), "http" | "https") => { Some(Ok(ref url)) if !matches!(url.scheme(), "http" | "https") => {
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::UnsupportedScheme);
"Location URL not an HTTP(S) scheme".into(),
));
}, },
Some(Ok(url)) => url, Some(Ok(url)) => url,
}; };
@ -1022,7 +1018,7 @@ pub async fn http_redirect_fetch(
// Step 7: If requests redirect count is 20, then return a network error. // Step 7: If requests redirect count is 20, then return a network error.
if request.redirect_count >= 20 { if request.redirect_count >= 20 {
return Response::network_error(NetworkError::Internal("Too many redirects".into())); return Response::network_error(NetworkError::RedirectError);
} }
// Step 8: Increase requests redirect count by 1. // Step 8: Increase requests redirect count by 1.
@ -1040,9 +1036,7 @@ pub async fn http_redirect_fetch(
let has_credentials = has_credentials(&location_url); let has_credentials = has_credentials(&location_url);
if request.mode == RequestMode::CorsMode && !same_origin && has_credentials { if request.mode == RequestMode::CorsMode && !same_origin && has_credentials {
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::CorsViolation);
"Cross-origin credentials check failed".into(),
));
} }
// Step 9 // Step 9
@ -1052,7 +1046,7 @@ pub async fn http_redirect_fetch(
// Step 10 // Step 10
if cors_flag && has_credentials { if cors_flag && has_credentials {
return Response::network_error(NetworkError::Internal("Credentials check failed".into())); return Response::network_error(NetworkError::CorsViolation);
} }
// Step 11: If internalResponses status is not 303, requests body is non-null, and requests // Step 11: If internalResponses status is not 303, requests body is non-null, and requests
@ -1060,7 +1054,7 @@ pub async fn http_redirect_fetch(
if response.actual_response().status != StatusCode::SEE_OTHER && if response.actual_response().status != StatusCode::SEE_OTHER &&
request.body.as_ref().is_some_and(|b| b.source_is_null()) 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 // Step 12
@ -1549,9 +1543,7 @@ async fn http_network_or_cache_fetch(
// The cache will not be updated, // The cache will not be updated,
// set its state to ready to construct. // set its state to ready to construct.
update_http_cache_state(context, http_request); update_http_cache_state(context, http_request);
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::CacheError);
"Couldn't find response in cache".into(),
));
} }
// Step 10.2 Let forwardResponse be the result of running HTTP-network fetch given httpFetchParams, // 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) == cross_origin_resource_policy_check(http_request, &response) ==
CrossOriginResourcePolicy::Blocked CrossOriginResourcePolicy::Blocked
{ {
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::CorsViolation);
"Cross-origin resource policy check failed".into(),
));
} }
// TODO(#33616): Step 11. Set responses URL list to a clone of httpRequests URL list. // TODO(#33616): Step 11. Set responses URL list to a clone of httpRequests URL list.
@ -1682,9 +1672,7 @@ async fn http_network_or_cache_fetch(
// Step 15.1 If requests window is "no-window", then return a network error. // Step 15.1 If requests window is "no-window", then return a network error.
if request_has_no_window { if request_has_no_window {
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::ResourceError);
"Can't find Window object".into(),
));
} }
// (Step 15.2 does not exist, requires testing on Proxy-Authenticate headers) // (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 { match fetch_terminated_receiver.recv().await {
Some(true) => { Some(true) => {
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::ConnectionFailure);
"Request body streaming failed.".into(),
));
}, },
Some(false) => {}, Some(false) => {},
_ => warn!("Failed to receive confirmation request was streamed without error."), _ => 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 meta_headers = meta.headers;
let cancellation_listener = context.cancellation_listener.clone(); let cancellation_listener = context.cancellation_listener.clone();
if cancellation_listener.cancelled() { 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![]); *res_body.lock().unwrap() = ResponseBody::Receiving(vec![]);
@ -2183,9 +2169,7 @@ async fn cors_preflight_fetch(
Some(methods) => methods.iter().collect(), Some(methods) => methods.iter().collect(),
// Substep 3 // Substep 3
None => { None => {
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::CorsViolation);
"CORS ACAM check failed".into(),
));
}, },
} }
} else { } else {
@ -2201,9 +2185,7 @@ async fn cors_preflight_fetch(
Some(names) => names.iter().collect(), Some(names) => names.iter().collect(),
// Substep 3 // Substep 3
None => { None => {
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::CorsViolation);
"CORS ACAH check failed".into(),
));
}, },
} }
} else { } else {
@ -2228,9 +2210,7 @@ async fn cors_preflight_fetch(
(request.credentials_mode == CredentialsMode::Include || (request.credentials_mode == CredentialsMode::Include ||
methods.iter().all(|m| m.as_ref() != "*")) methods.iter().all(|m| m.as_ref() != "*"))
{ {
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::CorsViolation);
"CORS method check failed".into(),
));
} }
debug!( debug!(
@ -2243,9 +2223,7 @@ async fn cors_preflight_fetch(
is_cors_non_wildcard_request_header_name(name) && is_cors_non_wildcard_request_header_name(name) &&
header_names.iter().all(|hn| hn != name) header_names.iter().all(|hn| hn != name)
}) { }) {
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::CorsViolation);
"CORS authorization check failed".into(),
));
} }
// Substep 7 // Substep 7
@ -2258,9 +2236,7 @@ async fn cors_preflight_fetch(
(request.credentials_mode == CredentialsMode::Include || (request.credentials_mode == CredentialsMode::Include ||
!header_names_contains_star) !header_names_contains_star)
{ {
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::CorsViolation);
"CORS headers check failed".into(),
));
} }
} }
@ -2290,7 +2266,7 @@ async fn cors_preflight_fetch(
} }
// Step 8 // 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) /// [CORS check](https://fetch.spec.whatwg.org#concept-cors-check)

View file

@ -19,18 +19,14 @@ pub fn fetch(request: &mut Request, url: ServoUrl, path_buf: PathBuf) -> Respons
if !pref!(network_local_directory_listing_enabled) { if !pref!(network_local_directory_listing_enabled) {
// If you want to be able to browse local directories, configure Servo prefs so that // If you want to be able to browse local directories, configure Servo prefs so that
// "network.local_directory_listing.enabled" is set to true. // "network.local_directory_listing.enabled" is set to true.
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::LocalDirectoryError);
"Local directory listing feature has not been enabled in preferences".into(),
));
} }
if !request.origin.is_opaque() { if !request.origin.is_opaque() {
// Checking for an opaque origin as a shorthand for user activation // Checking for an opaque origin as a shorthand for user activation
// as opposed to a request originating from a script. // as opposed to a request originating from a script.
// TODO(32534): carefully consider security of this approach. // TODO(32534): carefully consider security of this approach.
return Response::network_error(NetworkError::Internal( return Response::network_error(NetworkError::LocalDirectoryError);
"Cannot request local directory listing from non-local origin.".into(),
));
} }
let directory_contents = match std::fs::read_dir(path_buf.clone()) { let directory_contents = match std::fs::read_dir(path_buf.clone()) {

View file

@ -33,9 +33,7 @@ impl ProtocolHandler for BlobProtocolHander {
// Step 2. // Step 2.
if request.method != Method::GET { if request.method != Method::GET {
return Box::pin(ready(Response::network_error(NetworkError::Internal( return Box::pin(ready(Response::network_error(NetworkError::InvalidMethod)));
"Unexpected method for blob".into(),
))));
} }
let range_header = request.headers.typed_get::<Range>(); let range_header = request.headers.typed_get::<Range>();

View file

@ -47,9 +47,7 @@ impl ProtocolHandler for DataProtocolHander {
}, },
Err(_) => None, Err(_) => None,
} }
.unwrap_or_else(|| { .unwrap_or_else(|| Response::network_error(NetworkError::ResourceError));
Response::network_error(NetworkError::Internal("Decoding data URL failed".into()))
});
Box::pin(std::future::ready(response)) Box::pin(std::future::ready(response))
} }

View file

@ -34,9 +34,7 @@ impl ProtocolHandler for FileProtocolHander {
let url = request.current_url(); let url = request.current_url();
if request.method != Method::GET { if request.method != Method::GET {
return Box::pin(ready(Response::network_error(NetworkError::Internal( return Box::pin(ready(Response::network_error(NetworkError::InvalidMethod)));
"Unexpected method for file".into(),
))));
} }
let response = if let Ok(file_path) = url.to_file_path() { let response = if let Ok(file_path) = url.to_file_path() {
if file_path.is_dir() { if file_path.is_dir() {
@ -66,9 +64,7 @@ impl ProtocolHandler for FileProtocolHander {
}; };
let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file); let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file);
if reader.seek(SeekFrom::Start(range.start as u64)).is_err() { if reader.seek(SeekFrom::Start(range.start as u64)).is_err() {
return Box::pin(ready(Response::network_error(NetworkError::Internal( return Box::pin(ready(Response::network_error(NetworkError::InvalidMethod)));
"Unexpected method for file".into(),
))));
} }
// Set response status to 206 if Range header is present. // Set response status to 206 if Range header is present.
@ -98,12 +94,10 @@ impl ProtocolHandler for FileProtocolHander {
response response
} else { } else {
Response::network_error(NetworkError::Internal("Opening file failed".into())) Response::network_error(NetworkError::ResourceError)
} }
} else { } else {
Response::network_error(NetworkError::Internal( Response::network_error(NetworkError::ResourceError)
"Constructing file path failed".into(),
))
}; };
Box::pin(ready(response)) Box::pin(ready(response))

View file

@ -59,12 +59,7 @@ fn assert_parse(
}, },
None => { None => {
assert!(response.is_network_error()); assert!(response.is_network_error());
assert_eq!( assert_eq!(response.metadata().err(), Some(NetworkError::ResourceError));
response.metadata().err(),
Some(NetworkError::Internal(
"Decoding data URL failed".to_owned()
))
);
}, },
} }
} }

View file

@ -86,10 +86,7 @@ fn test_fetch_on_bad_port_is_network_error() {
let fetch_response = fetch(request, None); let fetch_response = fetch(request, None);
assert!(fetch_response.is_network_error()); assert!(fetch_response.is_network_error());
let fetch_error = fetch_response.get_network_error().unwrap(); let fetch_error = fetch_response.get_network_error().unwrap();
assert_eq!( assert_eq!(fetch_error, &NetworkError::InvalidPort)
fetch_error,
&NetworkError::Internal("Request attempted on bad port".into())
)
} }
#[test] #[test]

View file

@ -1141,7 +1141,7 @@ fn test_load_errors_when_there_a_redirect_loop() {
assert_eq!( assert_eq!(
response.get_network_error(), response.get_network_error(),
Some(&NetworkError::Internal("Too many redirects".to_owned())) Some(&NetworkError::RedirectError)
); );
} }

View file

@ -933,6 +933,18 @@ pub enum NetworkError {
SslValidation(String, Vec<u8>), SslValidation(String, Vec<u8>),
/// Crash error, to be converted to Resource::Crash in the HTML parser. /// Crash error, to be converted to Resource::Crash in the HTML parser.
Crash(String), Crash(String),
UnsupportedScheme,
CorsViolation,
ConnectionFailure,
Timeout,
RedirectError,
InvalidMethod,
ResourceError,
SecurityBlock,
MixedContent,
CacheError,
InvalidPort,
LocalDirectoryError,
} }
impl NetworkError { impl NetworkError {