sdk
sdk copied to clipboard
need more leniency in handling invalid Response cookies (avoid FormatException)
Dart 2.8.4 (Flutter 1.17.5) throws a FormatException when the cookie value in Set-Cookie header contains a space.
The following is from a Flutter app, but the underlying code throwing the error is in the Dart SDK.
I/flutter (24821): FormatException: Invalid character in cookie value, code unit: '32' (at character 16)
I/flutter (24821): cookievaluewith spaceinit
I/flutter (24821): ^
https://github.com/dart-lang/sdk/blob/master/sdk/lib/_http/http_headers.dart
for (int i = start; i < end; i++) {
int codeUnit = newValue.codeUnits[i];
if (!(codeUnit == 0x21 ||
(codeUnit >= 0x23 && codeUnit <= 0x2B) ||
(codeUnit >= 0x2D && codeUnit <= 0x3A) ||
(codeUnit >= 0x3C && codeUnit <= 0x5B) ||
(codeUnit >= 0x5D && codeUnit <= 0x7E))) {
throw new FormatException(
"Invalid character in cookie value, code unit: '$codeUnit'",
newValue,
i);
}
}
Spaces in cookie values (or names) do violate RFC 6265. No argument there.
However most clients comply with RFC 2965 instead (or even 2109), which is more lenient. Then they add additional leniency. As a Client, we sometimes have to deal with malformed responses, therefore leniency is good. Chrome and Firefox don't crash, they just accept the cookie with spaces (or other illegal characters) and deal with it. But with Dart, the error is thrown so low in the stack that any upstream Dart/Flutter library (Dio, http) can't catch the error, at least not with access to the rest of the Response.
Http libraries in other languages are more accommodating with the characters they accept, and also by avoiding throwing an error if possible, in favor of returning NULL or something else:
OKHttp (Java):
val cookieValue = setCookie.trimSubstring(pairEqualsSign + 1, cookiePairEnd)
if (cookieValue.indexOfControlOrNonAscii() != -1) return null
and indexOfControlOrNonAscii contains:
indexOfControlOrNonAscii: if (c <= '\u001f' || c >= '\u007f')
(space is char \u0020 or 0x20, thus not rejected by okhttp)
source-1
source-2
urllib (Python) as of April 2020:
# These quoting routines conform to the RFC2109 specification, which in # turn references the character definitions from RFC2068. They provide # a two-way quoting algorithm. Any non-text character is translated # into a 4 character sequence: a forward-slash followed by the # three-digit octal equivalent of the character. Any '' or '"' is # quoted with a preceding '' slash. # Because of the way browsers really handle cookies (as opposed to what # the RFC says) we also encode "," and ";". # # These are taken from RFC2068 and RFC2109. # _LegalChars is the list of chars which don't require "'s # _Translator hash-table for fast quoting #
_LegalChars = string.ascii_letters + string.digits + "!#$%&'*+-.^_`|~:"
_UnescapedChars = _LegalChars + ' ()/<=>?@[]{}'
Potential resolutions:
- Wrap cookie values with spaces (and some other illegal characters) in double-quotes. (what Python does)
- HTML encode (%20 or
) spaces inside cookie values - edit the line
if (!(codeUnit == 0x21 ||toif (!(codeUnit == 0x21 || codeUnit == 0x20 || - extract illegal characters from the cookie value and return the rest, i.e.
a;b c=>abc - Revert back to an earlier spec or clone the ruleset from another popular library/language
- Pass along untouched so a client library can deal with it along with access to the full response. Maybe log a warning or non-fatal error
- Other suggestions would be welcome. I think we all agree that if every web server would just follow the latest specs, everyone's lives could be made easier... unfortunately 20+ year old codebases are unlikely to change soon, since they work in popular browsers.
Similar issues have been raised before, and either partially fixed or not fixed: https://github.com/flutterchina/dio/issues/785 https://github.com/flutterchina/dio/issues/412 https://github.com/flutterchina/dio/issues/54 #37166 #33327 #33765
Thank you very much. 🎯 😊
Dart has attempted to build a library that adheres to recent IETF specs.
@sortie wrote in 2019:
Dart implements a strict version of RFC 6265 (HTTP State Management Mechanism) which standardizes the cookie format.
Unfortunately, some web servers out in the wild aren't fully compliant (or compliant to an older spec), which leads to unwanted and unnecessary failures in real-world use. To their credit, the Dart devs have recognized the need to "bend the rules" on occasion to improve real-world compatibility. The most notable example is with case-sensitive header names:
Header fields of HttpHeaders are case-insensitive according to specification. Implementation class _HttpHeaders will convert all header fields into lowercases by default. This is expected behavior. However, some servers do rely on cased header fields.
From RFC-2616 (1999):
Each header field consists of a name followed by a colon (":") and the field value. Field names are case-insensitive.
And case-insensitivity is confirmed in RFC-7230:
Each header field consists of a case-insensitive field name
However, despite the clarity of the specs, the Dart developer community allowed for preservation of header casing.
[Breaking Change Request] HttpHeaders allows cased header fields #39657
Update packages with HttpHeaders' preserveHeaderCase change https://github.com/dart-lang/http_multi_server/pull/21
I believe that invalid cookie values, which are also out of the client's control, deserve similar treatment. Perhaps a strictCookieRules flag (on by default) would, just like preserveHeaderValues, allow the developer more control when communicating with a non-compliant server. Thank you for your consideration.
To follow up on this and offer a temporary workaround:
By default, validating cookies in Dart is handled by http_headers.dart, the implementation is the _Cookie class. The class is defined by the abstract Cookie class in http.dart. Because _Cookie is private, it cannot be extended except within its own module -- and of course, if you do extend or re-implement, any code that relies on default Cookie needs to be updated to point to your custom extension or implementation.
Side note: Some packages facilitate replacing parts of the default stack with a custom implementation. Dio, for example, makes it easy to use your own HttpClient simply by swapping out the DefaultHttpClientAdapter with your own. (the hard part, of course, is creating a custom HttpClient. Huge credit to @shaxxx who did exactly that with the "alt_http" module, without his example I could not have figured out how all of this work and create a solution)
In the case of cookies, at least with Dio, it is not the HttpClient that validates the cookies, but the cookie manager. Seeing that this is a single small file, creating an alternative implementation will be far easier than creating (and maintaining) a new HttpClient. Specifically, pointing the method Cookie.fromSetCookieValue to a custom implementation of Cookie will allow us to change how cookies are validated.
CookieManager class can be extended, rather than entirely re-implemented:
class MyCookieManager extends CookieManager {
MyCookieManager(CookieJar cookieJar) : super(cookieJar);
@override
Future onResponse(Response response) async => _saveCookies(response);
@override
Future onError(DioError err) async => _saveCookies(err.response);
_saveCookies(Response response) {
if (response != null && response.headers != null) {
List<String> cookies = response.headers[HttpHeaders.setCookieHeader];
if (cookies != null) {
cookieJar.saveFromResponse(
response.request.uri,
// we're changing this ↙ line only... to use a custom class for cookie parsing
cookies.map((str) => MyCookie.fromSetCookieValue(str)).toList(),
);
}
}
}
}
And for MyCookie, as noted earlier it can't be extended, we must re-implement. Fortunately we can just copy the code of _Cookie from http_headers, with the changes you want to implement:
class MyCookie implements Cookie {
... copied code ...
// make your changes wherever appropriate in the class
if (!(codeUnit == 0x21 || codeUnit == 0x20 || // [allows spaces in cookie values]
... copied code ...
}
And because that copied code relies on a date parser from http_date.dart, we need to implement that too:
DateTime _parseCookieDate(String date) {
... copied code ... // suggest not making any changes
}
You can do this all in a single file, or use part and part of to combine 3 physical files into one logical file to Dart.
The last step is to change your Dio interceptor from the default to our custom version:
Dio()..interceptors.add(MyCookieManager(cookieJar))
Hopefully this will help anyone else finding themselves communicating with a non-compliant web server. I stated earlier that @shaxxx's example were hugely helpful, perhaps someone will find this post useful as well. This example is based on Dio, but it should be possible to adapt to any package that leverages the built-in Dart SDK HttpClient or associated classes (like Cookie).
Thanks for @2x2xplz, I have worked around this distressing problem. For that answer, I would like to provide a revised version for newer Dio and Dart, with some null check and arguments.
class MyCookieManager extends CookieManager {
MyCookieManager(CookieJar cookieJar) : super(cookieJar);
@override
Future onResponse(
Response response, ResponseInterceptorHandler handler) async =>
_saveCookies(response);
@override
Future onError(DioError err, ErrorInterceptorHandler handler) async =>
_saveCookies(err.response);
_saveCookies(Response? response) {
if (response != null) {
List<String>? cookies = response.headers[HttpHeaders.setCookieHeader];
if (cookies != null) {
cookieJar.saveFromResponse(
response.requestOptions.uri,
// we're changing this ↙ line only... to use a custom class for cookie parsing
cookies.map((str) => MyCookie.fromSetCookieValue(str)).toList(),
);
}
}
}
}
Hoping all of you watching at this have a nice day!(๑•̀ㅂ•́)و✧
This is the complete code based on the answers above:
import 'dart:io';
import 'package:cookie_jar/cookie_jar.dart';
import 'package:dio/dio.dart';
import 'package:dio_cookie_manager/dio_cookie_manager.dart';
class MyCookieManager extends CookieManager {
MyCookieManager(CookieJar cookieJar) : super(cookieJar);
@override
Future onResponse(Response response, ResponseInterceptorHandler handler) async => _saveCookies(response);
@override
Future onError(DioError err, ErrorInterceptorHandler handler) async => _saveCookies(err.response);
_saveCookies(Response? response) {
if (response != null) {
List<String>? cookies = response.headers[HttpHeaders.setCookieHeader];
if (cookies != null) {
cookieJar.saveFromResponse(
response.requestOptions.uri,
cookies.map((str) => MyCookie.fromSetCookieValue(str)).toList(),
);
}
}
}
}
class MyCookie implements Cookie {
String _name;
String _value;
DateTime? expires;
int? maxAge;
String? domain;
String? _path;
bool httpOnly = false;
bool secure = false;
MyCookie(String name, String value)
: _name = _validateName(name),
_value = _validateValue(value),
httpOnly = true;
String get name => _name;
String get value => _value;
String? get path => _path;
set path(String? newPath) {
_validatePath(newPath);
_path = newPath;
}
set name(String newName) {
_validateName(newName);
_name = newName;
}
set value(String newValue) {
_validateValue(newValue);
_value = newValue;
}
MyCookie.fromSetCookieValue(String value)
: _name = "",
_value = "" {
_parseSetCookieValue(value);
}
// Parse a 'set-cookie' header value according to the rules in RFC 6265.
void _parseSetCookieValue(String s) {
int index = 0;
bool done() => index == s.length;
String parseName() {
int start = index;
while (!done()) {
if (s[index] == "=") break;
index++;
}
return s.substring(start, index).trim();
}
String parseValue() {
int start = index;
while (!done()) {
if (s[index] == ";") break;
index++;
}
return s.substring(start, index).trim();
}
void parseAttributes() {
String parseAttributeName() {
int start = index;
while (!done()) {
if (s[index] == "=" || s[index] == ";") break;
index++;
}
return s.substring(start, index).trim().toLowerCase();
}
String parseAttributeValue() {
int start = index;
while (!done()) {
if (s[index] == ";") break;
index++;
}
return s.substring(start, index).trim().toLowerCase();
}
while (!done()) {
String name = parseAttributeName();
String value = "";
if (!done() && s[index] == "=") {
index++; // Skip the = character.
value = parseAttributeValue();
}
if (name == "expires") {
expires = _parseCookieDate(value);
} else if (name == "max-age") {
maxAge = int.parse(value);
} else if (name == "domain") {
domain = value;
} else if (name == "path") {
path = value;
} else if (name == "httponly") {
httpOnly = true;
} else if (name == "secure") {
secure = true;
}
if (!done()) index++; // Skip the ; character
}
}
_name = _validateName(parseName());
if (done() || _name.isEmpty) {
throw HttpException("Failed to parse header value [$s]");
}
index++; // Skip the = character.
_value = _validateValue(parseValue());
if (done()) return;
index++; // Skip the ; character.
parseAttributes();
}
String toString() {
StringBuffer sb = StringBuffer();
sb
..write(_name)
..write("=")
..write(_value);
var expires = this.expires;
if (expires != null) {
sb
..write("; Expires=")
..write(HttpDate.format(expires));
}
if (maxAge != null) {
sb
..write("; Max-Age=")
..write(maxAge);
}
if (domain != null) {
sb
..write("; Domain=")
..write(domain);
}
if (path != null) {
sb
..write("; Path=")
..write(path);
}
if (secure) sb.write("; Secure");
if (httpOnly) sb.write("; HttpOnly");
return sb.toString();
}
static String _validateName(String newName) {
const separators = ["(", ")", "<", ">", "@", ",", ";", ":", "\\", '"', "/", "[", "]", "?", "=", "{", "}"];
if (newName == null) throw ArgumentError.notNull("name");
for (int i = 0; i < newName.length; i++) {
int codeUnit = newName.codeUnitAt(i);
if (codeUnit <= 32 || codeUnit >= 127 || separators.contains(newName[i])) {
throw FormatException("Invalid character in cookie name, code unit: '$codeUnit'", newName, i);
}
}
return newName;
}
static String _validateValue(String newValue) {
if (newValue == null) throw 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 = 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 = newValue.codeUnits[i];
if (!(codeUnit == 0x21 || codeUnit == 0x20 || (codeUnit >= 0x23 && codeUnit <= 0x2B) || (codeUnit >= 0x2D && codeUnit <= 0x3A) || (codeUnit >= 0x3C && codeUnit <= 0x5B) || (codeUnit >= 0x5D && codeUnit <= 0x7E))) {
throw FormatException("Invalid character in cookie value, code unit: '$codeUnit'", newValue, i);
}
}
return newValue;
}
static void _validatePath(String? path) {
if (path == null) return;
for (int i = 0; i < path.length; i++) {
int codeUnit = path.codeUnitAt(i);
// According to RFC 6265, semicolon and controls should not occur in the
// path.
// path-value = <any CHAR except CTLs or ";">
// CTLs = %x00-1F / %x7F
if (codeUnit < 0x20 || codeUnit >= 0x7f || codeUnit == 0x3b /*;*/) {
throw FormatException("Invalid character in cookie path, code unit: '$codeUnit'");
}
}
}
static DateTime _parseCookieDate(String date) {
const List monthsLowerCase = ["jan", "feb", "mar", "apr", "may", "jun", "jul", "aug", "sep", "oct", "nov", "dec"];
int position = 0;
Never error() {
throw HttpException("Invalid cookie date $date");
}
bool isEnd() => position == date.length;
bool isDelimiter(String s) {
int char = s.codeUnitAt(0);
if (char == 0x09) return true;
if (char >= 0x20 && char <= 0x2F) return true;
if (char >= 0x3B && char <= 0x40) return true;
if (char >= 0x5B && char <= 0x60) return true;
if (char >= 0x7B && char <= 0x7E) return true;
return false;
}
bool isNonDelimiter(String s) {
int char = s.codeUnitAt(0);
if (char >= 0x00 && char <= 0x08) return true;
if (char >= 0x0A && char <= 0x1F) return true;
if (char >= 0x30 && char <= 0x39) return true; // Digit
if (char == 0x3A) return true; // ':'
if (char >= 0x41 && char <= 0x5A) return true; // Alpha
if (char >= 0x61 && char <= 0x7A) return true; // Alpha
if (char >= 0x7F && char <= 0xFF) return true; // Alpha
return false;
}
bool isDigit(String s) {
int char = s.codeUnitAt(0);
if (char > 0x2F && char < 0x3A) return true;
return false;
}
int getMonth(String month) {
if (month.length < 3) return -1;
return monthsLowerCase.indexOf(month.substring(0, 3));
}
int toInt(String s) {
int index = 0;
for (; index < s.length && isDigit(s[index]); index++);
return int.parse(s.substring(0, index));
}
var tokens = <String>[];
while (!isEnd()) {
while (!isEnd() && isDelimiter(date[position])) position++;
int start = position;
while (!isEnd() && isNonDelimiter(date[position])) position++;
tokens.add(date.substring(start, position).toLowerCase());
while (!isEnd() && isDelimiter(date[position])) position++;
}
String? timeStr;
String? dayOfMonthStr;
String? monthStr;
String? yearStr;
for (var token in tokens) {
if (token.isEmpty) continue;
if (timeStr == null && token.length >= 5 && isDigit(token[0]) && (token[1] == ":" || (isDigit(token[1]) && token[2] == ":"))) {
timeStr = token;
} else if (dayOfMonthStr == null && isDigit(token[0])) {
dayOfMonthStr = token;
} else if (monthStr == null && getMonth(token) >= 0) {
monthStr = token;
} else if (yearStr == null && token.length >= 2 && isDigit(token[0]) && isDigit(token[1])) {
yearStr = token;
}
}
if (timeStr == null || dayOfMonthStr == null || monthStr == null || yearStr == null) {
error();
}
int year = toInt(yearStr);
if (year >= 70 && year <= 99)
year += 1900;
else if (year >= 0 && year <= 69) year += 2000;
if (year < 1601) error();
int dayOfMonth = toInt(dayOfMonthStr);
if (dayOfMonth < 1 || dayOfMonth > 31) error();
int month = getMonth(monthStr) + 1;
var timeList = timeStr.split(":");
if (timeList.length != 3) error();
int hour = toInt(timeList[0]);
int minute = toInt(timeList[1]);
int second = toInt(timeList[2]);
if (hour > 23) error();
if (minute > 59) error();
if (second > 59) error();
return DateTime.utc(year, month, dayOfMonth, hour, minute, second, 0);
}
}
I have a same problem with Dio and Cookie manager, I implemented the final code and it does not give me any response, any solution?