From e5c83ec419a0e9a0a4e16342aa78d8809dd88d4e Mon Sep 17 00:00:00 2001 From: Sebastian C Date: Wed, 27 Aug 2025 18:41:11 -0500 Subject: [PATCH] script: Do not include fragments when comparing URLs in `CookieStore` (#38876) Fixes a check for empty options in `getAll(options)` and makes url comparison with exclude fragments set to true. Testing: New passing WPT tests Part of #37674 --------- Signed-off-by: Sebastian C --- components/script/dom/cookiestore.rs | 27 +++++++------------ components/url/lib.rs | 7 +++++ ...er_for_document_cookie.https.window.js.ini | 13 ++++----- ...and_set_cookie_headers.https.window.js.ini | 15 ++++++----- ...thandler_for_no_change.https.window.js.ini | 5 ++-- ...r_no_name_and_no_value.https.window.js.ini | 3 ++- ...o_name_equals_in_value.https.window.js.ini | 3 ++- ...o_name_multiple_values.https.window.js.ini | 3 ++- ...kieStore_getAll_arguments.https.any.js.ini | 12 --------- ...okieStore_getAll_multiple.https.any.js.ini | 3 --- ...e_getAll_set_creation_url.https.any.js.ini | 3 --- ...cookieStore_get_arguments.https.any.js.ini | 6 ----- .../cookieStore_opaque_origin.https.html.ini | 3 --- .../httponly_cookies.https.window.js.ini | 9 ++++--- 14 files changed, 46 insertions(+), 66 deletions(-) delete mode 100644 tests/wpt/meta/cookiestore/cookieStore_getAll_set_creation_url.https.any.js.ini diff --git a/components/script/dom/cookiestore.rs b/components/script/dom/cookiestore.rs index 9253b663fe2..1d445ff3ed2 100644 --- a/components/script/dom/cookiestore.rs +++ b/components/script/dom/cookiestore.rs @@ -259,12 +259,12 @@ impl CookieStoreMethods for CookieStore { // 6.1. Let parsed be the result of parsing options["url"] with settings’s API base URL. let parsed_url = ServoUrl::parse_with_base(Some(&global.api_base_url()), get_url); - // 6.2. If this’s relevant global object is a Window object and parsed does not equal url, + // 6.2. If this’s relevant global object is a Window object and parsed does not equal url with exclude fragments set to true, // then return a promise rejected with a TypeError. if let Some(_window) = DomRoot::downcast::(self.global()) { if parsed_url .as_ref() - .is_ok_and(|parsed| parsed.as_url() != creation_url.as_url()) + .is_ok_and(|parsed| !parsed.is_equal_excluding_fragments(creation_url)) { p.reject_error( Error::Type("URL does not match context".to_string()), @@ -354,7 +354,7 @@ impl CookieStoreMethods for CookieStore { // 2. Let origin be settings’s origin. let origin = global.origin(); - // 7. Let p be a new promise. + // 6. Let p be a new promise. let p = Promise::new(&global, can_gc); // 3. If origin is an opaque origin, then return a promise rejected with a "SecurityError" DOMException. @@ -366,26 +366,19 @@ impl CookieStoreMethods for CookieStore { // 4. Let url be settings’s creation URL. let creation_url = global.creation_url(); - // 5. If options is empty, then return a promise rejected with a TypeError. - // "is empty" is not strictly defined anywhere in the spec but the only value we require here is "url" - if options.url.is_none() && options.name.is_none() { - p.reject_error(Error::Type("Options cannot be empty".to_string()), can_gc); - return p; - } - let mut final_url = creation_url.clone(); - // 6. If options["url"] is present, then run these steps: + // 5. If options["url"] is present, then run these steps: if let Some(get_url) = &options.url { - // 6.1. Let parsed be the result of parsing options["url"] with settings’s API base URL. + // 5.1. Let parsed be the result of parsing options["url"] with settings’s API base URL. let parsed_url = ServoUrl::parse_with_base(Some(&global.api_base_url()), get_url); - // 6.2. If this’s relevant global object is a Window object and parsed does not equal url, + // If this’s relevant global object is a Window object and parsed does not equal url with exclude fragments set to true, // then return a promise rejected with a TypeError. if let Some(_window) = DomRoot::downcast::(self.global()) { if parsed_url .as_ref() - .is_ok_and(|parsed| parsed.as_url() != creation_url.as_url()) + .is_ok_and(|parsed| !parsed.is_equal_excluding_fragments(creation_url)) { p.reject_error( Error::Type("URL does not match context".to_string()), @@ -395,7 +388,7 @@ impl CookieStoreMethods for CookieStore { } } - // 6.3. If parsed’s origin and url’s origin are not the same origin, + // 5.3. If parsed’s origin and url’s origin are not the same origin, // then return a promise rejected with a TypeError. if parsed_url .as_ref() @@ -405,13 +398,13 @@ impl CookieStoreMethods for CookieStore { return p; } - // 6.4. Set url to parsed. + // 5.4. Set url to parsed. if let Ok(url) = parsed_url { final_url = url; } } - // 6. Run the following steps in parallel: + // 7. Run the following steps in parallel: let res = self.global() .resource_threads() diff --git a/components/url/lib.rs b/components/url/lib.rs index da44a794381..741f97dfdad 100644 --- a/components/url/lib.rs +++ b/components/url/lib.rs @@ -117,6 +117,13 @@ impl ServoUrl { scheme == "wss" } + /// + /// In the future this may be removed if the helper is added upstream in rust-url + /// see for details + pub fn is_equal_excluding_fragments(&self, other: &ServoUrl) -> bool { + self.0[..Position::AfterQuery] == other.0[..Position::AfterQuery] + } + pub fn as_str(&self) -> &str { self.0.as_str() } diff --git a/tests/wpt/meta/cookiestore/change_eventhandler_for_document_cookie.https.window.js.ini b/tests/wpt/meta/cookiestore/change_eventhandler_for_document_cookie.https.window.js.ini index f482971b1e8..613a54a7447 100644 --- a/tests/wpt/meta/cookiestore/change_eventhandler_for_document_cookie.https.window.js.ini +++ b/tests/wpt/meta/cookiestore/change_eventhandler_for_document_cookie.https.window.js.ini @@ -1,18 +1,19 @@ [change_eventhandler_for_document_cookie.https.window.html] + expected: TIMEOUT [document.cookie set/overwrite/delete observed by CookieStore] - expected: FAIL + expected: TIMEOUT [document.cookie set already-expired cookie should not be observed by CookieStore] - expected: FAIL + expected: NOTRUN [document.cookie duplicate cookie should not be observed by CookieStore] - expected: FAIL + expected: NOTRUN [CookieStore set/overwrite/delete observed by document.cookie] - expected: FAIL + expected: NOTRUN [CookieStore agrees with document.cookie on encoding non-ASCII cookies] - expected: FAIL + expected: NOTRUN [document.cookie agrees with CookieStore on encoding non-ASCII cookies] - expected: FAIL + expected: NOTRUN diff --git a/tests/wpt/meta/cookiestore/change_eventhandler_for_http_cookie_and_set_cookie_headers.https.window.js.ini b/tests/wpt/meta/cookiestore/change_eventhandler_for_http_cookie_and_set_cookie_headers.https.window.js.ini index e2ab8d878f1..c2087961854 100644 --- a/tests/wpt/meta/cookiestore/change_eventhandler_for_http_cookie_and_set_cookie_headers.https.window.js.ini +++ b/tests/wpt/meta/cookiestore/change_eventhandler_for_http_cookie_and_set_cookie_headers.https.window.js.ini @@ -1,21 +1,22 @@ [change_eventhandler_for_http_cookie_and_set_cookie_headers.https.window.html] + expected: TIMEOUT [HTTP set/overwrite/delete observed in CookieStore] - expected: FAIL + expected: TIMEOUT [HTTP set already-expired cookie should not be observed by CookieStore] - expected: FAIL + expected: NOTRUN [HTTP duplicate cookie should not be observed by CookieStore] - expected: FAIL + expected: NOTRUN [CookieStore agreed with HTTP headers agree on encoding non-ASCII cookies] - expected: FAIL + expected: NOTRUN [CookieStore set/overwrite/delete observed in HTTP headers] - expected: FAIL + expected: NOTRUN [HTTP headers agreed with CookieStore on encoding non-ASCII cookies] - expected: FAIL + expected: NOTRUN [Binary HTTP set/overwrite/delete observed in CookieStore] - expected: FAIL + expected: NOTRUN diff --git a/tests/wpt/meta/cookiestore/change_eventhandler_for_no_change.https.window.js.ini b/tests/wpt/meta/cookiestore/change_eventhandler_for_no_change.https.window.js.ini index d7ba44abba5..521e666cf90 100644 --- a/tests/wpt/meta/cookiestore/change_eventhandler_for_no_change.https.window.js.ini +++ b/tests/wpt/meta/cookiestore/change_eventhandler_for_no_change.https.window.js.ini @@ -1,6 +1,7 @@ [change_eventhandler_for_no_change.https.window.html] + expected: TIMEOUT [CookieStore duplicate cookie should not be observed] - expected: FAIL + expected: TIMEOUT [CookieStore duplicate partitioned cookie should not be observed] - expected: FAIL + expected: NOTRUN diff --git a/tests/wpt/meta/cookiestore/change_eventhandler_for_no_name_and_no_value.https.window.js.ini b/tests/wpt/meta/cookiestore/change_eventhandler_for_no_name_and_no_value.https.window.js.ini index b87dd5203c4..a9c0d61d89a 100644 --- a/tests/wpt/meta/cookiestore/change_eventhandler_for_no_name_and_no_value.https.window.js.ini +++ b/tests/wpt/meta/cookiestore/change_eventhandler_for_no_name_and_no_value.https.window.js.ini @@ -1,3 +1,4 @@ [change_eventhandler_for_no_name_and_no_value.https.window.html] + expected: TIMEOUT [Verify behavior of no-name and no-value cookies.] - expected: FAIL + expected: TIMEOUT diff --git a/tests/wpt/meta/cookiestore/change_eventhandler_for_no_name_equals_in_value.https.window.js.ini b/tests/wpt/meta/cookiestore/change_eventhandler_for_no_name_equals_in_value.https.window.js.ini index 5eeb11610bc..367f6352243 100644 --- a/tests/wpt/meta/cookiestore/change_eventhandler_for_no_name_equals_in_value.https.window.js.ini +++ b/tests/wpt/meta/cookiestore/change_eventhandler_for_no_name_equals_in_value.https.window.js.ini @@ -1,3 +1,4 @@ [change_eventhandler_for_no_name_equals_in_value.https.window.html] + expected: TIMEOUT [Verify that attempting to set a cookie with no name and with '=' in the value does not work.] - expected: FAIL + expected: TIMEOUT diff --git a/tests/wpt/meta/cookiestore/change_eventhandler_for_no_name_multiple_values.https.window.js.ini b/tests/wpt/meta/cookiestore/change_eventhandler_for_no_name_multiple_values.https.window.js.ini index 6f7ccd3037a..654dfbf2ba0 100644 --- a/tests/wpt/meta/cookiestore/change_eventhandler_for_no_name_multiple_values.https.window.js.ini +++ b/tests/wpt/meta/cookiestore/change_eventhandler_for_no_name_multiple_values.https.window.js.ini @@ -1,3 +1,4 @@ [change_eventhandler_for_no_name_multiple_values.https.window.html] + expected: TIMEOUT [Verify behavior of multiple no-name cookies] - expected: FAIL + expected: TIMEOUT diff --git a/tests/wpt/meta/cookiestore/cookieStore_getAll_arguments.https.any.js.ini b/tests/wpt/meta/cookiestore/cookieStore_getAll_arguments.https.any.js.ini index d941cedd62f..e9088211a61 100644 --- a/tests/wpt/meta/cookiestore/cookieStore_getAll_arguments.https.any.js.ini +++ b/tests/wpt/meta/cookiestore/cookieStore_getAll_arguments.https.any.js.ini @@ -2,17 +2,5 @@ expected: ERROR [cookieStore_getAll_arguments.https.any.html] - [cookieStore.getAll with no arguments] - expected: FAIL - - [cookieStore.getAll with empty options] - expected: FAIL - - [cookieStore.getAll with absolute url with fragment in options] - expected: FAIL - - [cookieStore.getAll with absolute different url in options] - expected: FAIL - [cookieStore.getAll with whitespace] expected: FAIL diff --git a/tests/wpt/meta/cookiestore/cookieStore_getAll_multiple.https.any.js.ini b/tests/wpt/meta/cookiestore/cookieStore_getAll_multiple.https.any.js.ini index 031177c873b..5cdeb496981 100644 --- a/tests/wpt/meta/cookiestore/cookieStore_getAll_multiple.https.any.js.ini +++ b/tests/wpt/meta/cookiestore/cookieStore_getAll_multiple.https.any.js.ini @@ -1,7 +1,4 @@ [cookieStore_getAll_multiple.https.any.html] - [cookieStore.getAll returns multiple cookies written by cookieStore.set] - expected: FAIL - [cookieStore_getAll_multiple.https.any.serviceworker.html] expected: ERROR diff --git a/tests/wpt/meta/cookiestore/cookieStore_getAll_set_creation_url.https.any.js.ini b/tests/wpt/meta/cookiestore/cookieStore_getAll_set_creation_url.https.any.js.ini deleted file mode 100644 index 92d48b35329..00000000000 --- a/tests/wpt/meta/cookiestore/cookieStore_getAll_set_creation_url.https.any.js.ini +++ /dev/null @@ -1,3 +0,0 @@ -[cookieStore_getAll_set_creation_url.https.any.html] - [cookieStore.set and cookieStore.getAll use the creation url] - expected: FAIL diff --git a/tests/wpt/meta/cookiestore/cookieStore_get_arguments.https.any.js.ini b/tests/wpt/meta/cookiestore/cookieStore_get_arguments.https.any.js.ini index cea0b853517..4e5a17f237d 100644 --- a/tests/wpt/meta/cookiestore/cookieStore_get_arguments.https.any.js.ini +++ b/tests/wpt/meta/cookiestore/cookieStore_get_arguments.https.any.js.ini @@ -2,11 +2,5 @@ expected: ERROR [cookieStore_get_arguments.https.any.html] - [cookieStore.get with absolute url with fragment in options] - expected: FAIL - - [cookieStore.get with absolute different url in options] - expected: FAIL - [cookieStore.get with whitespace] expected: FAIL diff --git a/tests/wpt/meta/cookiestore/cookieStore_opaque_origin.https.html.ini b/tests/wpt/meta/cookiestore/cookieStore_opaque_origin.https.html.ini index bf5e231e8f8..481e73304b7 100644 --- a/tests/wpt/meta/cookiestore/cookieStore_opaque_origin.https.html.ini +++ b/tests/wpt/meta/cookiestore/cookieStore_opaque_origin.https.html.ini @@ -1,7 +1,4 @@ [cookieStore_opaque_origin.https.html] expected: TIMEOUT - [cookieStore in non-sandboxed iframe should not throw] - expected: FAIL - [cookieStore in sandboxed iframe should throw SecurityError] expected: TIMEOUT diff --git a/tests/wpt/meta/cookiestore/httponly_cookies.https.window.js.ini b/tests/wpt/meta/cookiestore/httponly_cookies.https.window.js.ini index e518f59269c..2d885510f27 100644 --- a/tests/wpt/meta/cookiestore/httponly_cookies.https.window.js.ini +++ b/tests/wpt/meta/cookiestore/httponly_cookies.https.window.js.ini @@ -1,12 +1,13 @@ [httponly_cookies.https.window.html] + expected: TIMEOUT [HttpOnly cookies are not observed] - expected: FAIL + expected: TIMEOUT [HttpOnly cookies can not be set by document.cookie] - expected: FAIL + expected: NOTRUN [HttpOnly cookies can not be set by CookieStore] - expected: FAIL + expected: NOTRUN [HttpOnly cookies are not deleted/overwritten] - expected: FAIL + expected: NOTRUN