diff options
| -rw-r--r-- | net/http/http_auth_handler_basic.cc | 12 | ||||
| -rw-r--r-- | net/http/http_auth_handler_basic_unittest.cc | 38 | ||||
| -rw-r--r-- | net/http/http_auth_handler_digest.cc | 12 | ||||
| -rw-r--r-- | net/http/http_auth_handler_digest_unittest.cc | 19 | ||||
| -rw-r--r-- | net/http/http_network_transaction_unittest.cc | 4 |
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"), |