diff --git a/components/net/cookie.rs b/components/net/cookie.rs index cc696172bc7..73a53efbd4f 100644 --- a/components/net/cookie.rs +++ b/components/net/cookie.rs @@ -5,6 +5,7 @@ //! Implementation of cookie creation and matching as specified by //! http://tools.ietf.org/html/rfc6265 +use cookie_storage::CookieSource; use pub_domains::PUB_DOMAINS; use cookie_rs; @@ -23,15 +24,15 @@ pub struct Cookie { pub cookie: cookie_rs::Cookie, pub host_only: bool, pub persistent: bool, - pub created_at: Tm, + pub creation_time: Tm, pub last_access: Tm, - pub scheme: String, pub expiry_time: Tm, } impl Cookie { /// http://tools.ietf.org/html/rfc6265#section-5.3 - pub fn new_wrapped(mut cookie: cookie_rs::Cookie, request: &Url) -> Option { + pub fn new_wrapped(mut cookie: cookie_rs::Cookie, request: &Url, source: CookieSource) + -> Option { // Step 3 let (persistent, expiry_time) = match (&cookie.max_age, &cookie.expires) { (&Some(max_age), _) => (true, at(now().to_timespec() + Duration::seconds(max_age as i64))), @@ -67,19 +68,15 @@ impl Cookie { // Step 7 let mut path = cookie.path.unwrap_or("".to_owned()); if path.is_empty() || path.char_at(0) != '/' { - let mut url_path: String = "".to_owned(); - if let Some(paths) = request.path() { - for path in paths.iter() { - url_path.extend(path.chars()); - } - } - path = Cookie::default_path(url_path.as_slice()); + let url_path = request.serialize_path(); + let url_path = url_path.as_ref().map(|path| path.as_slice()); + path = Cookie::default_path(url_path.unwrap_or("")); } cookie.path = Some(path); // Step 10 - if cookie.httponly && !request.scheme.as_slice().starts_with("http") { + if cookie.httponly && source != CookieSource::HTTP { return None; } @@ -87,9 +84,8 @@ impl Cookie { cookie: cookie, host_only: host_only, persistent: persistent, - created_at: now(), + creation_time: now(), last_access: now(), - scheme: request.scheme.clone(), expiry_time: expiry_time, }) } @@ -100,13 +96,14 @@ impl Cookie { // http://tools.ietf.org/html/rfc6265#section-5.1.4 fn default_path(request_path: &str) -> String { - if request_path == "" || request_path.char_at(0) != '/' || request_path == "/" { - return "/".to_owned(); + if request_path == "" || request_path.char_at(0) != '/' || + request_path.chars().filter(|&c| c == '/').count() == 1 { + "/".to_owned() + } else if request_path.ends_with("/") { + request_path.slice_to(request_path.len()-1).to_owned() + } else { + request_path.to_owned() } - if request_path.ends_with("/") { - return request_path.slice_to(request_path.len()-1).to_string(); - } - return request_path.to_owned(); } // http://tools.ietf.org/html/rfc6265#section-5.1.4 @@ -131,7 +128,7 @@ impl Cookie { } // http://tools.ietf.org/html/rfc6265#section-5.4 step 1 - pub fn appropriate_for_url(&self, url: Url) -> bool { + pub fn appropriate_for_url(&self, url: &Url, source: CookieSource) -> bool { let domain = url.host().map(|host| host.serialize()); if self.host_only { if self.cookie.domain != domain { @@ -154,7 +151,7 @@ impl Cookie { if self.cookie.secure && url.scheme != "https".to_string() { return false; } - if self.cookie.httponly && !url.scheme.as_slice().starts_with("http") { + if self.cookie.httponly && source == CookieSource::NonHTTP { return false; } @@ -171,77 +168,67 @@ fn test_domain_match() { assert!(!Cookie::domain_match("bar.foo.com", "bar.com")); assert!(!Cookie::domain_match("bar.com", "baz.bar.com")); assert!(!Cookie::domain_match("foo.com", "bar.com")); + + assert!(!Cookie::domain_match("bar.com", "bbar.com")); + assert!(Cookie::domain_match("235.132.2.3", "235.132.2.3")); + assert!(!Cookie::domain_match("235.132.2.3", "1.1.1.1")); + assert!(!Cookie::domain_match("235.132.2.3", ".2.3")); } #[test] fn test_default_path() { assert!(Cookie::default_path("/foo/bar/baz/").as_slice() == "/foo/bar/baz"); - assert!(Cookie::default_path("/foo").as_slice() == "/foo"); + assert!(Cookie::default_path("/foo/").as_slice() == "/foo"); + assert!(Cookie::default_path("/foo").as_slice() == "/"); assert!(Cookie::default_path("/").as_slice() == "/"); assert!(Cookie::default_path("").as_slice() == "/"); + assert!(Cookie::default_path("foo").as_slice() == "/"); } #[test] fn fn_cookie_constructor() { + use cookie_storage::CookieSource; + let url = &Url::parse("http://example.com/foo").unwrap(); let gov_url = &Url::parse("http://gov.ac/foo").unwrap(); // cookie name/value test - assert!(Cookie::new(" baz ".to_string(), url).is_none()); - assert!(Cookie::new(" = bar ".to_string(), url).is_none()); - assert!(Cookie::new(" baz = ".to_string(), url).is_some()); + assert!(cookie_rs::Cookie::parse(" baz ").is_err()); + assert!(cookie_rs::Cookie::parse(" = bar ").is_err()); + assert!(cookie_rs::Cookie::parse(" baz = ").is_ok()); // cookie domains test - assert!(Cookie::new(" baz = bar; Domain = ".to_string(), url).is_some()); - assert!(Cookie::new(" baz = bar; Domain = ".to_string(), url).unwrap().domain.as_slice() == "example.com"); + let cookie = cookie_rs::Cookie::parse(" baz = bar; Domain = ").unwrap(); + assert!(Cookie::new_wrapped(cookie, url, CookieSource::HTTP).is_some()); + let cookie = cookie_rs::Cookie::parse(" baz = bar; Domain = ").unwrap(); + let cookie = Cookie::new_wrapped(cookie, url, CookieSource::HTTP).unwrap(); + assert!(cookie.cookie.domain.as_ref().unwrap().as_slice() == "example.com"); // cookie public domains test - assert!(Cookie::new(" baz = bar; Domain = gov.ac".to_string(), url).is_none()); - assert!(Cookie::new(" baz = bar; Domain = gov.ac".to_string(), gov_url).is_some()); + let cookie = cookie_rs::Cookie::parse(" baz = bar; Domain = gov.ac").unwrap(); + assert!(Cookie::new_wrapped(cookie.clone(), url, CookieSource::HTTP).is_none()); + assert!(Cookie::new_wrapped(cookie, gov_url, CookieSource::HTTP).is_some()); // cookie domain matching test - assert!(Cookie::new(" baz = bar ; Secure; Domain = bazample.com".to_string(), url).is_none()); + let cookie = cookie_rs::Cookie::parse(" baz = bar ; Secure; Domain = bazample.com").unwrap(); + assert!(Cookie::new_wrapped(cookie, url, CookieSource::HTTP).is_none()); - assert!(Cookie::new(" baz = bar ; Secure; Path = /foo/bar/".to_string(), url).is_some()); + let cookie = cookie_rs::Cookie::parse(" baz = bar ; Secure; Path = /foo/bar/").unwrap(); + assert!(Cookie::new_wrapped(cookie, url, CookieSource::HTTP).is_some()); - let cookie = Cookie::new(" baz = bar ; Secure; Path = /foo/bar/".to_string(), url).unwrap(); - assert!(cookie.value.as_slice() == "bar"); - assert!(cookie.name.as_slice() == "baz"); - assert!(cookie.secure); - assert!(cookie.path.as_slice() == "/foo/bar/"); - assert!(cookie.domain.as_slice() == "example.com"); + let cookie = cookie_rs::Cookie::parse(" baz = bar ; HttpOnly").unwrap(); + assert!(Cookie::new_wrapped(cookie, url, CookieSource::NonHTTP).is_none()); + + let cookie = cookie_rs::Cookie::parse(" baz = bar ; Secure; Path = /foo/bar/").unwrap(); + let cookie = Cookie::new_wrapped(cookie, url, CookieSource::HTTP).unwrap(); + assert!(cookie.cookie.value.as_slice() == "bar"); + assert!(cookie.cookie.name.as_slice() == "baz"); + assert!(cookie.cookie.secure); + assert!(cookie.cookie.path.as_ref().unwrap().as_slice() == "/foo/bar/"); + assert!(cookie.cookie.domain.as_ref().unwrap().as_slice() == "example.com"); assert!(cookie.host_only); let u = &Url::parse("http://example.com/foobar").unwrap(); - assert!(Cookie::new("foobar=value;path=/".to_string(), u).is_some()); -} - -#[deriving(Clone)] -pub struct CookieManager { - cookies: Vec, -} - -impl CookieManager { - pub fn new() -> CookieManager { - CookieManager { - cookies: Vec::new() - } - } - - pub fn add(&mut self, cookie: &Cookie) -> bool { - match self.cookies.iter().find(|x| { - x.cookie.domain == cookie.cookie.domain && - x.cookie.name == cookie.cookie.name && - x.cookie.path == cookie.cookie.path - }) { - Some(c) => { - if c.cookie.httponly && !cookie.scheme.as_slice().starts_with("http") { - return false - } - } - None => {} - } - self.cookies.push(cookie.clone()); - true - } + let cookie = cookie_rs::Cookie::parse("foobar=value;path=/").unwrap(); + assert!(Cookie::new_wrapped(cookie, u, CookieSource::HTTP).is_some()); } diff --git a/components/net/cookie_storage.rs b/components/net/cookie_storage.rs index bee2b36978f..dc69c146a5b 100644 --- a/components/net/cookie_storage.rs +++ b/components/net/cookie_storage.rs @@ -7,6 +7,16 @@ use url::Url; use cookie::Cookie; +use std::cmp::Ordering; + +/// The creator of a given cookie +#[derive(PartialEq, Copy)] +pub enum CookieSource { + /// An HTTP API + HTTP, + /// A non-HTTP API + NonHTTP, +} pub struct CookieStorage { cookies: Vec @@ -20,7 +30,7 @@ impl CookieStorage { } // http://tools.ietf.org/html/rfc6265#section-5.3 - pub fn remove(&mut self, cookie: &Cookie) -> Option { + pub fn remove(&mut self, cookie: &Cookie, source: CookieSource) -> Result, ()> { // Step 1 let position = self.cookies.iter().position(|c| { c.cookie.domain == cookie.cookie.domain && @@ -29,42 +39,68 @@ impl CookieStorage { }); if let Some(ind) = position { - Some(self.cookies.remove(ind)) + let c = self.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); + Err(()) + } } else { - None + Ok(None) } } // http://tools.ietf.org/html/rfc6265#section-5.3 - pub fn push(&mut self, mut cookie: Cookie, request: &Url) { - // Step 11 - if let Some(old_cookie) = self.remove(&cookie) { - // Step 11.2 - if old_cookie.cookie.httponly && !request.scheme.starts_with("http") { - self.cookies.push(old_cookie); - } else { - // Step 11.3 - cookie.created_at = old_cookie.created_at; - // Step 12 - self.cookies.push(cookie); - } + pub fn push(&mut self, mut cookie: Cookie, source: CookieSource) { + let old_cookie = self.remove(&cookie, source); + if old_cookie.is_err() { + // This new cookie is not allowed to overwrite an existing one. + return; } + + if cookie.cookie.value.is_empty() { + return; + } + + // Step 11 + if let Some(old_cookie) = old_cookie.unwrap() { + // Step 11.3 + cookie.creation_time = old_cookie.creation_time; + } + + // Step 12 + self.cookies.push(cookie); } // http://tools.ietf.org/html/rfc6265#section-5.4 - pub fn cookies_for_url(&mut self, url: Url) -> Option { + pub fn cookies_for_url(&mut self, url: &Url, 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.clone())); + info!(" === SENT COOKIE RESULT {}", c.appropriate_for_url(url, source)); // Step 1 - c.appropriate_for_url(url.clone()) + c.appropriate_for_url(url, source) }; - let mut url_cookies = self.cookies.iter_mut().filter(filterer); + // Step 2 + let mut url_cookies: Vec<&mut Cookie> = self.cookies.iter_mut().filter(filterer).collect(); + url_cookies.sort_by(|a, b| { + let a_path_len = a.cookie.path.as_ref().map(|p| p.len()).unwrap_or(0); + let b_path_len = b.cookie.path.as_ref().map(|p| p.len()).unwrap_or(0); + match a_path_len.cmp(&b_path_len) { + Ordering::Equal => { + let a_creation_time = a.creation_time.to_timespec(); + let b_creation_time = b.creation_time.to_timespec(); + a_creation_time.cmp(&b_creation_time) + } + result => result + } + }); - // TODO Step 2 - - let reducer = |&:acc: String, c: &mut Cookie| -> String { + let reducer = |&:acc: String, c: &mut &mut Cookie| -> String { // Step 3 c.touch(); @@ -74,7 +110,7 @@ impl CookieStorage { _ => acc + ";" }) + c.cookie.name.as_slice() + "=" + c.cookie.value.as_slice() }; - let result = url_cookies.fold("".to_string(), reducer); + let result = url_cookies.iter_mut().fold("".to_string(), reducer); info!(" === COOKIES SENT: {}", result); match result.len() { diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index d6d90f31473..69349234691 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -2,6 +2,7 @@ * 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 cookie_storage::CookieSource; use resource_task::{Metadata, TargetedLoadResponse, LoadData, start_sending_opt, ResponseSenders}; use resource_task::ControlMsg; use resource_task::ProgressMsg::{Payload, Done}; @@ -108,7 +109,7 @@ reason: \"certificate verify failed\" }]"; }; let (tx, rx) = channel(); - cookies_chan.send(ControlMsg::GetCookiesForUrl(url.clone(), tx)); + cookies_chan.send(ControlMsg::GetCookiesForUrl(url.clone(), tx, CookieSource::HTTP)); if let Some(cookies) = rx.recv().unwrap() { let mut v = Vec::new(); v.push(cookies.into_bytes()); @@ -189,7 +190,8 @@ reason: \"certificate verify failed\" }]"; } if let Some(&SetCookie(ref cookies)) = response.headers.get::() { - cookies_chan.send(ControlMsg::SetCookies(cookies.clone(), url.clone())); + cookies_chan.send(ControlMsg::SetCookies(cookies.clone(), url.clone(), + CookieSource::HTTP)); } if response.status.class() == StatusClass::Redirection { @@ -219,6 +221,8 @@ reason: \"certificate verify failed\" }]"; info!("redirecting to {}", new_url); url = new_url; + // According to https://tools.ietf.org/html/rfc7231#section-6.4.2, + // historically UAs have rewritten POST->GET on 301 and 302 responses. if load_data.method == Method::Post && (response.status == StatusCode::MovedPermanently || response.status == StatusCode::Found) { diff --git a/components/net/image_cache_task.rs b/components/net/image_cache_task.rs index fa8d189c943..ddf80570d10 100644 --- a/components/net/image_cache_task.rs +++ b/components/net/image_cache_task.rs @@ -555,13 +555,6 @@ mod tests { } fn mock_resource_task(on_load: Box) -> ResourceTask { - let cookie_chan = spawn_listener(move |port: Receiver| { - loop { - match port.recv() { - _ => { continue } - } - } - }); spawn_listener(move |port: Receiver| { loop { match port.recv().unwrap() { @@ -572,11 +565,11 @@ mod tests { eventual_consumer: response.consumer.clone(), }; let chan = start_sending(senders, Metadata::default( - Url::parse("file:///fake", cookie_chan.clone()).unwrap())); + Url::parse("file:///fake").unwrap())); on_load.invoke(chan); } - resource_task::ControlMsg::Cookies(_) => {} - resource_task::ControlMsg::Exit => break + resource_task::ControlMsg::Exit => break, + _ => {} } } }) @@ -718,13 +711,6 @@ mod tests { let (image_bin_sent_chan, image_bin_sent) = channel(); let (resource_task_exited_chan, resource_task_exited) = channel(); - let cookie_chan = spawn_listener(move |port: Receiver| { - loop { - match port.recv() { - _ => { continue } - } - } - }); let mock_resource_task = spawn_listener(move |port: Receiver| { loop { @@ -736,16 +722,16 @@ mod tests { eventual_consumer: response.consumer.clone(), }; let chan = start_sending(senders, Metadata::default( - Url::parse("file:///fake").unwrap()), cookie_chan.clone()); + Url::parse("file:///fake").unwrap())); chan.send(Payload(test_image_bin())); chan.send(Done(Ok(()))); image_bin_sent_chan.send(()); } - resource_task::ControlMsg::Cookies(_) => {} resource_task::ControlMsg::Exit => { resource_task_exited_chan.send(()); break } + _ => {} } } }); @@ -778,13 +764,6 @@ mod tests { let (image_bin_sent_chan, image_bin_sent) = channel(); let (resource_task_exited_chan, resource_task_exited) = channel(); - let cookie_chan = spawn_listener(move |port: Receiver| { - loop { - match port.recv() { - _ => { continue } - } - } - }); let mock_resource_task = spawn_listener(move |port: Receiver| { loop { match port.recv().unwrap() { @@ -795,16 +774,16 @@ mod tests { eventual_consumer: response.consumer.clone(), }; let chan = start_sending(senders, Metadata::default( - Url::parse("file:///fake").unwrap()), cookie_chan.clone()); + Url::parse("file:///fake").unwrap())); chan.send(Payload(test_image_bin())); chan.send(Done(Err("".to_string()))); image_bin_sent_chan.send(()); } - resource_task::ControlMsg::Cookies(_) => {} resource_task::ControlMsg::Exit => { resource_task_exited_chan.send(()); break } + _ => {} } } }); diff --git a/components/net/resource_task.rs b/components/net/resource_task.rs index 53c77f0f37e..1f100a514cf 100644 --- a/components/net/resource_task.rs +++ b/components/net/resource_task.rs @@ -11,7 +11,7 @@ use http_loader; use sniffer_task; use sniffer_task::SnifferTask; use cookie_rs::Cookie; -use cookie_storage::CookieStorage; +use cookie_storage::{CookieStorage, CookieSource}; use cookie; use util::task::spawn_named; @@ -31,9 +31,9 @@ pub enum ControlMsg { /// Request the data associated with a particular URL Load(LoadData), /// Store a set of cookies for a given originating URL - SetCookies(Vec, Url), + SetCookies(Vec, Url, CookieSource), /// Retrieve the stored cookies for a given URL - GetCookiesForUrl(Url, Sender>), + GetCookiesForUrl(Url, Sender>, CookieSource), Exit } @@ -205,17 +205,18 @@ struct ResourceManager { user_agent: Option, sniffer_task: SnifferTask, cookie_storage: CookieStorage, - cookies_chan: Sender, + resource_task: Sender, } impl ResourceManager { - fn new(from_client: Receiver, user_agent: Option, sniffer_task: SnifferTask, cookies_chan: Sender) -> ResourceManager { + fn new(from_client: Receiver, user_agent: Option, sniffer_task: SnifferTask, + resource_task: Sender) -> ResourceManager { ResourceManager { from_client: from_client, user_agent: user_agent, sniffer_task: sniffer_task, cookie_storage: CookieStorage::new(), - cookies_chan: cookies_chan, + resource_task: resource_task, } } } @@ -228,19 +229,15 @@ impl ResourceManager { ControlMsg::Load(load_data) => { self.load(load_data) } - ControlMsg::SetCookies(vector, request) => { + ControlMsg::SetCookies(vector, request, source) => { for cookie in vector.into_iter() { - if let Some(cookie) = cookie::Cookie::new_wrapped(cookie, &request) { - if cookie.cookie.value.is_empty() { - self.cookie_storage.remove(&cookie); - } else { - self.cookie_storage.push(cookie, &request); - } + if let Some(cookie) = cookie::Cookie::new_wrapped(cookie, &request, source) { + self.cookie_storage.push(cookie, source); } } } - ControlMsg::GetCookiesForUrl(url, consumer) => { - consumer.send(self.cookie_storage.cookies_for_url(url)); + ControlMsg::GetCookiesForUrl(url, consumer, source) => { + consumer.send(self.cookie_storage.cookies_for_url(&url, source)); } ControlMsg::Exit => { break @@ -266,7 +263,7 @@ impl ResourceManager { let loader = match load_data.url.scheme.as_slice() { "file" => from_factory(file_loader::factory), - "http" | "https" => http_loader::factory(self.cookies_chan.clone()), + "http" | "https" => http_loader::factory(self.resource_task.clone()), "data" => from_factory(data_loader::factory), "about" => from_factory(about_loader::factory), _ => { diff --git a/ports/cef/Cargo.lock b/ports/cef/Cargo.lock index fca6588c306..7aba998457c 100644 --- a/ports/cef/Cargo.lock +++ b/ports/cef/Cargo.lock @@ -153,6 +153,7 @@ name = "devtools_traits" version = "0.0.1" dependencies = [ "msg 0.0.1", + "util 0.0.1", ] [[package]] diff --git a/ports/gonk/Cargo.lock b/ports/gonk/Cargo.lock index a3581dd23f0..d24d9264541 100644 --- a/ports/gonk/Cargo.lock +++ b/ports/gonk/Cargo.lock @@ -124,6 +124,7 @@ name = "devtools_traits" version = "0.0.1" dependencies = [ "msg 0.0.1", + "util 0.0.1", ] [[package]]