Ignore 1xx informational headers.

net/spdy/spdy_stream.cc

Index: net/spdy/spdy_stream.cc
diff --git a/net/spdy/spdy_stream.cc b/net/spdy/spdy_stream.cc
index eb6176f94ce7b86bbb46da3e734b3c04c48ca5a4..9edead63d46aa2ec5712e9caf7ffaa531d557ae7 100644
--- a/net/spdy/spdy_stream.cc
+++ b/net/spdy/spdy_stream.cc
@@ -14,6 +14,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -379,11 +380,28 @@ void SpdyStream::OnHeadersReceived(const SpdyHeaderBlock& response_headers,
       // No header block has been received yet.
       DCHECK(response_headers_.empty());
 
-      if (response_headers.find(":status") == response_headers.end()) {
-        const std::string error("Response headers do not include :status.");
-        LogStreamError(ERR_SPDY_PROTOCOL_ERROR, error);
-        session_->ResetStream(stream_id_, RST_STREAM_PROTOCOL_ERROR, error);
-        return;
+      {
+        SpdyHeaderBlock::const_iterator it = response_headers.find(":status");
+        if (it == response_headers.end()) {
+          const std::string error("Response headers do not include :status.");
+          LogStreamError(ERR_SPDY_PROTOCOL_ERROR, error);
+          session_->ResetStream(stream_id_, RST_STREAM_PROTOCOL_ERROR, error);
+          return;
+        }
+
+        int status;
+        if (!StringToInt(it->second, &status)) {
+          const std::string error("Cannot parse :status.");

[Ryan Hamilton, 2016/12/06 20:02:18]

I wonder if there are any servers spitting out old spdy-style, aka http/1, style
status headers. Do we already require this in SpdyHttpStream (or somewhere
else?)

[Bence, 2016/12/06 20:10:13]

No, we do not.  That's why our unittests producing these incorrect headers did
not fail before this CL.

[Ryan Hamilton, 2016/12/06 23:48:14]

So if there are servers out there which return "200 OK", then Chrome will stop
working to them, right?

[Bence, 2016/12/07 00:22:12]

That is correct, and this is a functional change introduced by this CL.  8.1.2.4
of the HTTP/2 spec says ":status pseudo-header field is defined that carries the
HTTP status code field [...] HTTP/2 does not define a way to carry the version
or reason phrase that is included in an HTTP/1.1 status line".  That is, such a
server would not be in compliance with the spec.

We can (a) leave it as is, and still be able to relax the requirement before
branch point if users see this in the wild, or (b) relax the requirement and not
worry about it, or (c) relax the requirement, gather UMA data, and get back to
this later.  WDYT?

+          LogStreamError(ERR_SPDY_PROTOCOL_ERROR, error);
+          session_->ResetStream(stream_id_, RST_STREAM_PROTOCOL_ERROR, error);
+          return;
+        }
+
+        // Ignore informational headers.
+        // TODO(bnc): Add support for 103 Early Hints, https://crbug.com/671310.
+        if (status / 100 == 1) {
+          return;
+        }
       }

[Ryan Hamilton, 2016/12/06 20:02:18]

This is surprisingly clean. nice.

 
       response_state_ = READY_FOR_DATA_OR_TRAILERS;
@@ -440,7 +458,7 @@ void SpdyStream::OnHeadersReceived(const SpdyHeaderBlock& response_headers,
       const std::string error("Header block received after trailers.");
       LogStreamError(ERR_SPDY_PROTOCOL_ERROR, error);
       session_->ResetStream(stream_id_, RST_STREAM_PROTOCOL_ERROR, error);
-      return;
+      break;
   }
 }