diff options
author | vandebo@chromium.org <vandebo@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-07-30 23:21:59 +0000 |
---|---|---|
committer | vandebo@chromium.org <vandebo@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-07-30 23:21:59 +0000 |
commit | 9a9787ed17dce85531063d1fb2a07ce801c58de5 (patch) | |
tree | 8c5d68e2da23005d4604a03bea2fb2221affcc47 /net/http | |
parent | eb210d2cbe6e3dc88140bb8d53a9b02955fa5858 (diff) | |
download | chromium_src-9a9787ed17dce85531063d1fb2a07ce801c58de5.zip chromium_src-9a9787ed17dce85531063d1fb2a07ce801c58de5.tar.gz chromium_src-9a9787ed17dce85531063d1fb2a07ce801c58de5.tar.bz2 |
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.
BUG=49493
TEST=existing unit tests + manually visiting many SSL sites through a kerberized http proxy.
Review URL: http://codereview.chromium.org/3058013
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@54405 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net/http')
-rw-r--r-- | net/http/http_network_transaction.cc | 106 | ||||
-rw-r--r-- | net/http/http_network_transaction.h | 7 | ||||
-rw-r--r-- | net/http/http_network_transaction_unittest.cc | 105 | ||||
-rw-r--r-- | net/http/http_proxy_client_socket.cc | 52 | ||||
-rw-r--r-- | net/http/http_proxy_client_socket.h | 17 | ||||
-rw-r--r-- | net/http/http_proxy_client_socket_pool.cc | 14 | ||||
-rw-r--r-- | net/http/http_proxy_client_socket_pool.h | 9 | ||||
-rw-r--r-- | net/http/http_proxy_client_socket_pool_unittest.cc | 70 |
8 files changed, 289 insertions, 91 deletions
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc index f54db4f..e553ec4 100644 --- a/net/http/http_network_transaction.cc +++ b/net/http/http_network_transaction.cc @@ -337,7 +337,8 @@ int HttpNetworkTransaction::RestartWithAuth( if (target == HttpAuth::AUTH_PROXY && using_ssl_ && proxy_info_.is_http()) { DCHECK(establishing_tunnel_); - next_state_ = STATE_INIT_CONNECTION; + next_state_ = STATE_RESTART_TUNNEL_AUTH; + auth_controllers_[target] = NULL; ResetStateForRestart(); } else { PrepareForAuthRestart(target); @@ -538,6 +539,13 @@ int HttpNetworkTransaction::DoLoop(int result) { case STATE_INIT_CONNECTION_COMPLETE: rv = DoInitConnectionComplete(rv); break; + case STATE_RESTART_TUNNEL_AUTH: + DCHECK_EQ(OK, rv); + rv = DoRestartTunnelAuth(); + break; + case STATE_RESTART_TUNNEL_AUTH_COMPLETE: + rv = DoRestartTunnelAuthComplete(rv); + break; case STATE_GENERATE_PROXY_AUTH_TOKEN: DCHECK_EQ(OK, rv); rv = DoGenerateProxyAuthToken(); @@ -723,15 +731,6 @@ int HttpNetworkTransaction::DoInitConnection() { DCHECK(proxy_info_.proxy_server().is_valid()); next_state_ = STATE_INIT_CONNECTION_COMPLETE; - // Now that the proxy server has been resolved, create the auth_controllers_. - for (int i = 0; i < HttpAuth::AUTH_NUM_TARGETS; i++) { - HttpAuth::Target target = static_cast<HttpAuth::Target>(i); - if (!auth_controllers_[target].get()) - auth_controllers_[target] = new HttpAuthController(target, - AuthURL(target), - session_); - } - bool want_spdy_over_npn = alternate_protocol_mode_ == kUsingAlternateProtocol && alternate_protocol_ == HttpAlternateProtocols::NPN_SPDY_2; using_ssl_ = request_->url.SchemeIs("https") || @@ -780,15 +779,10 @@ int HttpNetworkTransaction::DoInitConnection() { request_->referrer, disable_resolver_cache); if (proxy_info_.is_http()) { - scoped_refptr<HttpAuthController> http_proxy_auth; - if (using_ssl_) { - http_proxy_auth = auth_controllers_[HttpAuth::AUTH_PROXY]; - establishing_tunnel_ = true; - } + establishing_tunnel_ = using_ssl_; http_proxy_params = new HttpProxySocketParams(proxy_tcp_params, request_->url, endpoint_, - http_proxy_auth, - using_ssl_); + session_, using_ssl_); } else { DCHECK(proxy_info_.is_socks()); char socks_version; @@ -896,14 +890,13 @@ int HttpNetworkTransaction::DoInitConnectionComplete(int result) { if (result == ERR_PROXY_AUTH_REQUESTED) { DCHECK(!ssl_started); - const HttpResponseInfo& tunnel_auth_response = - connection_->ssl_error_response_info(); - - response_.headers = tunnel_auth_response.headers; - response_.auth_challenge = tunnel_auth_response.auth_challenge; - headers_valid_ = true; - pending_auth_target_ = HttpAuth::AUTH_PROXY; - return OK; + // Other state (i.e. |using_ssl_|) suggests that |connection_| will have an + // SSL socket, but there was an error before that could happened. This + // puts the in progress HttpProxy socket into |connection_| in order to + // complete the auth. The tunnel restart code is carefully to remove it + // before returning control to the rest of this class. + connection_.reset(connection_->release_pending_http_proxy_connection()); + return HandleTunnelAuthFailure(result); } if ((!ssl_started && result < 0 && @@ -983,12 +976,44 @@ int HttpNetworkTransaction::DoInitConnectionComplete(int result) { return OK; } +int HttpNetworkTransaction::DoRestartTunnelAuth() { + next_state_ = STATE_RESTART_TUNNEL_AUTH_COMPLETE; + HttpProxyClientSocket* http_proxy_socket = + static_cast<HttpProxyClientSocket*>(connection_->socket()); + return http_proxy_socket->RestartWithAuth(&io_callback_); +} + +int HttpNetworkTransaction::DoRestartTunnelAuthComplete(int result) { + if (result == ERR_PROXY_AUTH_REQUESTED) + return HandleTunnelAuthFailure(result); + + if (result == OK) { + // Now that we've got the HttpProxyClientSocket connected. We have + // to release it as an idle socket into the pool and start the connection + // process from the beginning. Trying to pass it in with the + // SSLSocketParams might cause a deadlock since params are dispatched + // interchangeably. This request won't necessarily get this http proxy + // socket, but there will be forward progress. + connection_->Reset(); + establishing_tunnel_ = false; + next_state_ = STATE_INIT_CONNECTION; + return OK; + } + + return ReconsiderProxyAfterError(result); +} + int HttpNetworkTransaction::DoGenerateProxyAuthToken() { next_state_ = STATE_GENERATE_PROXY_AUTH_TOKEN_COMPLETE; if (!ShouldApplyProxyAuth()) return OK; - return auth_controllers_[HttpAuth::AUTH_PROXY]->MaybeGenerateAuthToken( - request_, &io_callback_, net_log_); + HttpAuth::Target target = HttpAuth::AUTH_PROXY; + if (!auth_controllers_[target].get()) + auth_controllers_[target] = new HttpAuthController(target, AuthURL(target), + session_); + return auth_controllers_[target]->MaybeGenerateAuthToken(request_, + &io_callback_, + net_log_); } int HttpNetworkTransaction::DoGenerateProxyAuthTokenComplete(int rv) { @@ -1000,10 +1025,15 @@ int HttpNetworkTransaction::DoGenerateProxyAuthTokenComplete(int rv) { int HttpNetworkTransaction::DoGenerateServerAuthToken() { next_state_ = STATE_GENERATE_SERVER_AUTH_TOKEN_COMPLETE; + HttpAuth::Target target = HttpAuth::AUTH_SERVER; + if (!auth_controllers_[target].get()) + auth_controllers_[target] = new HttpAuthController(target, AuthURL(target), + session_); if (!ShouldApplyServerAuth()) return OK; - return auth_controllers_[HttpAuth::AUTH_SERVER]->MaybeGenerateAuthToken( - request_, &io_callback_, net_log_); + return auth_controllers_[target]->MaybeGenerateAuthToken(request_, + &io_callback_, + net_log_); } int HttpNetworkTransaction::DoGenerateServerAuthTokenComplete(int rv) { @@ -1523,6 +1553,24 @@ void HttpNetworkTransaction::LogTransactionMetrics() const { } } +int HttpNetworkTransaction::HandleTunnelAuthFailure(int error) { + DCHECK(establishing_tunnel_); + DCHECK_EQ(ERR_PROXY_AUTH_REQUESTED, error); + HttpProxyClientSocket* http_proxy_socket = + static_cast<HttpProxyClientSocket*>(connection_->socket()); + + const HttpResponseInfo* tunnel_auth_response = + http_proxy_socket->GetResponseInfo(); + response_.headers = tunnel_auth_response->headers; + response_.auth_challenge = tunnel_auth_response->auth_challenge; + headers_valid_ = true; + + auth_controllers_[HttpAuth::AUTH_PROXY] = + http_proxy_socket->auth_controller(); + pending_auth_target_ = HttpAuth::AUTH_PROXY; + return OK; +} + int HttpNetworkTransaction::HandleCertificateError(int error) { DCHECK(using_ssl_); DCHECK(IsCertificateError(error)); diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h index e07e91d..695d7b9 100644 --- a/net/http/http_network_transaction.h +++ b/net/http/http_network_transaction.h @@ -91,6 +91,8 @@ class HttpNetworkTransaction : public HttpTransaction { STATE_RESOLVE_PROXY_COMPLETE, STATE_INIT_CONNECTION, STATE_INIT_CONNECTION_COMPLETE, + STATE_RESTART_TUNNEL_AUTH, + STATE_RESTART_TUNNEL_AUTH_COMPLETE, STATE_GENERATE_PROXY_AUTH_TOKEN, STATE_GENERATE_PROXY_AUTH_TOKEN_COMPLETE, STATE_GENERATE_SERVER_AUTH_TOKEN, @@ -134,6 +136,8 @@ class HttpNetworkTransaction : public HttpTransaction { int DoResolveProxyComplete(int result); int DoInitConnection(); int DoInitConnectionComplete(int result); + int DoRestartTunnelAuth(); + int DoRestartTunnelAuthComplete(int result); int DoGenerateProxyAuthToken(); int DoGenerateProxyAuthTokenComplete(int result); int DoGenerateServerAuthToken(); @@ -170,6 +174,9 @@ class HttpNetworkTransaction : public HttpTransaction { static void LogIOErrorMetrics(const ClientSocketHandle& handle); + // Called to handle an HTTP proxy tunnel request for auth. + int HandleTunnelAuthFailure(int error); + // Called to handle a certificate error. Returns OK if the error should be // ignored. Otherwise, stores the certificate in response_.ssl_info and // returns the same error code. diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index dcdf13f..5714543 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -1343,6 +1343,111 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthKeepAliveImpatientServer) { EXPECT_EQ(100, response->headers->GetContentLength()); } +// Test the request-challenge-retry sequence for basic auth, over a connection +// that requires a restart when setting up an SSL tunnel. +TEST_F(HttpNetworkTransactionTest, BasicAuthProxyNoKeepAlive) { + // Configure against proxy server "myproxy:70". + SessionDependencies session_deps(CreateFixedProxyService("myproxy:70")); + CapturingBoundNetLog log(CapturingNetLog::kUnbounded); + session_deps.net_log = log.bound().net_log(); + scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps)); + + scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction(session)); + + HttpRequestInfo request; + request.method = "GET"; + request.url = GURL("https://www.google.com/"); + // when the no authentication data flag is set. + request.load_flags = net::LOAD_DO_NOT_SEND_AUTH_DATA; + + // Since we have proxy, should try to establish tunnel. + MockWrite data_writes1[] = { + MockWrite("CONNECT www.google.com:443 HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Proxy-Connection: keep-alive\r\n\r\n"), + + // After calling trans->RestartWithAuth(), this is the request we should + // be issuing -- the final header line contains the credentials. + MockWrite("CONNECT www.google.com:443 HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Proxy-Connection: keep-alive\r\n" + "Proxy-Authorization: Basic Zm9vOmJhcg==\r\n\r\n"), + + MockWrite("GET / HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n\r\n"), + }; + + // The proxy responds to the connect with a 407, using a persistent + // connection. + MockRead data_reads1[] = { + // No credentials. + MockRead("HTTP/1.1 407 Proxy Authentication Required\r\n"), + MockRead("Proxy-Authenticate: Basic realm=\"MyRealm1\"\r\n"), + MockRead("Proxy-Connection: close\r\n\r\n"), + + MockRead("HTTP/1.1 200 Connection Established\r\n\r\n"), + + MockRead("HTTP/1.1 200 OK\r\n"), + MockRead("Content-Type: text/html; charset=iso-8859-1\r\n"), + MockRead("Content-Length: 100\r\n\r\n"), + MockRead(false, OK), + }; + + StaticSocketDataProvider data1(data_reads1, arraysize(data_reads1), + data_writes1, arraysize(data_writes1)); + session_deps.socket_factory.AddSocketDataProvider(&data1); + SSLSocketDataProvider ssl(true, OK); + session_deps.socket_factory.AddSSLSocketDataProvider(&ssl); + + TestCompletionCallback callback1; + + int rv = trans->Start(&request, &callback1, log.bound()); + EXPECT_EQ(ERR_IO_PENDING, rv); + + rv = callback1.WaitForResult(); + EXPECT_EQ(OK, rv); + size_t pos = ExpectLogContainsSomewhere( + log.entries(), 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS, + NetLog::PHASE_NONE); + ExpectLogContainsSomewhere( + log.entries(), pos, + NetLog::TYPE_HTTP_TRANSACTION_READ_TUNNEL_RESPONSE_HEADERS, + NetLog::PHASE_NONE); + + const HttpResponseInfo* response = trans->GetResponseInfo(); + ASSERT_FALSE(response == NULL); + + EXPECT_EQ(407, response->headers->response_code()); + EXPECT_TRUE(HttpVersion(1, 1) == response->headers->GetHttpVersion()); + + // The password prompt info should have been set in response->auth_challenge. + ASSERT_FALSE(response->auth_challenge.get() == NULL); + + EXPECT_EQ(L"myproxy:70", response->auth_challenge->host_and_port); + EXPECT_EQ(L"MyRealm1", response->auth_challenge->realm); + EXPECT_EQ(L"basic", response->auth_challenge->scheme); + + TestCompletionCallback callback2; + + rv = trans->RestartWithAuth(kFoo, kBar, &callback2); + EXPECT_EQ(ERR_IO_PENDING, rv); + + rv = callback2.WaitForResult(); + EXPECT_EQ(OK, rv); + + response = trans->GetResponseInfo(); + ASSERT_FALSE(response == NULL); + + EXPECT_TRUE(response->headers->IsKeepAlive()); + EXPECT_EQ(200, response->headers->response_code()); + EXPECT_EQ(100, response->headers->GetContentLength()); + EXPECT_TRUE(HttpVersion(1, 1) == response->headers->GetHttpVersion()); + + // The password prompt info should not be set. + EXPECT_TRUE(response->auth_challenge.get() == NULL); +} + // Test the request-challenge-retry sequence for basic auth, over a keep-alive // proxy connection, when setting up an SSL tunnel. TEST_F(HttpNetworkTransactionTest, BasicAuthProxyKeepAlive) { diff --git a/net/http/http_proxy_client_socket.cc b/net/http/http_proxy_client_socket.cc index 02c2666..1c76c36 100644 --- a/net/http/http_proxy_client_socket.cc +++ b/net/http/http_proxy_client_socket.cc @@ -50,18 +50,20 @@ void BuildTunnelRequest(const HttpRequestInfo* request_info, HttpProxyClientSocket::HttpProxyClientSocket( ClientSocketHandle* transport_socket, const GURL& request_url, - const HostPortPair& endpoint, const scoped_refptr<HttpAuthController>& auth, - bool tunnel) + const HostPortPair& endpoint, const HostPortPair& proxy_server, + const scoped_refptr<HttpNetworkSession>& session, bool tunnel) : ALLOW_THIS_IN_INITIALIZER_LIST( io_callback_(this, &HttpProxyClientSocket::OnIOComplete)), next_state_(STATE_NONE), user_callback_(NULL), transport_(transport_socket), endpoint_(endpoint), - auth_(auth), + auth_(tunnel ? + new HttpAuthController(HttpAuth::AUTH_PROXY, + GURL("http://" + proxy_server.ToString()), + session) : NULL), tunnel_(tunnel), net_log_(transport_socket->socket()->NetLog()) { - DCHECK_EQ(tunnel, auth != NULL); // Synthesize the bits of a request that we actually use. request_.url = request_url; request_.method = "GET"; @@ -126,13 +128,14 @@ int HttpProxyClientSocket::PrepareForAuthRestart() { } int HttpProxyClientSocket::DidDrainBodyForAuthRestart(bool keep_alive) { - int rc = OK; if (keep_alive && transport_->socket()->IsConnectedAndIdle()) { next_state_ = STATE_GENERATE_AUTH_TOKEN; transport_->set_is_reused(true); } else { + // This assumes that the underlying transport socket is a TCP socket, + // since only TCP sockets are restartable. + next_state_ = STATE_TCP_RESTART; transport_->socket()->Disconnect(); - rc = ERR_RETRY_CONNECTION; } // Reset the other member variables. @@ -140,7 +143,7 @@ int HttpProxyClientSocket::DidDrainBodyForAuthRestart(bool keep_alive) { http_stream_.reset(); request_headers_.clear(); response_ = HttpResponseInfo(); - return rc; + return OK; } void HttpProxyClientSocket::LogBlockedTunnelResponse(int response_code) const { @@ -159,19 +162,16 @@ void HttpProxyClientSocket::Disconnect() { } bool HttpProxyClientSocket::IsConnected() const { - return transport_->socket()->IsConnected(); + return next_state_ == STATE_DONE && transport_->socket()->IsConnected(); } bool HttpProxyClientSocket::IsConnectedAndIdle() const { - return transport_->socket()->IsConnectedAndIdle(); -} - -bool HttpProxyClientSocket::NeedsRestartWithAuth() const { - return next_state_ != STATE_DONE; + return next_state_ == STATE_DONE && + transport_->socket()->IsConnectedAndIdle(); } int HttpProxyClientSocket::Read(IOBuffer* buf, int buf_len, - CompletionCallback* callback) { + CompletionCallback* callback) { DCHECK(!user_callback_); if (next_state_ != STATE_DONE) { // We're trying to read the body of the response but we're still trying @@ -273,6 +273,13 @@ int HttpProxyClientSocket::DoLoop(int last_io_result) { case STATE_DRAIN_BODY_COMPLETE: rv = DoDrainBodyComplete(rv); break; + case STATE_TCP_RESTART: + DCHECK_EQ(OK, rv); + rv = DoTCPRestart(); + break; + case STATE_TCP_RESTART_COMPLETE: + rv = DoTCPRestartComplete(rv); + break; case STATE_DONE: break; default: @@ -280,8 +287,8 @@ int HttpProxyClientSocket::DoLoop(int last_io_result) { rv = ERR_UNEXPECTED; break; } - } while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE - && next_state_ != STATE_DONE); + } while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE && + next_state_ != STATE_DONE); return rv; } @@ -406,6 +413,19 @@ int HttpProxyClientSocket::DoDrainBodyComplete(int result) { return OK; } +int HttpProxyClientSocket::DoTCPRestart() { + next_state_ = STATE_TCP_RESTART_COMPLETE; + return transport_->socket()->Connect(&io_callback_); +} + +int HttpProxyClientSocket::DoTCPRestartComplete(int result) { + if (result != OK) + return result; + + next_state_ = STATE_GENERATE_AUTH_TOKEN; + return result; +} + int HttpProxyClientSocket::HandleAuthChallenge() { DCHECK(response_.headers); diff --git a/net/http/http_proxy_client_socket.h b/net/http/http_proxy_client_socket.h index 317b220..ef4f76c 100644 --- a/net/http/http_proxy_client_socket.h +++ b/net/http/http_proxy_client_socket.h @@ -35,7 +35,8 @@ class HttpProxyClientSocket : public ClientSocket { // this socket will establish an Http tunnel. HttpProxyClientSocket(ClientSocketHandle* transport_socket, const GURL& request_url, const HostPortPair& endpoint, - const scoped_refptr<HttpAuthController>& auth, + const HostPortPair& proxy_server, + const scoped_refptr<HttpNetworkSession>& session, bool tunnel); // On destruction Disconnect() is called. @@ -46,12 +47,12 @@ class HttpProxyClientSocket : public ClientSocket { // RestartWithAuth. int RestartWithAuth(CompletionCallback* callback); - // Indicates if RestartWithAuth needs to be called. i.e. if Connect - // returned PROXY_AUTH_REQUESTED. Only valid after Connect has been called. - bool NeedsRestartWithAuth() const; - const HttpResponseInfo* GetResponseInfo() const { - return response_.headers ? &response_ : NULL; + return response_.headers ? &response_ : NULL; + } + + const scoped_refptr<HttpAuthController>& auth_controller() { + return auth_; } // ClientSocket methods: @@ -85,6 +86,8 @@ class HttpProxyClientSocket : public ClientSocket { STATE_RESOLVE_CANONICAL_NAME_COMPLETE, STATE_DRAIN_BODY, STATE_DRAIN_BODY_COMPLETE, + STATE_TCP_RESTART, + STATE_TCP_RESTART_COMPLETE, STATE_DONE, }; @@ -112,6 +115,8 @@ class HttpProxyClientSocket : public ClientSocket { int DoReadHeadersComplete(int result); int DoDrainBody(); int DoDrainBodyComplete(int result); + int DoTCPRestart(); + int DoTCPRestartComplete(int result); CompletionCallbackImpl<HttpProxyClientSocket> io_callback_; State next_state_; diff --git a/net/http/http_proxy_client_socket_pool.cc b/net/http/http_proxy_client_socket_pool.cc index a142576..783c10f 100644 --- a/net/http/http_proxy_client_socket_pool.cc +++ b/net/http/http_proxy_client_socket_pool.cc @@ -7,6 +7,7 @@ #include "base/time.h" #include "googleurl/src/gurl.h" #include "net/base/net_errors.h" +#include "net/http/http_network_session.h" #include "net/http/http_proxy_client_socket.h" #include "net/socket/client_socket_factory.h" #include "net/socket/client_socket_handle.h" @@ -18,12 +19,12 @@ HttpProxySocketParams::HttpProxySocketParams( const scoped_refptr<TCPSocketParams>& proxy_server, const GURL& request_url, HostPortPair endpoint, - scoped_refptr<HttpAuthController> auth_controller, + scoped_refptr<HttpNetworkSession> session, bool tunnel) : tcp_params_(proxy_server), request_url_(request_url), endpoint_(endpoint), - auth_controller_(auth_controller), + session_(session), tunnel_(tunnel) { } @@ -133,19 +134,22 @@ int HttpProxyConnectJob::DoTCPConnectComplete(int result) { int HttpProxyConnectJob::DoHttpProxyConnect() { next_state_ = kStateHttpProxyConnectComplete; + const HostResolver::RequestInfo& tcp_destination = + params_->tcp_params()->destination(); + HostPortPair proxy_server(tcp_destination.hostname(), + tcp_destination.port()); // Add a HttpProxy connection on top of the tcp socket. socket_.reset(new HttpProxyClientSocket(tcp_socket_handle_.release(), params_->request_url(), params_->endpoint(), - params_->auth_controller(), + proxy_server, + params_->session(), params_->tunnel())); return socket_->Connect(&callback_); } int HttpProxyConnectJob::DoHttpProxyConnectComplete(int result) { - DCHECK_NE(result, ERR_RETRY_CONNECTION); - if (result == OK || result == ERR_PROXY_AUTH_REQUESTED) set_socket(socket_.release()); diff --git a/net/http/http_proxy_client_socket_pool.h b/net/http/http_proxy_client_socket_pool.h index 63d5c02..347a2427 100644 --- a/net/http/http_proxy_client_socket_pool.h +++ b/net/http/http_proxy_client_socket_pool.h @@ -26,12 +26,13 @@ namespace net { class ClientSocketFactory; class ConnectJobFactory; class HttpAuthController; +class HttpNetworkSession; class HttpProxySocketParams : public base::RefCounted<HttpProxySocketParams> { public: HttpProxySocketParams(const scoped_refptr<TCPSocketParams>& proxy_server, const GURL& request_url, HostPortPair endpoint, - scoped_refptr<HttpAuthController> auth_controller, + scoped_refptr<HttpNetworkSession> session, bool tunnel); const scoped_refptr<TCPSocketParams>& tcp_params() const { @@ -39,8 +40,8 @@ class HttpProxySocketParams : public base::RefCounted<HttpProxySocketParams> { } const GURL& request_url() const { return request_url_; } const HostPortPair& endpoint() const { return endpoint_; } - const scoped_refptr<HttpAuthController>& auth_controller() { - return auth_controller_; + const scoped_refptr<HttpNetworkSession>& session() { + return session_; } bool tunnel() const { return tunnel_; } @@ -51,7 +52,7 @@ class HttpProxySocketParams : public base::RefCounted<HttpProxySocketParams> { const scoped_refptr<TCPSocketParams> tcp_params_; const GURL request_url_; const HostPortPair endpoint_; - const scoped_refptr<HttpAuthController> auth_controller_; + const scoped_refptr<HttpNetworkSession> session_; const bool tunnel_; DISALLOW_COPY_AND_ASSIGN(HttpProxySocketParams); diff --git a/net/http/http_proxy_client_socket_pool_unittest.cc b/net/http/http_proxy_client_socket_pool_unittest.cc index 9400dd8..3e06c3b 100644 --- a/net/http/http_proxy_client_socket_pool_unittest.cc +++ b/net/http/http_proxy_client_socket_pool_unittest.cc @@ -6,12 +6,19 @@ #include "base/callback.h" #include "base/compiler_specific.h" +#include "base/string_util.h" +#include "net/base/mock_host_resolver.h" #include "net/base/net_errors.h" +#include "net/base/ssl_config_service_defaults.h" #include "net/base/test_completion_callback.h" +#include "net/http/http_auth_handler_factory.h" +#include "net/http/http_network_session.h" #include "net/http/http_proxy_client_socket.h" +#include "net/proxy/proxy_service.h" #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 { @@ -29,19 +36,34 @@ class HttpProxyClientSocketPoolTest : public ClientSocketPoolTest { tcp_histograms_(new ClientSocketPoolHistograms("MockTCP")), tcp_socket_pool_(new MockTCPClientSocketPool(kMaxSockets, kMaxSocketsPerGroup, tcp_histograms_, &tcp_client_socket_factory_)), + 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)), notunnel_socket_params_(new HttpProxySocketParams( ignored_tcp_socket_params_, GURL("http://host"), HostPortPair("host", 80), NULL, false)), - auth_controller_(new MockHttpAuthController), tunnel_socket_params_(new HttpProxySocketParams( ignored_tcp_socket_params_, GURL("http://host"), - HostPortPair("host", 80), auth_controller_, true)), + HostPortPair("host", 80), session_, true)), http_proxy_histograms_( new ClientSocketPoolHistograms("HttpProxyUnitTest")), pool_(new HttpProxyClientSocketPool(kMaxSockets, kMaxSocketsPerGroup, http_proxy_histograms_, NULL, tcp_socket_pool_, NULL)) { } + void AddAuthToCache() { + const string16 kFoo(ASCIIToUTF16("foo")); + const string16 kBar(ASCIIToUTF16("bar")); + session_->auth_cache()->Add(GURL("http://proxy/"), "MyRealm1", "Basic", + "Basic realm=MyRealm1", kFoo, kBar, "/"); + } + int StartRequest(const std::string& group_name, RequestPriority priority) { return StartRequestUsingPool( pool_, group_name, priority, tunnel_socket_params_); @@ -52,8 +74,10 @@ class HttpProxyClientSocketPoolTest : public ClientSocketPoolTest { MockClientSocketFactory tcp_client_socket_factory_; scoped_refptr<MockTCPClientSocketPool> tcp_socket_pool_; + MockClientSocketFactory socket_factory_; + scoped_ptr<HttpAuthHandlerFactory> http_auth_handler_factory_; + scoped_refptr<HttpNetworkSession> session_; scoped_refptr<HttpProxySocketParams> notunnel_socket_params_; - scoped_refptr<MockHttpAuthController> auth_controller_; scoped_refptr<HttpProxySocketParams> tunnel_socket_params_; scoped_refptr<ClientSocketPoolHistograms> http_proxy_histograms_; scoped_refptr<HttpProxyClientSocketPool> pool_; @@ -72,7 +96,7 @@ TEST_F(HttpProxyClientSocketPoolTest, NoTunnel) { EXPECT_TRUE(handle.socket()); HttpProxyClientSocket* tunnel_socket = static_cast<HttpProxyClientSocket*>(handle.socket()); - EXPECT_FALSE(tunnel_socket->NeedsRestartWithAuth()); + EXPECT_TRUE(tunnel_socket->IsConnected()); } TEST_F(HttpProxyClientSocketPoolTest, NeedAuth) { @@ -92,10 +116,6 @@ TEST_F(HttpProxyClientSocketPoolTest, NeedAuth) { arraysize(writes)); tcp_client_socket_factory_.AddSocketDataProvider(&data); - MockHttpAuthControllerData auth_data[] = { - MockHttpAuthControllerData(""), - }; - auth_controller_->SetMockAuthControllerData(auth_data, arraysize(auth_data)); ClientSocketHandle handle; TestCompletionCallback callback; @@ -110,7 +130,7 @@ TEST_F(HttpProxyClientSocketPoolTest, NeedAuth) { EXPECT_TRUE(handle.socket()); HttpProxyClientSocket* tunnel_socket = static_cast<HttpProxyClientSocket*>(handle.socket()); - EXPECT_TRUE(tunnel_socket->NeedsRestartWithAuth()); + EXPECT_FALSE(tunnel_socket->IsConnected()); } TEST_F(HttpProxyClientSocketPoolTest, HaveAuth) { @@ -119,7 +139,7 @@ TEST_F(HttpProxyClientSocketPoolTest, HaveAuth) { "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"), @@ -129,10 +149,7 @@ TEST_F(HttpProxyClientSocketPoolTest, HaveAuth) { data.set_connect_data(MockConnect(false, 0)); tcp_client_socket_factory_.AddSocketDataProvider(&data); - MockHttpAuthControllerData auth_data[] = { - MockHttpAuthControllerData("Proxy-Authorization: Basic Zm9vOmJheg=="), - }; - auth_controller_->SetMockAuthControllerData(auth_data, arraysize(auth_data)); + AddAuthToCache(); ClientSocketHandle handle; TestCompletionCallback callback; @@ -143,7 +160,7 @@ TEST_F(HttpProxyClientSocketPoolTest, HaveAuth) { EXPECT_TRUE(handle.socket()); HttpProxyClientSocket* tunnel_socket = static_cast<HttpProxyClientSocket*>(handle.socket()); - EXPECT_FALSE(tunnel_socket->NeedsRestartWithAuth()); + EXPECT_TRUE(tunnel_socket->IsConnected()); } TEST_F(HttpProxyClientSocketPoolTest, AsyncHaveAuth) { @@ -151,7 +168,7 @@ TEST_F(HttpProxyClientSocketPoolTest, AsyncHaveAuth) { 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"), @@ -160,10 +177,7 @@ TEST_F(HttpProxyClientSocketPoolTest, AsyncHaveAuth) { arraysize(writes)); tcp_client_socket_factory_.AddSocketDataProvider(&data); - MockHttpAuthControllerData auth_data[] = { - MockHttpAuthControllerData("Proxy-Authorization: Basic Zm9vOmJheg=="), - }; - auth_controller_->SetMockAuthControllerData(auth_data, arraysize(auth_data)); + AddAuthToCache(); ClientSocketHandle handle; TestCompletionCallback callback; @@ -178,7 +192,7 @@ TEST_F(HttpProxyClientSocketPoolTest, AsyncHaveAuth) { EXPECT_TRUE(handle.socket()); HttpProxyClientSocket* tunnel_socket = static_cast<HttpProxyClientSocket*>(handle.socket()); - EXPECT_FALSE(tunnel_socket->NeedsRestartWithAuth()); + EXPECT_TRUE(tunnel_socket->IsConnected()); } TEST_F(HttpProxyClientSocketPoolTest, TCPError) { @@ -205,7 +219,7 @@ TEST_F(HttpProxyClientSocketPoolTest, TunnelUnexpectedClose) { 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 Conn"), @@ -215,10 +229,7 @@ TEST_F(HttpProxyClientSocketPoolTest, TunnelUnexpectedClose) { arraysize(writes)); tcp_client_socket_factory_.AddSocketDataProvider(&data); - MockHttpAuthControllerData auth_data[] = { - MockHttpAuthControllerData("Proxy-Authorization: Basic Zm9vOmJheg=="), - }; - auth_controller_->SetMockAuthControllerData(auth_data, arraysize(auth_data)); + AddAuthToCache(); ClientSocketHandle handle; TestCompletionCallback callback; @@ -238,7 +249,7 @@ TEST_F(HttpProxyClientSocketPoolTest, TunnelSetupError) { 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 304 Not Modified\r\n\r\n"), @@ -247,10 +258,7 @@ TEST_F(HttpProxyClientSocketPoolTest, TunnelSetupError) { arraysize(writes)); tcp_client_socket_factory_.AddSocketDataProvider(&data); - MockHttpAuthControllerData auth_data[] = { - MockHttpAuthControllerData("Proxy-Authorization: Basic Zm9vOmJheg=="), - }; - auth_controller_->SetMockAuthControllerData(auth_data, arraysize(auth_data)); + AddAuthToCache(); ClientSocketHandle handle; TestCompletionCallback callback; |