Investigate whether resetting HTTP/2 stream is resonable when response contains status text. |
|||
Issue descriptionRFC 7540 Section 8.1.2.4 says that response ":status" header must be a status code, it must not contain a status text. (For example, "200" instead of "200 OK".) Up until https://crrev.com/2555563003 landed recently, Chromium was tolerant to responses that incorrectly included a status text. It is not known whether there are any servers that send such incorrect headers, and subsequently it is not known whether we can afford sending a RST_STREAM whenever such a header is encountered. Some data should be collected on this.
,
Feb 17 2017
,
Mar 28 2017
Looking at the past 28 days of data, the frequency of each bucket is: Headers do not include :status header field: less than 1e-8 :status header does not start with a number: less than 1e-8 :status header is an integer: approx 1.0 :status header is an integer followed by text: approx 4e-8 The spec says "This pseudo-header field MUST be included in all responses; otherwise, the response is malformed. [...] HTTP/2 does not define a way to carry the version or reason phrase that is included in an HTTP/1.1 status line." (RFC7540 Section 8.1.2.4). Given the very low number of response headers without :status, with one that does not start with a number, or with one that has bogus extra text following the number, I believe it is safe to reset the stream in such cases.
,
Mar 28 2017
Do you know if FF/IE/Safari enforce this spec behavior? My hunch is that a given server will always or never do the right thing. So while it's clear that very few requests hit this, I wonder if it'll end up knocking particular servers "offline".
,
Mar 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4ce14962179e9a25fdaaa595fc0cb093f79f4b79 commit 4ce14962179e9a25fdaaa595fc0cb093f79f4b79 Author: bnc <bnc@chromium.org> Date: Thu Mar 30 03:31:01 2017 Reset stream if HTTP/2 response status is invalid. * Reset stream if HTTP/2 response ":status" header is missing, does not start with a number, or has extra text after the number. * Modify unittests to document behavior. * Obsolete histogram. This is mostly a revert of https://crrev.com/2555183002. BUG= 672065 Review-Url: https://codereview.chromium.org/2779763005 Cr-Commit-Position: refs/heads/master@{#460644} [modify] https://crrev.com/4ce14962179e9a25fdaaa595fc0cb093f79f4b79/net/spdy/spdy_stream.cc [modify] https://crrev.com/4ce14962179e9a25fdaaa595fc0cb093f79f4b79/net/spdy/spdy_stream_unittest.cc [modify] https://crrev.com/4ce14962179e9a25fdaaa595fc0cb093f79f4b79/tools/metrics/histograms/histograms.xml
,
Mar 30 2017
Re #5: Firefox does not currently reject responses with invalid :status header value, but their developers show interest in changing the behavior to reject such responses, especially now that they see that Chromium is doing it. I do not know what other browsers do.
,
Mar 30 2017
Mozilla bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1352146. |
|||
►
Sign in to add a comment |
|||
Comment 1 by bugdroid1@chromium.org
, Dec 8 2016