SPDY - Handle incomplete headers during server push.

net/http/http_network_transaction.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/http/http_network_transaction.h"
 
 #include <set>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 242 matching lines @@
   return rv;
 }
 
 void HttpNetworkTransaction::PrepareForAuthRestart(HttpAuth::Target target) {
   DCHECK(HaveAuth(target));
   DCHECK(!stream_request_.get());
 
   bool keep_alive = false;
   // Even if the server says the connection is keep-alive, we have to be
   // able to find the end of each response in order to reuse the connection.
-  if (GetResponseHeaders()->IsKeepAlive() &&
+  HttpResponseHeaders* headers = GetResponseHeaders();
+  if (headers && headers->IsKeepAlive() &&

[Ryan Hamilton, 2012/08/03 19:52:14]

Is this change related to the SPDY server push bug?  If not, I think it should
be split into a new CL.

Either way, it would be great if there were a test.

[ramant (doing other things), 2012/08/03 20:50:40]

Deleted this change.

       stream_->CanFindEndOfResponse()) {
     // If the response body hasn't been completely read, we need to drain
     // it first.
     if (!stream_->IsResponseBodyComplete()) {
       next_state_ = STATE_DRAIN_BODY_FOR_AUTH_RESTART;
       read_buf_ = new IOBuffer(kDrainBodyBufferSize);  // A bit bucket.
       read_buf_len_ = kDrainBodyBufferSize;
       return;
     }
     keep_alive = true;
@@ skipping 639 matching lines @@
 
   bool keep_alive = false;
   if (stream_->IsResponseBodyComplete()) {
     // Note: Just because IsResponseBodyComplete is true, we're not
     // necessarily "done".  We're only "done" when it is the last
     // read on this HttpNetworkTransaction, which will be signified
     // by a zero-length read.
     // TODO(mbelshe): The keepalive property is really a property of
     //    the stream.  No need to compute it here just to pass back
     //    to the stream's Close function.
-    if (stream_->CanFindEndOfResponse())
-      keep_alive = GetResponseHeaders()->IsKeepAlive();
+    if (stream_->CanFindEndOfResponse()) {
+      HttpResponseHeaders* headers = GetResponseHeaders();

[Ryan Hamilton, 2012/08/03 19:52:14]

(Ditto)

[ramant (doing other things), 2012/08/03 20:50:40]

SpdyNetworkTransactionSpdy3Test..ServerPushWithNoStatusHeaderFrames causes this
crash. It looks like CanFindEndOfResponse is being true though there are no
response headers. That may be another bug. Will undo this change when we fix it.
Added a TODO. Will submit a separate CL for unit tests.

[Ryan Hamilton, 2012/08/03 21:10:01]

Does that test cause a crash with your new code, or with the new code.  If your
new test is crashing here with the new code, then I agree with you that this
change is required as part of this CL.

+      if (headers)
+        keep_alive = headers->IsKeepAlive();
+    }
   }
 
   // Clean up connection if we are done.
   if (done) {
     LogTransactionMetrics();
     stream_->LogNumRttVsBytesMetrics();
     stream_->Close(!keep_alive);
     // Note: we don't reset the stream here.  We've closed it, but we still
     // need it around so that callers can call methods such as
     // GetUploadProgress() and have them be meaningful.
@@ skipping 474 matching lines @@
       description = base::StringPrintf("Unknown state 0x%08X (%u)", state,
                                        state);
       break;
   }
   return description;
 }
 
 #undef STATE_CASE
 
 }  // namespace net