Skip to content

Commit 96cf889

Browse files
sortiecommit-bot@chromium.org
sortie
authored and
[email protected]
committed
[dart:io] Fix HeaderValue parsing, toString(), and support null values.
This is a breaking change. #40709 This change makes the HeaderValue parsing more strict in two invalid edge cases, supports parameters with null values as a feature, and fixes toString() so it always produces tokens or quoted-strings valid per RFC 7230 3.2.6. The empty parameter value without double quotes (which is not allowed by the standards) is now parsed as the empty string rather than null. E.g. HeaderValue.parse("v;a=").parameters now gives {"a": ""} rather than {"a": null}. Invalid inputs with unbalanced double quotes are now rejected. E.g. HeaderValue.parse('v;a="b').parameters will now throw a HttpException instead of giving {"a": "b"}. The HeaderValue.toString() method now supports parameters with null values by omitting the value. E.g.: HeaderValue("v", {"a": null, "b": "c"}).toString() now gives v; a; b=c This behavior can be used to implement some features in the Accept and Sec-WebSocket-Extensions headers. Likewise the empty value and values using characters outside of RFC 7230 3.2.6 tokens are now correctly implemented by double quoting such values with escape sequences. E.g.: HeaderValue("v", {"a": "A", "b": "(B)", "c": "", "d": "ø", "e": "\\\""}).toString() now gives v;a=A;b="(B)";c="";d="ø";e="\\\"" The NNBD migration required making subtle changes to some dart:io semantics in order to provide a better API. This change backports one of these semantic changes to the unmigrated SDK so any issues can be discovered now instead of blocking the future SDK unfork. Change-Id: Iafc790e03b6290232cac71fe14f995ce0f0b036b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/136620 Commit-Queue: Jonas Termansen <[email protected]> Reviewed-by: Lasse R.H. Nielsen <[email protected]>
1 parent 5f8802b commit 96cf889

File tree

9 files changed

+276
-94
lines changed

9 files changed

+276
-94
lines changed

‎CHANGELOG.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,37 @@ used (see Issue [39627][]).
7474
now contains Unix epoch timestamps instead of `null` for the `accessed`,
7575
`changed`, and `modified` getters.
7676

77+
* **Breaking change** [#40709](https://github.com/dart-lang/sdk/issues/40709):
78+
The `HeaderValue` class now parses more strictly in two invalid edge cases.
79+
This is the class used to parse the semicolon delimited parameters used in the
80+
`Accept`, `Authorization`, `Content-Type`, and other such HTTP headers.
81+
82+
The empty parameter value without double quotes (which is not allowed by the
83+
standards) is now parsed as the empty string rather than `null`. E.g.
84+
`HeaderValue.parse("v;a=").parameters` now gives `{"a": ""}` rather than
85+
`{"a": null}`.
86+
87+
Invalid inputs with unbalanced double quotes are now rejected. E.g.
88+
`HeaderValue.parse('v;a="b').parameters` will now throw a `HttpException`
89+
instead of giving `{"a": "b"}`.
90+
91+
* The `HeaderValue.toString()` method now supports parameters with `null` values
92+
by omitting the value. `HeaderValue("v", {"a": null, "b": "c"}).toString()`
93+
now gives `v; a; b=c`. This behavior can be used to implement some features in
94+
the `Accept` and `Sec-WebSocket-Extensions` headers.
95+
96+
Likewise the empty value and values using characters outside of
97+
[RFC 7230 tokens](https://tools.ietf.org/html/rfc7230#section-3.2.6) are now
98+
correctly implemented by double quoting such values with escape sequences.
99+
E.g:
100+
101+
```dart
102+
HeaderValue("v",
103+
{"a": "A", "b": "(B)", "c": "", "d": "ø", "e": "\\\""}).toString()
104+
```
105+
106+
now gives `v;a=A;b="(B)";c="";d="ø";e="\\\""`.
107+
77108
#### `dart:mirrors`
78109

79110
* Added `MirrorSystem.neverType`.

‎pkg/dev_compiler/tool/dart2js_nnbd_sdk_error_golden.txt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ ERROR|COMPILE_TIME_ERROR|INCONSISTENT_INHERITANCE|lib/_internal/js_runtime/lib/i
66
ERROR|COMPILE_TIME_ERROR|INCONSISTENT_INHERITANCE|lib/_internal/js_runtime/lib/interceptors.dart|1637|7|5|Superinterfaces don't have a valid override for '>>': int.>> (int Function(int)), JSNumber.>> (num Function(num)).
77
ERROR|COMPILE_TIME_ERROR|INCONSISTENT_INHERITANCE|lib/_internal/js_runtime/lib/interceptors.dart|1637|7|5|Superinterfaces don't have a valid override for '\|': int.\| (int Function(int)), JSNumber.\| (num Function(num)).
88
ERROR|COMPILE_TIME_ERROR|INCONSISTENT_INHERITANCE|lib/_internal/js_runtime/lib/interceptors.dart|1637|7|5|Superinterfaces don't have a valid override for '^': int.^ (int Function(int)), JSNumber.^ (num Function(num)).
9-
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1004|10|5|Non-nullable instance field 'value' must be initialized.
10-
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1030|8|6|Non-nullable instance field 'secure' must be initialized.
11-
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1036|8|8|Non-nullable instance field 'httpOnly' must be initialized.
12-
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1493|12|11|Non-nullable instance field 'idleTimeout' must be initialized.
13-
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1543|8|14|Non-nullable instance field 'autoUncompress' must be initialized.
9+
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1010|10|5|Non-nullable instance field 'value' must be initialized.
10+
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1036|8|6|Non-nullable instance field 'secure' must be initialized.
11+
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1042|8|8|Non-nullable instance field 'httpOnly' must be initialized.
12+
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1499|12|11|Non-nullable instance field 'idleTimeout' must be initialized.
13+
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1549|8|14|Non-nullable instance field 'autoUncompress' must be initialized.
1414
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|172|8|12|Non-nullable instance field 'autoCompress' must be initialized.
15-
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|991|10|4|Non-nullable instance field 'name' must be initialized.
15+
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|997|10|4|Non-nullable instance field 'name' must be initialized.
1616
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/io/io.dart|5589|12|8|Non-nullable instance field 'encoding' must be initialized.
1717
ERROR|STATIC_TYPE_WARNING|UNDEFINED_OPERATOR|lib/_internal/js_runtime/lib/interceptors.dart|1654|28|1|The operator '&' isn't defined for the type 'JSInt'.
1818
ERROR|STATIC_TYPE_WARNING|UNDEFINED_OPERATOR|lib/_internal/js_runtime/lib/interceptors.dart|1656|27|1|The operator '&' isn't defined for the type 'JSInt'.

‎pkg/dev_compiler/tool/dartdevc_nnbd_sdk_error_golden.txt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ ERROR|COMPILE_TIME_ERROR|BODY_MIGHT_COMPLETE_NORMALLY|lib/_internal/js_dev_runti
1515
ERROR|COMPILE_TIME_ERROR|BODY_MIGHT_COMPLETE_NORMALLY|lib/_internal/js_dev_runtime/private/foreign_helper.dart|221|8|37|The body might complete normally, which would cause 'null' to be returned, but the return type is a potentially non-nullable type.
1616
ERROR|COMPILE_TIME_ERROR|BODY_MIGHT_COMPLETE_NORMALLY|lib/_internal/js_dev_runtime/private/foreign_helper.dart|224|8|11|The body might complete normally, which would cause 'null' to be returned, but the return type is a potentially non-nullable type.
1717
ERROR|COMPILE_TIME_ERROR|BODY_MIGHT_COMPLETE_NORMALLY|lib/_internal/js_dev_runtime/private/foreign_helper.dart|228|6|11|The body might complete normally, which would cause 'null' to be returned, but the return type is a potentially non-nullable type.
18-
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1004|10|5|Non-nullable instance field 'value' must be initialized.
19-
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1030|8|6|Non-nullable instance field 'secure' must be initialized.
20-
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1036|8|8|Non-nullable instance field 'httpOnly' must be initialized.
21-
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1493|12|11|Non-nullable instance field 'idleTimeout' must be initialized.
22-
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1543|8|14|Non-nullable instance field 'autoUncompress' must be initialized.
18+
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1010|10|5|Non-nullable instance field 'value' must be initialized.
19+
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1036|8|6|Non-nullable instance field 'secure' must be initialized.
20+
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1042|8|8|Non-nullable instance field 'httpOnly' must be initialized.
21+
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1499|12|11|Non-nullable instance field 'idleTimeout' must be initialized.
22+
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|1549|8|14|Non-nullable instance field 'autoUncompress' must be initialized.
2323
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|172|8|12|Non-nullable instance field 'autoCompress' must be initialized.
24-
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|991|10|4|Non-nullable instance field 'name' must be initialized.
24+
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/_http/http.dart|997|10|4|Non-nullable instance field 'name' must be initialized.
2525
ERROR|COMPILE_TIME_ERROR|NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD|lib/io/io.dart|5589|12|8|Non-nullable instance field 'encoding' must be initialized.

‎sdk/lib/_http/http.dart

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,11 @@ abstract class HttpHeaders {
753753
* [HeaderValue] can be used to conveniently build and parse header
754754
* values on this form.
755755
*
756+
* Parameter values can be omitted, in which case the value is parsed as `null`.
757+
* Values can be doubled quoted to allow characters outside of the RFC 7230
758+
* token characters and backslash sequences can be used to represent the double
759+
* quote and backslash characters themselves.
760+
*
756761
* To build an [:accepts:] header with the value
757762
*
758763
* text/plain; q=0.3, text/html
@@ -779,7 +784,8 @@ abstract class HeaderValue {
779784
/**
780785
* Creates a new header value object setting the value and parameters.
781786
*/
782-
factory HeaderValue([String value = "", Map<String, String> parameters]) {
787+
factory HeaderValue(
788+
[String value = "", Map<String, String> parameters = const {}]) {
783789
return new _HeaderValue(value, parameters);
784790
}
785791

@@ -892,7 +898,7 @@ abstract class ContentType implements HeaderValue {
892898
* or in `parameters`, will have its value converted to lower-case.
893899
*/
894900
factory ContentType(String primaryType, String subType,
895-
{String charset, Map<String, String> parameters}) {
901+
{String charset, Map<String, String> parameters = const {}}) {
896902
return new _ContentType(primaryType, subType, charset, parameters);
897903
}
898904

‎sdk/lib/_http/http_headers.dart

Lines changed: 60 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -641,8 +641,8 @@ class _HeaderValue implements HeaderValue {
641641
Map<String, String> _parameters;
642642
Map<String, String> _unmodifiableParameters;
643643

644-
_HeaderValue([this._value = "", Map<String, String> parameters]) {
645-
if (parameters != null) {
644+
_HeaderValue([this._value = "", Map<String, String> parameters = const {}]) {
645+
if (parameters != null && parameters.isNotEmpty) {
646646
_parameters = new HashMap<String, String>.from(parameters);
647647
}
648648
}
@@ -659,26 +659,53 @@ class _HeaderValue implements HeaderValue {
659659

660660
String get value => _value;
661661

662-
void _ensureParameters() {
663-
if (_parameters == null) {
664-
_parameters = new HashMap<String, String>();
665-
}
666-
}
662+
Map<String, String> _ensureParameters() => _parameters ??= <String, String>{};
663+
664+
Map<String, String> get parameters =>
665+
_unmodifiableParameters ??= UnmodifiableMapView(_ensureParameters());
667666

668-
Map<String, String> get parameters {
669-
_ensureParameters();
670-
if (_unmodifiableParameters == null) {
671-
_unmodifiableParameters = new UnmodifiableMapView(_parameters);
667+
static bool _isToken(String token) {
668+
if (token.isEmpty) {
669+
return false;
672670
}
673-
return _unmodifiableParameters;
671+
final delimiters = "\"(),/:;<=>?@[\]{}";
672+
for (int i = 0; i < token.length; i++) {
673+
int codeUnit = token.codeUnitAt(i);
674+
if (codeUnit <= 32 ||
675+
codeUnit >= 127 ||
676+
delimiters.indexOf(token[i]) >= 0) {
677+
return false;
678+
}
679+
}
680+
return true;
674681
}
675682

676683
String toString() {
677684
StringBuffer sb = new StringBuffer();
678685
sb.write(_value);
679686
if (parameters != null && parameters.length > 0) {
680687
_parameters.forEach((String name, String value) {
681-
sb..write("; ")..write(name)..write("=")..write(value);
688+
sb..write("; ")..write(name);
689+
if (value != null) {
690+
sb.write("=");
691+
if (_isToken(value)) {
692+
sb.write(value);
693+
} else {
694+
sb.write('"');
695+
int start = 0;
696+
for (int i = 0; i < value.length; i++) {
697+
// Can use codeUnitAt here instead.
698+
int codeUnit = value.codeUnitAt(i);
699+
if (codeUnit == 92 /* backslash */ ||
700+
codeUnit == 34 /* double quote */) {
701+
sb.write(value.substring(start, i));
702+
sb.write(r'\');
703+
start = i;
704+
}
705+
}
706+
sb..write(value.substring(start))..write('"');
707+
}
708+
}
682709
});
683710
}
684711
return sb.toString();
@@ -716,8 +743,12 @@ class _HeaderValue implements HeaderValue {
716743
index++;
717744
}
718745

719-
void maybeExpect(String expected) {
720-
if (s[index] == expected) index++;
746+
bool maybeExpect(String expected) {
747+
if (done() || !s.startsWith(expected, index)) {
748+
return false;
749+
}
750+
index++;
751+
return true;
721752
}
722753

723754
void parseParameters() {
@@ -753,16 +784,15 @@ class _HeaderValue implements HeaderValue {
753784
index++;
754785
} else if (s[index] == "\"") {
755786
index++;
756-
break;
787+
return sb.toString();
757788
}
758789
sb.write(s[index]);
759790
index++;
760791
}
761-
return sb.toString();
792+
throw new HttpException("Failed to parse header value");
762793
} else {
763794
// Parse non-quoted value.
764-
var val = parseValue();
765-
return val == "" ? null : val;
795+
return parseValue();
766796
}
767797
}
768798

@@ -771,23 +801,18 @@ class _HeaderValue implements HeaderValue {
771801
if (done()) return;
772802
String name = parseParameterName();
773803
skipWS();
774-
if (done()) {
775-
parameters[name] = null;
776-
return;
777-
}
778-
maybeExpect("=");
779-
skipWS();
780-
if (done()) {
804+
if (maybeExpect("=")) {
805+
skipWS();
806+
String value = parseParameterValue();
807+
if (name == 'charset' && this is _ContentType) {
808+
// Charset parameter of ContentTypes are always lower-case.
809+
value = value.toLowerCase();
810+
}
811+
parameters[name] = value;
812+
skipWS();
813+
} else if (name.isNotEmpty) {
781814
parameters[name] = null;
782-
return;
783-
}
784-
String value = parseParameterValue();
785-
if (name == 'charset' && this is _ContentType && value != null) {
786-
// Charset parameter of ContentTypes are always lower-case.
787-
value = value.toLowerCase();
788815
}
789-
parameters[name] = value;
790-
skipWS();
791816
if (done()) return;
792817
// TODO: Implement support for multi-valued parameters.
793818
if (s[index] == valueSeparator) return;
@@ -821,7 +846,7 @@ class _ContentType extends _HeaderValue implements ContentType {
821846
parameters.forEach((String key, String value) {
822847
String lowerCaseKey = key.toLowerCase();
823848
if (lowerCaseKey == "charset") {
824-
value = value.toLowerCase();
849+
value = value?.toLowerCase();
825850
}
826851
this._parameters[lowerCaseKey] = value;
827852
});

‎sdk_nnbd/lib/_http/http.dart

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,11 @@ abstract class HttpHeaders {
772772
* [HeaderValue] can be used to conveniently build and parse header
773773
* values on this form.
774774
*
775+
* Parameter values can be omitted, in which case the value is parsed as `null`.
776+
* Values can be doubled quoted to allow characters outside of the RFC 7230
777+
* token characters and backslash sequences can be used to represent the double
778+
* quote and backslash characters themselves.
779+
*
775780
* To build an "accepts" header with the value
776781
*
777782
* text/plain; q=0.3, text/html
@@ -798,7 +803,8 @@ abstract class HeaderValue {
798803
/**
799804
* Creates a new header value object setting the value and parameters.
800805
*/
801-
factory HeaderValue([String value = "", Map<String, String>? parameters]) {
806+
factory HeaderValue(
807+
[String value = "", Map<String, String?> parameters = const {}]) {
802808
return new _HeaderValue(value, parameters);
803809
}
804810

@@ -826,7 +832,7 @@ abstract class HeaderValue {
826832
*
827833
* This map cannot be modified.
828834
*/
829-
Map<String, String> get parameters;
835+
Map<String, String?> get parameters;
830836

831837
/**
832838
* Returns the formatted string representation in the form:
@@ -916,7 +922,7 @@ abstract class ContentType implements HeaderValue {
916922
* or in `parameters`, will have its value converted to lower-case.
917923
*/
918924
factory ContentType(String primaryType, String subType,
919-
{String? charset, Map<String, String>? parameters}) {
925+
{String? charset, Map<String, String?> parameters = const {}}) {
920926
return new _ContentType(primaryType, subType, charset, parameters);
921927
}
922928

0 commit comments

Comments
 (0)