diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index 680e229e5c0..55d1c0ecd48 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -252,7 +252,7 @@ pub fn main_fetch(request: &mut Request, // Step 14. // We don't need to check whether response is a network error, // given its type would not be `Default` in this case. - let response = if response.response_type == ResponseType::Default { + let mut response = if response.response_type == ResponseType::Default { let response_type = match request.response_tainting { ResponseTainting::Basic => ResponseType::Basic, ResponseTainting::CorsTainting => ResponseType::Cors, @@ -264,18 +264,22 @@ pub fn main_fetch(request: &mut Request, }; let internal_error = { + // Tests for steps 17 and 18, before step 15 for borrowing concerns. + let response_is_network_error = response.is_network_error(); + let should_replace_with_nosniff_error = + !response_is_network_error && should_be_blocked_due_to_nosniff(request.type_, &response.headers); + // Step 15. - let network_error_response; - let internal_response = if let Some(error) = response.get_network_error() { - network_error_response = Response::network_error(error.clone()); - &network_error_response + let mut network_error_response = response.get_network_error().cloned().map(Response::network_error); + let internal_response = if let Some(error_response) = network_error_response.as_mut() { + error_response } else { - response.actual_response() + response.actual_response_mut() }; // Step 16. - if internal_response.url_list.borrow().is_empty() { - *internal_response.url_list.borrow_mut() = request.url_list.clone(); + if internal_response.url_list.is_empty() { + internal_response.url_list = request.url_list.clone(); } // Step 17. @@ -284,7 +288,7 @@ pub fn main_fetch(request: &mut Request, // TODO: handle blocking due to MIME type. let blocked_error_response; let internal_response = - if !response.is_network_error() && should_be_blocked_due_to_nosniff(request.type_, &response.headers) { + if should_replace_with_nosniff_error { // Defer rebinding result blocked_error_response = Response::network_error(NetworkError::Internal("Blocked by nosniff".into())); &blocked_error_response @@ -295,7 +299,7 @@ pub fn main_fetch(request: &mut Request, // Step 18. // We check `internal_response` since we did not mutate `response` // in the previous step. - let not_network_error = !response.is_network_error() && !internal_response.is_network_error(); + let not_network_error = !response_is_network_error && !internal_response.is_network_error(); if not_network_error && (is_null_body_status(&internal_response.status) || match request.method { Method::Head | Method::Connect => true, diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index 8e30aa5eb03..4712a8dc6ea 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -507,8 +507,7 @@ pub fn http_fetch(request: &mut Request, request.mode != RequestMode::NoCors) || (res.response_type == ResponseType::OpaqueRedirect && request.redirect_mode != RedirectMode::Manual) || - (res.url_list.borrow().len() > 1 && - request.redirect_mode != RedirectMode::Follow) || + (res.url_list.len() > 1 && request.redirect_mode != RedirectMode::Follow) || res.is_network_error() { return Response::network_error(NetworkError::Internal("Request failed".into())); } diff --git a/components/net_traits/response.rs b/components/net_traits/response.rs index 34faed00ee9..7d0bfe57000 100644 --- a/components/net_traits/response.rs +++ b/components/net_traits/response.rs @@ -10,7 +10,6 @@ use hyper::status::StatusCode; use hyper_serde::Serde; use servo_url::ServoUrl; use std::ascii::AsciiExt; -use std::cell::RefCell; use std::sync::{Arc, Mutex}; /// [Response type](https://fetch.spec.whatwg.org/#concept-response-type) @@ -81,7 +80,7 @@ pub struct Response { pub response_type: ResponseType, pub termination_reason: Option, url: Option, - pub url_list: RefCell>, + pub url_list: Vec, /// `None` can be considered a StatusCode of `0`. #[ignore_heap_size_of = "Defined in hyper"] pub status: Option, @@ -106,7 +105,7 @@ impl Response { response_type: ResponseType::Default, termination_reason: None, url: Some(url), - url_list: RefCell::new(Vec::new()), + url_list: vec![], status: Some(StatusCode::Ok), raw_status: Some((200, b"OK".to_vec())), headers: Headers::new(), @@ -124,7 +123,7 @@ impl Response { response_type: ResponseType::Error(e), termination_reason: None, url: None, - url_list: RefCell::new(vec![]), + url_list: vec![], status: None, raw_status: None, headers: Headers::new(), @@ -163,6 +162,14 @@ impl Response { } } + pub fn actual_response_mut(&mut self) -> &mut Response { + if self.return_internal && self.internal_response.is_some() { + &mut **self.internal_response.as_mut().unwrap() + } else { + self + } + } + pub fn to_actual(self) -> Response { if self.return_internal && self.internal_response.is_some() { *self.internal_response.unwrap() @@ -226,7 +233,7 @@ impl Response { }, ResponseType::Opaque => { - response.url_list = RefCell::new(vec![]); + response.url_list = vec![]; response.url = None; response.headers = Headers::new(); response.status = None; diff --git a/tests/unit/net/fetch.rs b/tests/unit/net/fetch.rs index 70c408f1776..7df0af9643c 100644 --- a/tests/unit/net/fetch.rs +++ b/tests/unit/net/fetch.rs @@ -418,7 +418,7 @@ fn test_fetch_response_is_opaque_filtered() { assert_eq!(fetch_response.response_type, ResponseType::Opaque); assert!(fetch_response.url().is_none()); - assert!(fetch_response.url_list.into_inner().len() == 0); + assert!(fetch_response.url_list.is_empty()); // this also asserts that status message is "the empty byte sequence" assert!(fetch_response.status.is_none()); assert_eq!(fetch_response.headers, Headers::new()); diff --git a/tests/unit/net/http_loader.rs b/tests/unit/net/http_loader.rs index be1370bfd1f..517a1c0741c 100644 --- a/tests/unit/net/http_loader.rs +++ b/tests/unit/net/http_loader.rs @@ -896,8 +896,7 @@ fn test_load_succeeds_with_a_redirect_loop() { let _ = server_b.close(); let response = response.to_actual(); - assert_eq!(*response.url_list.borrow(), - [url_a.clone(), url_b, url_a]); + assert_eq!(response.url_list, [url_a.clone(), url_b, url_a]); assert_eq!(*response.body.lock().unwrap(), ResponseBody::Done(b"Success".to_vec())); }