Ignore duplicate SYN_REPLYs on the same stream. Added a unit test.

BUG=45639
TEST=Added unit test, which caused an ASSERT fail before but now passes.

Review URL: http://codereview.chromium.org/2871015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50751 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 66 insertions, 4 deletions

diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc
index 810c5e4..c6e1cb0 100644
--- a/net/spdy/spdy_network_transaction_unittest.cc
+++ b/net/spdy/spdy_network_transaction_unittest.cc
@@ -682,6 +682,56 @@ TEST_F(SpdyNetworkTransactionTest, ResponseWithoutSynReply) {
   EXPECT_EQ(ERR_SYN_REPLY_NOT_RECEIVED, out.rv);
 }
 
+// Test that the transaction doesn't crash when we get two replies on the same
+// stream ID. See http://crbug.com/45639.
+TEST_F(SpdyNetworkTransactionTest, ResponseWithTwoSynReplies) {
+  SessionDependencies session_deps;
+  HttpNetworkSession* session = CreateSession(&session_deps);
+
+  // We disable SSL for this test.
+  SpdySession::SetSSLMode(false);
+
+  MockWrite writes[] = {
+    MockWrite(true, reinterpret_cast<const char*>(kGetSyn),
+              arraysize(kGetSyn)),
+  };
+
+  MockRead reads[] = {
+    MockRead(true, reinterpret_cast<const char*>(kGetSynReply),
+             arraysize(kGetSynReply)),
+    MockRead(true, reinterpret_cast<const char*>(kGetSynReply),
+             arraysize(kGetSynReply)),
+    MockRead(true, reinterpret_cast<const char*>(kGetBodyFrame),
+             arraysize(kGetBodyFrame)),
+    MockRead(true, 0, 0)  // EOF
+  };
+
+  HttpRequestInfo request;
+  request.method = "GET";
+  request.url = GURL("http://www.google.com/");
+  request.load_flags = 0;
+  scoped_refptr<DelayedSocketData> data(
+      new DelayedSocketData(1, reads, arraysize(reads),
+                            writes, arraysize(writes)));
+  session_deps.socket_factory.AddSocketDataProvider(data.get());
+
+  scoped_ptr<SpdyNetworkTransaction> trans(
+      new SpdyNetworkTransaction(session));
+
+  TestCompletionCallback callback;
+  int rv = trans->Start(&request, &callback, BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  rv = callback.WaitForResult();
+  EXPECT_EQ(OK, rv);
+
+  const HttpResponseInfo* response = trans->GetResponseInfo();
+  EXPECT_TRUE(response->headers != NULL);
+  EXPECT_TRUE(response->was_fetched_via_spdy);
+  std::string response_data;
+  rv = ReadTransaction(trans.get(), &response_data);
+  EXPECT_EQ(ERR_SPDY_PROTOCOL_ERROR, rv);
+}
+
 TEST_F(SpdyNetworkTransactionTest, CancelledTransaction) {
   MockWrite writes[] = {
     MockWrite(true, reinterpret_cast<const char*>(kGetSyn),
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index 3795bc1..4910646 100644..100755
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -949,6 +949,17 @@ void SpdySession::OnSynReply(const spdy::SpdySynReplyControlFrame& frame,
   LOG(INFO) << "SPDY SYN_REPLY RESPONSE HEADERS for stream: " << stream_id;
   DumpSpdyHeaders(*headers);
 
+  scoped_refptr<SpdyStream> stream = active_streams_[stream_id];
+  CHECK_EQ(stream->stream_id(), stream_id);
+  CHECK(!stream->cancelled());
+
+  if (stream->syn_reply_received()) {
+    LOG(WARNING) << "Received duplicate SYN_REPLY for stream " << stream_id;
+    CloseStream(stream->stream_id(), ERR_SPDY_PROTOCOL_ERROR);
+    return;
+  }
+  stream->set_syn_reply_received();
+
   // We record content declared as being pushed so that we don't
   // request a duplicate stream which is already scheduled to be
   // sent to us.
@@ -977,10 +988,6 @@ void SpdySession::OnSynReply(const spdy::SpdySynReplyControlFrame& frame,
     } while (start < content.length());
   }
 
-  scoped_refptr<SpdyStream> stream = active_streams_[stream_id];
-  CHECK_EQ(stream->stream_id(), stream_id);
-  CHECK(!stream->cancelled());
-
   const BoundNetLog& log = stream->net_log();
   if (log.HasListener()) {
     log.AddEvent(
diff --git a/net/spdy/spdy_stream.cc b/net/spdy/spdy_stream.cc
index 3fbde08..1166cd5 100644..100755
--- a/net/spdy/spdy_stream.cc
+++ b/net/spdy/spdy_stream.cc
@@ -17,6 +17,7 @@ SpdyStream::SpdyStream(
       priority_(0),
       pushed_(pushed),
       metrics_(Singleton<BandwidthMetrics>::get()),
+      syn_reply_received_(false),
       session_(session),
       delegate_(NULL),
       request_time_(base::Time::Now()),
diff --git a/net/spdy/spdy_stream.h b/net/spdy/spdy_stream.h
index 919bca8..6183be2 100644..100755
--- a/net/spdy/spdy_stream.h
+++ b/net/spdy/spdy_stream.h
@@ -90,6 +90,9 @@ class SpdyStream : public base::RefCounted<SpdyStream> {
   spdy::SpdyStreamId stream_id() const { return stream_id_; }
   void set_stream_id(spdy::SpdyStreamId stream_id) { stream_id_ = stream_id; }
 
+  bool syn_reply_received() const { return syn_reply_received_; }
+  void set_syn_reply_received() { syn_reply_received_ = true; }
+
   // For pushed streams, we track a path to identify them.
   const std::string& path() const { return path_; }
   void set_path(const std::string& path) { path_ = path; }
@@ -196,6 +199,7 @@ class SpdyStream : public base::RefCounted<SpdyStream> {
   int priority_;
   const bool pushed_;
   ScopedBandwidthMetrics metrics_;
+  bool syn_reply_received_;
 
   scoped_refptr<SpdySession> session_;