From 06ffdd68e85854790f48f0c59f638b415c8dd695 Mon Sep 17 00:00:00 2001 From: Bob Date: Tue, 1 Mar 2016 18:10:58 +0000 Subject: [PATCH] refactor http_loader hostname/htst order Changed hostname rewrite to happen inside obtain response after any htst changes. Removed url from load leaving just doc_url to avoid confusion --- components/net/http_loader.rs | 60 ++++++++++++++++------------------- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index 3d231423f93a..5b66c877579a 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -514,14 +514,13 @@ fn request_must_be_secured(url: &Url, hsts_list: &Arc>) -> bool pub fn modify_request_headers(headers: &mut Headers, url: &Url, - doc_url: &Url, user_agent: &str, cookie_jar: &Arc>, load_data: &LoadData) { // Ensure that the host header is set from the original url let host = Host { - hostname: doc_url.serialize_host().unwrap(), - port: doc_url.port_or_default() + hostname: url.serialize_host().unwrap(), + port: url.port_or_default() }; headers.set(host); headers.set(UserAgent(user_agent.to_owned())); @@ -534,7 +533,7 @@ pub fn modify_request_headers(headers: &mut Headers, // https://fetch.spec.whatwg.org/#http-network-or-cache-fetch step 12 if !headers.has::>() { - if let Some(auth) = auth_from_url(doc_url) { + if let Some(auth) = auth_from_url(url) { headers.set(auth); } } @@ -555,7 +554,6 @@ fn auth_from_url(doc_url: &Url) -> Option> { pub fn process_response_headers(response: &HttpResponse, url: &Url, - doc_url: &Url, cookie_jar: &Arc>, hsts_list: &Arc>, load_data: &LoadData) { @@ -568,7 +566,7 @@ pub fn process_response_headers(response: &HttpResponse, // https://fetch.spec.whatwg.org/#concept-http-network-fetch step 9 if load_data.credentials_flag { - set_cookies_from_response(doc_url.clone(), response, cookie_jar); + set_cookies_from_response(url.clone(), response, cookie_jar); } update_sts_list_from_response(url, response, hsts_list); } @@ -587,17 +585,18 @@ pub fn obtain_response(request_factory: &HttpRequestFactory, -> Result where A: HttpRequest + 'static { let response; + let connection_url = replace_hosts(&url); // loop trying connections in connection pool // they may have grown stale (disconnected), in which case we'll get // a ConnectionAborted error. this loop tries again with a new // connection. loop { - let mut req = try!(request_factory.create(url.clone(), method.clone())); + let mut req = try!(request_factory.create(connection_url.clone(), method.clone())); *req.headers_mut() = request_headers.clone(); if cancel_listener.is_cancelled() { - return Err(LoadError::Cancelled(url.clone(), "load cancelled".to_owned())); + return Err(LoadError::Cancelled(connection_url.clone(), "load cancelled".to_owned())); } if log_enabled!(log::LogLevel::Info) { @@ -633,7 +632,7 @@ pub fn obtain_response(request_factory: &HttpRequestFactory, if let Some(pipeline_id) = *pipeline_id { send_request_to_devtools( devtools_chan.clone(), request_id.clone().into(), - url.clone(), method.clone(), request_headers.clone(), + connection_url.clone(), method.clone(), request_headers.clone(), cloned_data, pipeline_id, time::now() ); } @@ -669,48 +668,44 @@ pub fn load(load_data: LoadData, let mut iters = 0; // URL of the document being loaded, as seen by all the higher-level code. let mut doc_url = load_data.url.clone(); - // URL that we actually fetch from the network, after applying the replacements - // specified in the hosts file. - let mut url = replace_hosts(&load_data.url); let mut redirected_to = HashSet::new(); let mut method = load_data.method.clone(); if cancel_listener.is_cancelled() { - return Err(LoadError::Cancelled(url, "load cancelled".to_owned())); + return Err(LoadError::Cancelled(doc_url, "load cancelled".to_owned())); } // If the URL is a view-source scheme then the scheme data contains the // real URL that should be used for which the source is to be viewed. // Change our existing URL to that and keep note that we are viewing // the source rather than rendering the contents of the URL. - let viewing_source = url.scheme == "view-source"; + let viewing_source = doc_url.scheme == "view-source"; if viewing_source { - url = inner_url(&load_data.url); - doc_url = url.clone(); + doc_url = inner_url(&load_data.url); } // Loop to handle redirects. loop { iters = iters + 1; - if &*url.scheme == "http" && request_must_be_secured(&url, &hsts_list) { - info!("{} is in the strict transport security list, requesting secure host", url); - url = secure_url(&url); + if &*doc_url.scheme == "http" && request_must_be_secured(&doc_url, &hsts_list) { + info!("{} is in the strict transport security list, requesting secure host", doc_url); + doc_url = secure_url(&doc_url); } if iters > max_redirects { - return Err(LoadError::MaxRedirects(url)); + return Err(LoadError::MaxRedirects(doc_url)); } - if &*url.scheme != "http" && &*url.scheme != "https" { - return Err(LoadError::UnsupportedScheme(url)); + if &*doc_url.scheme != "http" && &*doc_url.scheme != "https" { + return Err(LoadError::UnsupportedScheme(doc_url)); } if cancel_listener.is_cancelled() { - return Err(LoadError::Cancelled(url, "load cancelled".to_owned())); + return Err(LoadError::Cancelled(doc_url, "load cancelled".to_owned())); } - info!("requesting {}", url.serialize()); + info!("requesting {}", doc_url.serialize()); // Avoid automatically preserving request headers when redirects occur. // See https://bugzilla.mozilla.org/show_bug.cgi?id=401564 and @@ -726,13 +721,13 @@ pub fn load(load_data: LoadData, let request_id = uuid::Uuid::new_v4().to_simple_string(); - modify_request_headers(&mut request_headers, &url, &doc_url, &user_agent, &cookie_jar, &load_data); + modify_request_headers(&mut request_headers, &doc_url, &user_agent, &cookie_jar, &load_data); - let response = try!(obtain_response(request_factory, &url, &method, &request_headers, + let response = try!(obtain_response(request_factory, &doc_url, &method, &request_headers, &cancel_listener, &load_data.data, &load_data.method, &load_data.pipeline_id, iters, &devtools_chan, &request_id)); - process_response_headers(&response, &url, &doc_url, &cookie_jar, &hsts_list, &load_data); + process_response_headers(&response, &doc_url, &cookie_jar, &hsts_list, &load_data); // --- Loop if there's a redirect if response.status().class() == StatusClass::Redirection { @@ -742,7 +737,7 @@ pub fn load(load_data: LoadData, if c.preflight { return Err( LoadError::Cors( - url, + doc_url, "Preflight fetch inconsistent with main fetch".to_owned())); } else { // XXXManishearth There are some CORS-related steps here, @@ -757,10 +752,6 @@ pub fn load(load_data: LoadData, } }; - info!("redirecting to {}", new_doc_url); - url = replace_hosts(&new_doc_url); - doc_url = new_doc_url; - // According to https://tools.ietf.org/html/rfc7231#section-6.4.2, // historically UAs have rewritten POST->GET on 301 and 302 responses. if method == Method::Post && @@ -769,10 +760,13 @@ pub fn load(load_data: LoadData, method = Method::Get; } - if redirected_to.contains(&url) { + if redirected_to.contains(&new_doc_url) { return Err(LoadError::InvalidRedirect(doc_url, "redirect loop".to_owned())); } + info!("redirecting to {}", new_doc_url); + doc_url = new_doc_url; + redirected_to.insert(doc_url.clone()); continue; }