Add more tests cases for HttpAuth::HandleChallengeResponse.

This also improves the documentation for HandleChallengeResponse and clears the challenge_response arugment.

BUG=None
TEST=net_unittests --gtest_filter="*HandleChallengeResponse*"

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

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

3 files changed, 124 insertions, 48 deletions

diff --git a/net/http/http_auth.cc b/net/http/http_auth.cc
index ddd099c..e7002cf 100644
--- a/net/http/http_auth.cc
+++ b/net/http/http_auth.cc
@@ -60,7 +60,10 @@ HttpAuth::AuthorizationResult HttpAuth::HandleChallengeResponse(
     Target target,
     const std::set<std::string>& disabled_schemes,
     std::string* challenge_used) {
+  DCHECK(handler);
+  DCHECK(headers);
   DCHECK(challenge_used);
+  challenge_used->clear();
   const std::string& current_scheme = handler->scheme();
   if (disabled_schemes.find(current_scheme) != disabled_schemes.end())
     return HttpAuth::AUTHORIZATION_RESULT_REJECT;
diff --git a/net/http/http_auth.h b/net/http/http_auth.h
index ff1682b..3611612 100644
--- a/net/http/http_auth.h
+++ b/net/http/http_auth.h
@@ -121,7 +121,26 @@ class HttpAuth {
       const BoundNetLog& net_log,
       scoped_ptr<HttpAuthHandler>* handler);
 
-  // Handle a response to a previous authentication attempt.
+  // Handle a 401/407 response from a server/proxy after a previous
+  // authentication attempt. For connection-based authentication schemes, the
+  // new response may be another round in a multi-round authentication sequence.
+  // For request-based schemes, a 401/407 response is typically treated like a
+  // rejection of the previous challenge, except in the Digest case when a
+  // "stale" attribute is present.
+  //
+  // |handler| must be non-NULL, and is the HttpAuthHandler from the previous
+  // authentication round.
+  //
+  // |headers| must be non-NULL and contain the new HTTP response.
+  //
+  // |target| specifies whether the headers came from a server or proxy.
+  //
+  // |disabled_schemes| are the authentication schemes to ignore.
+  //
+  // |challenge_used| is the text of the authentication challenge used in
+  // support of the returned AuthorizationResult. If no headers were used for
+  // the result (for example, all headers have unknown authentication schemes),
+  // the value is cleared.
   static AuthorizationResult HandleChallengeResponse(
       HttpAuthHandler* handler,
       const HttpResponseHeaders* headers,
diff --git a/net/http/http_auth_unittest.cc b/net/http/http_auth_unittest.cc
index 85bed8c..3068135 100644
--- a/net/http/http_auth_unittest.cc
+++ b/net/http/http_auth_unittest.cc
@@ -42,6 +42,23 @@ HttpResponseHeaders* HeadersFromResponseText(const std::string& response) {
       HttpUtil::AssembleRawHeaders(response.c_str(), response.length()));
 }
 
+HttpAuth::AuthorizationResult HandleChallengeResponse(
+    bool connection_based,
+    const std::string& headers_text,
+    std::string* challenge_used) {
+  scoped_ptr<HttpAuthHandlerMock> mock_handler(
+      CreateMockHandler(connection_based));
+  std::set<std::string> disabled_schemes;
+  scoped_refptr<HttpResponseHeaders> headers(
+      HeadersFromResponseText(headers_text));
+  return HttpAuth::HandleChallengeResponse(
+      mock_handler.get(),
+      headers.get(),
+      HttpAuth::AUTH_SERVER,
+      disabled_schemes,
+      challenge_used);
+}
+
 }  // namespace
 
 TEST(HttpAuthTest, ChooseBestChallenge) {
@@ -130,58 +147,95 @@ TEST(HttpAuthTest, ChooseBestChallenge) {
   }
 }
 
-TEST(HttpAuthTest, HandleChallengeResponse_RequestBased) {
-  scoped_ptr<HttpAuthHandlerMock> mock_handler(CreateMockHandler(false));
-  std::set<std::string> disabled_schemes;
-  scoped_refptr<HttpResponseHeaders> headers(
-      HeadersFromResponseText(
-          "HTTP/1.1 401 Unauthorized\n"
-          "WWW-Authenticate: Mock token_here\n"));
+TEST(HttpAuthTest, HandleChallengeResponse) {
   std::string challenge_used;
-  EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_REJECT,
-            HttpAuth::HandleChallengeResponse(
-                mock_handler.get(),
-                headers.get(),
-                HttpAuth::AUTH_SERVER,
-                disabled_schemes,
-                &challenge_used));
+  const char* kMockChallenge =
+      "HTTP/1.1 401 Unauthorized\n"
+      "WWW-Authenticate: Mock token_here\n";
+  const char* kBasicChallenge =
+      "HTTP/1.1 401 Unauthorized\n"
+      "WWW-Authenticate: Basic realm=\"happy\"\n";
+  const char* kMissingChallenge =
+      "HTTP/1.1 401 Unauthorized\n";
+  const char* kEmptyChallenge =
+      "HTTP/1.1 401 Unauthorized\n"
+      "WWW-Authenticate: \n";
+  const char* kBasicAndMockChallenges =
+      "HTTP/1.1 401 Unauthorized\n"
+      "WWW-Authenticate: Basic realm=\"happy\"\n"
+      "WWW-Authenticate: Mock token_here\n";
+  const char* kTwoMockChallenges =
+      "HTTP/1.1 401 Unauthorized\n"
+      "WWW-Authenticate: Mock token_a\n"
+      "WWW-Authenticate: Mock token_b\n";
+
+  // Request based schemes should treat any new challenges as rejections of the
+  // previous authentication attempt. (There is a slight exception for digest
+  // authentication and the stale parameter, but that is covered in the
+  // http_auth_handler_digest_unittests).
+  EXPECT_EQ(
+      HttpAuth::AUTHORIZATION_RESULT_REJECT,
+      HandleChallengeResponse(false, kMockChallenge, &challenge_used));
   EXPECT_EQ("Mock token_here", challenge_used);
-}
 
-TEST(HttpAuthTest, HandleChallengeResponse_ConnectionBased) {
-  scoped_ptr<HttpAuthHandlerMock> mock_handler(CreateMockHandler(true));
-  std::set<std::string> disabled_schemes;
-  scoped_refptr<HttpResponseHeaders> headers(
-      HeadersFromResponseText(
-          "HTTP/1.1 401 Unauthorized\n"
-          "WWW-Authenticate: Mock token_here\n"));
-  std::string challenge_used;
-  EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
-            HttpAuth::HandleChallengeResponse(
-                mock_handler.get(),
-                headers.get(),
-                HttpAuth::AUTH_SERVER,
-                disabled_schemes,
-                &challenge_used));
+  EXPECT_EQ(
+      HttpAuth::AUTHORIZATION_RESULT_REJECT,
+      HandleChallengeResponse(false, kBasicChallenge, &challenge_used));
+  EXPECT_EQ("", challenge_used);
+
+  EXPECT_EQ(
+      HttpAuth::AUTHORIZATION_RESULT_REJECT,
+      HandleChallengeResponse(false, kMissingChallenge, &challenge_used));
+  EXPECT_EQ("", challenge_used);
+
+  EXPECT_EQ(
+      HttpAuth::AUTHORIZATION_RESULT_REJECT,
+      HandleChallengeResponse(false, kEmptyChallenge, &challenge_used));
+  EXPECT_EQ("", challenge_used);
+
+  EXPECT_EQ(
+      HttpAuth::AUTHORIZATION_RESULT_REJECT,
+      HandleChallengeResponse(false, kBasicAndMockChallenges, &challenge_used));
   EXPECT_EQ("Mock token_here", challenge_used);
-}
 
-TEST(HttpAuthTest, HandleChallengeResponse_ConnectionBasedNoMatch) {
-  scoped_ptr<HttpAuthHandlerMock> mock_handler(CreateMockHandler(true));
-  std::set<std::string> disabled_schemes;
-  scoped_refptr<HttpResponseHeaders> headers(
-      HeadersFromResponseText(
-          "HTTP/1.1 401 Unauthorized\n"
-          "WWW-Authenticate: Basic realm=\"happy\"\n"));
-  std::string challenge_used;
-  EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_REJECT,
-            HttpAuth::HandleChallengeResponse(
-                mock_handler.get(),
-                headers.get(),
-                HttpAuth::AUTH_SERVER,
-                disabled_schemes,
-                &challenge_used));
-  EXPECT_TRUE(challenge_used.empty());
+  EXPECT_EQ(
+      HttpAuth::AUTHORIZATION_RESULT_REJECT,
+      HandleChallengeResponse(false, kTwoMockChallenges, &challenge_used));
+  EXPECT_EQ("Mock token_a", challenge_used);
+
+  // Connection based schemes will treat new auth challenges for the same scheme
+  // as acceptance (and continuance) of the current approach. If there are
+  // no auth challenges for the same scheme, the response will be treated as
+  // a rejection.
+  EXPECT_EQ(
+      HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
+      HandleChallengeResponse(true, kMockChallenge, &challenge_used));
+  EXPECT_EQ("Mock token_here", challenge_used);
+
+  EXPECT_EQ(
+      HttpAuth::AUTHORIZATION_RESULT_REJECT,
+      HandleChallengeResponse(true, kBasicChallenge, &challenge_used));
+  EXPECT_EQ("", challenge_used);
+
+  EXPECT_EQ(
+      HttpAuth::AUTHORIZATION_RESULT_REJECT,
+      HandleChallengeResponse(true, kMissingChallenge, &challenge_used));
+  EXPECT_EQ("", challenge_used);
+
+  EXPECT_EQ(
+      HttpAuth::AUTHORIZATION_RESULT_REJECT,
+      HandleChallengeResponse(true, kEmptyChallenge, &challenge_used));
+  EXPECT_EQ("", challenge_used);
+
+  EXPECT_EQ(
+      HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
+      HandleChallengeResponse(true, kBasicAndMockChallenges, &challenge_used));
+  EXPECT_EQ("Mock token_here", challenge_used);
+
+  EXPECT_EQ(
+      HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
+      HandleChallengeResponse(true, kTwoMockChallenges, &challenge_used));
+  EXPECT_EQ("Mock token_a", challenge_used);
 }
 
 TEST(HttpAuthTest, ChallengeTokenizer) {