4 files changed, 61 insertions, 20 deletions

diff --git a/net/http/http_auth_controller.cc b/net/http/http_auth_controller.cc
index 8a342b8..52fc3f1 100644
--- a/net/http/http_auth_controller.cc
+++ b/net/http/http_auth_controller.cc
@@ -293,10 +293,11 @@ int HttpAuthController::HandleAuthChallenge(
         }
         break;
       case HttpAuth::AUTHORIZATION_RESULT_DIFFERENT_REALM:
-        // If the server asks for credentials for one realm and then
-        // rejects them, we remove the credentials from the cache
-        // unless it was in response to a preemptive authorization
-        // header.
+        // If the server changes the authentication realm in a
+        // subsequent challenge, invalidate cached credentials for the
+        // previous realm.  If the server rejects a preemptive
+        // authorization and requests credentials for a different
+        // realm, we keep the cached credentials.
         InvalidateCurrentHandler(
             (identity_.source == HttpAuth::IDENT_SRC_PATH_LOOKUP) ?
             INVALIDATE_HANDLER :
diff --git a/net/http/http_auth_controller.h b/net/http/http_auth_controller.h
index c4fc15e..d7faaac 100644
--- a/net/http/http_auth_controller.h
+++ b/net/http/http_auth_controller.h
@@ -89,7 +89,9 @@ class HttpAuthController : public base::RefCounted<HttpAuthController>,
   // cache entry's data and returns true.
   bool SelectPreemptiveAuth(const BoundNetLog& net_log);
 
-  // Invalidates the current handler, including cache.
+  // Invalidates the current handler.  If |action| is
+  // INVALIDATE_HANDLER_AND_CACHED_CREDENTIALS, then also invalidate
+  // the cached credentials used by the handler.
   void InvalidateCurrentHandler(InvalidateHandlerAction action);
 
   // Invalidates any auth cache entries after authentication has failed.
diff --git a/net/http/http_auth_handler_basic.cc b/net/http/http_auth_handler_basic.cc
index 9ed28e2..a51b8ba 100644
--- a/net/http/http_auth_handler_basic.cc
+++ b/net/http/http_auth_handler_basic.cc
@@ -59,10 +59,8 @@ HttpAuth::AuthorizationResult HttpAuthHandlerBasic::HandleAnotherChallenge(
   HttpUtil::NameValuePairsIterator parameters = challenge->param_pairs();
   std::string realm;
   while (parameters.GetNext()) {
-    if (LowerCaseEqualsASCII(parameters.name(), "realm")) {
+    if (LowerCaseEqualsASCII(parameters.name(), "realm"))
       realm = parameters.value();
-      break;
-    }
   }
   return (realm_ != realm)?
       HttpAuth::AUTHORIZATION_RESULT_DIFFERENT_REALM:
diff --git a/net/http/http_auth_handler_basic_unittest.cc b/net/http/http_auth_handler_basic_unittest.cc
index 0150579..51352da 100644
--- a/net/http/http_auth_handler_basic_unittest.cc
+++ b/net/http/http_auth_handler_basic_unittest.cc
@@ -5,6 +5,7 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "base/scoped_ptr.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "net/base/net_errors.h"
@@ -47,22 +48,53 @@ TEST(HttpAuthHandlerBasicTest, GenerateAuthToken) {
 }
 
 TEST(HttpAuthHandlerBasicTest, HandleAnotherChallenge) {
+  static const struct {
+    const char* challenge;
+    HttpAuth::AuthorizationResult expected_rv;
+  } tests[] = {
+    // The handler is initialized using this challenge.  The first
+    // time HandleAnotherChallenge is called with it should cause it
+    // to treat the second challenge as a rejection since it is for
+    // the same realm.
+    {
+      "Basic realm=\"First\"",
+      HttpAuth::AUTHORIZATION_RESULT_REJECT
+    },
+
+    // A challenge for a different realm.
+    {
+      "Basic realm=\"Second\"",
+      HttpAuth::AUTHORIZATION_RESULT_DIFFERENT_REALM
+    },
+
+    // Although RFC 2617 isn't explicit about this case, if there is
+    // more than one realm directive, we pick the last one.  So this
+    // challenge should be treated as being for "First" realm.
+    {
+      "Basic realm=\"Second\",realm=\"First\"",
+      HttpAuth::AUTHORIZATION_RESULT_REJECT
+    },
+
+    // And this one should be treated as if it was for "Second."
+    {
+      "basic realm=\"First\",realm=\"Second\"",
+      HttpAuth::AUTHORIZATION_RESULT_DIFFERENT_REALM
+    }
+  };
+
   GURL origin("http://www.example.com");
-  std::string challenge1 = "Basic realm=\"First\"";
-  std::string challenge2 = "Basic realm=\"Second\"";
   HttpAuthHandlerBasic::Factory factory;
   scoped_ptr<HttpAuthHandler> basic;
   EXPECT_EQ(OK, factory.CreateAuthHandlerFromString(
-      challenge1, HttpAuth::AUTH_SERVER, origin, BoundNetLog(), &basic));
-  HttpAuth::ChallengeTokenizer tok_first(challenge1.begin(),
-                                         challenge1.end());
-  EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_REJECT,
-            basic->HandleAnotherChallenge(&tok_first));
-
-  HttpAuth::ChallengeTokenizer tok_second(challenge2.begin(),
-                                          challenge2.end());
-  EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_DIFFERENT_REALM,
-            basic->HandleAnotherChallenge(&tok_second));
+      tests[0].challenge, HttpAuth::AUTH_SERVER, origin,
+      BoundNetLog(), &basic));
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
+    std::string challenge(tests[i].challenge);
+    HttpAuth::ChallengeTokenizer tok(challenge.begin(),
+                                     challenge.end());
+    EXPECT_EQ(tests[i].expected_rv, basic->HandleAnotherChallenge(&tok));
+  }
 }
 
 TEST(HttpAuthHandlerBasicTest, InitFromChallenge) {
@@ -131,6 +163,14 @@ TEST(HttpAuthHandlerBasicTest, InitFromChallenge) {
       ERR_INVALID_RESPONSE,
       ""
     },
+
+    // Although RFC 2617 isn't explicit about this case, if there is
+    // more than one realm directive, we pick the last one.
+    {
+      "Basic realm=\"foo\",realm=\"bar\"",
+      OK,
+      "bar",
+    },
   };
   HttpAuthHandlerBasic::Factory factory;
   GURL origin("http://www.example.com");