Skip to content

Commit

Permalink
Properly handle errors in the cookie's domain when setting a cookie w…
Browse files Browse the repository at this point in the history
…ith the Cookie Store API

https://bugs.webkit.org/show_bug.cgi?id=259529

Reviewed by Alex Christensen.

The spec (https://wicg.github.io/cookie-store/#set-cookie-algorithm)
dictates that in the set function, if the domain is not null, then if
the domain begins with a '.', the promise should be rejected with a
TypeError. If the domain does not begin with a '.' but the url's host
is not equal to domain and it does not end with a '.' followed by the
domain, the promise should be rejected with a TypeError. Additionally,
if the byte sequence length of the domain (in UTF8 format) is greater
than the maximum attribute value size (current 1024 bytes according to
https://wicg.github.io/cookie-store/#cookie-maximum-attribute-value-size),
then the promise should be rejected with a TypeError. This patch adds
these checks.

* LayoutTests/imported/w3c/web-platform-tests/cookie-store/cookieStore_delete_arguments.https.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/cookie-store/cookieStore_get_set_across_frames.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/cookie-store/cookieStore_opaque_origin.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/cookie-store/cookieStore_set_arguments.https.any-expected.txt:
* Source/WebCore/Modules/cookie-store/CookieStore.cpp:
(WebCore::CookieStore::set):

Canonical link: https://commits.webkit.org/266351@main
  • Loading branch information
RupinMittal committed Jul 27, 2023
1 parent 42c22e4 commit 58f6a2d
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@

PASS cookieStore.delete with positional name
PASS cookieStore.delete with name in options
FAIL cookieStore.delete domain starts with "." assert_unreached: Should have rejected: undefined Reached unreachable code
FAIL cookieStore.delete with domain that is not equal current host assert_unreached: Should have rejected: undefined Reached unreachable code
FAIL cookieStore.delete with domain set to the current hostname assert_equals: expected null but got object "[object Object]"
FAIL cookieStore.delete with domain set to a subdomain of the current hostname assert_unreached: Should have rejected: undefined Reached unreachable code
FAIL cookieStore.delete with domain set to a non-domain-matching suffix of the current hostname assert_unreached: Should have rejected: undefined Reached unreachable code
FAIL cookieStore.delete with path set to the current directory assert_equals: expected null but got object "[object Object]"
PASS cookieStore.delete domain starts with "."
PASS cookieStore.delete with domain that is not equal current host
PASS cookieStore.delete with domain set to the current hostname
PASS cookieStore.delete with domain set to a subdomain of the current hostname
PASS cookieStore.delete with domain set to a non-domain-matching suffix of the current hostname
PASS cookieStore.delete with path set to the current directory
PASS cookieStore.delete with path set to subdirectory of the current directory
FAIL cookieStore.delete with missing / at the end of path assert_equals: expected null but got object "[object Object]"
PASS cookieStore.delete with missing / at the end of path
PASS cookieStore.delete with path that does not start with /
FAIL cookieStore.delete with get result assert_equals: expected null but got object "[object Object]"
PASS cookieStore.delete with get result
FAIL cookieStore.delete with positional empty name promise_test: Unhandled rejection with value: object "TypeError: Type error"
FAIL cookieStore.delete with empty name in options promise_test: Unhandled rejection with value: object "TypeError: Type error"

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

FAIL cookieStore.get() sees cookieStore.set() in frame promise_test: Unhandled rejection with value: object "TypeError: null is not an object (evaluating 'frameCookie.value')"
FAIL cookieStore.get() in frame sees cookieStore.set() promise_test: Unhandled rejection with value: object "TypeError: null is not an object (evaluating 'cookie.value')"
FAIL cookieStore.get() in frame sees cookieStore.set() promise_test: Unhandled rejection with value: object "TypeError: The domain must be a part of the current host"

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

PASS cookieStore in non-sandboxed iframe should not throw
FAIL cookieStore in non-sandboxed iframe should not throw assert_equals: cookieStore ${apiCall} should not throw expected "no exception" but got "TypeError"
PASS cookieStore in sandboxed iframe should throw SecurityError

Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@ PASS cookieStore.set with expires set to a future Date
PASS cookieStore.set with expires set to a past Date
PASS cookieStore.set with expires set to a future timestamp
PASS cookieStore.set with expires set to a past timestamp
FAIL cookieStore.set domain starts with "." assert_unreached: Should have rejected: undefined Reached unreachable code
FAIL cookieStore.set with domain that is not equal current host assert_unreached: Should have rejected: undefined Reached unreachable code
PASS cookieStore.set domain starts with "."
PASS cookieStore.set with domain that is not equal current host
PASS cookieStore.set with domain set to the current hostname
FAIL cookieStore.set with domain set to a subdomain of the current hostname assert_unreached: Should have rejected: undefined Reached unreachable code
FAIL cookieStore.set with domain set to a non-domain-matching suffix of the current hostname assert_unreached: Should have rejected: undefined Reached unreachable code
FAIL cookieStore.set default domain is null and differs from current hostname assert_array_equals: expected property 0 to be "cookie-value1" but got "cookie-value" (expected array ["cookie-value1", "cookie-value2"] got ["cookie-value", "cookie-value2"])
PASS cookieStore.set with domain set to a subdomain of the current hostname
PASS cookieStore.set with domain set to a non-domain-matching suffix of the current hostname
FAIL cookieStore.set default domain is null and differs from current hostname assert_equals: expected 2 but got 1
PASS cookieStore.set with path set to the current directory
FAIL cookieStore.set with path set to a subdirectory of the current directory assert_equals: expected null but got object "[object Object]"
FAIL cookieStore.set default path is / assert_equals: expected 1 but got 2
PASS cookieStore.set with path set to a subdirectory of the current directory
PASS cookieStore.set default path is /
PASS cookieStore.set adds / to path that does not end with /
PASS cookieStore.set with path that does not start with /
FAIL cookieStore.set with get result assert_equals: expected "old-cookie-value" but got "cookie-value"
PASS cookieStore.set with get result

24 changes: 22 additions & 2 deletions Source/WebCore/Modules/cookie-store/CookieStore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,30 @@ void CookieStore::set(CookieInit&& options, Ref<DeferredPromise>&& promise)
Cookie cookie;
cookie.name = WTFMove(options.name);
cookie.value = WTFMove(options.value);
cookie.domain = options.domain.isNull() ? document.domain() : WTFMove(options.domain);
cookie.created = WallTime::now().secondsSinceEpoch().milliseconds();

cookie.domain = options.domain.isNull() ? document.domain() : WTFMove(options.domain);
auto& url = document.url();
if (!cookie.domain.isNull()) {
if (cookie.domain.startsWith('.')) {
promise->reject(Exception { TypeError, "The domain must not begin with a '.'"_s });
return;
}

auto host = url.host();
if (!host.endsWith(cookie.domain) || (host.length() > cookie.domain.length() && !StringView(host).substring(0, host.length() - cookie.domain.length()).endsWith('.'))) {
promise->reject(Exception { TypeError, "The domain must be a part of the current host"_s });
return;
}

// The maximum attribute value size is specified at https://wicg.github.io/cookie-store/#cookie-maximum-attribute-value-size.
static constexpr auto maximumAttributeValueSize = 1024;
if (cookie.domain.utf8().length() > maximumAttributeValueSize) {
promise->reject(Exception { TypeError, makeString("The size of the domain must not be greater than ", maximumAttributeValueSize, " bytes") });
return;
}
}

cookie.path = WTFMove(options.path);
if (!cookie.path.isNull()) {
if (!cookie.path.startsWith('/')) {
Expand All @@ -239,7 +260,6 @@ void CookieStore::set(CookieInit&& options, Ref<DeferredPromise>&& promise)
break;
}

auto& url = document.url();
auto& cookieJar = page->cookieJar();
auto completionHandler = [promise = WTFMove(promise)] (bool setSuccessfully) {
if (!setSuccessfully)
Expand Down

0 comments on commit 58f6a2d

Please sign in to comment.