Responds to more code review feedback

* Use regex from resource task
* Don't have an option of an HSTS list, default to empty

components/net/hsts.rs

@@ -2,20 +2,15 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use regex::Regex;
 use rustc_serialize::json::{decode};
 use time;
 use url::Url;
+use resource_task::{IPV4_REGEX, IPV6_REGEX};
 
 use std::str::{from_utf8};
 
 use util::resource_files::read_resource_file;
 
-static IPV4_REGEX: Regex = regex!(
-    r"^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$"
-);
-static IPV6_REGEX: Regex = regex!(r"^([a-fA-F0-9]{0,4}[:]?){1,8}(/\d{1,3})?$");
-
 #[derive(RustcDecodable, RustcEncodable, Clone)]
 pub struct HSTSEntry {
     pub host: String,
@@ -69,6 +64,12 @@ pub struct HSTSList {
 }
 
 impl HSTSList {
+    pub fn new() -> HSTSList {
+        HSTSList {
+            entries: vec![]
+        }
+    }
+
     pub fn new_from_preload(preload_content: &str) -> Option<HSTSList> {
         decode(preload_content).ok()
     }

components/net/http_loader.rs

@@ -36,7 +36,7 @@ use std::boxed::FnBox;
 
 pub fn factory(cookies_chan: Sender<ControlMsg>,
                devtools_chan: Option<Sender<DevtoolsControlMsg>>,
-               hsts_list: Option<HSTSList>)
+               hsts_list: HSTSList)
                -> Box<FnBox(LoadData, LoadConsumer, Arc<MIMEClassifier>) + Send> {
     box move |load_data, senders, classifier| {
         spawn_named("http_loader".to_owned(),
@@ -72,10 +72,10 @@ fn read_block<R: Read>(reader: &mut R) -> Result<ReadResult, ()> {
     }
 }
 
-fn request_must_be_secured(hsts_list: Option<&HSTSList>, url: &Url) -> bool {
+fn request_must_be_secured(hsts_list: &HSTSList, url: &Url) -> bool {
-    match (hsts_list.as_ref(), url.domain()) {
+    match url.domain() {
-        (Some(ref l), Some(ref h)) => {
+        Some(ref h) => {
-            l.is_host_secure(h)
+            hsts_list.is_host_secure(h)
         },
         _ => false
     }
@@ -86,7 +86,7 @@ fn load(mut load_data: LoadData,
         classifier: Arc<MIMEClassifier>,
         cookies_chan: Sender<ControlMsg>,
         devtools_chan: Option<Sender<DevtoolsControlMsg>>,
-        hsts_list: Option<HSTSList>) {
+        hsts_list: 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.
@@ -117,7 +117,7 @@ fn load(mut load_data: LoadData,
     loop {
         iters = iters + 1;
 
-        if request_must_be_secured(hsts_list.as_ref(), &url) {
+        if request_must_be_secured(&hsts_list, &url) {
             info!("{} is in the strict transport security list, requesting secure host", url);
             url = secure_url(&url);
         }

components/net/resource_task.rs

@@ -19,10 +19,7 @@ use util::opts;
 use util::task::spawn_named;
 use url::Url;
 
-use hsts::HSTSList;
+use hsts::{HSTSList, HSTSEntry, Subdomains, preload_hsts_domains};
-use hsts::HSTSEntry;
-use hsts::Subdomains;
-use hsts::preload_hsts_domains;
 
 use devtools_traits::{DevtoolsControlMsg};
 use hyper::header::{ContentType, Header, SetCookie, UserAgent};
@@ -39,10 +36,10 @@ use std::sync::Arc;
 use std::sync::mpsc::{channel, Receiver, Sender};
 
 static mut HOST_TABLE: Option<*mut HashMap<String, String>> = None;
-static IPV4_REGEX: Regex = regex!(
+pub static IPV4_REGEX: Regex = regex!(
     r"^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$"
 );
-static IPV6_REGEX: Regex = regex!(r"^([a-fA-F0-9]{0,4}[:]?){1,8}(/\d{1,3})?$");
+pub static IPV6_REGEX: Regex = regex!(r"^([a-fA-F0-9]{0,4}[:]?){1,8}(/\d{1,3})?$");
 
 pub fn global_init() {
     //TODO: handle bad file path
@@ -165,7 +162,10 @@ pub fn start_sending_opt(start_chan: LoadConsumer, metadata: Metadata) -> Result
 /// Create a ResourceTask
 pub fn new_resource_task(user_agent: Option<String>,
                          devtools_chan: Option<Sender<DevtoolsControlMsg>>) -> ResourceTask {
-    let hsts_preload = preload_hsts_domains();
+    let hsts_preload = match preload_hsts_domains() {
+        Some(list) => list,
+        None => HSTSList::new()
+    };
 
     let (setup_chan, setup_port) = channel();
     let setup_chan_clone = setup_chan.clone();
@@ -260,13 +260,13 @@ pub struct ResourceManager {
     resource_task: Sender<ControlMsg>,
     mime_classifier: Arc<MIMEClassifier>,
     devtools_chan: Option<Sender<DevtoolsControlMsg>>,
-    hsts_list: Option<HSTSList>
+    hsts_list: HSTSList
 }
 
 impl ResourceManager {
     pub fn new(user_agent: Option<String>,
            resource_task: Sender<ControlMsg>,
-           hsts_list: Option<HSTSList>,
+           hsts_list: HSTSList,
            devtools_channel: Option<Sender<DevtoolsControlMsg>>) -> ResourceManager {
         ResourceManager {
             user_agent: user_agent,
@@ -293,16 +293,11 @@ impl ResourceManager {
     }
 
     pub fn add_hsts_entry(&mut self, entry: HSTSEntry) {
-        if let Some(list) = self.hsts_list.as_mut() {
+        self.hsts_list.push(entry);
-            list.push(entry)
-        }
     }
 
     pub fn is_host_sts(&self, host: &str) -> bool {
-        match self.hsts_list.as_ref() {
+        self.hsts_list.is_host_secure(host)
-            Some(list) => list.is_host_secure(host),
-            None => false
-        }
     }
 
     fn load(&mut self, mut load_data: LoadData, consumer: LoadConsumer) {

tests/unit/net/hsts.rs

@@ -7,7 +7,6 @@ use net::hsts::HSTSEntry;
 use net::hsts::Subdomains;
 use net::hsts::secure_url;
 use net::resource_task::ResourceManager;
-use net_traits::LoadData;
 use std::sync::mpsc::channel;
 use url::Url;
 use time;
@@ -19,7 +18,7 @@ fn test_add_hsts_entry_to_resource_manager_adds_an_hsts_entry() {
     };
 
     let (tx, _) = channel();
-    let mut manager = ResourceManager::new(None, tx, Some(list), None);
+    let mut manager = ResourceManager::new(None, tx, list, None);
 
     let entry = HSTSEntry::new(
         "mozilla.org".to_string(), Subdomains::NotIncluded, None
@@ -134,10 +133,6 @@ fn test_push_entry_to_hsts_list_should_not_create_duplicate_entry() {
 
 #[test]
 fn test_push_multiple_entrie_to_hsts_list_should_add_them_all() {
-}
-
-#[test]
-fn test_push_entry_to_hsts_list_should_add_an_entry() {
     let mut list = HSTSList {
         entries: Vec::new()
     };
@@ -152,6 +147,19 @@ fn test_push_entry_to_hsts_list_should_add_an_entry() {
     assert!(list.is_host_secure("bugzilla.org"));
 }
 
+#[test]
+fn test_push_entry_to_hsts_list_should_add_an_entry() {
+    let mut list = HSTSList {
+        entries: Vec::new()
+    };
+
+    assert!(!list.is_host_secure("mozilla.org"));
+
+    list.push(HSTSEntry::new("mozilla.org".to_string(), Subdomains::Included, None).unwrap());
+
+    assert!(list.is_host_secure("mozilla.org"));
+}
+
 #[test]
 fn test_parse_hsts_preload_should_return_none_when_json_invalid() {
     let mock_preload_content = "derp";