Redesign CookieStorage and Implement Leave Secure Cookie Alone

CookieStorage has been refactored to use HashMap with base domain as the
key. Values of hashmap is vector of cookies.
CookieStorage now has max_per_host which restricts maximum cookies that
can be added per base domain.
Cookie eviction doesnot 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 exist, remove oldest accessed secure cookie
if new cookie being added is secure. Else ignore new cookie

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
+}

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),
         }

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(),

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");
+}

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;