diff --git a/components/net/cookie.rs b/components/net/cookie.rs index 1a6ec3ed7c7..34d327673f3 100644 --- a/components/net/cookie.rs +++ b/components/net/cookie.rs @@ -7,7 +7,7 @@ use std::borrow::ToOwned; use std::net::{Ipv4Addr, Ipv6Addr}; -use std::time::{Duration, SystemTime}; +use std::time::SystemTime; use cookie::Cookie; use net_traits::pub_domains::is_pub_domain; @@ -38,55 +38,108 @@ impl ServoCookie { request: &ServoUrl, source: CookieSource, ) -> Option { - Cookie::parse(cookie_str) - .ok() - .map(|cookie| ServoCookie::new_wrapped(cookie, request, source)) - .unwrap_or(None) + let cookie = Cookie::parse(cookie_str).ok()?; + ServoCookie::new_wrapped(cookie, request, source) } - /// + /// Steps 6-22 from pub fn new_wrapped( mut cookie: Cookie<'static>, request: &ServoUrl, source: CookieSource, ) -> Option { - // Step 3 - let max_age: Option = - cookie.max_age().and_then(|max_age| max_age.try_into().ok()); - let (persistent, expiry_time) = match (max_age, cookie.expires_datetime()) { - (Some(max_age), _) => (true, Some(SystemTime::now() + max_age)), - (_, Some(date_time)) => (true, Some(date_time.into())), - _ => (false, None), - }; + let persistent; + let expiry_time; + + // Step 6. If the cookie-attribute-list contains an attribute with an attribute-name of "Max-Age": + if let Some(max_age) = cookie.max_age() { + // 1. Set the cookie's persistent-flag to true. + persistent = true; + + // 2. Set the cookie's expiry-time to attribute-value of the last + // attribute in the cookie-attribute-list with an attribute-name of "Max-Age". + expiry_time = Some(SystemTime::now() + max_age); + } + // Otherwise, if the cookie-attribute-list contains an attribute with an attribute-name of "Expires": + else if let Some(date_time) = cookie.expires_datetime() { + // 1. Set the cookie's persistent-flag to true. + persistent = true; + + // 2. Set the cookie's expiry-time to attribute-value of the last attribute in the + // cookie-attribute-list with an attribute-name of "Expires". + expiry_time = Some(date_time.into()); + } + // Otherwise: + else { + // 1. Set the cookie's persistent-flag to false. + persistent = false; + + // 2. Set the cookie's expiry-time to the latest representable date. + expiry_time = None; + } let url_host = request.host_str().unwrap_or("").to_owned(); - // Step 4 - let mut domain = cookie.domain().unwrap_or("").to_owned(); + // Step 7. If the cookie-attribute-list contains an attribute with an attribute-name of "Domain": + let mut domain = if let Some(domain) = cookie.domain() { + // 1. Let the domain-attribute be the attribute-value of the last attribute in the + // cookie-attribute-list [..] + // NOTE: This is done by the cookie crate + domain.to_owned() + } + // Otherwise: + else { + // 1. Let the domain-attribute be the empty string. + String::new() + }; - // Step 5 + // TODO Step 8. If the domain-attribute contains a character that is not in the range of [USASCII] characters, + // abort these steps and ignore the cookie entirely. + // NOTE: (is this done by the cookies crate?) + + // Step 9. If the user agent is configured to reject "public suffixes" and the domain-attribute + // is a public suffix: if is_pub_domain(&domain) { + // 1. If the domain-attribute is identical to the canonicalized request-host: if domain == url_host { - domain = "".to_string(); - } else { + // 1. Let the domain-attribute be the empty string. + domain = String::new(); + } + // Otherwise: + else { + // 1.Abort these steps and ignore the cookie entirely. return None; } } - // Step 6 - let host_only = if !domain.is_empty() { + // Step 10. If the domain-attribute is non-empty: + let host_only; + if !domain.is_empty() { + // 1. If the canonicalized request-host does not domain-match the domain-attribute: if !ServoCookie::domain_match(&url_host, &domain) { + // 1. Abort these steps and ignore the cookie entirely. return None; } else { + // 1. Set the cookie's host-only-flag to false. + host_only = false; + + // 2. Set the cookie's domain to the domain-attribute. cookie.set_domain(domain); - false } - } else { + } + // Otherwise: + else { + // 1. Set the cookie's host-only-flag to true. + host_only = true; + + // 2. Set the cookie's domain to the canonicalized request-host. cookie.set_domain(url_host); - true }; - // Step 7 + // Step 11. If the cookie-attribute-list contains an attribute with an attribute-name of "Path", + // set the cookie's path to attribute-value of the last attribute in the cookie-attribute-list + // with both an attribute-name of "Path" and an attribute-value whose length is no more than 1024 octets. + // Otherwise, set the cookie's path to the default-path of the request-uri. let mut has_path_specified = true; let mut path = cookie .path() @@ -95,29 +148,81 @@ impl ServoCookie { "" }) .to_owned(); + // TODO: Why do we do this? if !path.starts_with('/') { path = ServoCookie::default_path(request.path()).to_string(); } cookie.set_path(path); - // Step 10 - if cookie.http_only().unwrap_or(false) && source == CookieSource::NonHTTP { + // Step 12. If the cookie-attribute-list contains an attribute with an attribute-name of "Secure", + // set the cookie's secure-only-flag to true. Otherwise, set the cookie's secure-only-flag to false. + let secure_only = cookie.secure().unwrap_or(false); + + // Step 13. If the request-uri does not denote a "secure" connection (as defined by the user agent), + // and the cookie's secure-only-flag is true, then abort these steps and ignore the cookie entirely. + if secure_only && !request.is_secure_scheme() { return None; } - // https://tools.ietf.org/html/draft-west-cookie-prefixes-04#section-4 - // Step 1 of cookie prefixes - if (cookie.name().starts_with("__Secure-") || cookie.name().starts_with("__Host-")) && - !(cookie.secure().unwrap_or(false) && request.is_secure_scheme()) + // Step 14. If the cookie-attribute-list contains an attribute with an attribute-name of "HttpOnly", + // set the cookie's http-only-flag to true. Otherwise, set the cookie's http-only-flag to false. + let http_only = cookie.http_only().unwrap_or(false); + + // Step 15. If the cookie was received from a "non-HTTP" API and the cookie's + // http-only-flag is true, abort these steps and ignore the cookie entirely. + if http_only && source == CookieSource::NonHTTP { + return None; + } + + // TODO: Step 16, Ignore cookies from insecure request uris based on existing cookies + + // TODO: Steps 17-19, same-site-flag + + // Step 20. If the cookie-name begins with a case-insensitive match for the string "__Secure-", + // abort these steps and ignore the cookie entirely unless the cookie's secure-only-flag is true. + let has_case_insensitive_prefix = |value: &str, prefix: &str| { + value + .get(..prefix.len()) + .map_or(false, |p| p.eq_ignore_ascii_case(prefix)) + }; + if has_case_insensitive_prefix(cookie.name(), "__Secure-") && + !cookie.secure().unwrap_or(false) { return None; } - // Step 2 of cookie prefixes - if cookie.name().starts_with("__Host-") && - !(host_only && has_path_specified && cookie.path().unwrap() == "/") - { - return None; + // Step 21. If the cookie-name begins with a case-insensitive match for the string "__Host-", + // abort these steps and ignore the cookie entirely unless the cookie meets all the following criteria: + if has_case_insensitive_prefix(cookie.name(), "__Host-") { + // 1. The cookie's secure-only-flag is true. + if !secure_only { + return None; + } + + // 2. The cookie's host-only-flag is true. + if !host_only { + return None; + } + + // 3. The cookie-attribute-list contains an attribute with an attribute-name of "Path", + // and the cookie's path is /. + if !has_path_specified || !cookie.path().is_some_and(|path| path == "/") { + return None; + } + } + + // Step 22. If the cookie-name is empty and either of the following conditions are true, + // abort these steps and ignore the cookie entirely: + if cookie.name().is_empty() { + // 1. the cookie-value begins with a case-insensitive match for the string "__Secure-" + if has_case_insensitive_prefix(cookie.value(), "__Secure-") { + return None; + } + + // 2. the cookie-value begins with a case-insensitive match for the string "__Host-" + if has_case_insensitive_prefix(cookie.value(), "__Host-") { + return None; + } } Some(ServoCookie { @@ -138,7 +243,7 @@ impl ServoCookie { self.expiry_time = Some(SystemTime::UNIX_EPOCH) } - // http://tools.ietf.org/html/rfc6265#section-5.1.4 + /// pub fn default_path(request_path: &str) -> &str { // Step 2 if !request_path.starts_with('/') { @@ -156,7 +261,7 @@ impl ServoCookie { &request_path[..rightmost_slash_idx] } - // http://tools.ietf.org/html/rfc6265#section-5.1.4 + /// pub fn path_match(request_path: &str, cookie_path: &str) -> bool { // A request-path path-matches a given cookie-path if at least one of // the following conditions holds: @@ -175,7 +280,7 @@ impl ServoCookie { )) } - // http://tools.ietf.org/html/rfc6265#section-5.1.3 + /// pub fn domain_match(string: &str, domain_string: &str) -> bool { let string = &string.to_lowercase(); let domain_string = &domain_string.to_lowercase(); @@ -187,7 +292,7 @@ impl ServoCookie { string.parse::().is_err()) } - // http://tools.ietf.org/html/rfc6265#section-5.4 step 1 + /// step 1 pub fn appropriate_for_url(&self, url: &ServoUrl, source: CookieSource) -> bool { let domain = url.host_str(); if self.host_only { diff --git a/components/net/tests/cookie.rs b/components/net/tests/cookie.rs index f36fbfeb2f9..cf8fcf4baf9 100644 --- a/components/net/tests/cookie.rs +++ b/components/net/tests/cookie.rs @@ -79,13 +79,17 @@ fn fn_cookie_constructor() { assert!(ServoCookie::new_wrapped(cookie, url, CookieSource::HTTP).is_none()); let cookie = cookie::Cookie::parse(" baz = bar ; Secure; Path = /foo/bar/").unwrap(); - assert!(ServoCookie::new_wrapped(cookie, url, CookieSource::HTTP).is_some()); + assert!( + ServoCookie::new_wrapped(cookie, url, CookieSource::HTTP).is_none(), + "Cookie with \"Secure\" attribute from non-secure source should be rejected" + ); let cookie = cookie::Cookie::parse(" baz = bar ; HttpOnly").unwrap(); assert!(ServoCookie::new_wrapped(cookie, url, CookieSource::NonHTTP).is_none()); + let secure_url = &ServoUrl::parse("https://example.com/foo").unwrap(); let cookie = cookie::Cookie::parse(" baz = bar ; Secure; Path = /foo/bar/").unwrap(); - let cookie = ServoCookie::new_wrapped(cookie, url, CookieSource::HTTP).unwrap(); + let cookie = ServoCookie::new_wrapped(cookie, secure_url, CookieSource::HTTP).unwrap(); assert_eq!(cookie.cookie.value(), "bar"); assert_eq!(cookie.cookie.name(), "baz"); assert!(cookie.cookie.secure().unwrap_or(false)); diff --git a/tests/wpt/meta/cookies/prefix/__host.document-cookie.html.ini b/tests/wpt/meta/cookies/prefix/__host.document-cookie.html.ini deleted file mode 100644 index 6f8ab2d4cb0..00000000000 --- a/tests/wpt/meta/cookies/prefix/__host.document-cookie.html.ini +++ /dev/null @@ -1,9 +0,0 @@ -[__host.document-cookie.html] - [__HoSt: Non-secure origin: 'Path=/;'] - expected: FAIL - - [__HoSt: Non-secure origin: 'Path=/;domain=web-platform.test'] - expected: FAIL - - [__HoSt: Non-secure origin: 'Path=/;MaxAge=10'] - expected: FAIL diff --git a/tests/wpt/meta/cookies/prefix/__host.document-cookie.https.html.ini b/tests/wpt/meta/cookies/prefix/__host.document-cookie.https.html.ini deleted file mode 100644 index cf3ab5fa14e..00000000000 --- a/tests/wpt/meta/cookies/prefix/__host.document-cookie.https.html.ini +++ /dev/null @@ -1,15 +0,0 @@ -[__host.document-cookie.https.html] - [__HoSt: Secure origin: Does not set 'Path=/;'] - expected: FAIL - - [__HoSt: Secure origin: Does not set 'Secure; Path=/; Domain=web-platform.test; '] - expected: FAIL - - [__HoSt: Secure origin: Does not set 'Path=/;MaxAge=10'] - expected: FAIL - - [__HoSt: Secure origin: Does not set 'Secure; Path=/; Domain=web-platform.test; MaxAge=10'] - expected: FAIL - - [__HoSt: Secure origin: Does not set 'Secure; Path=/cookies/resources/list.py'] - expected: FAIL diff --git a/tests/wpt/meta/cookies/prefix/__host.header.html.ini b/tests/wpt/meta/cookies/prefix/__host.header.html.ini deleted file mode 100644 index 2444b65c243..00000000000 --- a/tests/wpt/meta/cookies/prefix/__host.header.html.ini +++ /dev/null @@ -1,12 +0,0 @@ -[__host.header.html] - [__HoSt: Non-secure origin: Does not set 'Path=/;'] - expected: FAIL - - [__HoSt: Non-secure origin: Does not set 'Path=/;domain=web-platform.test'] - expected: FAIL - - [__HoSt: Non-secure origin: Does not set 'Path=/;MaxAge=10'] - expected: FAIL - - [__HoSt: Non-secure origin: Does not set 'Path=/;HttpOnly'] - expected: FAIL diff --git a/tests/wpt/meta/cookies/prefix/__host.header.https.html.ini b/tests/wpt/meta/cookies/prefix/__host.header.https.html.ini deleted file mode 100644 index 33d9e1ada62..00000000000 --- a/tests/wpt/meta/cookies/prefix/__host.header.https.html.ini +++ /dev/null @@ -1,21 +0,0 @@ -[__host.header.https.html] - [__HoSt: Secure origin: Does not set 'Path=/;'] - expected: FAIL - - [__HoSt: Secure origin: Does not set 'Secure; Path=/; Domain=web-platform.test; '] - expected: FAIL - - [__HoSt: Secure origin: Does not set 'Path=/;MaxAge=10'] - expected: FAIL - - [__HoSt: Secure origin: Does not set 'Secure; Path=/; Domain=web-platform.test; MaxAge=10'] - expected: FAIL - - [__HoSt: Secure origin: Does not set 'Path=/;HttpOnly'] - expected: FAIL - - [__HoSt: Secure origin: Does not set 'Secure; Path=/; Domain=web-platform.test; HttpOnly'] - expected: FAIL - - [__HoSt: Secure origin: Does not set 'Secure; Path=/cookies/resources/list.py'] - expected: FAIL diff --git a/tests/wpt/meta/cookies/prefix/__secure.document-cookie.html.ini b/tests/wpt/meta/cookies/prefix/__secure.document-cookie.html.ini deleted file mode 100644 index 265fc3f0301..00000000000 --- a/tests/wpt/meta/cookies/prefix/__secure.document-cookie.html.ini +++ /dev/null @@ -1,9 +0,0 @@ -[__secure.document-cookie.html] - [__SeCuRe: Non-secure origin: Should not set 'Path=/;'] - expected: FAIL - - [__SeCuRe: Non-secure origin: Should not set 'Path=/;MaxAge=10'] - expected: FAIL - - [__SeCuRe: Non-secure origin: Should not set 'Path=/;domain=web-platform.test'] - expected: FAIL diff --git a/tests/wpt/meta/cookies/prefix/__secure.document-cookie.https.html.ini b/tests/wpt/meta/cookies/prefix/__secure.document-cookie.https.html.ini deleted file mode 100644 index 5e070e58ba2..00000000000 --- a/tests/wpt/meta/cookies/prefix/__secure.document-cookie.https.html.ini +++ /dev/null @@ -1,9 +0,0 @@ -[__secure.document-cookie.https.html] - [__SeCuRe: Secure origin: Should not set 'Path=/;'] - expected: FAIL - - [__SeCuRe: Secure origin: Should not set 'Path=/;MaxAge=10'] - expected: FAIL - - [__SeCuRe: Secure origin: Should not set 'Path=/;domain=web-platform.test'] - expected: FAIL diff --git a/tests/wpt/meta/cookies/prefix/__secure.header.html.ini b/tests/wpt/meta/cookies/prefix/__secure.header.html.ini deleted file mode 100644 index 142f4756a83..00000000000 --- a/tests/wpt/meta/cookies/prefix/__secure.header.html.ini +++ /dev/null @@ -1,12 +0,0 @@ -[__secure.header.html] - [__SeCuRe: Non-secure origin: Should not set 'Path=/;'] - expected: FAIL - - [__SeCuRe: Non-secure origin: Should not set 'Path=/;domain=web-platform.test'] - expected: FAIL - - [__SeCuRe: Non-secure origin: Should not set 'Path=/;MaxAge=10'] - expected: FAIL - - [__SeCuRe: Non-secure origin: Should not set 'Path=/;HttpOnly'] - expected: FAIL diff --git a/tests/wpt/meta/cookies/prefix/__secure.header.https.html.ini b/tests/wpt/meta/cookies/prefix/__secure.header.https.html.ini deleted file mode 100644 index 10db3a84195..00000000000 --- a/tests/wpt/meta/cookies/prefix/__secure.header.https.html.ini +++ /dev/null @@ -1,12 +0,0 @@ -[__secure.header.https.html] - [__SeCuRe: secure origin: Should not set 'Path=/;'] - expected: FAIL - - [__SeCuRe: secure origin: Should not set 'Path=/;MaxAge=10'] - expected: FAIL - - [__SeCuRe: secure origin: Should not set 'Path=/;HttpOnly'] - expected: FAIL - - [__SeCuRe: secure origin: Should not set 'Path=/;domain=not-web-platform.test'] - expected: FAIL diff --git a/tests/wpt/meta/cookies/prefix/document-cookie.non-secure.html.ini b/tests/wpt/meta/cookies/prefix/document-cookie.non-secure.html.ini deleted file mode 100644 index fcbb39e1912..00000000000 --- a/tests/wpt/meta/cookies/prefix/document-cookie.non-secure.html.ini +++ /dev/null @@ -1,18 +0,0 @@ -[document-cookie.non-secure.html] - [__SeCuRe: Non-secure origin: 'Path=/;'] - expected: FAIL - - [__SeCuRe: Non-secure origin: 'Path=/;domain=web-platform.test'] - expected: FAIL - - [__SeCuRe: Non-secure origin: 'Path=/;MaxAge=10'] - expected: FAIL - - [__HoSt: Non-secure origin: 'Path=/; '] - expected: FAIL - - [__HoSt: Non-secure origin: 'Path=/; domain=web-platform.test'] - expected: FAIL - - [__HoSt: Non-secure origin: 'Path=/; MaxAge=10'] - expected: FAIL