HttpStreamParser: Reject headers with nulls in them. #13663
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wpt-pr-bot
approved these changes
Oct 22, 2018
While the HTTP spec further limits what values are legal, nulls are particularly concerning, and it's safest just to reject them. See discussion here: whatwg/xhr#165 Chrome will be the first browser to reject nulls in responses, despite there being wpt tests for this, so we'll have to keep an eye out for breakages. For reference, 0x00 through 0x1F aren't allowed in header values or fields, (https://tools.ietf.org/html/rfc7230#section-3.2 - VCHAR excludes those characters). CRs and LFs are of course needed, and 0x0C and 0x0B are allowed by other specs for particular header parsers, strangely. This CL does not affect other code that can generate HTTP response headers, which still uses the old behavior of just removing nulls. ServiceWorkers, extensions, WebPackages, Dial (?), and various tests still inherit the old behavior, since they create headers directly with a method that can't fail. It does introduce a new helper method, however, that they should eventually be switched to use: HttpResponseHeaders::TryToCreate(). We should probably put off conversion until this successfully makes it to stable. Bug: 832086 Change-Id: Ib75ac03a6a298238cafb41eaa5f046c082fd0bdf Reviewed-on: https://chromium-review.googlesource.com/c/1291812 Reviewed-by: Asanka Herath <[email protected]> Commit-Queue: Matt Menke <[email protected]> Cr-Commit-Position: refs/heads/master@{#601776}
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-1291812
branch
from
4497304
to
d0df0bd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While the HTTP spec further limits what values are legal, nulls are
particularly concerning, and it's safest just to reject them. See
discussion here: whatwg/xhr#165
Chrome will be the first browser to reject nulls in responses, despite
there being wpt tests for this, so we'll have to keep an eye out for
breakages.
For reference, 0x00 through 0x1F aren't allowed in header values or
fields, (https://tools.ietf.org/html/rfc7230#section-3.2 - VCHAR
excludes those characters). CRs and LFs are of course needed, and
0x0C and 0x0B are allowed by other specs for particular
header parsers, strangely.
This CL does not affect other code that can generate HTTP response
headers, which still uses the old behavior of just removing nulls.
ServiceWorkers, extensions, WebPackages, Dial (?), and various tests
still inherit the old behavior, since they create headers directly
with a method that can't fail. It does introduce a new helper method,
however, that they should eventually be switched to use:
HttpResponseHeaders::TryToCreate(). We should probably put off
conversion until this successfully makes it to stable.
Bug: 832086
Change-Id: Ib75ac03a6a298238cafb41eaa5f046c082fd0bdf
Reviewed-on: https://chromium-review.googlesource.com/c/1291812
Reviewed-by: Asanka Herath <[email protected]>
Commit-Queue: Matt Menke <[email protected]>
Cr-Commit-Position: refs/heads/master@{#601776}