Remove static "set_fixed_cnonce" in favor of NonceGenerator objects.

Trying to simplify cleanup for more unit tests.

BUG=None
TEST=net_unittests

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

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

5 files changed, 94 insertions, 23 deletions

diff --git a/net/http/http_auth_handler_digest.cc b/net/http/http_auth_handler_digest.cc
index fe3b8be..517558f 100644
--- a/net/http/http_auth_handler_digest.cc
+++ b/net/http/http_auth_handler_digest.cc
@@ -45,16 +45,19 @@ namespace net {
 // auth-int |          | req-method:req-uri:MD5(req-entity-body)  |
 //=====================+==========================================+
 
+HttpAuthHandlerDigest::NonceGenerator::NonceGenerator() {
+}
 
-//static
-bool HttpAuthHandlerDigest::fixed_cnonce_ = false;
+HttpAuthHandlerDigest::NonceGenerator::~NonceGenerator() {
+}
 
-// static
-std::string HttpAuthHandlerDigest::GenerateNonce() {
+HttpAuthHandlerDigest::DynamicNonceGenerator::DynamicNonceGenerator() {
+}
+
+std::string HttpAuthHandlerDigest::DynamicNonceGenerator::GenerateNonce()
+    const {
   // This is how mozilla generates their cnonce -- a 16 digit hex string.
   static const char domain[] = "0123456789abcdef";
-  if (fixed_cnonce_)
-    return std::string(domain);
   std::string cnonce;
   cnonce.reserve(16);
   for (int i = 0; i < 16; ++i)
@@ -62,6 +65,15 @@ std::string HttpAuthHandlerDigest::GenerateNonce() {
   return cnonce;
 }
 
+HttpAuthHandlerDigest::FixedNonceGenerator::FixedNonceGenerator(
+    const std::string& nonce)
+    : nonce_(nonce) {
+}
+
+std::string HttpAuthHandlerDigest::FixedNonceGenerator::GenerateNonce() const {
+  return nonce_;
+}
+
 // static
 std::string HttpAuthHandlerDigest::QopToString(QualityOfProtection qop) {
   switch (qop) {
@@ -91,11 +103,14 @@ std::string HttpAuthHandlerDigest::AlgorithmToString(
   }
 }
 
-HttpAuthHandlerDigest::HttpAuthHandlerDigest(int nonce_count)
+HttpAuthHandlerDigest::HttpAuthHandlerDigest(
+    int nonce_count, const NonceGenerator* nonce_generator)
     : stale_(false),
       algorithm_(ALGORITHM_UNSPECIFIED),
       qop_(QOP_UNSPECIFIED),
-      nonce_count_(nonce_count) {
+      nonce_count_(nonce_count),
+      nonce_generator_(nonce_generator) {
+  DCHECK(nonce_generator_);
 }
 
 HttpAuthHandlerDigest::~HttpAuthHandlerDigest() {
@@ -108,7 +123,7 @@ int HttpAuthHandlerDigest::GenerateAuthTokenImpl(
     CompletionCallback* callback,
     std::string* auth_token) {
   // Generate a random client nonce.
-  std::string cnonce = GenerateNonce();
+  std::string cnonce = nonce_generator_->GenerateNonce();
 
   // Extract the request method and path -- the meaning of 'path' is overloaded
   // in certain cases, to be a hostname.
@@ -327,12 +342,18 @@ bool HttpAuthHandlerDigest::ParseChallengeProperty(const std::string& name,
   return true;
 }
 
-HttpAuthHandlerDigest::Factory::Factory() {
+HttpAuthHandlerDigest::Factory::Factory()
+    : nonce_generator_(new DynamicNonceGenerator()) {
 }
 
 HttpAuthHandlerDigest::Factory::~Factory() {
 }
 
+void HttpAuthHandlerDigest::Factory::set_nonce_generator(
+    const NonceGenerator* nonce_generator) {
+  nonce_generator_.reset(nonce_generator);
+}
+
 int HttpAuthHandlerDigest::Factory::CreateAuthHandler(
     HttpAuth::ChallengeTokenizer* challenge,
     HttpAuth::Target target,
@@ -344,7 +365,7 @@ int HttpAuthHandlerDigest::Factory::CreateAuthHandler(
   // TODO(cbentzel): Move towards model of parsing in the factory
   //                 method and only constructing when valid.
   scoped_ptr<HttpAuthHandler> tmp_handler(
-      new HttpAuthHandlerDigest(digest_nonce_count));
+      new HttpAuthHandlerDigest(digest_nonce_count, nonce_generator_.get()));
   if (!tmp_handler->InitFromChallenge(challenge, target, origin, net_log))
     return ERR_INVALID_RESPONSE;
   handler->swap(tmp_handler);
diff --git a/net/http/http_auth_handler_digest.h b/net/http/http_auth_handler_digest.h
index ba5d92f..25cf16a 100644
--- a/net/http/http_auth_handler_digest.h
+++ b/net/http/http_auth_handler_digest.h
@@ -8,7 +8,9 @@
 
 #include <string>
 
+#include "base/basictypes.h"
 #include "base/gtest_prod_util.h"
+#include "base/scoped_ptr.h"
 #include "base/string16.h"
 #include "net/http/http_auth_handler.h"
 #include "net/http/http_auth_handler_factory.h"
@@ -18,6 +20,43 @@ namespace net {
 // Code for handling http digest authentication.
 class HttpAuthHandlerDigest : public HttpAuthHandler {
  public:
+  // A NonceGenerator is a simple interface for generating client nonces.
+  // Unit tests can override the default client nonce behavior with fixed
+  // nonce generation to get reproducible results.
+  class NonceGenerator {
+   public:
+    NonceGenerator();
+    virtual ~NonceGenerator();
+
+    // Generates a client nonce.
+    virtual std::string GenerateNonce() const = 0;
+   private:
+    DISALLOW_COPY_AND_ASSIGN(NonceGenerator);
+  };
+
+  // DynamicNonceGenerator does a random shuffle of 16
+  // characters to generate a client nonce.
+  class DynamicNonceGenerator : public NonceGenerator {
+   public:
+    DynamicNonceGenerator();
+    virtual std::string GenerateNonce() const;
+   private:
+    DISALLOW_COPY_AND_ASSIGN(DynamicNonceGenerator);
+  };
+
+  // FixedNonceGenerator always uses the same string specified at
+  // construction time as the client nonce.
+  class FixedNonceGenerator : public NonceGenerator {
+   public:
+    explicit FixedNonceGenerator(const std::string& nonce);
+
+    virtual std::string GenerateNonce() const;
+
+   private:
+    const std::string nonce_;
+    DISALLOW_COPY_AND_ASSIGN(FixedNonceGenerator);
+  };
+
   class Factory : public HttpAuthHandlerFactory {
    public:
     Factory();
@@ -30,6 +69,12 @@ class HttpAuthHandlerDigest : public HttpAuthHandler {
                                   int digest_nonce_count,
                                   const BoundNetLog& net_log,
                                   scoped_ptr<HttpAuthHandler>* handler);
+
+    // This factory owns the passed in |nonce_generator|.
+    void set_nonce_generator(const NonceGenerator* nonce_generator);
+
+   private:
+    scoped_ptr<const NonceGenerator> nonce_generator_;
   };
 
   HttpAuth::AuthorizationResult HandleAnotherChallenge(
@@ -70,7 +115,12 @@ class HttpAuthHandlerDigest : public HttpAuthHandler {
     QOP_AUTH,
   };
 
-  explicit HttpAuthHandlerDigest(int nonce_count);
+  // |nonce_count| indicates how many times the server-specified nonce has
+  // been used so far.
+  // |nonce_generator| is used to create a client nonce, and is not owned by
+  // the handler. The lifetime of the |nonce_generator| must exceed that of this
+  // handler.
+  HttpAuthHandlerDigest(int nonce_count, const NonceGenerator* nonce_generator);
   ~HttpAuthHandlerDigest();
 
   // Parse the challenge, saving the results into this instance.
@@ -110,11 +160,6 @@ class HttpAuthHandlerDigest : public HttpAuthHandler {
                                   const std::string& cnonce,
                                   int nonce_count) const;
 
-  // Forces cnonce to be the same each time. This is used for unit tests.
-  static void SetFixedCnonce(bool fixed_cnonce) {
-    fixed_cnonce_ = fixed_cnonce;
-  }
-
   // Information parsed from the challenge.
   std::string nonce_;
   std::string domain_;
@@ -124,9 +169,7 @@ class HttpAuthHandlerDigest : public HttpAuthHandler {
   QualityOfProtection qop_;
 
   int nonce_count_;
-
-  // Forces the cnonce to be the same each time, for unit tests.
-  static bool fixed_cnonce_;
+  const NonceGenerator* nonce_generator_;
 };
 
 }  // namespace net
diff --git a/net/http/http_auth_handler_digest_unittest.cc b/net/http/http_auth_handler_digest_unittest.cc
index 2c43710..ff37e99 100644
--- a/net/http/http_auth_handler_digest_unittest.cc
+++ b/net/http/http_auth_handler_digest_unittest.cc
@@ -463,7 +463,7 @@ TEST(HttpAuthHandlerDigest, HandleAnotherChallenge) {
       default_challenge, HttpAuth::AUTH_SERVER, origin, BoundNetLog(),
       &handler);
   EXPECT_EQ(OK, rv);
-
+  ASSERT_TRUE(handler.get() != NULL);
   HttpAuth::ChallengeTokenizer tok_default(default_challenge.begin(),
                                            default_challenge.end());
   EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_REJECT,
diff --git a/net/http/http_auth_handler_factory.h b/net/http/http_auth_handler_factory.h
index 16e60c4..2879aed 100644
--- a/net/http/http_auth_handler_factory.h
+++ b/net/http/http_auth_handler_factory.h
@@ -24,6 +24,8 @@ class HttpAuthHandler;
 class HttpAuthHandlerRegistryFactory;
 
 // An HttpAuthHandlerFactory is used to create HttpAuthHandler objects.
+// The HttpAuthHandlerFactory object _must_ outlive any of the HttpAuthHandler
+// objects that it creates.
 class HttpAuthHandlerFactory {
  public:
   HttpAuthHandlerFactory() : url_security_manager_(NULL) {}
@@ -49,7 +51,7 @@ class HttpAuthHandlerFactory {
   // challenge specified by |*challenge|. |challenge| must point to a valid
   // non-NULL tokenizer.
   //
-  // If an HttpAuthHandler object  is successfully created it is passed back to
+  // If an HttpAuthHandler object is successfully created it is passed back to
   // the caller through |*handler| and OK is returned.
   //
   // If |*challenge| specifies an unsupported authentication scheme, |*handler|
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 881a037..6a389af 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -4088,8 +4088,13 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthCacheAndPreauth) {
 // are started with the same nonce.
 TEST_F(HttpNetworkTransactionTest, DigestPreAuthNonceCount) {
   SessionDependencies session_deps;
+  HttpAuthHandlerDigest::Factory* digest_factory =
+      new HttpAuthHandlerDigest::Factory();
+  HttpAuthHandlerDigest::FixedNonceGenerator* nonce_generator =
+      new HttpAuthHandlerDigest::FixedNonceGenerator("0123456789abcdef");
+  digest_factory->set_nonce_generator(nonce_generator);
+  session_deps.http_auth_handler_factory.reset(digest_factory);
   scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps));
-  HttpAuthHandlerDigest::SetFixedCnonce(true);
 
   // Transaction 1: authenticate (foo, bar) on MyRealm1
   {