[dart:_http] Require Cookie name and value to be valid.
This is a breaking change. https://github.com/dart-lang/sdk/issues/37192
This change makes the name and value positional optional parameters in the
Cookie class constructor mandatory by changing the signature from
Cookie([String name, String value])
to
Cookie(String name, String value)
The parameters were already effectively mandatory as a bug introduced in
Dart 1.3.0 (2014) meant the name and value parameters could not be null, and
any such uses already threw a noSuchMethod exception because null did not
have a length getter. As such, this is not a breaking change but adopts the
current behavior as a null name and value was already of questionable use.
Breaking change: This change adds validation to the String name and String
value setters, which had not been validating the fields at all, unlike the
constructor. This also forbids the name and value from being set to null.
That meant potentially invalid cookies could be sent to servers if the
cookie was modified after construction. This change adds the validation to
follow the rule of least surprise.
The documentation has been updated accordingly and improved a bit.
Closes https://github.com/dart-lang/sdk/issues/37192
Closes https://github.com/dart-lang/sdk/issues/29463
Change-Id: Iffed3dc265ca9c68142c4372522913f9d1ff4d51
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/103840
Commit-Queue: Jonas Termansen <sortie@google.com>
Reviewed-by: Lasse R.H. Nielsen <lrn@google.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ddc6377..dac2a39 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -50,6 +50,32 @@
[29456]: https://github.com/dart-lang/sdk/issues/29456
+#### `dart:io`
+
+* **Breaking Change:** The `Cookie` class's constructor's `name` and `value`
+ optional positional parameters are now mandatory (Issue [37192][]). The
+ signature changes from:
+
+ Cookie([String name, String value])
+
+ to
+
+ Cookie(String name, String value)
+
+ However, it has not been possible to set `name` and `value` to null since Dart
+ 1.3.0 (2014) where a bug made it impossible. Any code not using both
+ parameters or setting any to null would necessarily get a noSuchMethod
+ exception at runtime. This change catches such erroneous uses at compile time.
+ Since code could not previously correctly omit the parameters, this is not
+ really a breaking change.
+
+* **Breaking Change:** The `Cookie` class's `name` and `value` setters now
+ validates that the strings are made from the allowed character set and are not
+ null (Issue [37192][]). The constructor already made these checks and this
+ fixes the loophole where the setters didn't also validate.
+
+[37192]: https://github.com/dart-lang/sdk/issues/37192
+
### Dart VM
### Tools
diff --git a/sdk/lib/_http/http.dart b/sdk/lib/_http/http.dart
index 1e2560f..a759348 100644
--- a/sdk/lib/_http/http.dart
+++ b/sdk/lib/_http/http.dart
@@ -912,60 +912,78 @@
}
/**
- * Representation of a cookie. For cookies received by the server as
- * Cookie header values only [:name:] and [:value:] fields will be
- * set. When building a cookie for the 'set-cookie' header in the server
- * and when receiving cookies in the client as 'set-cookie' headers all
- * fields can be used.
+ * Representation of a cookie. For cookies received by the server as Cookie
+ * header values only [name] and [value] properties will be set. When building a
+ * cookie for the 'set-cookie' header in the server and when receiving cookies
+ * in the client as 'set-cookie' headers all fields can be used.
*/
abstract class Cookie {
/**
- * Gets and sets the name.
+ * The name of the cookie.
+ *
+ * Must be a `token` as specified in RFC 6265.
+ *
+ * The allowed characters in a `token` are the visible ASCII characters,
+ * U+0021 (`!`) through U+007E (`~`), except the separator characters:
+ * `(`, `)`, `<`, `>`, `@`, `,`, `;`, `:`, `\`, `"`, `/`, `[`, `]`, `?`, `=`,
+ * `{`, and `}`.
*/
String name;
/**
- * Gets and sets the value.
+ * The value of the cookie.
+ *
+ * Must be a `cookie-value` as specified in RFC 6265.
+ *
+ * The allowed characters in a cookie value are the visible ASCII characters,
+ * U+0021 (`!`) through U+007E (`~`) except the characters:
+ * `"`, `,`, `;` and `\`.
+ * Cookie values may be wrapped in a single pair of double quotes
+ * (U+0022, `"`).
*/
String value;
/**
- * Gets and sets the expiry date.
+ * The time at which the cookie expires.
*/
DateTime expires;
/**
- * Gets and sets the max age. A value of [:0:] means delete cookie
- * now.
+ * The number of seconds until the cookie expires. A zero or negative value
+ * means the cookie has expired.
*/
int maxAge;
/**
- * Gets and sets the domain.
+ * The domain the cookie applies to.
*/
String domain;
/**
- * Gets and sets the path.
+ * The path within the [domain] the cookie applies to.
*/
String path;
/**
- * Gets and sets whether this cookie is secure.
+ * Whether to only send this cookie on secure connections.
*/
bool secure;
/**
- * Gets and sets whether this cookie is HTTP only.
+ * Whether the cookie is only sent in the HTTP request and is not made
+ * available to client side scripts.
*/
bool httpOnly;
/**
- * Creates a new cookie optionally setting the name and value.
+ * Creates a new cookie setting the name and value.
+ *
+ * [name] and [value] must be composed of valid characters according to RFC
+ * 6265.
*
* By default the value of `httpOnly` will be set to `true`.
*/
- factory Cookie([String name, String value]) => new _Cookie(name, value);
+ factory Cookie(String name, String value) => new _Cookie(name, value);
/**
* Creates a new cookie by parsing a header value from a 'set-cookie'
diff --git a/sdk/lib/_http/http_headers.dart b/sdk/lib/_http/http_headers.dart
index dc9ff84..7601485 100644
--- a/sdk/lib/_http/http_headers.dart
+++ b/sdk/lib/_http/http_headers.dart
@@ -829,8 +829,8 @@
}
class _Cookie implements Cookie {
- String name;
- String value;
+ String _name;
+ String _value;
DateTime expires;
int maxAge;
String domain;
@@ -838,10 +838,22 @@
bool httpOnly = false;
bool secure = false;
- _Cookie([this.name, this.value]) {
- // Default value of httponly is true.
- httpOnly = true;
- _validate();
+ _Cookie(String name, String value)
+ : _name = _validateName(name),
+ _value = _validateValue(value),
+ httpOnly = true;
+
+ String get name => _name;
+ String get value => _value;
+
+ set name(String newName) {
+ _validateName(newName);
+ _name = newName;
+ }
+
+ set value(String newValue) {
+ _validateValue(newValue);
+ _value = newValue;
}
_Cookie.fromSetCookieValue(String value) {
@@ -924,13 +936,12 @@
}
}
- name = parseName();
- if (done() || name.length == 0) {
+ _name = _validateName(parseName());
+ if (done() || _name.length == 0) {
throw new HttpException("Failed to parse header value [$s]");
}
index++; // Skip the = character.
- value = parseValue();
- _validate();
+ _value = _validateValue(parseValue());
if (done()) return;
index++; // Skip the ; character.
parseAttributes();
@@ -938,7 +949,7 @@
String toString() {
StringBuffer sb = new StringBuffer();
- sb..write(name)..write("=")..write(value);
+ sb..write(_name)..write("=")..write(_value);
if (expires != null) {
sb..write("; Expires=")..write(HttpDate.format(expires));
}
@@ -956,7 +967,7 @@
return sb.toString();
}
- void _validate() {
+ static String _validateName(String newName) {
const separators = const [
"(",
")",
@@ -976,31 +987,36 @@
"{",
"}"
];
- for (int i = 0; i < name.length; i++) {
- int codeUnit = name.codeUnits[i];
+ if (newName == null) throw new ArgumentError.notNull("name");
+ for (int i = 0; i < newName.length; i++) {
+ int codeUnit = newName.codeUnits[i];
if (codeUnit <= 32 ||
codeUnit >= 127 ||
- separators.indexOf(name[i]) >= 0) {
+ separators.indexOf(newName[i]) >= 0) {
throw new FormatException(
"Invalid character in cookie name, code unit: '$codeUnit'",
- name,
+ newName,
i);
}
}
+ return newName;
+ }
+ static String _validateValue(String newValue) {
+ if (newValue == null) throw new ArgumentError.notNull("value");
// 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) {
+ int end = newValue.length;
+ if (2 <= newValue.length &&
+ newValue.codeUnits[start] == 0x22 &&
+ newValue.codeUnits[end - 1] == 0x22) {
start++;
end--;
}
for (int i = start; i < end; i++) {
- int codeUnit = value.codeUnits[i];
+ int codeUnit = newValue.codeUnits[i];
if (!(codeUnit == 0x21 ||
(codeUnit >= 0x23 && codeUnit <= 0x2B) ||
(codeUnit >= 0x2D && codeUnit <= 0x3A) ||
@@ -1008,9 +1024,10 @@
(codeUnit >= 0x5D && codeUnit <= 0x7E))) {
throw new FormatException(
"Invalid character in cookie value, code unit: '$codeUnit'",
- value,
+ newValue,
i);
}
}
+ return newValue;
}
}
