diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index 1f044885abf..e0227a0637f 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -513,14 +513,14 @@ fn request_must_be_secured(url: &Url, hsts_list: &Arc>) -> bool } pub fn modify_request_headers(headers: &mut Headers, - doc_url: &Url, + 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); @@ -538,11 +538,11 @@ pub fn modify_request_headers(headers: &mut Headers, set_default_accept_encoding(headers); // https://fetch.spec.whatwg.org/#concept-http-network-or-cache-fetch step 11 if load_data.credentials_flag { - set_request_cookies(doc_url.clone(), headers, cookie_jar); + set_request_cookies(url.clone(), headers, cookie_jar); // 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); } } @@ -563,7 +563,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) { @@ -576,7 +575,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); } @@ -595,17 +594,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) { @@ -677,48 +677,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 @@ -736,11 +732,11 @@ pub fn load(load_data: LoadData, 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 { @@ -750,7 +746,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, @@ -765,10 +761,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 && @@ -777,10 +769,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; } diff --git a/tests/unit/net/http_loader.rs b/tests/unit/net/http_loader.rs index 70d4944b7ae..3f0aa134862 100644 --- a/tests/unit/net/http_loader.rs +++ b/tests/unit/net/http_loader.rs @@ -18,10 +18,10 @@ use hyper::status::StatusCode; use msg::constellation_msg::PipelineId; use net::cookie::Cookie; use net::cookie_storage::CookieStorage; -use net::hsts::{HSTSList}; +use net::hsts::{HSTSList, HSTSEntry}; use net::http_loader::{load, LoadError, HttpRequestFactory, HttpRequest, HttpResponse}; use net::resource_thread::CancellationListener; -use net_traits::{LoadData, CookieSource, LoadContext}; +use net_traits::{LoadData, CookieSource, LoadContext, IncludeSubdomains}; use std::borrow::Cow; use std::io::{self, Write, Read, Cursor}; use std::sync::mpsc::Receiver; @@ -813,6 +813,50 @@ fn test_load_sets_requests_cookies_header_for_url_by_getting_cookies_from_the_re &CancellationListener::new(None)); } +#[test] +fn test_load_sends_secure_cookie_if_http_changed_to_https_due_to_entry_in_hsts_store() { + let url = url!("http://mozilla.com"); + let secured_url = url!("https://mozilla.com"); + + let hsts_list = Arc::new(RwLock::new(HSTSList::new())); + let cookie_jar = Arc::new(RwLock::new(CookieStorage::new())); + + { + let mut hsts_list = hsts_list.write().unwrap(); + let entry = HSTSEntry::new( + "mozilla.com".to_owned(), IncludeSubdomains::Included, Some(1000000) + ).unwrap(); + hsts_list.push(entry); + } + + { + let mut cookie_jar = cookie_jar.write().unwrap(); + let cookie_url = secured_url.clone(); + let mut cookie_pair = CookiePair::new("mozillaIs".to_owned(), "theBest".to_owned()); + cookie_pair.secure = true; + + let cookie = Cookie::new_wrapped( + cookie_pair, + &cookie_url, + CookieSource::HTTP + ).unwrap(); + cookie_jar.push(cookie, CookieSource::HTTP); + } + + let mut load_data = LoadData::new(LoadContext::Browsing, url, None); + load_data.data = Some(<[_]>::to_vec("Yay!".as_bytes())); + + let mut headers = Headers::new(); + headers.set_raw("Cookie".to_owned(), vec![<[_]>::to_vec("mozillaIs=theBest".as_bytes())]); + + let _ = load::( + load_data.clone(), hsts_list, cookie_jar, None, + &AssertMustIncludeHeadersRequestFactory { + expected_headers: headers, + body: <[_]>::to_vec(&*load_data.data.unwrap()) + }, DEFAULT_USER_AGENT.to_owned(), &CancellationListener::new(None)); +} + #[test] fn test_load_sends_cookie_if_nonhttp() { let url = url!("http://mozilla.com");