HttpStreamParser: Don't reuse sockets which receive unparsed data.

net/http/http_network_transaction_unittest.cc

 // Copyright 2013 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 <math.h>  // ceil
 #include <stdarg.h>
 #include <stdint.h>
 #include <limits>
@@ skipping 34 matching lines @@
 #include "net/http/http_auth_challenge_tokenizer.h"
 #include "net/http/http_auth_handler_digest.h"
 #include "net/http/http_auth_handler_mock.h"
 #include "net/http/http_auth_handler_ntlm.h"
 #include "net/http/http_auth_scheme.h"
 #include "net/http/http_basic_state.h"
 #include "net/http/http_basic_stream.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_network_session_peer.h"
 #include "net/http/http_request_headers.h"
+#include "net/http/http_response_info.h"
 #include "net/http/http_server_properties_impl.h"
 #include "net/http/http_stream.h"
 #include "net/http/http_stream_factory.h"
 #include "net/http/http_stream_parser.h"
 #include "net/http/http_transaction_test_util.h"
 #include "net/log/net_log.h"
 #include "net/log/test_net_log.h"
 #include "net/log/test_net_log_entry.h"
 #include "net/log/test_net_log_util.h"
 #include "net/proxy/mock_proxy_resolver.h"
@@ skipping 1899 matching lines @@
   const HttpResponseInfo* response = trans->GetResponseInfo();
   ASSERT_TRUE(response);
   ASSERT_TRUE(response->headers);
   EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
   std::string response_data;
   rv = ReadTransaction(trans.get(), &response_data);
   EXPECT_EQ(OK, rv);
   EXPECT_EQ("hello", response_data);
 }
 
+// Sockets that receive extra data after a response is complete should not be
+// reused.
+TEST_P(HttpNetworkTransactionTest, KeepAliveWithUnusedData1) {
+  scoped_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
+  MockWrite data_writes1[] = {
+      MockWrite("HEAD / HTTP/1.1\r\n"
+                "Host: www.borked.com\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockRead data_reads1[] = {
+      MockRead("HTTP/1.1 200 OK\r\n"
+               "Connection: keep-alive\r\n"
+               "Content-Length: 22\r\n\r\n"
+               "This server is borked."),
+  };
+
+  MockWrite data_writes2[] = {
+      MockWrite("GET /foo HTTP/1.1\r\n"
+                "Host: www.borked.com\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockRead data_reads2[] = {
+      MockRead("HTTP/1.1 200 OK\r\n"
+               "Content-Length: 3\r\n\r\n"
+               "foo"),
+  };
+  StaticSocketDataProvider data1(data_reads1, arraysize(data_reads1),
+                                 data_writes1, arraysize(data_writes1));
+  session_deps_.socket_factory->AddSocketDataProvider(&data1);
+  StaticSocketDataProvider data2(data_reads2, arraysize(data_reads2),
+                                 data_writes2, arraysize(data_writes2));
+  session_deps_.socket_factory->AddSocketDataProvider(&data2);
+
+  TestCompletionCallback callback;
+  HttpRequestInfo request1;
+  request1.method = "HEAD";
+  request1.url = GURL("http://www.borked.com/");
+
+  scoped_ptr<HttpTransaction> trans1(
+      new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
+  int rv = trans1->Start(&request1, callback.callback(), BoundNetLog());
+  EXPECT_EQ(OK, callback.GetResult(rv));
+
+  const HttpResponseInfo* response1 = trans1->GetResponseInfo();
+  ASSERT_TRUE(response1);
+  ASSERT_TRUE(response1->headers);
+  EXPECT_EQ(200, response1->headers->response_code());
+  EXPECT_TRUE(response1->headers->IsKeepAlive());
+
+  std::string response_data1;
+  EXPECT_EQ(OK, ReadTransaction(trans1.get(), &response_data1));
+  EXPECT_EQ("", response_data1);
+  // Deleting the transaction attempts to release the socket back into the
+  // socket pool.
+  trans1.reset();
+
+  HttpRequestInfo request2;
+  request2.method = "GET";
+  request2.url = GURL("http://www.borked.com/foo");
+
+  scoped_ptr<HttpTransaction> trans2(
+      new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
+  rv = trans2->Start(&request2, callback.callback(), BoundNetLog());
+  EXPECT_EQ(OK, callback.GetResult(rv));
+
+  const HttpResponseInfo* response2 = trans2->GetResponseInfo();
+  ASSERT_TRUE(response2);
+  ASSERT_TRUE(response2->headers);
+  EXPECT_EQ(200, response2->headers->response_code());
+
+  std::string response_data2;
+  EXPECT_EQ(OK, ReadTransaction(trans2.get(), &response_data2));
+  EXPECT_EQ("foo", response_data2);
+}
+
+TEST_P(HttpNetworkTransactionTest, KeepAliveWithUnusedData2) {
+  scoped_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
+  MockWrite data_writes1[] = {
+      MockWrite("GET / HTTP/1.1\r\n"
+                "Host: www.borked.com\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockRead data_reads1[] = {
+      MockRead("HTTP/1.1 200 OK\r\n"
+               "Connection: keep-alive\r\n"
+               "Content-Length: 22\r\n\r\n"
+               "This server is borked."
+               "Bonus data!"),
+  };
+
+  MockWrite data_writes2[] = {
+      MockWrite("GET /foo HTTP/1.1\r\n"
+                "Host: www.borked.com\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockRead data_reads2[] = {
+      MockRead("HTTP/1.1 200 OK\r\n"
+               "Content-Length: 3\r\n\r\n"
+               "foo"),
+  };
+  StaticSocketDataProvider data1(data_reads1, arraysize(data_reads1),
+                                 data_writes1, arraysize(data_writes1));
+  session_deps_.socket_factory->AddSocketDataProvider(&data1);
+  StaticSocketDataProvider data2(data_reads2, arraysize(data_reads2),
+                                 data_writes2, arraysize(data_writes2));
+  session_deps_.socket_factory->AddSocketDataProvider(&data2);
+
+  TestCompletionCallback callback;
+  HttpRequestInfo request1;
+  request1.method = "GET";
+  request1.url = GURL("http://www.borked.com/");
+
+  scoped_ptr<HttpTransaction> trans1(
+      new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
+  int rv = trans1->Start(&request1, callback.callback(), BoundNetLog());
+  EXPECT_EQ(OK, callback.GetResult(rv));
+
+  const HttpResponseInfo* response1 = trans1->GetResponseInfo();
+  ASSERT_TRUE(response1);
+  ASSERT_TRUE(response1->headers);
+  EXPECT_EQ(200, response1->headers->response_code());
+  EXPECT_TRUE(response1->headers->IsKeepAlive());
+
+  std::string response_data1;
+  EXPECT_EQ(OK, ReadTransaction(trans1.get(), &response_data1));
+  EXPECT_EQ("This server is borked.", response_data1);
+  // Deleting the transaction attempts to release the socket back into the
+  // socket pool.
+  trans1.reset();
+
+  HttpRequestInfo request2;
+  request2.method = "GET";
+  request2.url = GURL("http://www.borked.com/foo");
+
+  scoped_ptr<HttpTransaction> trans2(
+      new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
+  rv = trans2->Start(&request2, callback.callback(), BoundNetLog());
+  EXPECT_EQ(OK, callback.GetResult(rv));
+
+  const HttpResponseInfo* response2 = trans2->GetResponseInfo();
+  ASSERT_TRUE(response2);
+  ASSERT_TRUE(response2->headers);
+  EXPECT_EQ(200, response2->headers->response_code());
+
+  std::string response_data2;
+  EXPECT_EQ(OK, ReadTransaction(trans2.get(), &response_data2));
+  EXPECT_EQ("foo", response_data2);
+}
+
+TEST_P(HttpNetworkTransactionTest, KeepAliveWithUnusedData3) {
+  scoped_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
+  MockWrite data_writes1[] = {
+      MockWrite("GET / HTTP/1.1\r\n"
+                "Host: www.borked.com\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockRead data_reads1[] = {
+      MockRead("HTTP/1.1 200 OK\r\n"
+               "Connection: keep-alive\r\n"
+               "Transfer-Encoding: chunked\r\n\r\n"),
+      MockRead("16\r\nThis server is borked.\r\n"),
+      MockRead("0\r\n\r\nBonus data!"),
+  };
+
+  MockWrite data_writes2[] = {
+      MockWrite("GET /foo HTTP/1.1\r\n"
+                "Host: www.borked.com\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockRead data_reads2[] = {
+      MockRead("HTTP/1.1 200 OK\r\n"
+               "Content-Length: 3\r\n\r\n"
+               "foo"),
+  };
+  StaticSocketDataProvider data1(data_reads1, arraysize(data_reads1),
+                                 data_writes1, arraysize(data_writes1));
+  session_deps_.socket_factory->AddSocketDataProvider(&data1);
+  StaticSocketDataProvider data2(data_reads2, arraysize(data_reads2),
+                                 data_writes2, arraysize(data_writes2));
+  session_deps_.socket_factory->AddSocketDataProvider(&data2);
+
+  TestCompletionCallback callback;
+  HttpRequestInfo request1;
+  request1.method = "GET";
+  request1.url = GURL("http://www.borked.com/");
+
+  scoped_ptr<HttpTransaction> trans1(
+      new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
+  int rv = trans1->Start(&request1, callback.callback(), BoundNetLog());
+  EXPECT_EQ(OK, callback.GetResult(rv));
+
+  const HttpResponseInfo* response1 = trans1->GetResponseInfo();
+  ASSERT_TRUE(response1);
+  ASSERT_TRUE(response1->headers);
+  EXPECT_EQ(200, response1->headers->response_code());
+  EXPECT_TRUE(response1->headers->IsKeepAlive());
+
+  std::string response_data1;
+  EXPECT_EQ(OK, ReadTransaction(trans1.get(), &response_data1));
+  EXPECT_EQ("This server is borked.", response_data1);
+  // Deleting the transaction attempts to release the socket back into the
+  // socket pool.
+  trans1.reset();
+
+  HttpRequestInfo request2;
+  request2.method = "GET";
+  request2.url = GURL("http://www.borked.com/foo");
+
+  scoped_ptr<HttpTransaction> trans2(
+      new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
+  rv = trans2->Start(&request2, callback.callback(), BoundNetLog());
+  EXPECT_EQ(OK, callback.GetResult(rv));
+
+  const HttpResponseInfo* response2 = trans2->GetResponseInfo();
+  ASSERT_TRUE(response2);
+  ASSERT_TRUE(response2->headers);
+  EXPECT_EQ(200, response2->headers->response_code());
+
+  std::string response_data2;
+  EXPECT_EQ(OK, ReadTransaction(trans2.get(), &response_data2));
+  EXPECT_EQ("foo", response_data2);
+}
+
+// This is a little different from the others - it tests the case that the
+// HttpStreamParser doesn't know if there's extra data on a socket or not when
+// the HttpNetworkTransaction is torn down, because the response body hasn't
+// been read from yet, but the request goes through the HttpResponseBodyDrainer.
+TEST_P(HttpNetworkTransactionTest, KeepAliveWithUnusedData4) {
+  scoped_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
+  MockWrite data_writes1[] = {
+      MockWrite("GET / HTTP/1.1\r\n"
+                "Host: www.borked.com\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockRead data_reads1[] = {
+      MockRead("HTTP/1.1 200 OK\r\n"
+               "Connection: keep-alive\r\n"
+               "Transfer-Encoding: chunked\r\n\r\n"),
+      MockRead("16\r\nThis server is borked.\r\n"),
+      MockRead("0\r\n\r\nBonus data!"),
+  };
+
+  MockWrite data_writes2[] = {
+      MockWrite("GET /foo HTTP/1.1\r\n"
+                "Host: www.borked.com\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockRead data_reads2[] = {
+      MockRead("HTTP/1.1 200 OK\r\n"
+               "Content-Length: 3\r\n\r\n"
+               "foo"),
+  };
+  StaticSocketDataProvider data1(data_reads1, arraysize(data_reads1),
+                                 data_writes1, arraysize(data_writes1));
+  session_deps_.socket_factory->AddSocketDataProvider(&data1);
+  StaticSocketDataProvider data2(data_reads2, arraysize(data_reads2),
+                                 data_writes2, arraysize(data_writes2));
+  session_deps_.socket_factory->AddSocketDataProvider(&data2);
+
+  TestCompletionCallback callback;
+  HttpRequestInfo request1;
+  request1.method = "GET";
+  request1.url = GURL("http://www.borked.com/");
+
+  scoped_ptr<HttpTransaction> trans1(
+      new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
+  int rv = trans1->Start(&request1, callback.callback(), BoundNetLog());
+  EXPECT_EQ(OK, callback.GetResult(rv));
+
+  const HttpResponseInfo* response1 = trans1->GetResponseInfo();
+  ASSERT_TRUE(response1);
+  ASSERT_TRUE(response1->headers);
+  EXPECT_EQ(200, response1->headers->response_code());
+  EXPECT_TRUE(response1->headers->IsKeepAlive());
+
+  // Deleting the transaction creates an HttpResponseBodyDrainer to read the
+  // response body.
+  trans1.reset();
+
+  // Let the HttpResponseBodyDrainer drain the socket.  It should determine the
+  // socket can't be reused, rather than returning it to the socket pool.
+  base::RunLoop().RunUntilIdle();
+
+  HttpRequestInfo request2;
+  request2.method = "GET";
+  request2.url = GURL("http://www.borked.com/foo");
+
+  scoped_ptr<HttpTransaction> trans2(
+      new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
+  rv = trans2->Start(&request2, callback.callback(), BoundNetLog());
+  EXPECT_EQ(OK, callback.GetResult(rv));
+
+  const HttpResponseInfo* response2 = trans2->GetResponseInfo();
+  ASSERT_TRUE(response2);
+  ASSERT_TRUE(response2->headers);
+  EXPECT_EQ(200, response2->headers->response_code());
+
+  std::string response_data2;
+  EXPECT_EQ(OK, ReadTransaction(trans2.get(), &response_data2));
+  EXPECT_EQ("foo", response_data2);
+}
+
 // Test the request-challenge-retry sequence for basic auth.
 // (basic auth is the easiest to mock, because it has no randomness).
 TEST_P(HttpNetworkTransactionTest, BasicAuth) {
   HttpRequestInfo request;
   request.method = "GET";
   request.url = GURL("http://www.example.org/");
   request.load_flags = 0;
 
   TestNetLog log;
   session_deps_.net_log = &log;
@@ skipping 5090 matching lines @@
   scoped_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
   scoped_ptr<HttpNetworkTransaction> trans(
       new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
 
   // Setup some state (which we expect ResetStateForRestart() will clear).
   trans->read_buf_ = new IOBuffer(15);
   trans->read_buf_len_ = 15;
   trans->request_headers_.SetHeader("Authorization", "NTLM");
 
   // Setup state in response_
-  HttpResponseInfo* response = &trans->response_;
+  HttpResponseInfo* response = trans->response_.get();
   response->auth_challenge = new AuthChallengeInfo();
   response->ssl_info.cert_status = static_cast<CertStatus>(-1);  // Nonsensical.
   response->response_time = base::Time::Now();
   response->was_cached = true;  // (Wouldn't ever actually be true...)
 
   { // Setup state for response_.vary_data
     HttpRequestInfo request;
     std::string temp("HTTP/1.1 200 OK\nVary: foo, bar\n\n");
     std::replace(temp.begin(), temp.end(), '\n', '\0');
     scoped_refptr<HttpResponseHeaders> headers(new HttpResponseHeaders(temp));
@@ skipping 7426 matching lines @@
   }
 
   bool GetRemoteEndpoint(IPEndPoint* endpoint) override { return false; }
 
   Error GetSignedEKMForTokenBinding(crypto::ECPrivateKey* key,
                                     std::vector<uint8_t>* out) override {
     ADD_FAILURE();
     return ERR_NOT_IMPLEMENTED;
   }
 
-  void Drain(HttpNetworkSession* session) override { ADD_FAILURE(); }
+  void Drain(HttpNetworkSession* session,
+             scoped_ptr<HttpResponseInfo> response_info) override {
+    ADD_FAILURE();
+  }
 
   void PopulateNetErrorDetails(NetErrorDetails* details) override { return; }
 
   void SetPriority(RequestPriority priority) override { priority_ = priority; }
 
   UploadProgress GetUploadProgress() const override { return UploadProgress(); }
 
   HttpStream* RenewStreamForAuth() override { return NULL; }
 
  private:
@@ skipping 216 matching lines @@
   }
 
   bool GetRemoteEndpoint(IPEndPoint* endpoint) override { return false; }
 
   Error GetSignedEKMForTokenBinding(crypto::ECPrivateKey* key,
                                     std::vector<uint8_t>* out) override {
     ADD_FAILURE();
     return ERR_NOT_IMPLEMENTED;
   }
 
-  void Drain(HttpNetworkSession* session) override { NOTREACHED(); }
+  void Drain(HttpNetworkSession* session,
+             scoped_ptr<HttpResponseInfo> response_info) override {
+    NOTREACHED();
+  }
 
   void PopulateNetErrorDetails(NetErrorDetails* details) override { return; }
 
   void SetPriority(RequestPriority priority) override { NOTREACHED(); }
 
   UploadProgress GetUploadProgress() const override {
     NOTREACHED();
     return UploadProgress();
   }
 
@@ skipping 1173 matching lines @@
   base::MessageLoop::current()->RunUntilIdle();
 
   EXPECT_TRUE(trans.GetResponseInfo()->was_fetched_via_spdy);
   HttpRequestHeaders headers;
   ASSERT_TRUE(trans.GetFullRequestHeaders(&headers));
   EXPECT_TRUE(headers.HasHeader(HttpRequestHeaders::kTokenBinding));
 }
 #endif  // !defined(OS_IOS)
 
 }  // namespace net