Recommit 54405 - Fix late binding induced mismatch of Socket and AuthController

ClientSocketPool treats all pending SocketParams as interchangeable. Therefore they can not contain any connection specific data. This only affects the Http Proxy tunnel case. The lowest risk change to fix this problem is to create the HttpAuthController in the HttpProxyClientSocket. If we get a 407 and need to restart the Tunnel, the pending HttpProxyClientSocket is returned to the HttpNetworkTransaction in the additional error state of the connection and then complete the auth in a pair of states in the HttpNetworkTransaction. This reintroduces a dependency between tunnel setup and the HttpNetworkTransaction, but that will need to be fixed at a later date.

Original Review URL: http://codereview.chromium.org/3058013

BUG=49493
TEST=existing unit tests + manually visiting many SSL sites through a kerberized http proxy.

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

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

1 files changed, 53 insertions, 183 deletions

diff --git a/net/socket/ssl_client_socket_pool_unittest.cc b/net/socket/ssl_client_socket_pool_unittest.cc
index efc4795..047a647 100644
--- a/net/socket/ssl_client_socket_pool_unittest.cc
+++ b/net/socket/ssl_client_socket_pool_unittest.cc
@@ -6,13 +6,14 @@
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
+#include "base/string_util.h"
 #include "base/time.h"
 #include "net/base/auth.h"
 #include "net/base/mock_host_resolver.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
 #include "net/base/ssl_config_service_defaults.h"
-#include "net/http/http_auth_controller.h"
+#include "net/http/http_auth_handler_factory.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_response_headers.h"
@@ -20,6 +21,7 @@
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/client_socket_pool_histograms.h"
 #include "net/socket/socket_test_util.h"
+#include "net/spdy/spdy_session_pool.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
@@ -32,7 +34,16 @@ const int kMaxSocketsPerGroup = 6;
 class SSLClientSocketPoolTest : public ClientSocketPoolTest {
  protected:
   SSLClientSocketPoolTest()
-      : direct_tcp_socket_params_(new TCPSocketParams(
+      : http_auth_handler_factory_(HttpAuthHandlerFactory::CreateDefault()),
+        session_(new HttpNetworkSession(new MockHostResolver,
+                                        ProxyService::CreateNull(),
+                                        &socket_factory_,
+                                        new SSLConfigServiceDefaults,
+                                        new SpdySessionPool(),
+                                        http_auth_handler_factory_.get(),
+                                        NULL,
+                                        NULL)),
+        direct_tcp_socket_params_(new TCPSocketParams(
             HostPortPair("host", 443), MEDIUM, GURL(), false)),
         tcp_socket_pool_(new MockTCPClientSocketPool(
             kMaxSockets,
@@ -41,6 +52,9 @@ class SSLClientSocketPoolTest : public ClientSocketPoolTest {
             &socket_factory_)),
         proxy_tcp_socket_params_(new TCPSocketParams(
             HostPortPair("proxy", 443), MEDIUM, GURL(), false)),
+        http_proxy_socket_params_(new HttpProxySocketParams(
+            proxy_tcp_socket_params_, GURL("http://host"),
+            HostPortPair("host", 80), session_, true)),
         http_proxy_socket_pool_(new HttpProxyClientSocketPool(
             kMaxSockets,
             kMaxSocketsPerGroup,
@@ -74,33 +88,30 @@ class SSLClientSocketPoolTest : public ClientSocketPoolTest {
         NULL);
   }
 
-  scoped_refptr<SSLSocketParams> SSLParams(
-      ProxyServer::Scheme proxy, struct MockHttpAuthControllerData* auth_data,
-      size_t auth_data_len, bool want_spdy_over_ssl, bool want_spdy_over_npn) {
-    scoped_refptr<HttpProxySocketParams> http_proxy_params;
-    if (proxy == ProxyServer::SCHEME_HTTP) {
-      scoped_refptr<MockHttpAuthController> auth_controller =
-         new MockHttpAuthController();
-      auth_controller->SetMockAuthControllerData(auth_data, auth_data_len);
-      http_proxy_params = new HttpProxySocketParams(proxy_tcp_socket_params_,
-                                                    GURL("http://host"),
-                                                    HostPortPair("host", 80),
-                                                    auth_controller, true);
-    }
-
+  scoped_refptr<SSLSocketParams> SSLParams(ProxyServer::Scheme proxy,
+                                           bool want_spdy_over_npn) {
     return make_scoped_refptr(new SSLSocketParams(
         proxy == ProxyServer::SCHEME_DIRECT ? direct_tcp_socket_params_ : NULL,
-        http_proxy_params,
+        proxy == ProxyServer::SCHEME_HTTP ? http_proxy_socket_params_ : NULL,
         proxy == ProxyServer::SCHEME_SOCKS5 ? socks_socket_params_ : NULL,
         proxy,
         "host",
         ssl_config_,
         0,
-        want_spdy_over_ssl,
+        false,
         want_spdy_over_npn));
   }
 
+  void AddAuthToCache() {
+    const string16 kFoo(ASCIIToUTF16("foo"));
+    const string16 kBar(ASCIIToUTF16("bar"));
+    session_->auth_cache()->Add(GURL("http://proxy:443/"), "MyRealm1", "Basic",
+                                "Basic realm=MyRealm1", kFoo, kBar, "/");
+  }
+
   MockClientSocketFactory socket_factory_;
+  scoped_ptr<HttpAuthHandlerFactory> http_auth_handler_factory_;
+  scoped_refptr<HttpNetworkSession> session_;
 
   scoped_refptr<TCPSocketParams> direct_tcp_socket_params_;
   scoped_refptr<MockTCPClientSocketPool> tcp_socket_pool_;
@@ -123,7 +134,7 @@ TEST_F(SSLClientSocketPoolTest, TCPFail) {
 
   CreatePool(true /* tcp pool */, false, false);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_DIRECT,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   int rv = handle.Init("a", params, MEDIUM, NULL, pool_, BoundNetLog());
@@ -140,7 +151,7 @@ TEST_F(SSLClientSocketPoolTest, TCPFailAsync) {
 
   CreatePool(true /* tcp pool */, false, false);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_DIRECT,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -164,7 +175,7 @@ TEST_F(SSLClientSocketPoolTest, BasicDirect) {
 
   CreatePool(true /* tcp pool */, false, false);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_DIRECT,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -182,7 +193,7 @@ TEST_F(SSLClientSocketPoolTest, BasicDirectAsync) {
 
   CreatePool(true /* tcp pool */, false, false);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_DIRECT,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -204,7 +215,7 @@ TEST_F(SSLClientSocketPoolTest, DirectCertError) {
 
   CreatePool(true /* tcp pool */, false, false);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_DIRECT,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -226,7 +237,7 @@ TEST_F(SSLClientSocketPoolTest, DirectSSLError) {
 
   CreatePool(true /* tcp pool */, false, false);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_DIRECT,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -251,7 +262,7 @@ TEST_F(SSLClientSocketPoolTest, DirectWithNPN) {
 
   CreatePool(true /* tcp pool */, false, false);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_DIRECT,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -277,7 +288,7 @@ TEST_F(SSLClientSocketPoolTest, DirectNoSPDY) {
 
   CreatePool(true /* tcp pool */, false, false);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_DIRECT,
-                                                    NULL, 0, false, true);
+                                                    true);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -302,7 +313,7 @@ TEST_F(SSLClientSocketPoolTest, DirectGotSPDY) {
 
   CreatePool(true /* tcp pool */, false, false);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_DIRECT,
-                                                    NULL, 0, false, true);
+                                                    true);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -333,7 +344,7 @@ TEST_F(SSLClientSocketPoolTest, DirectGotBonusSPDY) {
 
   CreatePool(true /* tcp pool */, false, false);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_DIRECT,
-                                                    NULL, 0, false, true);
+                                                    true);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -361,7 +372,7 @@ TEST_F(SSLClientSocketPoolTest, SOCKSFail) {
 
   CreatePool(false, true /* http proxy pool */, true /* socks pool */);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_SOCKS5,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -379,7 +390,7 @@ TEST_F(SSLClientSocketPoolTest, SOCKSFailAsync) {
 
   CreatePool(false, true /* http proxy pool */, true /* socks pool */);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_SOCKS5,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -403,7 +414,7 @@ TEST_F(SSLClientSocketPoolTest, SOCKSBasic) {
 
   CreatePool(false, true /* http proxy pool */, true /* socks pool */);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_SOCKS5,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -421,7 +432,7 @@ TEST_F(SSLClientSocketPoolTest, SOCKSBasicAsync) {
 
   CreatePool(false, true /* http proxy pool */, true /* socks pool */);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_SOCKS5,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -442,7 +453,7 @@ TEST_F(SSLClientSocketPoolTest, HttpProxyFail) {
 
   CreatePool(false, true /* http proxy pool */, true /* socks pool */);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_HTTP,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -460,7 +471,7 @@ TEST_F(SSLClientSocketPoolTest, HttpProxyFailAsync) {
 
   CreatePool(false, true /* http proxy pool */, true /* socks pool */);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_HTTP,
-                                                    NULL, 0, false, false);
+                                                    false);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -481,7 +492,7 @@ TEST_F(SSLClientSocketPoolTest, HttpProxyBasic) {
                 "CONNECT host:80 HTTP/1.1\r\n"
                 "Host: host\r\n"
                 "Proxy-Connection: keep-alive\r\n"
-                "Proxy-Authorization: Basic Zm9vOmJheg==\r\n\r\n"),
+                "Proxy-Authorization: Basic Zm9vOmJhcg==\r\n\r\n"),
   };
   MockRead reads[] = {
       MockRead(false, "HTTP/1.1 200 Connection Established\r\n\r\n"),
@@ -490,17 +501,12 @@ TEST_F(SSLClientSocketPoolTest, HttpProxyBasic) {
                                 arraysize(writes));
   data.set_connect_data(MockConnect(false, OK));
   socket_factory_.AddSocketDataProvider(&data);
-  MockHttpAuthControllerData auth_data[] = {
-      MockHttpAuthControllerData("Proxy-Authorization: Basic Zm9vOmJheg=="),
-  };
+  AddAuthToCache();
   SSLSocketDataProvider ssl(false, OK);
   socket_factory_.AddSSLSocketDataProvider(&ssl);
 
   CreatePool(false, true /* http proxy pool */, true /* socks pool */);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_HTTP,
-                                                    auth_data,
-                                                    arraysize(auth_data),
-                                                    false,
                                                     false);
 
   ClientSocketHandle handle;
@@ -516,7 +522,7 @@ TEST_F(SSLClientSocketPoolTest, HttpProxyBasicAsync) {
       MockWrite("CONNECT host:80 HTTP/1.1\r\n"
                 "Host: host\r\n"
                 "Proxy-Connection: keep-alive\r\n"
-                "Proxy-Authorization: Basic Zm9vOmJheg==\r\n\r\n"),
+                "Proxy-Authorization: Basic Zm9vOmJhcg==\r\n\r\n"),
   };
   MockRead reads[] = {
       MockRead("HTTP/1.1 200 Connection Established\r\n\r\n"),
@@ -524,17 +530,12 @@ TEST_F(SSLClientSocketPoolTest, HttpProxyBasicAsync) {
   StaticSocketDataProvider data(reads, arraysize(reads), writes,
                                 arraysize(writes));
   socket_factory_.AddSocketDataProvider(&data);
-  MockHttpAuthControllerData auth_data[] = {
-      MockHttpAuthControllerData("Proxy-Authorization: Basic Zm9vOmJheg=="),
-  };
+  AddAuthToCache();
   SSLSocketDataProvider ssl(true, OK);
   socket_factory_.AddSSLSocketDataProvider(&ssl);
 
   CreatePool(false, true /* http proxy pool */, true /* socks pool */);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_HTTP,
-                                                    auth_data,
-                                                    arraysize(auth_data),
-                                                    false,
                                                     false);
 
   ClientSocketHandle handle;
@@ -564,135 +565,11 @@ TEST_F(SSLClientSocketPoolTest, NeedProxyAuth) {
   StaticSocketDataProvider data(reads, arraysize(reads), writes,
                                 arraysize(writes));
   socket_factory_.AddSocketDataProvider(&data);
-  MockHttpAuthControllerData auth_data[] = {
-      MockHttpAuthControllerData(""),
-  };
-  SSLSocketDataProvider ssl(true, OK);
-  socket_factory_.AddSSLSocketDataProvider(&ssl);
-
-  CreatePool(false, true /* http proxy pool */, true /* socks pool */);
-  scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_HTTP,
-                                                    auth_data,
-                                                    arraysize(auth_data),
-                                                    false,
-                                                    false);
-
-  ClientSocketHandle handle;
-  TestCompletionCallback callback;
-  int rv = handle.Init("a", params, MEDIUM, &callback, pool_, BoundNetLog());
-  EXPECT_EQ(ERR_IO_PENDING, rv);
-  EXPECT_FALSE(handle.is_initialized());
-  EXPECT_FALSE(handle.socket());
-
-  EXPECT_EQ(ERR_PROXY_AUTH_REQUESTED, callback.WaitForResult());
-  EXPECT_FALSE(handle.is_initialized());
-  EXPECT_FALSE(handle.socket());
-  EXPECT_FALSE(handle.is_ssl_error());
-  const HttpResponseInfo& tunnel_info = handle.ssl_error_response_info();
-  EXPECT_EQ(tunnel_info.headers->response_code(), 407);
-}
-
-TEST_F(SSLClientSocketPoolTest, DoProxyAuth) {
-  MockWrite writes[] = {
-      MockWrite("CONNECT host:80 HTTP/1.1\r\n"
-                "Host: host\r\n"
-                "Proxy-Connection: keep-alive\r\n\r\n"),
-      MockWrite("CONNECT host:80 HTTP/1.1\r\n"
-                "Host: host\r\n"
-                "Proxy-Connection: keep-alive\r\n"
-                "Proxy-Authorization: Basic Zm9vOmJheg==\r\n\r\n"),
-  };
-  MockRead reads[] = {
-      MockRead("HTTP/1.1 407 Proxy Authentication Required\r\n"),
-      MockRead("Proxy-Authenticate: Basic realm=\"MyRealm1\"\r\n"),
-      MockRead("Content-Length: 10\r\n\r\n"),
-      MockRead("0123456789"),
-      MockRead("HTTP/1.1 200 Connection Established\r\n\r\n"),
-  };
-  StaticSocketDataProvider data(reads, arraysize(reads), writes,
-                                arraysize(writes));
-  socket_factory_.AddSocketDataProvider(&data);
-  MockHttpAuthControllerData auth_data[] = {
-      MockHttpAuthControllerData(""),
-      MockHttpAuthControllerData("Proxy-Authorization: Basic Zm9vOmJheg=="),
-  };
-  SSLSocketDataProvider ssl(true, OK);
-  socket_factory_.AddSSLSocketDataProvider(&ssl);
-
-  CreatePool(false, true /* http proxy pool */, true /* socks pool */);
-  scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_HTTP,
-                                                    auth_data,
-                                                    arraysize(auth_data),
-                                                    false,
-                                                    false);
-
-  ClientSocketHandle handle;
-  TestCompletionCallback callback;
-  int rv = handle.Init("a", params, MEDIUM, &callback, pool_, BoundNetLog());
-  EXPECT_EQ(ERR_IO_PENDING, rv);
-  EXPECT_FALSE(handle.is_initialized());
-  EXPECT_FALSE(handle.socket());
-
-  EXPECT_EQ(ERR_PROXY_AUTH_REQUESTED, callback.WaitForResult());
-  EXPECT_FALSE(handle.is_initialized());
-  EXPECT_FALSE(handle.socket());
-  EXPECT_FALSE(handle.is_ssl_error());
-  const HttpResponseInfo& tunnel_info = handle.ssl_error_response_info();
-  EXPECT_EQ(tunnel_info.headers->response_code(), 407);
-
-  params->http_proxy_params()->auth_controller()->ResetAuth(string16(),
-                                                            string16());
-  rv = handle.Init("a", params, MEDIUM, &callback, pool_, BoundNetLog());
-  EXPECT_EQ(ERR_IO_PENDING, rv);
-  EXPECT_FALSE(handle.is_initialized());
-  EXPECT_FALSE(handle.socket());
-
-  // Test that http://crbug.com/49325 doesn't regress.
-  EXPECT_EQ(handle.GetLoadState(), LOAD_STATE_ESTABLISHING_PROXY_TUNNEL);
-
-  EXPECT_EQ(OK, callback.WaitForResult());
-  EXPECT_TRUE(handle.is_initialized());
-  EXPECT_TRUE(handle.socket());
-}
-
-TEST_F(SSLClientSocketPoolTest, DoProxyAuthNoKeepAlive) {
-  MockWrite writes1[] = {
-      MockWrite("CONNECT host:80 HTTP/1.1\r\n"
-                "Host: host\r\n"
-                "Proxy-Connection: keep-alive\r\n\r\n"),
-  };
-  MockWrite writes2[] = {
-      MockWrite("CONNECT host:80 HTTP/1.1\r\n"
-                "Host: host\r\n"
-                "Proxy-Connection: keep-alive\r\n"
-                "Proxy-Authorization: Basic Zm9vOmJheg==\r\n\r\n"),
-  };
-  MockRead reads1[] = {
-      MockRead("HTTP/1.1 407 Proxy Authentication Required\r\n"),
-      MockRead("Proxy-Authenticate: Basic realm=\"MyRealm1\"\r\n\r\n"),
-      MockRead("Content0123456789"),
-  };
-  MockRead reads2[] = {
-      MockRead("HTTP/1.1 200 Connection Established\r\n\r\n"),
-  };
-  StaticSocketDataProvider data1(reads1, arraysize(reads1), writes1,
-                                arraysize(writes1));
-  socket_factory_.AddSocketDataProvider(&data1);
-  StaticSocketDataProvider data2(reads2, arraysize(reads2), writes2,
-                                arraysize(writes2));
-  socket_factory_.AddSocketDataProvider(&data2);
-  MockHttpAuthControllerData auth_data[] = {
-      MockHttpAuthControllerData(""),
-      MockHttpAuthControllerData("Proxy-Authorization: Basic Zm9vOmJheg=="),
-  };
   SSLSocketDataProvider ssl(true, OK);
   socket_factory_.AddSSLSocketDataProvider(&ssl);
 
   CreatePool(false, true /* http proxy pool */, true /* socks pool */);
   scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_HTTP,
-                                                    auth_data,
-                                                    arraysize(auth_data),
-                                                    false,
                                                     false);
 
   ClientSocketHandle handle;
@@ -708,17 +585,10 @@ TEST_F(SSLClientSocketPoolTest, DoProxyAuthNoKeepAlive) {
   EXPECT_FALSE(handle.is_ssl_error());
   const HttpResponseInfo& tunnel_info = handle.ssl_error_response_info();
   EXPECT_EQ(tunnel_info.headers->response_code(), 407);
-
-  params->http_proxy_params()->auth_controller()->ResetAuth(string16(),
-                                                            string16());
-  rv = handle.Init("a", params, MEDIUM, &callback, pool_, BoundNetLog());
-  EXPECT_EQ(ERR_IO_PENDING, rv);
-  EXPECT_FALSE(handle.is_initialized());
-  EXPECT_FALSE(handle.socket());
-
-  EXPECT_EQ(OK, callback.WaitForResult());
-  EXPECT_TRUE(handle.is_initialized());
-  EXPECT_TRUE(handle.socket());
+  scoped_ptr<ClientSocketHandle> tunnel_handle(
+      handle.release_pending_http_proxy_connection());
+  EXPECT_TRUE(tunnel_handle->socket());
+  EXPECT_FALSE(tunnel_handle->socket()->IsConnected());
 }
 
 // It would be nice to also test the timeouts in SSLClientSocketPool.