Add HTTP OWS handling to single range header parsing

[dlrobertson]

Add HTTP optional white space handling to the single range header parser.

Normative changes happen as part of #1520.

---

Preview | Diff

[annevk]

@jakearchibald @youennf @rayankans does any of you think this boolean is worth having for CORS? I think I'd rather allow whitespace in CORS for the Range header so we can keep Range header parsing consistent. In particular this would allow an arbitrary number of 0x09 and 0x20 after bytes=. I don't see the security benefit.

(We also use this parser for blob: URL requests and there we already allow whitespace.)

[annevk]

If we keep this we should name this disallowWhitespace so it can default to false. Although as long as we don't export it we should probably keep it as a required argument.

[dlrobertson]

Would this still be the case following the discussion in #1070

[dlrobertson]

@jakearchibald @youennf @rayankans does any of you think this boolean is worth having for CORS? I think I'd rather allow whitespace in CORS for the Range header so we can keep Range header parsing consistent. In particular this would allow an arbitrary number of 0x09 and 0x20 after bytes=. I don't see the security benefit.

(We also use this parser for blob: URL requests and there we already allow whitespace.)

I've tested Blink and WebKit and both disallow the whitespace allowed in HTTP-RANGE as tested here. Would it make sense to remove the boolean, but add a note that some implementations don't allow this optional whitespace?

[annevk]

It seems that as per more recent discussion in #1070 I misunderstood the ABNF and no whitespace is allowed after bytes= in theory. Let's get to the bottom of that and then revisit this as it might well be that the status quo is correct now.

[dlrobertson]

If we keep this we should name this disallowWhitespace so it can default to false. Although as long as we don't export it we should probably keep it as a required argument.

Updated to use disallowWhitespace. Sorry for the delay, I thought I had already done this.

[annevk]

This only removes whitespace after = if I understand it correctly. Whereas #1070 (comment) argues for removing it in 6 places.

[dlrobertson]

HTTP 2616 allowed whitespace:

whitespace	supported by parse-single-range
Before =	✅
After =	✅
Before -	✅
After -	✅
Before ,	❌
After ,	❌

Added some extra cases to web-platform-tests/wpt#37569

[annevk]

Thanks, that makes sense as we don't support multiple values here. One nit and with that this is good to go.

[annevk]

This looks great, thanks!

Blocked on speced/bikeshed#2476 unfortunately.

[dlrobertson]

Blocked on speced/bikeshed#2476 unfortunately.

No problem! Thanks for the review... Will I need to rebase once the fix for speced/bikeshed#2476 lands?

[annevk]

No, this is all good. I also looked at #1520 again and apart from invoking this algorithm with true I think that's also good (and I can make that change when merging if it all looks good to you).