From df95340f0c80795fc5d71302fb4e6ad00c38b96d Mon Sep 17 00:00:00 2001 From: Jonas Termansen Date: Thu, 23 May 2019 10:05:36 +0000 Subject: [PATCH] [dart:_http] Fix parsing of empty cookies and double quotes. Commit a9ad427 introduced a bug that assumed the cookie value was at least one character, but the cookie value can also be empty. RFC 6265 5.2 does not specify any special behavior for double quotes and as such they should be considered part of the value. This change stops stripping those double quotes and instead preserves them. The io/http_cookie_test test was skipped because it was considered flaky. This change dusts it off and tests the new behavior. This change adds the exact offsets and source to the FormatExceptions to help the caller understand why a malformed cookie was rejected. Fixes https://github.com/dart-lang/sdk/issues/33327 Fixes https://github.com/dart-lang/sdk/issues/35804 Change-Id: I3479ba48be5763c485bd3ca5b5d2d86d283df971 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/91221 Commit-Queue: Jonas Termansen Reviewed-by: Zach Anderson Reviewed-by: William Hesse --- CHANGELOG.md | 9 ++++++ sdk/lib/_http/http_headers.dart | 22 ++++++++++---- tests/standalone_2/io/http_cookie_test.dart | 30 ++++++++++++++----- tests/standalone_2/standalone_2_kernel.status | 1 - 4 files changed, 49 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fda523921d9..aaac854e089 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ ### Core library +#### `dart:io` + +* Fixed `Cookie` class interoperability with certain websites by allowing the + cookie values to be the empty string (Issue [35804][]) and not stripping + double quotes from the value (Issue [33327][]) in accordance with RFC 6265. + +[33327]: https://github.com/dart-lang/sdk/issues/33327 +[35804]: https://github.com/dart-lang/sdk/issues/35804 + ### Dart VM ### Tools diff --git a/sdk/lib/_http/http_headers.dart b/sdk/lib/_http/http_headers.dart index ea5eafff067..dc9ff8476a6 100644 --- a/sdk/lib/_http/http_headers.dart +++ b/sdk/lib/_http/http_headers.dart @@ -982,14 +982,24 @@ class _Cookie implements Cookie { codeUnit >= 127 || separators.indexOf(name[i]) >= 0) { throw new FormatException( - "Invalid character in cookie name, code unit: '$codeUnit'"); + "Invalid character in cookie name, code unit: '$codeUnit'", + name, + i); } } - if (value[0] == '"' && value[value.length - 1] == '"') { - value = value.substring(1, value.length - 1); + // Per RFC 6265, consider surrounding "" as part of the value, but otherwise + // double quotes are not allowed. + int start = 0; + int end = value.length; + if (2 <= value.length && + value.codeUnits[start] == 0x22 && + value.codeUnits[end - 1] == 0x22) { + start++; + end--; } - for (int i = 0; i < value.length; i++) { + + for (int i = start; i < end; i++) { int codeUnit = value.codeUnits[i]; if (!(codeUnit == 0x21 || (codeUnit >= 0x23 && codeUnit <= 0x2B) || @@ -997,7 +1007,9 @@ class _Cookie implements Cookie { (codeUnit >= 0x3C && codeUnit <= 0x5B) || (codeUnit >= 0x5D && codeUnit <= 0x7E))) { throw new FormatException( - "Invalid character in cookie value, code unit: '$codeUnit'"); + "Invalid character in cookie value, code unit: '$codeUnit'", + value, + i); } } } diff --git a/tests/standalone_2/io/http_cookie_test.dart b/tests/standalone_2/io/http_cookie_test.dart index 50cad66d990..bfe9302aeb2 100644 --- a/tests/standalone_2/io/http_cookie_test.dart +++ b/tests/standalone_2/io/http_cookie_test.dart @@ -2,8 +2,8 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -import "dart:async"; import "dart:io"; + import "package:expect/expect.dart"; void testCookies() { @@ -59,12 +59,28 @@ void testCookies() { } void testValidateCookieWithDoubleQuotes() { - try { - Cookie cookie = Cookie('key', '"double-quoted-value"'); - } catch (e) { - Expect.fail("Unexpected error $e.\n" - "Unable to parse cookie with value in double-quote characters."); - } + Expect.equals(Cookie('key', 'value').toString(), 'key=value; HttpOnly'); + Expect.equals(Cookie('key', '').toString(), 'key=; HttpOnly'); + Expect.equals(Cookie('key', '""').toString(), 'key=""; HttpOnly'); + Expect.equals(Cookie('key', '"value"').toString(), 'key="value"; HttpOnly'); + Expect.equals(Cookie.fromSetCookieValue('key=value; HttpOnly').toString(), + 'key=value; HttpOnly'); + Expect.equals( + Cookie.fromSetCookieValue('key=; HttpOnly').toString(), 'key=; HttpOnly'); + Expect.equals(Cookie.fromSetCookieValue('key=""; HttpOnly').toString(), + 'key=""; HttpOnly'); + Expect.equals(Cookie.fromSetCookieValue('key="value"; HttpOnly').toString(), + 'key="value"; HttpOnly'); + Expect.throwsFormatException(() => Cookie('key', '"')); + Expect.throwsFormatException(() => Cookie('key', '"""')); + Expect.throwsFormatException(() => Cookie('key', '"x""')); + Expect.throwsFormatException(() => Cookie('key', '"x"y"')); + Expect.throwsFormatException( + () => Cookie.fromSetCookieValue('key="; HttpOnly')); + Expect.throwsFormatException( + () => Cookie.fromSetCookieValue('key="""; HttpOnly')); + Expect.throwsFormatException( + () => Cookie.fromSetCookieValue('key="x""; HttpOnly')); } void main() { diff --git a/tests/standalone_2/standalone_2_kernel.status b/tests/standalone_2/standalone_2_kernel.status index 69cdf980f41..1441048b7f2 100644 --- a/tests/standalone_2/standalone_2_kernel.status +++ b/tests/standalone_2/standalone_2_kernel.status @@ -106,7 +106,6 @@ io/file_fuzz_test: RuntimeError, Pass io/http_client_connect_test: Skip # Flaky. io/http_close_test: Crash io/http_content_length_test: Skip # Flaky. -io/http_cookie_test: Skip # Flaky io/http_proxy_advanced_test: Skip # Flaky io/http_proxy_test: Skip # Flaky. io/http_response_deadline_test: Skip # Flaky.