Commit 898306df authored by Joshua Bell's avatar Joshua Bell Committed by Commit Bot

Cookie Store: disallow '=' in cookies with empty names

Per the explainer[1] and tests: "Cookies with an empty name cannot be
set using values containing = as this would result in ambiguous
serializations in the majority of current browsers."

Also, fix a test glitch now that this restriction is implemented.

[1] https://github.com/WICG/cookie-store/blob/gh-pages/explainer.md

Bug: 729800
Change-Id: I9ed02885c217cbdb4c86d8fd236d49c6c56b6e96
Reviewed-on: https://chromium-review.googlesource.com/994145
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: default avatarVictor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548563}
parent f60afee8
This is a testharness.js-based test.
FAIL Verify that attempting to set a cookie with no name and with '=' in the value does not work. assert_unreached: Should have rejected: Expected promise rejection when setting a cookie with no name and "=" in value Reached unreachable code
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Verify that attempting to set a cookie with no name and with '=' in the value does not work. assert_unreached: Should have rejected: Expected promise rejection when setting a cookie with no name and "=" in value Reached unreachable code
Harness: the test ran to completion.
...@@ -3,10 +3,10 @@ ...@@ -3,10 +3,10 @@
cookie_test(async t => { cookie_test(async t => {
let eventPromise = observeNextCookieChangeEvent(); let eventPromise = observeNextCookieChangeEvent();
await cookieStore.set('', 'first-value'); await cookieStore.set('', 'first-value');
const actual1 = assert_equals(
(await cookieStore.getAll('')).map(({ value }) => value).join(';'); (await cookieStore.getAll('')).map(({ value }) => value).join(';'),
const expected1 = 'first-value'; 'first-value',
assert_equals(actual1, expected1); 'Cookie with no name and normal value should have been set');
await verifyCookieChangeEvent( await verifyCookieChangeEvent(
eventPromise, {changed: [{name: '', value: 'first-value'}]}, eventPromise, {changed: [{name: '', value: 'first-value'}]},
'Observed no-name change'); 'Observed no-name change');
...@@ -16,31 +16,26 @@ cookie_test(async t => { ...@@ -16,31 +16,26 @@ cookie_test(async t => {
new TypeError(), new TypeError(),
cookieStore.set('', 'suspicious-value=resembles-name-and-value'), cookieStore.set('', 'suspicious-value=resembles-name-and-value'),
'Expected promise rejection when setting a cookie with' + 'Expected promise rejection when setting a cookie with' +
' no name and "=" in value'); ' no name and "=" in value (via arguments)');
await promise_rejects(
t,
new TypeError(),
cookieStore.set(
{name: '', value: 'suspicious-value=resembles-name-and-value'}),
'Expected promise rejection when setting a cookie with' +
' no name and "=" in value (via options)');
const actual2 =
(await cookieStore.getAll('')).map(({ value }) => value).join(';');
const expected2 = 'first-value';
assert_equals(actual2, expected2);
assert_equals( assert_equals(
await getCookieString(), (await cookieStore.getAll('')).map(({ value }) => value).join(';'),
'first-value', 'first-value',
'Earlier cookie jar after rejected'); 'Cookie with no name should still have previous value');
eventPromise = observeNextCookieChangeEvent(); eventPromise = observeNextCookieChangeEvent();
await cookieStore.delete(''); await cookieStore.delete('');
await verifyCookieChangeEvent( await verifyCookieChangeEvent(
eventPromise, {deleted: [{name: '', value: ''}]}, eventPromise, {deleted: [{name: ''}]},
'Observed no-name deletion'); 'Observed no-name deletion');
assert_equals(
await getCookieString(),
undefined,
'Empty cookie jar after cleanup');
assert_equals(
await getCookieStringHttp(),
undefined,
'Empty HTTP cookie jar after cleanup');
}, "Verify that attempting to set a cookie with no name and with '=' in" + }, "Verify that attempting to set a cookie with no name and with '=' in" +
" the value does not work."); " the value does not work.");
...@@ -127,6 +127,13 @@ network::mojom::blink::CanonicalCookiePtr ToCanonicalCookie( ...@@ -127,6 +127,13 @@ network::mojom::blink::CanonicalCookiePtr ToCanonicalCookie(
canonical_cookie->value = value; canonical_cookie->value = value;
} }
if (canonical_cookie->name.IsEmpty() &&
canonical_cookie->value.Contains('=')) {
exception_state.ThrowTypeError(
"Cookie value cannot contain '=' if the name is empty.");
return nullptr;
}
if (options.hasExpires()) if (options.hasExpires())
canonical_cookie->expiry = WTF::Time::FromJavaTime(options.expires()); canonical_cookie->expiry = WTF::Time::FromJavaTime(options.expires());
// The expires option is not set in CookieStoreSetOptions for session // The expires option is not set in CookieStoreSetOptions for session
......
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