Auto merge of #14445 - mrnayak:netSecurity, r=jdm

Redesign CookieStorage and Implement Leave Secure Cookie Alone

CookieStorage has been refactored to use HashMap with the base domain as the key. Values of hashmap are vector of cookies.
CookieStorage now has max_per_host which restricts maximum cookies that can be added per base domain.
Cookie eviction does not take place if max_per_host is not reached.
Cookie eviction logic implemented here does following steps
1) Evict all expired cookies
2) Remove oldest accessed non-secure cookie If any
3) When no non-secure cookie exists, remove oldest accessed secure cookie if new cookie being added is secure. Else ignore new cookie

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14445)
<!-- Reviewable:end -->

5 files changed, 195 insertions, 21 deletions

diff --git a/components/net/cookie_storage.rs b/components/net/cookie_storage.rs
index 71eed2ec7ad..21c47e1f17b 100644
--- a/components/net/cookie_storage.rs
+++ b/components/net/cookie_storage.rs
@@ -8,41 +8,51 @@
 use cookie::Cookie;
 use cookie_rs;
 use net_traits::CookieSource;
+use net_traits::pub_domains::reg_suffix;
 use servo_url::ServoUrl;
 use std::cmp::Ordering;
+use std::collections::HashMap;
+use time::Tm;
+
+extern crate time;
 
 #[derive(Clone, Debug, RustcDecodable, RustcEncodable)]
 pub struct CookieStorage {
     version: u32,
-    cookies: Vec<Cookie>
+    cookies_map: HashMap<String, Vec<Cookie>>,
+    max_per_host: usize,
 }
 
 impl CookieStorage {
-    pub fn new() -> CookieStorage {
+    pub fn new(max_cookies: usize) -> CookieStorage {
         CookieStorage {
             version: 1,
-            cookies: Vec::new()
+            cookies_map: HashMap::new(),
+            max_per_host: max_cookies,
         }
     }
 
     // http://tools.ietf.org/html/rfc6265#section-5.3
     pub fn remove(&mut self, cookie: &Cookie, source: CookieSource) -> Result<Option<Cookie>, ()> {
+        let domain = reg_host(cookie.cookie.domain.as_ref().unwrap_or(&"".to_string()));
+        let cookies = self.cookies_map.entry(domain).or_insert(vec![]);
+
         // Step 1
-        let position = self.cookies.iter().position(|c| {
+        let position = cookies.iter().position(|c| {
             c.cookie.domain == cookie.cookie.domain &&
             c.cookie.path == cookie.cookie.path &&
             c.cookie.name == cookie.cookie.name
         });
 
         if let Some(ind) = position {
-            let c = self.cookies.remove(ind);
+            let c = cookies.remove(ind);
 
             // http://tools.ietf.org/html/rfc6265#section-5.3 step 11.2
             if !c.cookie.httponly || source == CookieSource::HTTP {
                 Ok(Some(c))
             } else {
                 // Undo the removal.
-                self.cookies.push(c);
+                cookies.push(c);
                 Err(())
             }
         } else {
@@ -65,7 +75,20 @@ impl CookieStorage {
         }
 
         // Step 12
-        self.cookies.push(cookie);
+        let domain = reg_host(&cookie.cookie.domain.as_ref().unwrap_or(&"".to_string()));
+        let mut cookies = self.cookies_map.entry(domain).or_insert(vec![]);
+
+        if cookies.len() == self.max_per_host {
+            let old_len = cookies.len();
+            cookies.retain(|c| !is_cookie_expired(&c));
+            let new_len = cookies.len();
+
+            // https://datatracker.ietf.org/doc/draft-ietf-httpbis-cookie-alone
+            if new_len == old_len && !evict_one_cookie(cookie.cookie.secure, cookies) {
+                return;
+            }
+        }
+        cookies.push(cookie);
     }
 
     pub fn cookie_comparator(a: &Cookie, b: &Cookie) -> Ordering {
@@ -87,14 +110,21 @@ impl CookieStorage {
     pub fn cookies_for_url(&mut self, url: &ServoUrl, source: CookieSource) -> Option<String> {
         let filterer = |c: &&mut Cookie| -> bool {
             info!(" === SENT COOKIE : {} {} {:?} {:?}",
-                  c.cookie.name, c.cookie.value, c.cookie.domain, c.cookie.path);
-            info!(" === SENT COOKIE RESULT {}", c.appropriate_for_url(url, source));
+                  c.cookie.name,
+                  c.cookie.value,
+                  c.cookie.domain,
+                  c.cookie.path);
+            info!(" === SENT COOKIE RESULT {}",
+                  c.appropriate_for_url(url, source));
             // Step 1
             c.appropriate_for_url(url, source)
         };
 
         // Step 2
-        let mut url_cookies: Vec<&mut Cookie> = self.cookies.iter_mut().filter(filterer).collect();
+        let domain = reg_host(url.host_str().unwrap_or(""));
+        let cookies = self.cookies_map.entry(domain).or_insert(vec![]);
+
+        let mut url_cookies: Vec<&mut Cookie> = cookies.iter_mut().filter(filterer).collect();
         url_cookies.sort_by(|a, b| CookieStorage::cookie_comparator(*a, *b));
 
         let reducer = |acc: String, c: &mut &mut Cookie| -> String {
@@ -104,7 +134,7 @@ impl CookieStorage {
             // Step 4
             (match acc.len() {
                 0 => acc,
-                _ => acc + "; "
+                _ => acc + "; ",
             }) + &c.cookie.name + "=" + &c.cookie.value
         };
         let result = url_cookies.iter_mut().fold("".to_owned(), reducer);
@@ -112,15 +142,60 @@ impl CookieStorage {
         info!(" === COOKIES SENT: {}", result);
         match result.len() {
             0 => None,
-            _ => Some(result)
+            _ => Some(result),
         }
     }
 
-    pub fn cookies_data_for_url<'a>(&'a mut self, url: &'a ServoUrl,
-                                    source: CookieSource) -> Box<Iterator<Item=cookie_rs::Cookie> + 'a> {
-        Box::new(self.cookies.iter_mut().filter(move |c| { c.appropriate_for_url(url, source) }).map(|c| {
+    pub fn cookies_data_for_url<'a>(&'a mut self,
+                                    url: &'a ServoUrl,
+                                    source: CookieSource)
+                                    -> Box<Iterator<Item = cookie_rs::Cookie> + 'a> {
+        let domain = reg_host(url.host_str().unwrap_or(""));
+        let cookies = self.cookies_map.entry(domain).or_insert(vec![]);
+
+        Box::new(cookies.iter_mut().filter(move |c| c.appropriate_for_url(url, source)).map(|c| {
             c.touch();
             c.cookie.clone()
         }))
     }
 }
+fn reg_host<'a>(url: &'a str) -> String {
+    reg_suffix(url).to_string()
+}
+
+fn is_cookie_expired(cookie: &Cookie) -> bool {
+    match cookie.expiry_time {
+        Some(t) => t.to_timespec() <= time::get_time(),
+        None => false,
+    }
+}
+
+fn evict_one_cookie(is_secure_cookie: bool, cookies: &mut Vec<Cookie>) -> bool {
+    // Remove non-secure cookie with oldest access time
+    let oldest_accessed: Option<(usize, Tm)> = get_oldest_accessed(false, cookies);
+
+    if let Some((index, _)) = oldest_accessed {
+        cookies.remove(index);
+    } else {
+        // All secure cookies were found
+        if !is_secure_cookie {
+            return false;
+        }
+        let oldest_accessed: Option<(usize, Tm)> = get_oldest_accessed(true, cookies);
+        if let Some((index, _)) = oldest_accessed {
+            cookies.remove(index);
+        }
+    }
+    return true;
+}
+
+fn get_oldest_accessed(is_secure_cookie: bool, cookies: &mut Vec<Cookie>) -> Option<(usize, Tm)> {
+    let mut oldest_accessed: Option<(usize, Tm)> = None;
+    for (i, c) in cookies.iter().enumerate() {
+        if (c.cookie.secure == is_secure_cookie) &&
+           oldest_accessed.as_ref().map_or(true, |a| c.last_access < a.1) {
+            oldest_accessed = Some((i, c.last_access));
+        }
+    }
+    oldest_accessed
+}
diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs
index 7becd00637f..88d37877f54 100644
--- a/components/net/http_loader.rs
+++ b/components/net/http_loader.rs
@@ -79,7 +79,7 @@ impl HttpState {
     pub fn new() -> HttpState {
         HttpState {
             hsts_list: Arc::new(RwLock::new(HstsList::new())),
-            cookie_jar: Arc::new(RwLock::new(CookieStorage::new())),
+            cookie_jar: Arc::new(RwLock::new(CookieStorage::new(150))),
             auth_cache: Arc::new(RwLock::new(AuthCache::new())),
             blocked_content: Arc::new(None),
         }
diff --git a/components/net/resource_thread.rs b/components/net/resource_thread.rs
index 24feb34dd9c..0a4cc89d806 100644
--- a/components/net/resource_thread.rs
+++ b/components/net/resource_thread.rs
@@ -185,7 +185,7 @@ fn create_resource_groups(config_dir: Option<&Path>)
                           -> (ResourceGroup, ResourceGroup) {
     let mut hsts_list = HstsList::from_servo_preload();
     let mut auth_cache = AuthCache::new();
-    let mut cookie_jar = CookieStorage::new();
+    let mut cookie_jar = CookieStorage::new(150);
     if let Some(config_dir) = config_dir {
         read_json_from_file(&mut auth_cache, config_dir, "auth_cache.json");
         read_json_from_file(&mut hsts_list, config_dir, "hsts_list.json");
@@ -198,7 +198,7 @@ fn create_resource_groups(config_dir: Option<&Path>)
         connector: create_http_connector(),
     };
     let private_resource_group = ResourceGroup {
-        cookie_jar: Arc::new(RwLock::new(CookieStorage::new())),
+        cookie_jar: Arc::new(RwLock::new(CookieStorage::new(150))),
         auth_cache: Arc::new(RwLock::new(AuthCache::new())),
         hsts_list: Arc::new(RwLock::new(HstsList::new())),
         connector: create_http_connector(),
diff --git a/tests/unit/net/cookie.rs b/tests/unit/net/cookie.rs
index 2a8e4d19b1e..aa6519571b8 100644
--- a/tests/unit/net/cookie.rs
+++ b/tests/unit/net/cookie.rs
@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use cookie_rs;
+use hyper::header::{Header, SetCookie};
 use net::cookie::Cookie;
 use net::cookie_storage::CookieStorage;
 use net_traits::CookieSource;
@@ -110,8 +111,7 @@ fn delay_to_ensure_different_timestamp() {
 }
 
 #[cfg(not(target_os = "windows"))]
-fn delay_to_ensure_different_timestamp() {
-}
+fn delay_to_ensure_different_timestamp() {}
 
 #[test]
 fn test_sort_order() {
@@ -132,3 +132,102 @@ fn test_sort_order() {
     assert!(CookieStorage::cookie_comparator(&a_prime, &a) == Ordering::Greater);
     assert!(CookieStorage::cookie_comparator(&a, &a) == Ordering::Equal);
 }
+
+
+fn add_retrieve_cookies(set_location: &str,
+                        set_cookies: &[String],
+                        final_location: &str)
+                        -> String {
+    let mut storage = CookieStorage::new(5);
+    let url = ServoUrl::parse(set_location).unwrap();
+    let source = CookieSource::HTTP;
+
+    // Add all cookies to the store
+    for str_cookie in set_cookies {
+        let bytes = str_cookie.to_string().into_bytes();
+        let header = Header::parse_header(&[bytes]).unwrap();
+        let SetCookie(cookies) = header;
+        for bare_cookie in cookies {
+            let cookie = Cookie::new_wrapped(bare_cookie, &url, source).unwrap();
+            storage.push(cookie, source);
+        }
+    }
+
+    // Get cookies for the test location
+    let url = ServoUrl::parse(final_location).unwrap();
+    storage.cookies_for_url(&url, source).unwrap_or("".to_string())
+}
+
+
+#[test]
+fn test_cookie_eviction_expired() {
+    let mut vec = Vec::new();
+    for i in 1..6 {
+        let st = format!("extra{}=bar; Secure; expires=Sun, 18-Apr-2000 21:06:29 GMT",
+                         i);
+        vec.push(st);
+    }
+    vec.push("foo=bar; Secure; expires=Sun, 18-Apr-2027 21:06:29 GMT".to_owned());
+    let r = add_retrieve_cookies("https://home.example.org:8888/cookie-parser?0001",
+                                 &vec, "https://home.example.org:8888/cookie-parser-result?0001");
+    assert_eq!(&r, "foo=bar");
+}
+
+
+#[test]
+fn test_cookie_eviction_all_secure_one_nonsecure() {
+    let mut vec = Vec::new();
+    for i in 1..5 {
+        let st = format!("extra{}=bar; Secure; expires=Sun, 18-Apr-2026 21:06:29 GMT",
+                         i);
+        vec.push(st);
+    }
+    vec.push("foo=bar; expires=Sun, 18-Apr-2026 21:06:29 GMT".to_owned());
+    vec.push("foo2=bar; Secure; expires=Sun, 18-Apr-2028 21:06:29 GMT".to_owned());
+    let r = add_retrieve_cookies("https://home.example.org:8888/cookie-parser?0001",
+                                 &vec, "https://home.example.org:8888/cookie-parser-result?0001");
+    assert_eq!(&r, "extra1=bar; extra2=bar; extra3=bar; extra4=bar; foo2=bar");
+}
+
+
+#[test]
+fn test_cookie_eviction_all_secure_new_nonsecure() {
+    let mut vec = Vec::new();
+    for i in 1..6 {
+        let st = format!("extra{}=bar; Secure; expires=Sun, 18-Apr-2026 21:06:29 GMT",
+                         i);
+        vec.push(st);
+    }
+    vec.push("foo=bar; expires=Sun, 18-Apr-2077 21:06:29 GMT".to_owned());
+    let r = add_retrieve_cookies("https://home.example.org:8888/cookie-parser?0001",
+                                 &vec, "https://home.example.org:8888/cookie-parser-result?0001");
+    assert_eq!(&r, "extra1=bar; extra2=bar; extra3=bar; extra4=bar; extra5=bar");
+}
+
+
+#[test]
+fn test_cookie_eviction_all_nonsecure_new_secure() {
+    let mut vec = Vec::new();
+    for i in 1..6 {
+        let st = format!("extra{}=bar; expires=Sun, 18-Apr-2026 21:06:29 GMT", i);
+        vec.push(st);
+    }
+    vec.push("foo=bar; Secure; expires=Sun, 18-Apr-2077 21:06:29 GMT".to_owned());
+    let r = add_retrieve_cookies("https://home.example.org:8888/cookie-parser?0001",
+                                 &vec, "https://home.example.org:8888/cookie-parser-result?0001");
+    assert_eq!(&r, "extra2=bar; extra3=bar; extra4=bar; extra5=bar; foo=bar");
+}
+
+
+#[test]
+fn test_cookie_eviction_all_nonsecure_new_nonsecure() {
+    let mut vec = Vec::new();
+    for i in 1..6 {
+        let st = format!("extra{}=bar; expires=Sun, 18-Apr-2026 21:06:29 GMT", i);
+        vec.push(st);
+    }
+    vec.push("foo=bar; expires=Sun, 18-Apr-2077 21:06:29 GMT".to_owned());
+    let r = add_retrieve_cookies("https://home.example.org:8888/cookie-parser?0001",
+                                 &vec, "https://home.example.org:8888/cookie-parser-result?0001");
+    assert_eq!(&r, "extra2=bar; extra3=bar; extra4=bar; extra5=bar; foo=bar");
+}
diff --git a/tests/unit/net/cookie_http_state.rs b/tests/unit/net/cookie_http_state.rs
index 4f40157dd8e..819aaabb5f2 100644
--- a/tests/unit/net/cookie_http_state.rs
+++ b/tests/unit/net/cookie_http_state.rs
@@ -10,7 +10,7 @@ use servo_url::ServoUrl;
 
 
 fn run(set_location: &str, set_cookies: &[&str], final_location: &str) -> String {
-    let mut storage = CookieStorage::new();
+    let mut storage = CookieStorage::new(150);
     let url = ServoUrl::parse(set_location).unwrap();
     let source = CookieSource::HTTP;