auth-int qop is ignored for Digest authentication

net/http/http_auth_handler_digest.cc

Index: net/http/http_auth_handler_digest.cc
===================================================================
--- net/http/http_auth_handler_digest.cc	(revision 63735)
+++ net/http/http_auth_handler_digest.cc	(working copy)
@@ -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.
@@ -66,13 +64,13 @@
 
 // static
 std::string HttpAuthHandlerDigest::QopToString(int qop) {

[eroman, 2010/11/11 23:02:58]

I think this function should be changed to either just take a
QualityOfProtection parameter, or else to return a comma-separated string.

My preference is the former, since it makes it obvious we are dealing with just
a single value, and the caller should have already decided on the policy of
which QOP to use.

-  switch (qop) {
-    case QOP_AUTH:
-      return "auth";
-    case QOP_AUTH_INT:
-      return "auth-int";
-    default:
-      return "";
+  // qop is a bitmask, so report highest priority one first.
+  if (qop & QOP_AUTH_INT) {
+    return "auth-int";
+  } else if (qop & QOP_AUTH) {

[eroman, 2010/11/11 23:02:58]

style: we avoid "else" when using returns.
As in:

if (xxx)
  return AAA;
if (yyy)
  return BBB;

+    return "auth";
+  } else {
+    return "";
   }
 }
 
@@ -309,13 +307,20 @@
   } else if (LowerCaseEqualsASCII(name, "qop")) {
     // Parse the comma separated list of qops.
     HttpUtil::ValuesIterator qop_values(value.begin(), value.end(), ',');
+    int qop = QOP_UNSPECIFIED;
     while (qop_values.GetNext()) {
       if (LowerCaseEqualsASCII(qop_values.value(), "auth")) {
-        qop_ |= QOP_AUTH;
+        qop |= QOP_AUTH;
       } else if (LowerCaseEqualsASCII(qop_values.value(), "auth-int")) {
-        qop_ |= QOP_AUTH_INT;
+        qop |= QOP_AUTH_INT;
       }
     }
+    // TODO(cbentzel): Since auth-int isn't currently supported, fail
+    // parsing if it is the only qop option available.
+    // http://crbug.com/45194
+    if (qop == QOP_AUTH_INT)
+      return false;

[eroman, 2010/11/11 23:02:58]

I suggest outputting some sort of warning or log message.

+    qop_ = (qop & ~QOP_AUTH_INT);
   } else {
     DVLOG(1) << "Skipping unrecognized digest property";
     // TODO(eroman): perhaps we should fail instead of silently skipping?