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

net/spdy/spdy_stream.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/spdy/spdy_stream.h"
 
 #include <limits>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 12 matching lines @@
 #include "net/log/net_log_capture_mode.h"
 #include "net/log/net_log_event_type.h"
 #include "net/spdy/spdy_buffer_producer.h"
 #include "net/spdy/spdy_http_utils.h"
 #include "net/spdy/spdy_session.h"
 
 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,
     const std::string* description,
     NetLogCaptureMode /* capture_mode */) {
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
   dict->SetInteger("stream_id", static_cast<int>(stream_id));
   dict->SetInteger("status", status);
   dict->SetString("description", *description);
   return std::move(dict);
@@ skipping 326 matching lines @@
 }
 
 void SpdyStream::SetRequestTime(base::Time t) {
   request_time_ = t;
 }
 
 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;
 
       switch (type_) {
         case SPDY_BIDIRECTIONAL_STREAM:
         case SPDY_REQUEST_RESPONSE_STREAM:
           // A bidirectional stream or a request/response stream is ready for
           // the response headers only after request headers are sent.
           if (io_state_ == STATE_IDLE) {
@@ skipping 17 matching lines @@
           break;
       }
 
       DCHECK_NE(io_state_, STATE_IDLE);
 
       response_time_ = response_time;
       recv_first_byte_time_ = recv_first_byte_time;
       SaveResponseHeaders(response_headers);
 
       break;
-
+    }
     case READY_FOR_DATA_OR_TRAILERS:
       // Second header block is trailers.
       if (type_ == SPDY_PUSH_STREAM) {
         const std::string error("Trailers not supported for push stream.");
         LogStreamError(ERR_SPDY_PROTOCOL_ERROR, error);
         session_->ResetStream(stream_id_, RST_STREAM_PROTOCOL_ERROR, error);
         return;
       }
 
       response_state_ = TRAILERS_RECEIVED;
@@ skipping 448 matching lines @@
       description = base::StringPrintf("Unknown state 0x%08X (%u)", state,
                                        state);
       break;
   }
   return description;
 }
 
 #undef STATE_CASE
 
 }  // namespace net