HttpAuthHandlerFactory::CreateDefault requires a HostResolver argument.

Previously the default registry factory would crash if it saw a Negotiate header when trying to do a CNAME lookup because of a NULL resolver. Since then I've changed HttpAuthHandlerNegotiate to skip the CNAME lookup step when the resolver_ is NULL. 

So, one option is to make CreateDefault accept an optional HostResolver argument. My concern is that since passing in NULL is the easier choice, people will do it and encounter user problems when going through a Negotiate proxy. Hence, I force the correct behavior.

BUG=56819
TEST=net_unittests, try fetch client through a Negotiate authenticating proxy.

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

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

14 files changed, 51 insertions, 23 deletions

diff --git a/chrome/browser/net/connection_tester.cc b/chrome/browser/net/connection_tester.cc
index 5f71795..4f9d943 100644
--- a/chrome/browser/net/connection_tester.cc
+++ b/chrome/browser/net/connection_tester.cc
@@ -54,7 +54,8 @@ class ExperimentURLRequestContext : public URLRequestContext {
     // experiment being run.
     ftp_transaction_factory_ = new net::FtpNetworkLayer(host_resolver_);
     ssl_config_service_ = new net::SSLConfigServiceDefaults;
-    http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault();
+    http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault(
+        host_resolver_);
     http_transaction_factory_ = new net::HttpCache(
         net::HttpNetworkLayer::CreateFactory(host_resolver_, proxy_service_,
             ssl_config_service_, http_auth_handler_factory_, NULL, NULL),
diff --git a/chrome/service/net/service_url_request_context.cc b/chrome/service/net/service_url_request_context.cc
index e7069a5..9455c00 100644
--- a/chrome/service/net/service_url_request_context.cc
+++ b/chrome/service/net/service_url_request_context.cc
@@ -37,7 +37,8 @@ ServiceURLRequestContext::ServiceURLRequestContext() {
           proxy_config_service, false, 0u, this, NULL, NULL);
   ftp_transaction_factory_ = new net::FtpNetworkLayer(host_resolver_);
   ssl_config_service_ = new net::SSLConfigServiceDefaults;
-  http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault();
+  http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault(
+      host_resolver_);
   http_transaction_factory_ = new net::HttpCache(
       net::HttpNetworkLayer::CreateFactory(host_resolver_,
                                            proxy_service_,
diff --git a/chrome_frame/test/test_server_test.cc b/chrome_frame/test/test_server_test.cc
index dbbbd20..4fac1f7 100644
--- a/chrome_frame/test/test_server_test.cc
+++ b/chrome_frame/test/test_server_test.cc
@@ -66,7 +66,8 @@ class URLRequestTestContext : public URLRequestContext {
                                       NULL);
     proxy_service_ = net::ProxyService::CreateDirect();
     ssl_config_service_ = new net::SSLConfigServiceDefaults;
-    http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault();
+    http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault(
+        host_resolver_);
     http_transaction_factory_ = new net::HttpCache(
         net::HttpNetworkLayer::CreateFactory(host_resolver_, proxy_service_,
             ssl_config_service_, http_auth_handler_factory_, NULL, NULL),
diff --git a/net/http/http_auth_handler_factory.cc b/net/http/http_auth_handler_factory.cc
index c9e0fb9..4ad7fe8 100644
--- a/net/http/http_auth_handler_factory.cc
+++ b/net/http/http_auth_handler_factory.cc
@@ -39,15 +39,19 @@ int HttpAuthHandlerFactory::CreatePreemptiveAuthHandlerFromString(
 }
 
 // static
-HttpAuthHandlerRegistryFactory* HttpAuthHandlerFactory::CreateDefault() {
+HttpAuthHandlerRegistryFactory* HttpAuthHandlerFactory::CreateDefault(
+    HostResolver* host_resolver) {
+  DCHECK(host_resolver);
   HttpAuthHandlerRegistryFactory* registry_factory =
       new HttpAuthHandlerRegistryFactory();
   registry_factory->RegisterSchemeFactory(
       "basic", new HttpAuthHandlerBasic::Factory());
   registry_factory->RegisterSchemeFactory(
       "digest", new HttpAuthHandlerDigest::Factory());
-  registry_factory->RegisterSchemeFactory(
-      "negotiate", new HttpAuthHandlerNegotiate::Factory());
+  HttpAuthHandlerNegotiate::Factory* negotiate_factory =
+      new HttpAuthHandlerNegotiate::Factory();
+  negotiate_factory->set_host_resolver(host_resolver);
+  registry_factory->RegisterSchemeFactory("negotiate", negotiate_factory);
   registry_factory->RegisterSchemeFactory(
       "ntlm", new HttpAuthHandlerNTLM::Factory());
   return registry_factory;
@@ -89,7 +93,7 @@ HttpAuthHandlerRegistryFactory* HttpAuthHandlerRegistryFactory::Create(
     HttpAuthHandlerNegotiate::Factory* negotiate_factory =
         new HttpAuthHandlerNegotiate::Factory();
     negotiate_factory->set_url_security_manager(security_manager);
-    DCHECK(host_resolver != NULL || negotiate_disable_cname_lookup);
+    DCHECK(host_resolver || negotiate_disable_cname_lookup);
     negotiate_factory->set_host_resolver(host_resolver);
     negotiate_factory->set_disable_cname_lookup(negotiate_disable_cname_lookup);
     negotiate_factory->set_use_port(negotiate_enable_port);
diff --git a/net/http/http_auth_handler_factory.h b/net/http/http_auth_handler_factory.h
index 887c6cd..06d0f83 100644
--- a/net/http/http_auth_handler_factory.h
+++ b/net/http/http_auth_handler_factory.h
@@ -106,7 +106,11 @@ class HttpAuthHandlerFactory {
   // Creates a standard HttpAuthHandlerRegistryFactory. The caller is
   // responsible for deleting the factory.
   // The default factory supports Basic, Digest, NTLM, and Negotiate schemes.
-  static HttpAuthHandlerRegistryFactory* CreateDefault();
+  //
+  // |host_resolver| is used by the Negotiate authentication handler to perform
+  // CNAME lookups to generate a Kerberos SPN for the server. It must be
+  // non-NULL.
+  static HttpAuthHandlerRegistryFactory* CreateDefault(HostResolver* resolver);
 
  private:
   // The URL security manager
diff --git a/net/http/http_auth_handler_factory_unittest.cc b/net/http/http_auth_handler_factory_unittest.cc
index f845c8a..173e951 100644
--- a/net/http/http_auth_handler_factory_unittest.cc
+++ b/net/http/http_auth_handler_factory_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/scoped_ptr.h"
+#include "net/base/mock_host_resolver.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_auth_handler.h"
 #include "net/http/http_auth_handler_factory.h"
@@ -95,9 +96,10 @@ TEST(HttpAuthHandlerFactoryTest, RegistryFactory) {
 }
 
 TEST(HttpAuthHandlerFactoryTest, DefaultFactory) {
+  scoped_refptr<HostResolver> host_resolver(new MockHostResolver());
   URLSecurityManagerAllow url_security_manager;
   scoped_ptr<HttpAuthHandlerRegistryFactory> http_auth_handler_factory(
-      HttpAuthHandlerFactory::CreateDefault());
+      HttpAuthHandlerFactory::CreateDefault(host_resolver));
   http_auth_handler_factory->SetURLSecurityManager(
       "negotiate", &url_security_manager);
   GURL server_origin("http://www.example.com");
diff --git a/net/http/http_auth_unittest.cc b/net/http/http_auth_unittest.cc
index a3f2b28..cc62214 100644
--- a/net/http/http_auth_unittest.cc
+++ b/net/http/http_auth_unittest.cc
@@ -8,6 +8,7 @@
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "base/string_util.h"
+#include "net/base/mock_host_resolver.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_auth.h"
 #include "net/http/http_auth_filter.h"
@@ -97,8 +98,9 @@ TEST(HttpAuthTest, ChooseBestChallenge) {
   GURL origin("http://www.example.com");
   std::set<std::string> disabled_schemes;
   URLSecurityManagerAllow url_security_manager;
+  scoped_refptr<HostResolver> host_resolver(new MockHostResolver());
   scoped_ptr<HttpAuthHandlerRegistryFactory> http_auth_handler_factory(
-      HttpAuthHandlerFactory::CreateDefault());
+      HttpAuthHandlerFactory::CreateDefault(host_resolver));
   http_auth_handler_factory->SetURLSecurityManager(
       "negotiate", &url_security_manager);
 
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 2bcd753..b04a0a5 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -112,7 +112,8 @@ struct SessionDependencies {
       : host_resolver(new MockHostResolver),
         proxy_service(ProxyService::CreateDirect()),
         ssl_config_service(new SSLConfigServiceDefaults),
-        http_auth_handler_factory(HttpAuthHandlerFactory::CreateDefault()),
+        http_auth_handler_factory(
+            HttpAuthHandlerFactory::CreateDefault(host_resolver)),
         spdy_session_pool(new SpdySessionPool(NULL)),
         net_log(NULL) {}
 
@@ -121,7 +122,8 @@ struct SessionDependencies {
       : host_resolver(new MockHostResolver),
         proxy_service(proxy_service),
         ssl_config_service(new SSLConfigServiceDefaults),
-        http_auth_handler_factory(HttpAuthHandlerFactory::CreateDefault()),
+        http_auth_handler_factory(
+            HttpAuthHandlerFactory::CreateDefault(host_resolver)),
         spdy_session_pool(new SpdySessionPool(NULL)),
         net_log(NULL) {}
 
diff --git a/net/http/http_proxy_client_socket_pool_unittest.cc b/net/http/http_proxy_client_socket_pool_unittest.cc
index 0a27d62..6d03c19 100644
--- a/net/http/http_proxy_client_socket_pool_unittest.cc
+++ b/net/http/http_proxy_client_socket_pool_unittest.cc
@@ -50,8 +50,10 @@ class HttpProxyClientSocketPoolTest : public TestWithHttpParam {
             kMaxSocketsPerGroup, tcp_histograms_, &tcp_client_socket_factory_)),
         ssl_socket_pool_(new MockSSLClientSocketPool(kMaxSockets,
             kMaxSocketsPerGroup, tcp_histograms_, &tcp_client_socket_factory_)),
-        http_auth_handler_factory_(HttpAuthHandlerFactory::CreateDefault()),
-        session_(new HttpNetworkSession(new MockHostResolver,
+        host_resolver_(new MockHostResolver),
+        http_auth_handler_factory_(
+            HttpAuthHandlerFactory::CreateDefault(host_resolver_)),
+        session_(new HttpNetworkSession(host_resolver_,
                                         ProxyService::CreateDirect(),
                                         &socket_factory_,
                                         new SSLConfigServiceDefaults,
@@ -118,6 +120,7 @@ class HttpProxyClientSocketPoolTest : public TestWithHttpParam {
   scoped_refptr<MockSSLClientSocketPool> ssl_socket_pool_;
 
   MockClientSocketFactory socket_factory_;
+  scoped_refptr<HostResolver> host_resolver_;
   scoped_ptr<HttpAuthHandlerFactory> http_auth_handler_factory_;
   scoped_refptr<HttpNetworkSession> session_;
   scoped_refptr<ClientSocketPoolHistograms> http_proxy_histograms_;
diff --git a/net/socket/ssl_client_socket_pool_unittest.cc b/net/socket/ssl_client_socket_pool_unittest.cc
index 22a5441..62d7aac 100644
--- a/net/socket/ssl_client_socket_pool_unittest.cc
+++ b/net/socket/ssl_client_socket_pool_unittest.cc
@@ -35,8 +35,10 @@ const int kMaxSocketsPerGroup = 6;
 class SSLClientSocketPoolTest : public ClientSocketPoolTest {
  protected:
   SSLClientSocketPoolTest()
-      : http_auth_handler_factory_(HttpAuthHandlerFactory::CreateDefault()),
-        session_(new HttpNetworkSession(new MockHostResolver,
+      : host_resolver_(new MockHostResolver),
+        http_auth_handler_factory_(HttpAuthHandlerFactory::CreateDefault(
+            host_resolver_)),
+        session_(new HttpNetworkSession(host_resolver_,
                                         ProxyService::CreateDirect(),
                                         &socket_factory_,
                                         new SSLConfigServiceDefaults,
@@ -116,6 +118,7 @@ class SSLClientSocketPoolTest : public ClientSocketPoolTest {
   }
 
   MockClientSocketFactory socket_factory_;
+  scoped_refptr<HostResolver> host_resolver_;
   scoped_ptr<HttpAuthHandlerFactory> http_auth_handler_factory_;
   scoped_refptr<HttpNetworkSession> session_;
 
diff --git a/net/spdy/spdy_test_util.h b/net/spdy/spdy_test_util.h
index c30cb5c..be2f7d0 100644
--- a/net/spdy/spdy_test_util.h
+++ b/net/spdy/spdy_test_util.h
@@ -248,7 +248,6 @@ spdy::SpdyFrame* ConstructSpdyGetSynReply(const char* const extra_headers[],
 // Returns a SpdyFrame.
 spdy::SpdyFrame* ConstructSpdyGetSynReplyRedirect(int stream_id);
 
-
 // Constructs a standard SPDY POST SYN packet.
 // |extra_headers| are the extra header-value pairs, which typically
 // will vary the most between calls.
@@ -304,7 +303,8 @@ class SpdySessionDependencies {
         ssl_config_service(new SSLConfigServiceDefaults),
         socket_factory(new MockClientSocketFactory),
         deterministic_socket_factory(new DeterministicMockClientSocketFactory),
-        http_auth_handler_factory(HttpAuthHandlerFactory::CreateDefault()),
+        http_auth_handler_factory(
+            HttpAuthHandlerFactory::CreateDefault(host_resolver)),
         spdy_session_pool(new SpdySessionPool(NULL)) {
           // Note: The CancelledTransaction test does cleanup by running all
           // tasks in the message loop (RunAllPending).  Unfortunately, that
@@ -322,9 +322,11 @@ class SpdySessionDependencies {
         ssl_config_service(new SSLConfigServiceDefaults),
         socket_factory(new MockClientSocketFactory),
         deterministic_socket_factory(new DeterministicMockClientSocketFactory),
-        http_auth_handler_factory(HttpAuthHandlerFactory::CreateDefault()),
+        http_auth_handler_factory(
+            HttpAuthHandlerFactory::CreateDefault(host_resolver)),
         spdy_session_pool(new SpdySessionPool(NULL)) {}
 
+  // NOTE: host_resolver must be ordered before http_auth_handler_factory.
   scoped_refptr<MockHostResolverBase> host_resolver;
   scoped_refptr<ProxyService> proxy_service;
   scoped_refptr<SSLConfigService> ssl_config_service;
@@ -365,7 +367,8 @@ class SpdyURLRequestContext : public URLRequestContext {
     proxy_service_ = ProxyService::CreateDirect();
     spdy_session_pool_ = new SpdySessionPool(NULL);
     ssl_config_service_ = new SSLConfigServiceDefaults;
-    http_auth_handler_factory_ = HttpAuthHandlerFactory::CreateDefault();
+    http_auth_handler_factory_ = HttpAuthHandlerFactory::CreateDefault(
+        host_resolver_);
     http_transaction_factory_ = new net::HttpCache(
         new HttpNetworkLayer(&socket_factory_,
                              host_resolver_,
diff --git a/net/tools/fetch/fetch_client.cc b/net/tools/fetch/fetch_client.cc
index 9adf20b..435dc93 100644
--- a/net/tools/fetch/fetch_client.cc
+++ b/net/tools/fetch/fetch_client.cc
@@ -145,7 +145,7 @@ int main(int argc, char**argv) {
       net::SSLConfigService::CreateSystemSSLConfigService());
   net::HttpTransactionFactory* factory = NULL;
   scoped_ptr<net::HttpAuthHandlerFactory> http_auth_handler_factory(
-      net::HttpAuthHandlerFactory::CreateDefault());
+      net::HttpAuthHandlerFactory::CreateDefault(host_resolver));
   if (use_cache) {
     factory = new net::HttpCache(host_resolver, proxy_service,
         ssl_config_service, http_auth_handler_factory.get(), NULL, NULL,
diff --git a/net/url_request/url_request_unittest.h b/net/url_request/url_request_unittest.h
index 9ee9591..5900d61 100644
--- a/net/url_request/url_request_unittest.h
+++ b/net/url_request/url_request_unittest.h
@@ -156,7 +156,8 @@ class TestURLRequestContext : public URLRequestContext {
   void Init() {
     ftp_transaction_factory_ = new net::FtpNetworkLayer(host_resolver_);
     ssl_config_service_ = new net::SSLConfigServiceDefaults;
-    http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault();
+    http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault(
+        host_resolver_);
     http_transaction_factory_ = new net::HttpCache(
         net::HttpNetworkLayer::CreateFactory(host_resolver_,
                                              proxy_service_,
diff --git a/webkit/tools/test_shell/test_shell_request_context.cc b/webkit/tools/test_shell/test_shell_request_context.cc
index 82e7d01..b6e7b59 100644
--- a/webkit/tools/test_shell/test_shell_request_context.cc
+++ b/webkit/tools/test_shell/test_shell_request_context.cc
@@ -66,7 +66,8 @@ void TestShellRequestContext::Init(
                                              false, 0, NULL, NULL, NULL);
   ssl_config_service_ = net::SSLConfigService::CreateSystemSSLConfigService();
 
-  http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault();
+  http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault(
+      host_resolver_);
 
   net::HttpCache::DefaultBackend* backend = new net::HttpCache::DefaultBackend(
       cache_path.empty() ? net::MEMORY_CACHE : net::DISK_CACHE,