Do not reset stream if HTTP/2 response contains status text.

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 9edead63d46aa2ec5712e9caf7ffaa531d557ae7..958aaf0178608be23f3e9f028d783488725767f4 100644
--- a/net/spdy/spdy_stream.cc
+++ b/net/spdy/spdy_stream.cc
@@ -30,6 +30,43 @@ namespace net {
 
 namespace {
 
+enum StatusHeader {
+  STATUS_HEADER_NOT_INCLUDED = 0,
+  STATUS_HEADER_DOES_NOT_START_WITH_NUMBER = 1,
+  STATUS_HEADER_IS_NUMBER = 2,
+  STATUS_HEADER_HAS_STATUS_TEXT = 3,
+  STATUS_HEADER_MAX = STATUS_HEADER_HAS_STATUS_TEXT
+};
+
+StatusHeader ParseStatusHeaderImpl(const SpdyHeaderBlock& response_headers,
+                                   int* status) {
+  SpdyHeaderBlock::const_iterator it = response_headers.find(":status");
+  if (it == response_headers.end())
+    return STATUS_HEADER_NOT_INCLUDED;
+
+  // Save status in |*status| even if some text follows the status code.
+  base::StringPiece status_string = it->second;
+  base::StringPiece::size_type end = status_string.find(' ');
+  if (!StringToInt(status_string.substr(0, end), status))
+    return STATUS_HEADER_DOES_NOT_START_WITH_NUMBER;
+
+  // Check if status header could be parsed as a single number,
+  // but keep |*status| correct even if it cannot.
+  int unused;
+  if (StringToInt(status_string, &unused))

[Ryan Hamilton, 2016/12/07 19:11:39]

nit: Instead of calling StringToInt twice, it seems like you could call it once
with the invocation on line 50. Then if it fails, return HAS_TEXT or
DOES_NOT_START based on if end == StringPiece::end(), right?

[Bence, 2016/12/07 20:33:18]

Done.

+    return STATUS_HEADER_IS_NUMBER;
+
+  return STATUS_HEADER_HAS_STATUS_TEXT;
+}
+
+StatusHeader ParseStatusHeader(const SpdyHeaderBlock& response_headers,
+                               int* status) {
+  StatusHeader status_header = ParseStatusHeaderImpl(response_headers, status);
+  UMA_HISTOGRAM_ENUMERATION("Net.Http2ResponseStatusHeader", status_header,
+                            STATUS_HEADER_MAX + 1);
+  return status_header;
+}
+
 std::unique_ptr<base::Value> NetLogSpdyStreamErrorCallback(
     SpdyStreamId stream_id,
     int status,
@@ -376,32 +413,34 @@ void SpdyStream::OnHeadersReceived(const SpdyHeaderBlock& response_headers,
                                    base::Time response_time,
                                    base::TimeTicks recv_first_byte_time) {
   switch (response_state_) {
-    case READY_FOR_HEADERS:
+    case READY_FOR_HEADERS: {
       // No header block has been received yet.
       DCHECK(response_headers_.empty());
-
-      {
-        SpdyHeaderBlock::const_iterator it = response_headers.find(":status");
-        if (it == response_headers.end()) {
+      int status;
+      switch (ParseStatusHeader(response_headers, &status)) {
+        case STATUS_HEADER_NOT_INCLUDED: {
           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)) {
+        case STATUS_HEADER_DOES_NOT_START_WITH_NUMBER: {
           const std::string error("Cannot parse :status.");
           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;
-        }
+        // Intentional fallthrough for the following two cases,
+        // to maintain compatibility with broken servers that include
+        // status text in the response.
+        case STATUS_HEADER_IS_NUMBER:
+        case STATUS_HEADER_HAS_STATUS_TEXT:
+          // Ignore informational headers.
+          // TODO(bnc): Add support for 103 Early Hints,
+          // https://crbug.com/671310.
+          if (status / 100 == 1) {
+            return;
+          }
       }
 
       response_state_ = READY_FOR_DATA_OR_TRAILERS;
@@ -439,7 +478,7 @@ void SpdyStream::OnHeadersReceived(const SpdyHeaderBlock& response_headers,
       SaveResponseHeaders(response_headers);
 
       break;
-
+    }
     case READY_FOR_DATA_OR_TRAILERS:
       // Second header block is trailers.
       if (type_ == SPDY_PUSH_STREAM) {