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
This commit is contained in:
Bob 2016-03-01 18:10:58 +00:00
parent 759099c78d
commit 06ffdd68e8

View file

@ -514,14 +514,13 @@ fn request_must_be_secured(url: &Url, hsts_list: &Arc<RwLock<HSTSList>>) -> bool
pub fn modify_request_headers(headers: &mut Headers,
url: &Url,
doc_url: &Url,
user_agent: &str,
cookie_jar: &Arc<RwLock<CookieStorage>>,
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::<Authorization<Basic>>() {
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<Authorization<Basic>> {
pub fn process_response_headers(response: &HttpResponse,
url: &Url,
doc_url: &Url,
cookie_jar: &Arc<RwLock<CookieStorage>>,
hsts_list: &Arc<RwLock<HSTSList>>,
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<A>(request_factory: &HttpRequestFactory<R=A>,
-> Result<A::R, LoadError> 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<A>(request_factory: &HttpRequestFactory<R=A>,
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<A>(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<A>(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<A>(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<A>(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<A>(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;
}