Improve unit test coverage of HttpAuthHandlerDigest parsing.

Also, if the only qop is auth-int, use a different handler since this is not currently handled.

BUG=45194
TEST=net_unittests --gtest_filter="*HttpAuthHandlerDigest*"

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

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

3 files changed, 195 insertions, 31 deletions

diff --git a/net/http/http_auth_handler_digest.cc b/net/http/http_auth_handler_digest.cc
index 82aa9af..fe3b8be 100644
--- a/net/http/http_auth_handler_digest.cc
+++ b/net/http/http_auth_handler_digest.cc
@@ -18,8 +18,6 @@
 #include "net/http/http_request_info.h"
 #include "net/http/http_util.h"
 
-// TODO(eroman): support qop=auth-int
-
 namespace net {
 
 // Digest authentication is specified in RFC 2617.
@@ -65,25 +63,30 @@ std::string HttpAuthHandlerDigest::GenerateNonce() {
 }
 
 // static
-std::string HttpAuthHandlerDigest::QopToString(int qop) {
+std::string HttpAuthHandlerDigest::QopToString(QualityOfProtection qop) {
   switch (qop) {
+    case QOP_UNSPECIFIED:
+      return "";
     case QOP_AUTH:
       return "auth";
-    case QOP_AUTH_INT:
-      return "auth-int";
     default:
+      NOTREACHED();
       return "";
   }
 }
 
 // static
-std::string HttpAuthHandlerDigest::AlgorithmToString(int algorithm) {
+std::string HttpAuthHandlerDigest::AlgorithmToString(
+    DigestAlgorithm algorithm) {
   switch (algorithm) {
+    case ALGORITHM_UNSPECIFIED:
+      return "";
     case ALGORITHM_MD5:
       return "MD5";
     case ALGORITHM_MD5_SESS:
       return "MD5-sess";
     default:
+      NOTREACHED();
       return "";
   }
 }
@@ -91,7 +94,7 @@ std::string HttpAuthHandlerDigest::AlgorithmToString(int algorithm) {
 HttpAuthHandlerDigest::HttpAuthHandlerDigest(int nonce_count)
     : stale_(false),
       algorithm_(ALGORITHM_UNSPECIFIED),
-      qop_(0),
+      qop_(QOP_UNSPECIFIED),
       nonce_count_(nonce_count) {
 }
 
@@ -308,12 +311,13 @@ bool HttpAuthHandlerDigest::ParseChallengeProperty(const std::string& name,
     }
   } else if (LowerCaseEqualsASCII(name, "qop")) {
     // Parse the comma separated list of qops.
+    // auth is the only supported qop, and all other values are ignored.
     HttpUtil::ValuesIterator qop_values(value.begin(), value.end(), ',');
+    qop_ = QOP_UNSPECIFIED;
     while (qop_values.GetNext()) {
       if (LowerCaseEqualsASCII(qop_values.value(), "auth")) {
-        qop_ |= QOP_AUTH;
-      } else if (LowerCaseEqualsASCII(qop_values.value(), "auth-int")) {
-        qop_ |= QOP_AUTH_INT;
+        qop_ = QOP_AUTH;
+        break;
       }
     }
   } else {
diff --git a/net/http/http_auth_handler_digest.h b/net/http/http_auth_handler_digest.h
index 0c38641..ba5d92f 100644
--- a/net/http/http_auth_handler_digest.h
+++ b/net/http/http_auth_handler_digest.h
@@ -63,12 +63,11 @@ class HttpAuthHandlerDigest : public HttpAuthHandler {
     ALGORITHM_MD5_SESS,
   };
 
-  // Possible values for "qop" -- may be or-ed together if there were
-  // multiple comma separated values.
+  // Possible values for QualityOfProtection.
+  // auth-int is not supported, see http://crbug.com/62890 for justification.
   enum QualityOfProtection {
-    QOP_UNSPECIFIED = 0,
-    QOP_AUTH = 1 << 0,
-    QOP_AUTH_INT = 1 << 1,
+    QOP_UNSPECIFIED,
+    QOP_AUTH,
   };
 
   explicit HttpAuthHandlerDigest(int nonce_count);
@@ -86,8 +85,8 @@ class HttpAuthHandlerDigest : public HttpAuthHandler {
   static std::string GenerateNonce();
 
   // Convert enum value back to string.
-  static std::string QopToString(int qop);
-  static std::string AlgorithmToString(int algorithm);
+  static std::string QopToString(QualityOfProtection qop);
+  static std::string AlgorithmToString(DigestAlgorithm algorithm);
 
   // Extract the method and path of the request, as needed by
   // the 'A2' production. (path may be a hostname for proxy).
@@ -122,7 +121,7 @@ class HttpAuthHandlerDigest : public HttpAuthHandler {
   std::string opaque_;
   bool stale_;
   DigestAlgorithm algorithm_;
-  int qop_;  // Bitfield of QualityOfProtection
+  QualityOfProtection qop_;
 
   int nonce_count_;
 
diff --git a/net/http/http_auth_handler_digest_unittest.cc b/net/http/http_auth_handler_digest_unittest.cc
index 8026613..2c43710 100644
--- a/net/http/http_auth_handler_digest_unittest.cc
+++ b/net/http/http_auth_handler_digest_unittest.cc
@@ -28,7 +28,7 @@ TEST(HttpAuthHandlerDigestTest, ParseChallenge) {
     int parsed_algorithm;
     int parsed_qop;
   } tests[] = {
-    {
+    { // Check that a minimal challenge works correctly.
       "Digest nonce=\"xyz\", realm=\"Thunder Bluff\"",
       true,
       "Thunder Bluff",
@@ -40,6 +40,80 @@ TEST(HttpAuthHandlerDigestTest, ParseChallenge) {
       HttpAuthHandlerDigest::QOP_UNSPECIFIED
     },
 
+    { // Realm does not need to be quoted, even though RFC2617 requires it.
+      "Digest nonce=\"xyz\", realm=ThunderBluff",
+      true,
+      "ThunderBluff",
+      "xyz",
+      "",
+      "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_UNSPECIFIED
+    },
+
+    { // We allow the realm to be omitted, and will default it to empty string.
+      // See http://crbug.com/20984.
+      "Digest nonce=\"xyz\"",
+      true,
+      "",
+      "xyz",
+      "",
+      "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_UNSPECIFIED
+    },
+
+    { // Try with realm set to empty string.
+      "Digest realm=\"\", nonce=\"xyz\"",
+      true,
+      "",
+      "xyz",
+      "",
+      "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_UNSPECIFIED
+    },
+
+    { // At a minimum, a nonce must be provided.
+      "Digest realm=\"Thunder Bluff\"",
+      false,
+      "",
+      "",
+      "",
+      "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_UNSPECIFIED
+    },
+
+    { // The nonce does not need to be quoted, even though RFC2617
+      // requires it.
+      "Digest nonce=xyz, realm=\"Thunder Bluff\"",
+      true,
+      "Thunder Bluff",
+      "xyz",
+      "",
+      "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_UNSPECIFIED
+    },
+
+    { // Unknown authentication parameters are ignored.
+      "Digest nonce=\"xyz\", realm=\"Thunder Bluff\", foo=\"bar\"",
+      true,
+      "Thunder Bluff",
+      "xyz",
+      "",
+      "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_UNSPECIFIED
+    },
+
     { // Check that when algorithm has an unsupported value, parsing fails.
       "Digest nonce=\"xyz\", algorithm=\"awezum\", realm=\"Thunder\"",
       false,
@@ -53,7 +127,8 @@ TEST(HttpAuthHandlerDigestTest, ParseChallenge) {
       HttpAuthHandlerDigest::QOP_UNSPECIFIED
     },
 
-    { // Check that algorithm's value is case insensitive.
+    { // Check that algorithm's value is case insensitive, and that MD5 is
+      // a supported algorithm.
       "Digest nonce=\"xyz\", algorithm=\"mD5\", realm=\"Oblivion\"",
       true,
       "Oblivion",
@@ -65,9 +140,8 @@ TEST(HttpAuthHandlerDigestTest, ParseChallenge) {
       HttpAuthHandlerDigest::QOP_UNSPECIFIED
     },
 
-    { // Check that md5-sess is recognized, as is single QOP
-      "Digest nonce=\"xyz\", algorithm=\"md5-sess\", "
-      "realm=\"Oblivion\", qop=\"auth\"",
+    { // Check that md5-sess is a supported algorithm.
+      "Digest nonce=\"xyz\", algorithm=\"md5-sess\", realm=\"Oblivion\"",
       true,
       "Oblivion",
       "xyz",
@@ -75,33 +149,119 @@ TEST(HttpAuthHandlerDigestTest, ParseChallenge) {
       "",
       false,
       HttpAuthHandlerDigest::ALGORITHM_MD5_SESS,
+      HttpAuthHandlerDigest::QOP_UNSPECIFIED,
+    },
+
+    { // Check that qop's value is case insensitive, and that auth is known.
+      "Digest nonce=\"xyz\", realm=\"Oblivion\", qop=\"aUth\"",
+      true,
+      "Oblivion",
+      "xyz",
+      "",
+      "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
       HttpAuthHandlerDigest::QOP_AUTH
     },
 
-    { // We allow the realm to be omitted, and will default it to empty string.
-      // See http://crbug.com/20984.
-      "Digest nonce=\"xyz\"",
+    { // auth-int is not handled, but will fall back to default qop.
+      "Digest nonce=\"xyz\", realm=\"Oblivion\", qop=\"auth-int\"",
+      true,
+      "Oblivion",
+      "xyz",
+      "",
+      "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_UNSPECIFIED
+    },
+
+    { // Unknown qop values are ignored.
+      "Digest nonce=\"xyz\", realm=\"Oblivion\", qop=\"auth,foo\"",
       true,
+      "Oblivion",
+      "xyz",
+      "",
       "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_AUTH
+    },
+
+    { // If auth-int is included with auth, then use auth.
+      "Digest nonce=\"xyz\", realm=\"Oblivion\", qop=\"auth,auth-int\"",
+      true,
+      "Oblivion",
+      "xyz",
+      "",
+      "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_AUTH
+    },
+
+    { // Opaque parameter parsing should work correctly.
+      "Digest nonce=\"xyz\", realm=\"Thunder Bluff\", opaque=\"foobar\"",
+      true,
+      "Thunder Bluff",
       "xyz",
       "",
+      "foobar",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_UNSPECIFIED
+    },
+
+    { // Opaque parameters do not need to be quoted, even though RFC2617
+      // seems to require it.
+      "Digest nonce=\"xyz\", realm=\"Thunder Bluff\", opaque=foobar",
+      true,
+      "Thunder Bluff",
+      "xyz",
       "",
+      "foobar",
       false,
       HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
       HttpAuthHandlerDigest::QOP_UNSPECIFIED
     },
 
-    { // Try with realm set to empty string.
-      "Digest realm=\"\", nonce=\"xyz\"",
+    { // Domain can be parsed.
+      "Digest nonce=\"xyz\", realm=\"Thunder Bluff\", "
+      "domain=\"http://intranet.example.com/protection\"",
       true,
+      "Thunder Bluff",
+      "xyz",
+      "http://intranet.example.com/protection",
       "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_UNSPECIFIED
+    },
+
+    { // Multiple domains can be parsed.
+      "Digest nonce=\"xyz\", realm=\"Thunder Bluff\", "
+      "domain=\"http://intranet.example.com/protection http://www.google.com\"",
+      true,
+      "Thunder Bluff",
       "xyz",
+      "http://intranet.example.com/protection http://www.google.com",
+      "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_UNSPECIFIED
+    },
+
+    { // If a non-Digest scheme is somehow passed in, it should be rejected.
+      "Basic realm=\"foo\"",
+      false,
+      "",
+      "",
       "",
       "",
       false,
       HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
       HttpAuthHandlerDigest::QOP_UNSPECIFIED
-    }
+    },
   };
 
   GURL origin("http://www.example.com");
@@ -118,9 +278,10 @@ TEST(HttpAuthHandlerDigestTest, ParseChallenge) {
       EXPECT_EQ(OK, rv);
     } else {
       EXPECT_NE(OK, rv);
+      EXPECT_TRUE(handler.get() == NULL);
       continue;
     }
-    ASSERT_TRUE(handler != NULL);
+    ASSERT_TRUE(handler.get() != NULL);
     HttpAuthHandlerDigest* digest =
         static_cast<HttpAuthHandlerDigest*>(handler.get());
     EXPECT_STREQ(tests[i].parsed_realm, digest->realm_.c_str());
@@ -291,7 +452,7 @@ TEST(HttpAuthHandlerDigestTest, AssembleCredentials) {
   }
 }
 
-TEST(HttpAuthHandlerDigest, HandleAnotherChallenge_Failed) {
+TEST(HttpAuthHandlerDigest, HandleAnotherChallenge) {
   scoped_ptr<HttpAuthHandlerDigest::Factory> factory(
       new HttpAuthHandlerDigest::Factory());
   scoped_ptr<HttpAuthHandler> handler;