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

net/http/http_network_transaction.cc

Index: net/http/http_network_transaction.cc
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 5f7c4bd99743851434119aee7608fc88bfce0635..773c09bb1f101e7707228f21c1731f490b16ea9c 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -107,8 +107,9 @@ HttpNetworkTransaction::HttpNetworkTransaction(RequestPriority priority,
       io_callback_(base::Bind(&HttpNetworkTransaction::OnIOComplete,
                               base::Unretained(this))),
       session_(session),
-      request_(NULL),
+      request_(nullptr),
       priority_(priority),
+      response_(new HttpResponseInfo()),
       headers_valid_(false),
       server_ssl_failure_state_(SSL_FAILURE_NONE),
       fallback_error_code_(ERR_SSL_INAPPROPRIATE_FALLBACK),
@@ -140,7 +141,7 @@ HttpNetworkTransaction::~HttpNetworkTransaction() {
     } else {
       // Otherwise, we try to drain the response body.
       HttpStream* stream = stream_.release();
-      stream->Drain(session_);
+      stream->Drain(session_, std::move(response_));
     }
   }
 
@@ -160,7 +161,7 @@ int HttpNetworkTransaction::Start(const HttpRequestInfo* request_info,
   }
 
   if (request_->load_flags & LOAD_PREFETCH)
-    response_.unused_since_prefetch = true;
+    response_->unused_since_prefetch = true;
 
   // Channel ID is disabled if privacy mode is enabled for this request.
   if (request_->privacy_mode == PRIVACY_MODE_ENABLED)
@@ -202,13 +203,14 @@ int HttpNetworkTransaction::RestartWithCertificate(
   DCHECK(!stream_.get());
   DCHECK_EQ(STATE_NONE, next_state_);
 
-  SSLConfig* ssl_config = response_.cert_request_info->is_proxy ?
-      &proxy_ssl_config_ : &server_ssl_config_;
+  SSLConfig* ssl_config = response_->cert_request_info->is_proxy
+                              ? &proxy_ssl_config_
+                              : &server_ssl_config_;
   ssl_config->send_client_cert = true;
   ssl_config->client_cert = client_cert;
   ssl_config->client_private_key = client_private_key;
   session_->ssl_client_auth_cache()->Add(
-      response_.cert_request_info->host_and_port, client_cert,
+      response_->cert_request_info->host_and_port, client_cert,
       client_private_key);
   // Reset the other member variables.
   // Note: this is necessary only with SSL renegotiation.
@@ -382,7 +384,7 @@ int64_t HttpNetworkTransaction::GetTotalSentBytes() const {
 void HttpNetworkTransaction::DoneReading() {}
 
 const HttpResponseInfo* HttpNetworkTransaction::GetResponseInfo() const {
-  return &response_;
+  return response_.get();
 }
 
 LoadState HttpNetworkTransaction::GetLoadState() const {
@@ -485,13 +487,13 @@ void HttpNetworkTransaction::OnStreamReady(const SSLConfig& used_ssl_config,
   stream_.reset(stream);
   server_ssl_config_ = used_ssl_config;
   proxy_info_ = used_proxy_info;
-  response_.was_npn_negotiated = stream_request_->was_npn_negotiated();
-  response_.npn_negotiated_protocol = SSLClientSocket::NextProtoToString(
+  response_->was_npn_negotiated = stream_request_->was_npn_negotiated();
+  response_->npn_negotiated_protocol = SSLClientSocket::NextProtoToString(
       stream_request_->protocol_negotiated());
-  response_.was_fetched_via_spdy = stream_request_->using_spdy();
-  response_.was_fetched_via_proxy = !proxy_info_.is_direct();
-  if (response_.was_fetched_via_proxy && !proxy_info_.is_empty())
-    response_.proxy_server = proxy_info_.proxy_server().host_port_pair();
+  response_->was_fetched_via_spdy = stream_request_->using_spdy();
+  response_->was_fetched_via_proxy = !proxy_info_.is_direct();
+  if (response_->was_fetched_via_proxy && !proxy_info_.is_empty())
+    response_->proxy_server = proxy_info_.proxy_server().host_port_pair();
   OnIOComplete(OK);
 }
 
@@ -531,7 +533,7 @@ void HttpNetworkTransaction::OnCertificateError(
   DCHECK(stream_request_.get());
   DCHECK(!stream_.get());
 
-  response_.ssl_info = ssl_info;
+  response_->ssl_info = ssl_info;
   server_ssl_config_ = used_ssl_config;
 
   // TODO(mbelshe):  For now, we're going to pass the error through, and that
@@ -552,8 +554,8 @@ void HttpNetworkTransaction::OnNeedsProxyAuth(
   DCHECK_EQ(STATE_CREATE_STREAM_COMPLETE, next_state_);
 
   establishing_tunnel_ = true;
-  response_.headers = proxy_response.headers;
-  response_.auth_challenge = proxy_response.auth_challenge;
+  response_->headers = proxy_response.headers;
+  response_->auth_challenge = proxy_response.auth_challenge;
   headers_valid_ = true;
   server_ssl_config_ = used_ssl_config;
   proxy_info_ = used_proxy_info;
@@ -570,7 +572,7 @@ void HttpNetworkTransaction::OnNeedsClientAuth(
   DCHECK_EQ(STATE_CREATE_STREAM_COMPLETE, next_state_);
 
   server_ssl_config_ = used_ssl_config;
-  response_.cert_request_info = cert_info;
+  response_->cert_request_info = cert_info;
   OnIOComplete(ERR_SSL_CLIENT_AUTH_CERT_NEEDED);
 }
 
@@ -584,7 +586,7 @@ void HttpNetworkTransaction::OnHttpsProxyTunnelResponse(
   CopyConnectionAttemptsFromStreamRequest();
 
   headers_valid_ = true;
-  response_ = response_info;
+  *response_ = response_info;
   server_ssl_config_ = used_ssl_config;
   proxy_info_ = used_proxy_info;
   if (stream_) {
@@ -795,7 +797,7 @@ int HttpNetworkTransaction::DoCreateStream() {
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "424359 HttpNetworkTransaction::DoCreateStream"));
 
-  response_.network_accessed = true;
+  response_->network_accessed = true;
 
   next_state_ = STATE_CREATE_STREAM_COMPLETE;
   if (ForWebSocketHandshake()) {
@@ -1020,7 +1022,7 @@ int HttpNetworkTransaction::BuildRequestHeaders(
       !before_proxy_headers_sent_callback_.is_null())
     before_proxy_headers_sent_callback_.Run(proxy_info_, &request_headers_);
 
-  response_.did_use_http_auth =
+  response_->did_use_http_auth =
       request_headers_.HasHeader(HttpRequestHeaders::kAuthorization) ||
       request_headers_.HasHeader(HttpRequestHeaders::kProxyAuthorization);
   return OK;
@@ -1097,7 +1099,7 @@ int HttpNetworkTransaction::DoSendRequest() {
   send_start_time_ = base::TimeTicks::Now();
   next_state_ = STATE_SEND_REQUEST_COMPLETE;
 
-  return stream_->SendRequest(request_headers_, &response_, io_callback_);
+  return stream_->SendRequest(request_headers_, response_.get(), io_callback_);
 }
 
 int HttpNetworkTransaction::DoSendRequestComplete(int result) {
@@ -1127,8 +1129,8 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
     // TODO(wtc): Need a test case for this code path!
     DCHECK(stream_.get());
     DCHECK(IsSecureRequest());
-    response_.cert_request_info = new SSLCertRequestInfo;
-    stream_->GetSSLCertRequestInfo(response_.cert_request_info.get());
+    response_->cert_request_info = new SSLCertRequestInfo;
+    stream_->GetSSLCertRequestInfo(response_->cert_request_info.get());
     result = HandleCertificateRequest(result);
     if (result == OK)
       return result;
@@ -1146,22 +1148,22 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
   // TODO(davidben): Consider moving this to HttpBasicStream, It's a little
   // bizarre for SPDY. Assuming this logic is useful at all.
   // TODO(davidben): Bubble the error code up so we do not cache?
-  if (result == ERR_CONNECTION_CLOSED && response_.headers.get())
+  if (result == ERR_CONNECTION_CLOSED && response_->headers.get())
     result = OK;
 
   if (result < 0)
     return HandleIOError(result);
 
-  DCHECK(response_.headers.get());
+  DCHECK(response_->headers.get());
 
   // On a 408 response from the server ("Request Timeout") on a stale socket,
   // retry the request.
   // Headers can be NULL because of http://crbug.com/384554.
-  if (response_.headers.get() && response_.headers->response_code() == 408 &&
+  if (response_->headers.get() && response_->headers->response_code() == 408 &&
       stream_->IsConnectionReused()) {
     net_log_.AddEventWithNetErrorCode(
         NetLog::TYPE_HTTP_TRANSACTION_RESTART_AFTER_ERROR,
-        response_.headers->response_code());
+        response_->headers->response_code());
     // This will close the socket - it would be weird to try and reuse it, even
     // if the server doesn't actually close it.
     ResetConnectionAndRequestForResend();
@@ -1170,16 +1172,16 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
 
   // Like Net.HttpResponseCode, but only for MAIN_FRAME loads.
   if (request_->load_flags & LOAD_MAIN_FRAME) {
-    const int response_code = response_.headers->response_code();
+    const int response_code = response_->headers->response_code();
     UMA_HISTOGRAM_ENUMERATION(
         "Net.HttpResponseCode_Nxx_MainFrame", response_code/100, 10);
   }
 
   net_log_.AddEvent(
       NetLog::TYPE_HTTP_TRANSACTION_READ_RESPONSE_HEADERS,
-      base::Bind(&HttpResponseHeaders::NetLogCallback, response_.headers));
+      base::Bind(&HttpResponseHeaders::NetLogCallback, response_->headers));
 
-  if (response_.headers->GetHttpVersion() < HttpVersion(1, 0)) {
+  if (response_->headers->GetHttpVersion() < HttpVersion(1, 0)) {
     // HTTP/0.9 doesn't support the PUT method, so lack of response headers
     // indicates a buggy server.  See:
     // https://bugzilla.mozilla.org/show_bug.cgi?id=193921
@@ -1193,18 +1195,18 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
   // We treat any other 1xx in this same way (although in practice getting
   // a 1xx that isn't a 100 is rare).
   // Unless this is a WebSocket request, in which case we pass it on up.
-  if (response_.headers->response_code() / 100 == 1 &&
+  if (response_->headers->response_code() / 100 == 1 &&
       !ForWebSocketHandshake()) {
-    response_.headers = new HttpResponseHeaders(std::string());
+    response_->headers = new HttpResponseHeaders(std::string());
     next_state_ = STATE_READ_HEADERS;
     return OK;
   }
 
   session_->http_stream_factory()->ProcessAlternativeServices(
-      session_, response_.headers.get(), HostPortPair::FromURL(request_->url));
+      session_, response_->headers.get(), HostPortPair::FromURL(request_->url));
 
   if (IsSecureRequest())
-    stream_->GetSSLInfo(&response_.ssl_info);
+    stream_->GetSSLInfo(&response_->ssl_info);
 
   int rv = HandleAuthChallenge();
   if (rv != OK)
@@ -1328,7 +1330,7 @@ int HttpNetworkTransaction::HandleCertificateRequest(int error) {
   scoped_refptr<X509Certificate> client_cert;
   scoped_refptr<SSLPrivateKey> client_private_key;
   bool found_cached_cert = session_->ssl_client_auth_cache()->Lookup(
-      response_.cert_request_info->host_and_port, &client_cert,
+      response_->cert_request_info->host_and_port, &client_cert,
       &client_private_key);
   if (!found_cached_cert)
     return error;
@@ -1338,7 +1340,7 @@ int HttpNetworkTransaction::HandleCertificateRequest(int error) {
   // CertificateRequest message.
   if (client_cert.get()) {
     const std::vector<std::string>& cert_authorities =
-        response_.cert_request_info->cert_authorities;
+        response_->cert_request_info->cert_authorities;
 
     bool cert_still_valid = cert_authorities.empty() ||
         client_cert->IsIssuedByEncoded(cert_authorities);
@@ -1349,8 +1351,9 @@ int HttpNetworkTransaction::HandleCertificateRequest(int error) {
   // TODO(davidben): Add a unit test which covers this path; we need to be
   // able to send a legitimate certificate and also bypass/clear the
   // SSL session cache.
-  SSLConfig* ssl_config = response_.cert_request_info->is_proxy ?
-      &proxy_ssl_config_ : &server_ssl_config_;
+  SSLConfig* ssl_config = response_->cert_request_info->is_proxy
+                              ? &proxy_ssl_config_
+                              : &server_ssl_config_;
   ssl_config->send_client_cert = true;
   ssl_config->client_cert = client_cert;
   ssl_config->client_private_key = client_private_key;
@@ -1522,11 +1525,11 @@ void HttpNetworkTransaction::ResetStateForAuthRestart() {
   send_end_time_ = base::TimeTicks();
 
   pending_auth_target_ = HttpAuth::AUTH_NONE;
-  read_buf_ = NULL;
+  read_buf_ = nullptr;
   read_buf_len_ = 0;
   headers_valid_ = false;
   request_headers_.Clear();
-  response_ = HttpResponseInfo();
+  *response_ = HttpResponseInfo();
   establishing_tunnel_ = false;
   remote_endpoint_ = IPEndPoint();
   net_error_details_.quic_broken = false;
@@ -1609,7 +1612,7 @@ void HttpNetworkTransaction::RecordSSLFallbackMetrics(int result) {
 }
 
 HttpResponseHeaders* HttpNetworkTransaction::GetResponseHeaders() const {
-  return response_.headers.get();
+  return response_->headers.get();
 }
 
 bool HttpNetworkTransaction::ShouldResendRequest() const {
@@ -1665,7 +1668,7 @@ int HttpNetworkTransaction::HandleAuthChallenge() {
     return ERR_UNEXPECTED_PROXY_AUTH;
 
   int rv = auth_controllers_[target]->HandleAuthChallenge(
-      headers, response_.ssl_info,
+      headers, response_->ssl_info,
       (request_->load_flags & LOAD_DO_NOT_SEND_AUTH_DATA) != 0, false,
       net_log_);
   if (auth_controllers_[target]->HaveAuthHandler())
@@ -1674,7 +1677,7 @@ int HttpNetworkTransaction::HandleAuthChallenge() {
   scoped_refptr<AuthChallengeInfo> auth_info =
       auth_controllers_[target]->auth_info();
   if (auth_info.get())
-      response_.auth_challenge = auth_info;
+    response_->auth_challenge = auth_info;
 
   return rv;
 }