Detach the ClientSocketHandle from an HttpStream so the same socket can be reused for multiple rounds of

NTLM authentication.

BUG=57261
TEST=Start chrome with --auth-schemes=NTLM and --proxy-server pointing to a Microsoft Forefront Threat Management Gateway proxy configurated for Integrated Authentication. Assuming the user is part of the same domain as the proxy, authentication should work transparently, and the user should not be presented with auth prompts. Also, net_unittests should pass.

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

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

13 files changed, 91 insertions, 11 deletions

diff --git a/net/http/http_basic_stream.cc b/net/http/http_basic_stream.cc
index d90cb73..4303291 100644
--- a/net/http/http_basic_stream.cc
+++ b/net/http/http_basic_stream.cc
@@ -19,6 +19,7 @@ HttpBasicStream::HttpBasicStream(ClientSocketHandle* connection)
 int HttpBasicStream::InitializeStream(const HttpRequestInfo* request_info,
                                       const BoundNetLog& net_log,
                                       CompletionCallback* callback) {
+  DCHECK(!IsDetached());
   parser_.reset(new HttpStreamParser(connection_.get(), request_info,
                                      read_buf_, net_log));
   return OK;
@@ -30,59 +31,80 @@ int HttpBasicStream::SendRequest(const std::string& headers,
                                  HttpResponseInfo* response,
                                  CompletionCallback* callback) {
   DCHECK(parser_.get());
+  DCHECK(!IsDetached());
   return parser_->SendRequest(headers, request_body, response, callback);
 }
 
 HttpBasicStream::~HttpBasicStream() {}
 
 uint64 HttpBasicStream::GetUploadProgress() const {
+  DCHECK(!IsDetached());
   return parser_->GetUploadProgress();
 }
 
 int HttpBasicStream::ReadResponseHeaders(CompletionCallback* callback) {
+  DCHECK(!IsDetached());
   return parser_->ReadResponseHeaders(callback);
 }
 
 const HttpResponseInfo* HttpBasicStream::GetResponseInfo() const {
+  DCHECK(!IsDetached());
   return parser_->GetResponseInfo();
 }
 
 int HttpBasicStream::ReadResponseBody(IOBuffer* buf, int buf_len,
                                       CompletionCallback* callback) {
+  DCHECK(!IsDetached());
   return parser_->ReadResponseBody(buf, buf_len, callback);
 }
 
 void HttpBasicStream::Close(bool not_reusable) {
+  DCHECK(!IsDetached());
   parser_->Close(not_reusable);
 }
 
 bool HttpBasicStream::IsResponseBodyComplete() const {
+  DCHECK(!IsDetached());
   return parser_->IsResponseBodyComplete();
 }
 
 bool HttpBasicStream::CanFindEndOfResponse() const {
+  DCHECK(!IsDetached());
   return parser_->CanFindEndOfResponse();
 }
 
 bool HttpBasicStream::IsMoreDataBuffered() const {
+  DCHECK(!IsDetached());
   return parser_->IsMoreDataBuffered();
 }
 
 bool HttpBasicStream::IsConnectionReused() const {
+  DCHECK(!IsDetached());
   return parser_->IsConnectionReused();
 }
 
 void HttpBasicStream::SetConnectionReused() {
+  DCHECK(!IsDetached());
   parser_->SetConnectionReused();
 }
 
+ClientSocketHandle* HttpBasicStream::DetachConnection() {
+  return connection_.release();
+}
+
 void HttpBasicStream::GetSSLInfo(SSLInfo* ssl_info) {
+  DCHECK(!IsDetached());
   parser_->GetSSLInfo(ssl_info);
 }
 
 void HttpBasicStream::GetSSLCertRequestInfo(
     SSLCertRequestInfo* cert_request_info) {
+  DCHECK(!IsDetached());
   parser_->GetSSLCertRequestInfo(cert_request_info);
 }
 
+bool HttpBasicStream::IsDetached() const {
+  return connection_.get() == NULL;
+}
+
 }  // namespace net
diff --git a/net/http/http_basic_stream.h b/net/http/http_basic_stream.h
index a572bfc..2da2f4d 100644
--- a/net/http/http_basic_stream.h
+++ b/net/http/http_basic_stream.h
@@ -63,11 +63,15 @@ class HttpBasicStream : public HttpStream {
 
   virtual void SetConnectionReused();
 
+  virtual ClientSocketHandle* DetachConnection();
+
   virtual void GetSSLInfo(SSLInfo* ssl_info);
 
   virtual void GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info);
 
  private:
+  bool IsDetached() const;
+
   scoped_refptr<GrowableIOBuffer> read_buf_;
 
   scoped_ptr<HttpStreamParser> parser_;
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 7cf599e..cab9831 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -329,8 +329,11 @@ void HttpNetworkTransaction::DidDrainBodyForAuthRestart(bool keep_alive) {
       // We should call connection_->set_idle_time(), but this doesn't occur
       // often enough to be worth the trouble.
       stream_->SetConnectionReused();
+      DCHECK(!auth_connection_.get());
+      auth_connection_.reset(stream_->DetachConnection());
+    } else {
+      stream_->Close(true);
     }
-    stream_->Close(!keep_alive);
     next_state_ = STATE_CREATE_STREAM;
   }
 
@@ -585,13 +588,15 @@ int HttpNetworkTransaction::DoLoop(int result) {
 int HttpNetworkTransaction::DoCreateStream() {
   next_state_ = STATE_CREATE_STREAM_COMPLETE;
 
-  session_->http_stream_factory()->RequestStream(request_,
-                                                 &ssl_config_,
-                                                 &proxy_info_,
-                                                 this,
-                                                 net_log_,
-                                                 session_,
-                                                 &stream_request_);
+  session_->http_stream_factory()->RequestStream(
+      request_,
+      &ssl_config_,
+      &proxy_info_,
+      auth_connection_.release(),
+      this,
+      net_log_,
+      session_,
+      &stream_request_);
   return ERR_IO_PENDING;
 }
 
diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h
index f0080e2..d52b8ec 100644
--- a/net/http/http_network_transaction.h
+++ b/net/http/http_network_transaction.h
@@ -24,6 +24,7 @@
 
 namespace net {
 
+class ClientSocketHandle;
 class HttpAuthController;
 class HttpNetworkSession;
 class HttpStream;
@@ -208,6 +209,10 @@ class HttpNetworkTransaction : public HttpTransaction,
   scoped_refptr<StreamFactory::StreamRequestJob> stream_request_;
   scoped_ptr<HttpStream> stream_;
 
+  // Reuse the same connection for each round of a connection-based HTTP
+  // authentication scheme.
+  scoped_ptr<ClientSocketHandle> auth_connection_;
+
   // True if we've validated the headers that the stream parser has returned.
   bool headers_valid_;
 
diff --git a/net/http/http_response_body_drainer_unittest.cc b/net/http/http_response_body_drainer_unittest.cc
index 6226b98..4858a27 100644
--- a/net/http/http_response_body_drainer_unittest.cc
+++ b/net/http/http_response_body_drainer_unittest.cc
@@ -94,6 +94,8 @@ class MockHttpStream : public HttpStream {
   virtual bool IsMoreDataBuffered() const { return false; }
   virtual bool IsConnectionReused() const { return false; }
   virtual void SetConnectionReused() {}
+  virtual ClientSocketHandle* DetachConnection() { return NULL; }
+
   virtual void GetSSLInfo(SSLInfo* ssl_info) {}
   virtual void GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info) {}
 
diff --git a/net/http/http_stream.h b/net/http/http_stream.h
index 211267d..601d01b 100644
--- a/net/http/http_stream.h
+++ b/net/http/http_stream.h
@@ -22,6 +22,7 @@
 namespace net {
 
 class BoundNetLog;
+class ClientSocketHandle;
 class HttpResponseInfo;
 class IOBuffer;
 class SSLCertRequestInfo;
@@ -108,6 +109,17 @@ class HttpStream {
   virtual bool IsConnectionReused() const = 0;
   virtual void SetConnectionReused() = 0;
 
+  // Detach the connection from this HttpStream. The caller is responsible
+  // for deleting the handle. After this is called, none of the other HttpStream
+  // methods should be called.
+  //
+  // The return value may be NULL. In that case, the underlying connection
+  // is either unavailable, or can be consistently rediscoverable.
+  //
+  // TODO(cbentzel): Consider ResetForAuth() approach instead.
+  // http://crbug.com/58192
+  virtual ClientSocketHandle* DetachConnection() = 0;
+
   // Get the SSLInfo associated with this stream's connection.  This should
   // only be called for streams over SSL sockets, otherwise the behavior is
   // undefined.
diff --git a/net/http/http_stream_factory.cc b/net/http/http_stream_factory.cc
index 03237ab..bbe2878 100644
--- a/net/http/http_stream_factory.cc
+++ b/net/http/http_stream_factory.cc
@@ -46,13 +46,20 @@ void HttpStreamFactory::RequestStream(
     const HttpRequestInfo* request_info,
     SSLConfig* ssl_config,
     ProxyInfo* proxy_info,
+    ClientSocketHandle* connection,
     StreamFactory::StreamRequestDelegate* delegate,
     const BoundNetLog& net_log,
     const scoped_refptr<HttpNetworkSession>& session,
     scoped_refptr<StreamRequestJob>* stream) {
   DCHECK(stream != NULL);
   *stream = new HttpStreamRequest(this, session);
-  (*stream)->Start(request_info, ssl_config, proxy_info, delegate, net_log);
+  (*stream)->Start(
+      request_info,
+      ssl_config,
+      proxy_info,
+      connection,
+      delegate,
+      net_log);
 }
 
 void HttpStreamFactory::AddTLSIntolerantServer(const GURL& url) {
diff --git a/net/http/http_stream_factory.h b/net/http/http_stream_factory.h
index 5c6f179..804ef35 100644
--- a/net/http/http_stream_factory.h
+++ b/net/http/http_stream_factory.h
@@ -33,6 +33,7 @@ class HttpStreamFactory : public StreamFactory,
   virtual void RequestStream(const HttpRequestInfo* info,
                              SSLConfig* ssl_config,
                              ProxyInfo* proxy_info,
+                             ClientSocketHandle* connection,
                              StreamRequestDelegate* delegate,
                              const BoundNetLog& net_log,
                              const scoped_refptr<HttpNetworkSession>& session,
diff --git a/net/http/http_stream_request.cc b/net/http/http_stream_request.cc
index 86cf0ef..35b2145 100644
--- a/net/http/http_stream_request.cc
+++ b/net/http/http_stream_request.cc
@@ -54,7 +54,6 @@ HttpStreamRequest::HttpStreamRequest(
       session_(session),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           io_callback_(this, &HttpStreamRequest::OnIOComplete)),
-      connection_(new ClientSocketHandle),
       factory_(factory),
       delegate_(NULL),
       next_state_(STATE_NONE),
@@ -91,6 +90,7 @@ HttpStreamRequest::~HttpStreamRequest() {
 void HttpStreamRequest::Start(const HttpRequestInfo* request_info,
                               SSLConfig* ssl_config,
                               ProxyInfo* proxy_info,
+                              ClientSocketHandle* connection,
                               StreamFactory::StreamRequestDelegate* delegate,
                               const BoundNetLog& net_log) {
   CHECK_EQ(STATE_NONE, next_state_);
@@ -101,7 +101,15 @@ void HttpStreamRequest::Start(const HttpRequestInfo* request_info,
   proxy_info_ = proxy_info;
   delegate_ = delegate;
   net_log_ = net_log;
-  next_state_ = STATE_RESOLVE_PROXY;
+  if (connection) {
+    DCHECK(!using_spdy_);
+    DCHECK(connection->is_initialized());
+    connection_.reset(connection);
+    next_state_ = STATE_CREATE_STREAM;
+  } else {
+    connection_.reset(new ClientSocketHandle);
+    next_state_ = STATE_RESOLVE_PROXY;
+  }
   int rv = RunLoop(OK);
   DCHECK_EQ(ERR_IO_PENDING, rv);
 }
diff --git a/net/http/http_stream_request.h b/net/http/http_stream_request.h
index a60c96d..10e6525 100644
--- a/net/http/http_stream_request.h
+++ b/net/http/http_stream_request.h
@@ -42,6 +42,7 @@ class HttpStreamRequest : public StreamFactory::StreamRequestJob {
   virtual void Start(const HttpRequestInfo* request_info,
                      SSLConfig* ssl_config,
                      ProxyInfo* proxy_info,
+                     ClientSocketHandle* connection,
                      StreamFactory::StreamRequestDelegate* delegate,
                      const BoundNetLog& net_log);
   virtual void Cancel();
diff --git a/net/http/stream_factory.h b/net/http/stream_factory.h
index 9ec85f1..6e89c53 100644
--- a/net/http/stream_factory.h
+++ b/net/http/stream_factory.h
@@ -13,6 +13,7 @@
 namespace net {
 
 class BoundNetLog;
+class ClientSocketHandle;
 class HostPortPair;
 class HttpAlternateProtocols;
 class HttpAuthController;
@@ -83,6 +84,7 @@ class StreamFactory {
     virtual void Start(const HttpRequestInfo* request_info,
                        SSLConfig* ssl_config,
                        ProxyInfo* proxy_info,
+                       ClientSocketHandle* connection,
                        StreamRequestDelegate* delegate,
                        const BoundNetLog& net_log) = 0;
 
@@ -128,6 +130,7 @@ class StreamFactory {
   virtual void RequestStream(const HttpRequestInfo* info,
                              SSLConfig* ssl_config,
                              ProxyInfo* proxy_info,
+                             ClientSocketHandle* connection,
                              StreamRequestDelegate* delegate,
                              const BoundNetLog& net_log,
                              const scoped_refptr<HttpNetworkSession>& session,
diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc
index 0e3962e..aa86604 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/net/spdy/spdy_http_stream.cc
@@ -381,4 +381,12 @@ void SpdyHttpStream::GetSSLCertRequestInfo(
   stream_->GetSSLCertRequestInfo(cert_request_info);
 }
 
+ClientSocketHandle* SpdyHttpStream::DetachConnection() {
+  // DetachConnection is currently used to ensure that multi-round HTTP
+  // authentication takes place on the same connection. Since SpdyHttpStream's
+  // for the same domain will always map to the same SpdySession, NULL can
+  // be returned.
+  return NULL;
+}
+
 }  // namespace net
diff --git a/net/spdy/spdy_http_stream.h b/net/spdy/spdy_http_stream.h
index eab605c..a8e1a2c 100644
--- a/net/spdy/spdy_http_stream.h
+++ b/net/spdy/spdy_http_stream.h
@@ -91,6 +91,8 @@ class SpdyHttpStream : public SpdyStream::Delegate, public HttpStream {
     // SPDY doesn't need an indicator here.
   }
 
+  virtual ClientSocketHandle* DetachConnection();
+
   virtual void GetSSLInfo(SSLInfo* ssl_info);
   virtual void GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info);