Commit 4851f5ff authored by Dylan Cutler's avatar Dylan Cutler Committed by Commit Bot

Have CookieStore reject cookies with no name and no value.

The CookieStore spec recently changed to disallow setting cookies
using this API:

https://github.com/WICG/cookie-store/pull/150

This CL updates the implementation of CookieStore in Blink to
perform this additional check and updates the web platform
tests to reflect the new changes to the spec.

Bug: 1108964
Change-Id: I1bc60332551308498509c0e8bbc6823bd8375ab8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2316622Reviewed-by: default avatarAyu Ishii <ayui@chromium.org>
Commit-Queue: Dylan Cutler <dylancutler@google.com>
Cr-Commit-Position: refs/heads/master@{#791963}
parent e760d895
......@@ -77,6 +77,11 @@ base::Optional<CanonicalCookie> ToCanonicalCookie(
"Cookie value cannot contain '=' if the name is empty");
return base::nullopt;
}
if (name.IsEmpty() && value.IsEmpty()) {
exception_state.ThrowTypeError(
"Cookie name and value both cannot be empty");
return base::nullopt;
}
base::Time expires = options->hasExpiresNonNull()
? base::Time::FromJavaTime(options->expiresNonNull())
......@@ -307,7 +312,7 @@ ScriptPromise CookieStore::Delete(ScriptState* script_state,
CookieInit* set_options = CookieInit::Create();
set_options->setName(name);
set_options->setValue(g_empty_string);
set_options->setValue("deleted");
set_options->setExpires(0);
return DoWrite(script_state, set_options, exception_state);
}
......@@ -317,7 +322,7 @@ ScriptPromise CookieStore::Delete(ScriptState* script_state,
ExceptionState& exception_state) {
CookieInit* set_options = CookieInit::Create();
set_options->setName(options->name());
set_options->setValue(g_empty_string);
set_options->setValue("deleted");
set_options->setExpires(0);
set_options->setDomain(options->domain());
set_options->setPath(options->path());
......
......@@ -14,15 +14,25 @@ cookie_test(async t => {
eventPromise, {changed: [{name: '', value: 'first-value'}]},
'Observed no-name change');
eventPromise = observeNextCookieChangeEvent();
await cookieStore.set('', '');
const actual2 =
(await cookieStore.getAll('')).map(({ value }) => value).join(';');
const expected2 = '';
assert_equals(actual2, expected2);
await verifyCookieChangeEvent(
eventPromise, {changed: [{name: '', value: ''}]},
'Observed no-name change');
await promise_rejects_js(
t,
TypeError,
cookieStore.set('', ''),
'Expected promise rejection when setting a cookie with' +
' no name and no value');
await promise_rejects_js(
t,
TypeError,
cookieStore.set({name: '', value: ''}),
'Expected promise rejection when setting a cookie with' +
' no name and no value');
const cookies = await cookieStore.getAll('');
assert_equals(cookies.length, 1);
assert_equals(cookies[0].name, '');
assert_equals(cookies[0].value, 'first-value',
'Cookie with no name should still have previous value.');
eventPromise = observeNextCookieChangeEvent();
await cookieStore.delete('');
......
......@@ -147,3 +147,25 @@ promise_test(async testCase => {
const cookie = await cookieStore.get('cookie-name');
assert_equals(cookie, null);
}, 'cookieStore.delete with get result');
promise_test(async testCase => {
await cookieStore.set('', 'cookie-value');
testCase.add_cleanup(async () => {
await cookieStore.delete('');
});
await cookieStore.delete('');
const cookie = await cookieStore.get('');
assert_equals(cookie, null);
}, 'cookieStore.delete with positional empty name');
promise_test(async testCase => {
await cookieStore.set('', 'cookie-value');
testCase.add_cleanup(async () => {
await cookieStore.delete('');
});
await cookieStore.delete({ name: '' });
const cookie = await cookieStore.get('');
assert_equals(cookie, null);
}, 'cookieStore.delete with empty name in options');
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment