Change combined values - #813

@ddragana @MattMenke2 @youennf @mnot please take a look. I think the one further improvement here could be (and why I wrote "more closely match" above) to not include the space (0x20) as HTTP doesn't seem to do that either when combining (for the non-naïve case), but I'm rather worried about compatibility.

Firefox passes the tests. Chrome and Safari would have some work to do if we agree on this. (I haven't tested Edge yet since BrowserStack keeps being annoying with local tests.)

The other thing I'm interested in is if we should use this same definition when defining parsers for particular headers, such as Content-Type, or if we should use a version that does not include the space. This ends up mattering a bit in edge cases unfortunately (double quotes split across headers). I guess ideally we'd fix the API here, but ugh.

@wisniewskit I suppose you might have thoughts on this too.

I'm concerned about ignoring empty response header fields. Something like:

Content-Length:
Content-Length: 10

Seems more like something that should be rejected than treated as the logical equivalent to "Content-Length: 10". Seems like allowing it would potentially allow for more shenanigans around deliberately tricking (Or accidentally confusing) proxies and security software.

An intermediary that combines would make that Content-Length: 10 and you wouldn't be wiser. Unless we change the HTTP standard I'm not sure there's much we can do there?

Note also that Content-Length: ,10 (not the combined value, I know, but interesting) ends up being treated as 10. And 10, too! (Arguably we could/should fix these.)

Given that HTTP combining is a thing, I'm not sure what alternative strategy we have. Not ignore empty/whitespace values and contradict the HTTP standard?

Unless we change the spec? What spec requires ignoring empty response headers? Or do you mean your proposed changes here?

Under what pre-existing spec is "Content-Length: ,10" the same as "Content-Length: 10"? Am I missing something?

Sigh, I got misled by https://tools.ietf.org/html/rfc7230#section-7 and the discussion at httpwg/http-extensions#596 (comment). But that should happen at the level of parsing individual headers, it shouldn't happen at this level. My bad, this is a bug in Firefox.

Thanks a lot for catching this!

Under what pre-existing spec is "Content-Length: ,10" the same as "Content-Length: 10"? Am I missing something?

No spec, but this is what browsers do at the moment. (Whereas two separate headers does result in a network error.)

Closing as this was bogus. I'm changing the expected results in web-platform-tests/wpt#13471 and will file a bug against Firefox for eliding empty/whitespace header values when there's multiple headers of the same name.

The Content-Length discussion is probably best had in web-platform-tests/wpt#10548. It'd be ideal if we could make ,10 and 10, a network error.

Under what pre-existing spec is "Content-Length: ,10" the same as "Content-Length: 10"? Am I missing something?

No spec, but this is what browsers do at the moment. (Whereas two separate headers does result in a network error.)

I'm surprised that Chrome does this, but you're absolutely correct - deep within Chrome's parser is code that just throws away empty HTTP response headers.