Strip TAB & SPACE from name & value passed to CookieStore methods

annevk · annevk · commit da64dee537b1 · 2025-08-08T18:05:56.000+02:00
https://bugs.webkit.org/show_bug.cgi?id=297118 Reviewed by NOBODY (OOPS!). The specification already covers set() and whatwg/cookiestore#281 will cover get(), getAll(), and delete() for consistency. subscribe() and unsubscribe() are not covered by this PR. They are not shipping and have various outstanding specification issues. This also removes an unused argument from CookieStore::MainThreadBridge::set. While using trim() I also discovered that the comments in Source/WTF/wtf/text/StringImpl.cpp were out-of-date so this updates those as well.
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_delete_arguments.https.any-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_delete_arguments.https.any-expected.txt
@@ -16,4 +16,5 @@ FAIL cookieStore.delete with positional empty name promise_test: Unhandled rejec
 FAIL cookieStore.delete with empty name in options promise_test: Unhandled rejection with value: object "TypeError: Type error"
 PASS cookieStore.delete with maximum cookie name size
 PASS cookieStore.delete with a __Host- prefix should not have a domain
+PASS cookieStore.delete with whitespace
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_delete_arguments.https.any.js b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_delete_arguments.https.any.js
@@ -220,3 +220,12 @@ promise_test(async testCase => {
   const cookie = await cookieStore.get('cookie-name');
   assert_equals(cookie, null);
 }, 'cookieStore.delete with a __Host- prefix should not have a domain');
+
+promise_test(async testCase => {
+  await cookieStore.set('cookie-name', 'cookie-value');
+  testCase.add_cleanup(async () => {
+    await cookieStore.delete('cookie-name');
+  });
+  const cookie = await cookieStore.delete(' cookie-name \t');
+  assert_equals(cookie, undefined);
+}, 'cookieStore.delete with whitespace');
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_delete_arguments.https.any.serviceworker-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_delete_arguments.https.any.serviceworker-expected.txt
@@ -16,4 +16,5 @@ FAIL cookieStore.delete with positional empty name promise_test: Unhandled rejec
 FAIL cookieStore.delete with empty name in options promise_test: Unhandled rejection with value: object "TypeError: Type error"
 PASS cookieStore.delete with maximum cookie name size
 PASS cookieStore.delete with a __Host- prefix should not have a domain
+PASS cookieStore.delete with whitespace
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_getAll_arguments.https.any-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_getAll_arguments.https.any-expected.txt
@@ -10,4 +10,5 @@ PASS cookieStore.getAll with invalid url path in options
 PASS cookieStore.getAll with invalid url host in options
 PASS cookieStore.getAll with absolute url with fragment in options
 PASS cookieStore.getAll with absolute different url in options
+PASS cookieStore.getAll with whitespace
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_getAll_arguments.https.any.js b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_getAll_arguments.https.any.js
@@ -194,3 +194,14 @@ promise_test(async testCase => {
     assert_equals(cookies[0].value, 'cookie-value');
   }
 }, 'cookieStore.getAll with absolute different url in options');
+
+promise_test(async testCase => {
+  await cookieStore.set('cookie-name', 'cookie-value');
+  testCase.add_cleanup(async () => {
+    await cookieStore.delete('cookie-name');
+  });
+  const cookies = await cookieStore.getAll(' cookie-name \t');
+  assert_equals(cookies.length, 1);
+  assert_equals(cookies[0].name, 'cookie-name');
+  assert_equals(cookies[0].value, 'cookie-value');
+}, 'cookieStore.getAll with whitespace');
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_getAll_arguments.https.any.serviceworker-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_getAll_arguments.https.any.serviceworker-expected.txt
@@ -10,4 +10,5 @@ PASS cookieStore.getAll with same-origin url path in options
 PASS cookieStore.getAll with invalid url host in options
 PASS cookieStore.getAll with absolute url with fragment in options
 PASS cookieStore.getAll with absolute different url in options
+PASS cookieStore.getAll with whitespace
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_get_arguments.https.any-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_get_arguments.https.any-expected.txt
@@ -10,4 +10,5 @@ PASS cookieStore.get with invalid url path in options
 PASS cookieStore.get with invalid url host in options
 PASS cookieStore.get with absolute url with fragment in options
 PASS cookieStore.get with absolute different url in options
+PASS cookieStore.get with whitespace
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_get_arguments.https.any.js b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_get_arguments.https.any.js
@@ -142,3 +142,13 @@ promise_test(async testCase => {
     assert_equals(cookie.value, 'cookie-value');
   }
 }, 'cookieStore.get with absolute different url in options');
+
+promise_test(async testCase => {
+  await cookieStore.set('cookie-name', 'cookie-value');
+  testCase.add_cleanup(async () => {
+    await cookieStore.delete('cookie-name');
+  });
+  const cookie = await cookieStore.get(' cookie-name \t');
+  assert_equals(cookie.name, 'cookie-name');
+  assert_equals(cookie.value, 'cookie-value');
+}, 'cookieStore.get with whitespace');
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_get_arguments.https.any.serviceworker-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_get_arguments.https.any.serviceworker-expected.txt
@@ -10,4 +10,5 @@ PASS cookieStore.get with same-origin url path in options
 PASS cookieStore.get with invalid url host in options
 PASS cookieStore.get with absolute url with fragment in options
 PASS cookieStore.get with absolute different url in options
+PASS cookieStore.get with whitespace
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_set_arguments.https.any-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_set_arguments.https.any-expected.txt
@@ -51,6 +51,6 @@ PASS cookieStore.set with get result
 PASS cookieStore.set checks if the path is too long
 PASS cookieStore.set checks if the domain is too long
 PASS cookieStore.set with a __Host- prefix should not have a domain
-FAIL cookieStore.set with whitespace only name and value assert_unreached: Should have rejected: undefined Reached unreachable code
-FAIL cookieStore.set with whitespace at begining or end assert_unreached: Should have rejected: undefined Reached unreachable code
+PASS cookieStore.set with whitespace only name and value
+PASS cookieStore.set with whitespace at begining or end
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_set_arguments.https.any.js b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_set_arguments.https.any.js
@@ -390,26 +390,29 @@ promise_test(async testCase => {
 }, 'cookieStore.set with whitespace only name and value');
 
 promise_test(async testCase => {
-  testCase.add_cleanup(async () => {
-    await cookieStore.delete('a b');
-  });
-  await cookieStore.set('a b', 'x y');
-  const cookie = await cookieStore.get('a b');
-  assert_equals(cookie.value, "x y");
-
-  await promise_rejects_js(testCase, TypeError, cookieStore.set(
-      { name: 'a  ',
-        value: 'x' }));
-
-  await promise_rejects_js(testCase, TypeError, cookieStore.set(
-      { name: '  a',
-        value: 'x' }));
-
-  await promise_rejects_js(testCase, TypeError, cookieStore.set(
-      { name: 'a',
-        value: 'x ' }));
+  const tests = [
+    { set: ['a b', 'x y'], get: ['a b', 'x y'] },
+    { set: ['a ', 'x'], get: ['a', 'x'] },
+    { set: ['a\t', 'x'], get: ['a', 'x'] },
+    { set: ['  a', 'x'], get: ['a', 'x'] },
+    { set: [' \t a', 'x'], get: ['a', 'x'] },
+    { set: [' \t a  \t\t', 'x'], get: ['a', 'x'] },
+    { set: ['a', 'x '], get: ['a', 'x'] },
+    { set: ['a', 'x\t'], get: ['a', 'x'] },
+    { set: ['a', ' x'], get: ['a', 'x'] },
+    { set: ['a', ' \t x'], get: ['a', 'x'] },
+    { set: ['a', ' \t x  \t\t'], get: ['a', 'x'] },
+  ];
+
+  for (const t of tests) {
+    await cookieStore.set(t.set[0], t.set[1]);
+    let cookie = await cookieStore.get(t.get[0]);
+    assert_equals(cookie.value, t.get[1]);
+
+    await cookieStore.delete(t.get[0]);
+
+    cookie = await cookieStore.get(t.get[0]);
+    assert_equals(cookie, null);
+  }
 
-  await promise_rejects_js(testCase, TypeError, cookieStore.set(
-      { name: 'a',
-        value: 'x ' }));
 }, 'cookieStore.set with whitespace at begining or end');
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_set_arguments.https.any.serviceworker-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_set_arguments.https.any.serviceworker-expected.txt
@@ -51,6 +51,6 @@ PASS cookieStore.set with get result
 PASS cookieStore.set checks if the path is too long
 PASS cookieStore.set checks if the domain is too long
 PASS cookieStore.set with a __Host- prefix should not have a domain
-FAIL cookieStore.set with whitespace only name and value assert_unreached: Should have rejected: undefined Reached unreachable code
-FAIL cookieStore.set with whitespace at begining or end assert_unreached: Should have rejected: undefined Reached unreachable code
+PASS cookieStore.set with whitespace only name and value
+PASS cookieStore.set with whitespace at begining or end
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_set_domain_parsing.sub.https.html b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_set_domain_parsing.sub.https.html
@@ -3,8 +3,6 @@
   <head>
     <script src="/resources/testharness.js"></script>
     <script src="/resources/testharnessreport.js"></script>
-    <script src="/resources/testdriver.js"></script>
-    <script src="/resources/testdriver-vendor.js"></script>
   </head>
   <body>
     <script>
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_set_domain_parsing.tentative.sub.https.html b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/cookieStore_set_domain_parsing.tentative.sub.https.html
@@ -3,8 +3,6 @@
   <head>
     <script src="/resources/testharness.js"></script>
     <script src="/resources/testharnessreport.js"></script>
-    <script src="/resources/testdriver.js"></script>
-    <script src="/resources/testdriver-vendor.js"></script>
   </head>
   <body>
     <script>
diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookiestore/resources/domain_parsing-child.sub.https.html b/LayoutTests/imported/w3c/web-platform-tests/cookiestore/resources/domain_parsing-child.sub.https.html
@@ -2,8 +2,6 @@
 <html>
 <head>
   <script src="/resources/testharness.js"></script>
-  <script src="/resources/testdriver.js"></script>
-  <script src="/resources/testdriver-vendor.js"></script>
 </head>
 <body>
   <script>
diff --git a/Source/WTF/wtf/text/StringImpl.cpp b/Source/WTF/wtf/text/StringImpl.cpp
@@ -781,16 +781,16 @@ template<typename CharacterType, typename CodeUnitPredicate> inline Ref<StringIm
 
     unsigned start = 0;
     unsigned end = span.size() - 1;
-    
-    // skip white space from start
+
+    // skip matched characters from start
     while (start <= end && predicate(span[start]))
         ++start;
-    
-    // only white space
+
+    // only matched characters
     if (start > end) 
         return *empty();
 
-    // skip white space from end
+    // skip matched characters from end
     while (end && predicate(span[end]))
         --end;
 
diff --git a/Source/WebCore/Modules/cookie-store/CookieStore.cpp b/Source/WebCore/Modules/cookie-store/CookieStore.cpp
@@ -79,7 +79,7 @@ class CookieStore::MainThreadBridge : public ThreadSafeRefCounted<MainThreadBrid
 
     void get(CookieStoreGetOptions&&, URL&&, Function<void(CookieStore&, ExceptionOr<Vector<Cookie>>&&)>&&);
     void getAll(CookieStoreGetOptions&&, URL&&, Function<void(CookieStore&, ExceptionOr<Vector<Cookie>>&&)>&&);
-    void set(CookieInit&& options, Cookie&&, URL&&, Function<void(CookieStore&, std::optional<Exception>&&)>&&);
+    void set(Cookie&&, URL&&, Function<void(CookieStore&, std::optional<Exception>&&)>&&);
 
     void detach() { m_cookieStore = nullptr; }
 
@@ -185,11 +185,11 @@ void CookieStore::MainThreadBridge::getAll(CookieStoreGetOptions&& options, URL&
     ensureOnMainThread(WTFMove(getAllCookies));
 }
 
-void CookieStore::MainThreadBridge::set(CookieInit&& options, Cookie&& cookie, URL&& url, Function<void(CookieStore&, std::optional<Exception>&&)>&& completionHandler)
+void CookieStore::MainThreadBridge::set(Cookie&& cookie, URL&& url, Function<void(CookieStore&, std::optional<Exception>&&)>&& completionHandler)
 {
     ASSERT(m_cookieStore);
 
-    auto setCookie = [this, protectedThis = Ref { *this }, options = crossThreadCopy(WTFMove(options)), cookie = crossThreadCopy(WTFMove(cookie)), url = crossThreadCopy(WTFMove(url)), completionHandler = WTFMove(completionHandler)](ScriptExecutionContext& context) mutable {
+    auto setCookie = [this, protectedThis = Ref { *this }, cookie = crossThreadCopy(WTFMove(cookie)), url = crossThreadCopy(WTFMove(url)), completionHandler = WTFMove(completionHandler)](ScriptExecutionContext& context) mutable {
         Ref document = downcast<Document>(context);
         WeakPtr page = document->page();
         if (!page) {
@@ -234,6 +234,13 @@ CookieStore::~CookieStore()
     m_mainThreadBridge->detach();
 }
 
+static String normalize(const String& string)
+{
+    if (string.contains(isTabOrSpace<char16_t>))
+        return string.trim(isTabOrSpace);
+    return string;
+}
+
 static bool containsInvalidCharacters(const String& string)
 {
     // The invalid characters are specified at https://wicg.github.io/cookie-store/#set-a-cookie.
@@ -287,6 +294,9 @@ void CookieStore::get(CookieStoreGetOptions&& options, Ref<DeferredPromise>&& pr
         options.url = nullString();
     }
 
+    if (!options.name.isNull())
+        options.name = normalize(options.name);
+
     m_promises.add(++m_nextPromiseIdentifier, WTFMove(promise));
     auto completionHandler = [promiseIdentifier = m_nextPromiseIdentifier](CookieStore& cookieStore, ExceptionOr<Vector<Cookie>>&& result) {
         auto promise = cookieStore.takePromise(promiseIdentifier);
@@ -350,6 +360,9 @@ void CookieStore::getAll(CookieStoreGetOptions&& options, Ref<DeferredPromise>&&
         options.url = nullString();
     }
 
+    if (!options.name.isNull())
+        options.name = normalize(options.name);
+
     m_promises.add(++m_nextPromiseIdentifier, WTFMove(promise));
     auto completionHandler = [promiseIdentifier = m_nextPromiseIdentifier](CookieStore& cookieStore, ExceptionOr<Vector<Cookie>>&& result) {
         auto promise = cookieStore.takePromise(promiseIdentifier);
@@ -404,8 +417,8 @@ void CookieStore::set(CookieInit&& options, Ref<DeferredPromise>&& promise)
     Cookie cookie;
     cookie.created = WallTime::now().secondsSinceEpoch().milliseconds();
 
-    cookie.name = WTFMove(options.name);
-    cookie.value = WTFMove(options.value);
+    cookie.name = normalize(options.name);
+    cookie.value = normalize(options.value);
 
     if (containsInvalidCharacters(cookie.name)) {
         promise->reject(Exception { ExceptionCode::TypeError, "The cookie name must not contain '\u003B', '\u007F', or any C0 control character except '\u0009'."_s });
@@ -527,7 +540,7 @@ void CookieStore::set(CookieInit&& options, Ref<DeferredPromise>&& promise)
             promise->resolve();
     };
 
-    m_mainThreadBridge->set(WTFMove(options), WTFMove(cookie), WTFMove(url), WTFMove(completionHandler));
+    m_mainThreadBridge->set(WTFMove(cookie), WTFMove(url), WTFMove(completionHandler));
 }
 
 void CookieStore::remove(String&& name, Ref<DeferredPromise>&& promise)
@@ -555,7 +568,7 @@ void CookieStore::remove(CookieStoreDeleteOptions&& options, Ref<DeferredPromise
     }
 
     CookieInit initOptions;
-    initOptions.name = WTFMove(options.name);
+    initOptions.name = normalize(options.name);
     initOptions.value = emptyString();
     initOptions.domain = WTFMove(options.domain);
     initOptions.path = WTFMove(options.path);
