-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Description
Note: This proposed behavior has been amended, see below for the current proposal
Intended change
The HeaderValue class will now parse parameters without a value as having the empty string as value instead of a null value. This is the class used to parse semicolon delimited parameters used in the Accept, Authorization, Content-Type, and other such HTTP headers. RFC 7231 section 3.1.1.1 and other locations require such parameters to have a value, but HeaderValue was parsing empty parameters as null value which was wrong according to the specification. The Sec-WebSocket-Extensions websocket header is a special case that allows absent values. The HeaderValues class now parses absent values and empty values as the empty string. For instance
HeaderValue.parse('a; b=B; c; d=; e=E').parametersused to give {b: "B", c: null, d: null, e: "E"} but now gives {b: "B", c: "", d: "", e: "E"}.
Additionally HeaderValue.toString() was incorrectly not quoting the empty parameter values, which wasn't allowed by the specification. I.e.
new HeaderValue("a", {"b": ""})used to give text/plain; q= but now gives text/plain; q="".
Rationale
The non-nullable-by-default migration required making subtle changes to some dart:io semantics in order to provide a better API. The HeaderValue.parameters map never contains null values except if parameters not allowed by the standard have been parsed, or in the special case of Sec-WebSocket-Extensions. To avoid code using these parameters from needing to null check the parsed parameter values, these invalid parameters are instead parsed as the empty string.
These absent values also did not survice a roundtrip through HeaderValue as HeaderValue.parse('a; b') used to give a; b=null where null has no special meaning, but will now give a; b="" which is closer to the original input. That means HeaderValue could not and still cannot be used and to construct the Sec-WebSocket-Extensions parameters that don't have values.
Expected impact
HTTP headers such as Accept, Authorization, Content-Type that abide by the standard will be unaffected.
Code handling the Sec-WebSocket-Extensions using HeaderValue may be affected if it doesn't simply check if the parameter is set in the map, but instead handles an empty string differently from null. The relevant code and tests in the SDK has been updated.
It's hard to rule out if someone has built other non-standard extensions, where the class has been used for valueless parameters, such as foo; bar where the header's value is foo with the bar parameter, which is handled distinctly from if the bar parameter had a value (foo; bar=qux). Though the HeaderValue can only receive such invalid parameters, as it was not able to construct them, instead producing foo; bar=null.
Steps for mitigation
If code relies on HeaderValue returns has a null value for valueless parameters, it'll need to be updated to allow the empty string as well. However, if code relies on being able to tell a parameter without a value apart from a parameter with the empty string, then head then the HeaderValue class can't solve that problem anymore and one will instead need to build a custom parser.
Alternatively if this sort of non-standard valueless parameters turns out to exist in practice or there's non-SDK code parsing Sec-WebSocket-Extensions, we can decide not to make this breaking change, and embrace this non-standard behavior as a feature.
Implementation
This change was implemented in https://dart-review.googlesource.com/c/sdk/+/136620.
Note: This proposed behavior has been amended, see below for the current proposal