From c1518adba86a3124c950fdc2399865d73afea8c4 Mon Sep 17 00:00:00 2001 From: Raghav Date: Sun, 4 Dec 2016 16:29:38 -0500 Subject: [PATCH] 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 | 105 ++++++++++++++++++++++++---- components/net/http_loader.rs | 2 +- components/net/resource_thread.rs | 4 +- tests/unit/net/cookie.rs | 103 ++++++++++++++++++++++++++- tests/unit/net/cookie_http_state.rs | 2 +- 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 + cookies_map: HashMap>, + 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, ()> { + 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 { 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 + '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 + '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) -> 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) -> 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;