Redesign CookieStorage and Implement Leave Secure Cookie Alone

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,23 +134,68 @@ 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);
 
         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;