Skip to content

Commit

Permalink
refactor http_loader hostname/htst order
Browse files Browse the repository at this point in the history 
Changed hostname rewrite to happen inside obtain response after any htst
changes.  Removed  url from load leaving just doc_url to avoid confusion
  • Loading branch information
@bobthekingofegypt
bobthekingofegypt committed Mar 1, 2016
1 parent 759099c commit 06ffdd6
Showing 1 changed file with 27 additions and 33 deletions.
60 changes: 27 additions & 33 deletions components/net/http_loader.rs
View file
Expand Up @@ -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()));
Expand All @@ -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);
}
}
Expand All @@ -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) {
Expand All @@ -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);
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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()
);
}
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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 &&
Expand All @@ -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;
}
Expand Down

0 comments on commit 06ffdd6

Please sign in to comment.