Moves the HSTS replacement code to http_loader

This respects STS for redirects as well.

components/net/hsts.rs

@@ -9,7 +9,6 @@ use url::Url;
 
 use std::str::{from_utf8};
 
-use net_traits::LoadData;
 use util::resource_files::read_resource_file;
 
 static IPV4_REGEX: Regex = regex!(
@@ -64,7 +63,7 @@ impl HSTSEntry {
     }
 }
 
-#[derive(RustcDecodable, RustcEncodable)]
+#[derive(RustcDecodable, RustcEncodable, Clone)]
 pub struct HSTSList {
     pub entries: Vec<HSTSEntry>
 }
@@ -126,20 +125,13 @@ pub fn preload_hsts_domains() -> Option<HSTSList> {
     })
 }
 
-pub fn secure_load_data(load_data: &LoadData) -> LoadData {
-    if &*load_data.url.scheme == "http" {
-        let mut secure_load_data = load_data.clone();
-        let mut secure_url = load_data.url.clone();
+pub fn secure_url(url: &Url) -> Url {
+    if &*url.scheme == "http" {
+        let mut secure_url = url.clone();
         secure_url.scheme = "https".to_string();
-        // The Url struct parses the port for a known scheme only once.
-        // Updating the scheme doesn't update the port internally, resulting in
-        // HTTPS connections attempted on port 80. Serialising and re-parsing
-        // the Url is a hack to get around this.
-        secure_load_data.url = Url::parse(&secure_url.serialize()).unwrap();
-
-        secure_load_data
+        Url::parse(&secure_url.serialize()).unwrap()
     } else {
-        load_data.clone()
+        url.clone()
     }
 }
 

components/net/http_loader.rs

@@ -7,6 +7,7 @@ use net_traits::ProgressMsg::{Payload, Done};
 use devtools_traits::{DevtoolsControlMsg, NetworkEvent};
 use mime_classifier::MIMEClassifier;
 use resource_task::{start_sending_opt, start_sending_sniffed_opt};
+use hsts::{HSTSList, secure_url};
 
 use log;
 use std::collections::HashSet;
@@ -33,11 +34,13 @@ use uuid;
 use std::borrow::ToOwned;
 use std::boxed::FnBox;
 
-pub fn factory(cookies_chan: Sender<ControlMsg>, devtools_chan: Option<Sender<DevtoolsControlMsg>>)
+pub fn factory(cookies_chan: Sender<ControlMsg>,
+               devtools_chan: Option<Sender<DevtoolsControlMsg>>,
+               hsts_list: Option<HSTSList>)
                -> Box<FnBox(LoadData, LoadConsumer, Arc<MIMEClassifier>) + Send> {
     box move |load_data, senders, classifier| {
         spawn_named("http_loader".to_owned(),
-                    move || load(load_data, senders, classifier, cookies_chan, devtools_chan))
+                    move || load(load_data, senders, classifier, cookies_chan, devtools_chan, hsts_list))
     }
 }
 
@@ -69,8 +72,21 @@ fn read_block<R: Read>(reader: &mut R) -> Result<ReadResult, ()> {
     }
 }
 
-fn load(mut load_data: LoadData, start_chan: LoadConsumer, classifier: Arc<MIMEClassifier>,
-        cookies_chan: Sender<ControlMsg>, devtools_chan: Option<Sender<DevtoolsControlMsg>>) {
+fn request_must_be_secured(hsts_list: Option<&HSTSList>, url: &Url) -> bool {
+    match (hsts_list.as_ref(), url.domain()) {
+        (Some(ref l), Some(ref h)) => {
+            l.is_host_secure(h)
+        },
+        _ => false
+    }
+}
+
+fn load(mut load_data: LoadData,
+        start_chan: LoadConsumer,
+        classifier: Arc<MIMEClassifier>,
+        cookies_chan: Sender<ControlMsg>,
+        devtools_chan: Option<Sender<DevtoolsControlMsg>>,
+        hsts_list: Option<HSTSList>) {
     // FIXME: At the time of writing this FIXME, servo didn't have any central
     //        location for configuration. If you're reading this and such a
     //        repository DOES exist, please update this constant to use it.
@@ -101,6 +117,11 @@ fn load(mut load_data: LoadData, start_chan: LoadConsumer, classifier: Arc<MIMEC
     loop {
         iters = iters + 1;
 
+        if request_must_be_secured(hsts_list.as_ref(), &url) {
+            info!("{} is in the strict transport security list, requesting secure host", url);
+            url = secure_url(&url);
+        }
+
         if iters > max_redirects {
             send_error(url, "too many redirects".to_string(), start_chan);
             return;

components/net/resource_task.rs

@@ -23,7 +23,6 @@ use hsts::HSTSList;
 use hsts::HSTSEntry;
 use hsts::Subdomains;
 use hsts::preload_hsts_domains;
-use hsts::secure_load_data;
 
 use devtools_traits::{DevtoolsControlMsg};
 use hyper::header::{ContentType, Header, SetCookie, UserAgent};
@@ -293,25 +292,16 @@ impl ResourceManager {
         }
     }
 
-    pub fn is_host_sts(&self, host: &str) -> bool {
-        match self.hsts_list.as_ref() {
-            Some(list) => list.is_host_secure(host),
-            None => false
-        }
-    }
-
     pub fn add_hsts_entry(&mut self, entry: HSTSEntry) {
         if let Some(list) = self.hsts_list.as_mut() {
             list.push(entry)
         }
     }
 
-    fn load_data_must_be_secured(&self, load_data: &LoadData) -> bool {
-        match (self.hsts_list.as_ref(), load_data.url.domain()) {
-            (Some(ref l), Some(ref h)) => {
-                l.is_host_secure(h)
-            },
-            _ => false
+    pub fn is_host_sts(&self, host: &str) -> bool {
+        match self.hsts_list.as_ref() {
+            Some(list) => list.is_host_secure(host),
+            None => false
         }
     }
 
@@ -326,10 +316,6 @@ impl ResourceManager {
             load_data.preserved_headers.set(UserAgent(ua.clone()));
         });
 
-        if self.load_data_must_be_secured(&load_data) {
-            load_data = secure_load_data(&load_data)
-        }
-
         fn from_factory(factory: fn(LoadData, LoadConsumer, Arc<MIMEClassifier>))
                         -> Box<FnBox(LoadData, LoadConsumer, Arc<MIMEClassifier>) + Send> {
             box move |load_data, senders, classifier| {
@@ -340,7 +326,7 @@ impl ResourceManager {
         let loader = match &*load_data.url.scheme {
             "file" => from_factory(file_loader::factory),
             "http" | "https" | "view-source" =>
-                http_loader::factory(self.resource_task.clone(), self.devtools_chan.clone()),
+                http_loader::factory(self.resource_task.clone(), self.devtools_chan.clone(), self.hsts_list.clone()),
             "data" => from_factory(data_loader::factory),
             "about" => from_factory(about_loader::factory),
             _ => {

tests/unit/net/hsts.rs

@@ -5,7 +5,7 @@
 use net::hsts::HSTSList;
 use net::hsts::HSTSEntry;
 use net::hsts::Subdomains;
-use net::hsts::secure_load_data;
+use net::hsts::secure_url;
 use net::resource_task::ResourceManager;
 use net_traits::LoadData;
 use std::sync::mpsc::channel;
@@ -248,26 +248,26 @@ fn test_hsts_list_with_expired_entry_is_not_is_host_secure() {
 }
 
 #[test]
-fn test_secure_load_data_does_not_change_explicit_port() {
-    let load_data = LoadData::new(Url::parse("http://mozilla.org:8080/").unwrap(), None);
-    let secure = secure_load_data(&load_data);
+fn test_secure_url_does_not_change_explicit_port() {
+    let url = Url::parse("http://mozilla.org:8080/").unwrap();
+    let secure = secure_url(&url);
 
-    assert!(secure.url.port().unwrap() == 8080u16);
+    assert!(secure.port().unwrap() == 8080u16);
 }
 
 #[test]
-fn test_secure_load_data_does_not_affect_non_http_schemas() {
-    let load_data = LoadData::new(Url::parse("file://mozilla.org").unwrap(), None);
-    let secure = secure_load_data(&load_data);
+fn test_secure_url_does_not_affect_non_http_schemas() {
+    let url = Url::parse("file://mozilla.org").unwrap();
+    let secure = secure_url(&url);
 
-    assert_eq!(&secure.url.scheme, "file");
+    assert_eq!(&secure.scheme, "file");
 }
 
 #[test]
-fn test_secure_load_data_forces_an_http_host_in_list_to_https() {
-    let load_data = LoadData::new(Url::parse("http://mozilla.org").unwrap(), None);
-    let secure = secure_load_data(&load_data);
+fn test_secure_url_forces_an_http_host_in_list_to_https() {
+    let url = Url::parse("http://mozilla.org").unwrap();
+    let secure = secure_url(&url);
 
-    assert_eq!(&secure.url.scheme, "https");
+    assert_eq!(&secure.scheme, "https");
 }