Allow the realm in BASIC and DIGEST challenges to not be specified.

This goes against RFC 2617 which states they are required parameters, but apparently there are servers which do this, and other browsers are less strict.

Also allow the empty string as a valid realm value (previously this was being disallowed as an implementation bug to check if it was not specified).

BUG=12565,20984

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

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

5 files changed, 74 insertions, 11 deletions

diff --git a/net/http/http_auth_handler_basic.cc b/net/http/http_auth_handler_basic.cc
index 698b0ab..0052dbf 100644
--- a/net/http/http_auth_handler_basic.cc
+++ b/net/http/http_auth_handler_basic.cc
@@ -10,6 +10,14 @@
 
 namespace net {
 
+// Note that if a realm was not specified, we will default it to "";
+// so specifying 'Basic realm=""' is equivalent to 'Basic'.
+//
+// This is more generous than RFC 2617, which is pretty clear in the
+// production of challenge that realm is required.
+//
+// We allow it to be compatibility with certain embedded webservers that don't
+// include a realm (see http://crbug.com/20984.)
 bool HttpAuthHandlerBasic::Init(std::string::const_iterator challenge_begin,
                                 std::string::const_iterator challenge_end) {
   scheme_ = "basic";
@@ -22,13 +30,13 @@ bool HttpAuthHandlerBasic::Init(std::string::const_iterator challenge_begin,
       !LowerCaseEqualsASCII(challenge_tok.scheme(), "basic"))
     return false;
 
-  // Extract the realm.
+  // Extract the realm (may be missing).
   while (challenge_tok.GetNext()) {
     if (LowerCaseEqualsASCII(challenge_tok.name(), "realm"))
       realm_ = challenge_tok.unquoted_value();
   }
 
-  return challenge_tok.valid() && !realm_.empty();
+  return challenge_tok.valid();
 }
 
 std::string HttpAuthHandlerBasic::GenerateCredentials(
diff --git a/net/http/http_auth_handler_basic_unittest.cc b/net/http/http_auth_handler_basic_unittest.cc
index ca2004c..d7a1437 100644
--- a/net/http/http_auth_handler_basic_unittest.cc
+++ b/net/http/http_auth_handler_basic_unittest.cc
@@ -27,8 +27,9 @@ TEST(HttpAuthHandlerBasicTest, GenerateCredentials) {
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
     std::string challenge = "Basic realm=\"Atlantis\"";
     scoped_refptr<HttpAuthHandlerBasic> basic = new HttpAuthHandlerBasic;
-    basic->InitFromChallenge(challenge.begin(), challenge.end(),
-                             HttpAuth::AUTH_SERVER, origin);
+    bool ok = basic->InitFromChallenge(challenge.begin(), challenge.end(),
+                                       HttpAuth::AUTH_SERVER, origin);
+    EXPECT_TRUE(ok);
     std::string credentials = basic->GenerateCredentials(tests[i].username,
                                                          tests[i].password,
                                                          NULL, NULL);
@@ -36,4 +37,37 @@ TEST(HttpAuthHandlerBasicTest, GenerateCredentials) {
   }
 }
 
+TEST(HttpAuthHandlerBasicTest, InitFromChallenge) {
+  static const struct {
+    const char* challenge;
+    bool expected_success;
+    const char* expected_realm;
+  } tests[] = {
+    // No realm (we allow this even though realm is supposed to be required
+    // according to RFC 2617.)
+    {
+      "Basic",
+      true,
+      "",
+    },
+
+    // Realm is empty string.
+    {
+      "Basic realm=\"\"",
+      true,
+      "",
+    },
+  };
+  GURL origin("http://www.example.com");
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
+    std::string challenge = tests[i].challenge;
+    scoped_refptr<HttpAuthHandlerBasic> basic = new HttpAuthHandlerBasic;
+    bool ok = basic->InitFromChallenge(challenge.begin(), challenge.end(),
+                                       HttpAuth::AUTH_SERVER, origin);
+    EXPECT_EQ(tests[i].expected_success, ok);
+    if (ok)
+      EXPECT_EQ(tests[i].expected_realm, basic->realm());
+  }
+}
+
 } // namespace net
diff --git a/net/http/http_auth_handler_digest.cc b/net/http/http_auth_handler_digest.cc
index 759d697..a0c443c 100644
--- a/net/http/http_auth_handler_digest.cc
+++ b/net/http/http_auth_handler_digest.cc
@@ -187,7 +187,7 @@ std::string HttpAuthHandlerDigest::AssembleCredentials(
 
 // The digest challenge header looks like:
 //   WWW-Authenticate: Digest
-//     realm="<realm-value>"
+//     [realm="<realm-value>"]
 //     nonce="<nonce-value>"
 //     [domain="<list-of-URIs>"]
 //     [opaque="<opaque-token-value>"]
@@ -195,6 +195,14 @@ std::string HttpAuthHandlerDigest::AssembleCredentials(
 //     [algorithm="<digest-algorithm>"]
 //     [qop="<list-of-qop-values>"]
 //     [<extension-directive>]
+//
+// Note that according to RFC 2617 (section 1.2) the realm is required.
+// However we allow it to be omitted, in which case it will default to the
+// empty string.
+//
+// This allowance is for better compatibility with webservers that fail to
+// send the realm (See http://crbug.com/20984 for an instance where a
+// webserver was not sending the realm with a BASIC challenge).
 bool HttpAuthHandlerDigest::ParseChallenge(
     std::string::const_iterator challenge_begin,
     std::string::const_iterator challenge_end) {
@@ -229,7 +237,7 @@ bool HttpAuthHandlerDigest::ParseChallenge(
     return false; // FAIL
 
   // Check that a minimum set of properties were provided.
-  if (realm_.empty() || nonce_.empty())
+  if (nonce_.empty())
     return false; // FAIL
 
   return true;
diff --git a/net/http/http_auth_handler_digest_unittest.cc b/net/http/http_auth_handler_digest_unittest.cc
index 5a27292..eb5649b 100644
--- a/net/http/http_auth_handler_digest_unittest.cc
+++ b/net/http/http_auth_handler_digest_unittest.cc
@@ -36,7 +36,7 @@ TEST(HttpAuthHandlerDigestTest, ParseChallenge) {
       HttpAuthHandlerDigest::QOP_UNSPECIFIED
     },
 
-    {// Check that when algorithm has an unsupported value, parsing fails.
+    { // Check that when algorithm has an unsupported value, parsing fails.
       "Digest nonce=\"xyz\", algorithm=\"awezum\", realm=\"Thunder\"",
       false,
       // The remaining values don't matter (but some have been set already).
@@ -74,9 +74,22 @@ TEST(HttpAuthHandlerDigestTest, ParseChallenge) {
       HttpAuthHandlerDigest::QOP_AUTH
     },
 
-    { // The realm can't be missing.
+    { // We allow the realm to be omitted, and will default it to empty string.
+      // See http://crbug.com/20984.
       "Digest nonce=\"xyz\"",
-      false, // FAILED parse.
+      true,
+      "",
+      "xyz",
+      "",
+      "",
+      false,
+      HttpAuthHandlerDigest::ALGORITHM_UNSPECIFIED,
+      HttpAuthHandlerDigest::QOP_UNSPECIFIED
+    },
+
+    { // Try with realm set to empty string.
+      "Digest realm=\"\", nonce=\"xyz\"",
+      true,
       "",
       "xyz",
       "",
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 32185c4..0148a77 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -666,7 +666,7 @@ TEST_F(HttpNetworkTransactionTest, BasicAuth) {
     MockRead("HTTP/1.0 401 Unauthorized\r\n"),
     // Give a couple authenticate options (only the middle one is actually
     // supported).
-    MockRead("WWW-Authenticate: Basic\r\n"),  // Malformed
+    MockRead("WWW-Authenticate: Basic invalid\r\n"),  // Malformed.
     MockRead("WWW-Authenticate: Basic realm=\"MyRealm1\"\r\n"),
     MockRead("WWW-Authenticate: UNSUPPORTED realm=\"FOO\"\r\n"),
     MockRead("Content-Type: text/html; charset=iso-8859-1\r\n"),
@@ -1398,7 +1398,7 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthProxyThenServer) {
     MockRead("HTTP/1.0 407 Unauthorized\r\n"),
     // Give a couple authenticate options (only the middle one is actually
     // supported).
-    MockRead("Proxy-Authenticate: Basic\r\n"),  // Malformed
+    MockRead("Proxy-Authenticate: Basic invalid\r\n"),  // Malformed.
     MockRead("Proxy-Authenticate: Basic realm=\"MyRealm1\"\r\n"),
     MockRead("Proxy-Authenticate: UNSUPPORTED realm=\"FOO\"\r\n"),
     MockRead("Content-Type: text/html; charset=iso-8859-1\r\n"),