Disallow multiple HEADERS frames on pushed streams.

net/spdy/spdy_http_stream.cc

Index: net/spdy/spdy_http_stream.cc
diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc
index 1d739b22504083ec9395b5c0b0b4503798da0ff1..36e02a26c99ec531844cfbff5c8a854f551809ca 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/net/spdy/spdy_http_stream.cc
@@ -42,7 +42,7 @@ SpdyHttpStream::SpdyHttpStream(const base::WeakPtr<SpdySession>& spdy_session,
       closed_stream_sent_bytes_(0),
       request_info_(NULL),
       response_info_(NULL),
-      response_headers_status_(RESPONSE_HEADERS_ARE_INCOMPLETE),
+      response_headers_complete_(false),
       user_buffer_len_(0),
       request_body_buf_size_(0),
       buffered_read_callback_pending_(false),
@@ -105,7 +105,7 @@ int SpdyHttpStream::ReadResponseHeaders(const CompletionCallback& callback) {
   CHECK(stream_.get());
 
   // Check if we already have the response headers. If so, return synchronously.
-  if (response_headers_status_ == RESPONSE_HEADERS_ARE_COMPLETE) {
+  if (response_headers_complete_) {
     CHECK(!stream_->IsIdle());
     return OK;
   }
@@ -301,7 +301,7 @@ void SpdyHttpStream::Cancel() {
   }
 }
 
-void SpdyHttpStream::OnRequestHeadersSent() {
+void SpdyHttpStream::OnHeadersSent() {
   if (HasUploadData()) {
     ReadAndSendRequestBodyData();
   } else {
@@ -309,9 +309,10 @@ void SpdyHttpStream::OnRequestHeadersSent() {
   }
 }
 
-SpdyResponseHeadersStatus SpdyHttpStream::OnResponseHeadersUpdated(
+void SpdyHttpStream::OnHeadersReceived(
     const SpdyHeaderBlock& response_headers) {
-  CHECK_EQ(response_headers_status_, RESPONSE_HEADERS_ARE_INCOMPLETE);
+  DCHECK(!response_headers_complete_);
+  response_headers_complete_ = true;
 
   if (!response_info_) {
     DCHECK_EQ(stream_->type(), SPDY_PUSH_STREAM);
@@ -319,13 +320,11 @@ SpdyResponseHeadersStatus SpdyHttpStream::OnResponseHeadersUpdated(
     response_info_ = push_response_info_.get();
   }
 
-  if (!SpdyHeadersToHttpResponse(response_headers, response_info_)) {
-    // We do not have complete headers yet.
-    return RESPONSE_HEADERS_ARE_INCOMPLETE;
-  }
+  const bool headers_valid =
+      SpdyHeadersToHttpResponse(response_headers, response_info_);
+  DCHECK(headers_valid);

[Ryan Hamilton, 2016/11/23 20:58:43]

Why is this a DCHECK as opposed to an error? If the peer sends us bogus headers,
could we hit this?

[Bence, 2016/11/23 21:15:58]

Nope.  SpdyHeadersToHttpResponse() only fails if there is no ":status", and now
SpdyStream checks this before calling OnHeadersReceived.

Also, what kind of error would it be?  Should we generate a RST_STREAM here by
calling back to |stream_|?  Or should this method have a return value?  In the
latter case, would we give up surfacing an error string in NetLog?

 
   response_info_->response_time = stream_->response_time();
-  response_headers_status_ = RESPONSE_HEADERS_ARE_COMPLETE;
   // Don't store the SSLInfo in the response here, HttpNetworkTransaction
   // will take care of that part.
   response_info_->was_alpn_negotiated = was_alpn_negotiated_;
@@ -339,12 +338,10 @@ SpdyResponseHeadersStatus SpdyHttpStream::OnResponseHeadersUpdated(
   if (!response_callback_.is_null()) {
     DoResponseCallback(OK);
   }
-
-  return RESPONSE_HEADERS_ARE_COMPLETE;
 }
 
 void SpdyHttpStream::OnDataReceived(std::unique_ptr<SpdyBuffer> buffer) {
-  CHECK_EQ(response_headers_status_, RESPONSE_HEADERS_ARE_COMPLETE);
+  DCHECK(response_headers_complete_);
 
   // Note that data may be received for a SpdyStream prior to the user calling
   // ReadResponseBody(), therefore user_buffer_ may be NULL.  This may often