Fix malformed Digest Authorization header for multi-value qop (fixes #441)

lib/httpclient/auth.rb

@@ -393,6 +393,17 @@ def calc_cred(req, user, passwd, param)
       a_1 = "#{user}:#{param['realm']}:#{passwd}"
       a_2 = "#{method}:#{path}"
       qop = param['qop']
+      if qop
+        # A server may offer several qop-options as a comma-separated list
+        # (RFC 2617/7616: WWW-Authenticate carries qop="auth,auth-int"). The
+        # Authorization request header, however, sends exactly ONE unquoted
+        # qop-value (message-qop = "qop" "=" qop-value). Pick "auth" when it is
+        # offered (auth-int also requires hashing the entity body), else the
+        # first option. Both the response digest and the header below then use
+        # this single value, so they stay consistent. See issue #441.
+        qop_options = qop.split(',').map(&:strip)
+        qop = qop_options.include?('auth') ? 'auth' : qop_options.first
+      end
       nonce = param['nonce']
       cnonce = nil
       if qop || param['algorithm'] =~ /MD5-sess/
@@ -412,7 +423,7 @@ def calc_cred(req, user, passwd, param)
         @nonce_count += 1
         message_digest << ('%08x' % @nonce_count)
         message_digest << cnonce
-        message_digest << param['qop']
+        message_digest << qop
       end
       message_digest << Digest::MD5.hexdigest(a_2)
       header = []
@@ -425,7 +436,7 @@ def calc_cred(req, user, passwd, param)
       end
       if qop
         header << "nc=#{'%08x' % @nonce_count}"
-        header << "qop=#{param['qop']}"
+        header << "qop=#{qop}"
       end
       header << "response=\"#{Digest::MD5.hexdigest(message_digest.join(":"))}\""
       header << "algorithm=#{algorithm}"
@@ -442,12 +453,13 @@ def generate_cnonce
 
     def parse_challenge_param(param_str)
       param = {}
-      param_str.scan(/\s*([^\,]+(?:\\.[^\,]*)*)/).each do |str|
-        key, value = str[0].scan(/\A([^=]+)=(.*)\z/)[0]
-        if /\A"(.*)"\z/ =~ value
-          value = $1.gsub(/\\(.)/, '\1')
-        end
-        param[key] = value
+      # Split into `key = value` pairs. A value is either a quoted string (which
+      # may itself contain commas, e.g. qop="auth,auth-int") or a bare token.
+      # Splitting naively on every comma breaks a quoted comma-separated list
+      # such as qop="auth,auth-int" into qop=%{"auth} + junk, which then yields a
+      # malformed Authorization header (qop="auth, response="...). See issue #441.
+      param_str.scan(/(\w+)\s*=\s*(?:"((?:[^"\\]|\\.)*)"|([^,]*))/).each do |key, quoted, bare|
+        param[key] = quoted ? quoted.gsub(/\\(.)/, '\1') : bare.strip
       end
       param
     end

test/test_auth.rb

@@ -320,6 +320,27 @@ def test_perfer_digest
     assert_match(/^Authorization: Digest/, str)
   end
 
+  def test_digest_auth_multi_value_qop
+    # RFC 2617/7616: a server MAY offer several qop-options as a quoted,
+    # comma-separated list in WWW-Authenticate (qop="auth,auth-int"), but the
+    # client's Authorization header carries exactly ONE unquoted qop-value.
+    # Regression for issue #441: parse_challenge_param split on the comma INSIDE
+    # the quotes, so the request header came out malformed as
+    #   qop="auth, response="...  (unclosed qop quote swallowing response=)
+    # which strict servers reject.
+    c = HTTPClient.new
+    c.set_auth('http://example.com/', 'admin', 'admin')
+    c.test_loopback_http_response << %{HTTP/1.0 401 Unauthorized\nWWW-Authenticate: Digest realm="foo", qop="auth,auth-int", nonce="nonce", opaque="xyz", stale=false\nContent-Length: 2\n\nNG}
+    c.test_loopback_http_response << "HTTP/1.0 200 OK\nContent-Length: 2\n\nOK"
+    c.debug_dev = str = ''.dup
+    c.get_content('http://example.com/')
+    auth = str[/^Authorization: Digest.*$/]
+    assert(auth, 'no Authorization header sent')
+    assert_match(/(?:\A|, )qop=auth(?:,|\z)/, auth)   # single, unquoted qop-value
+    assert_no_match(/qop="/, auth)                    # never a quoted qop in the request
+    assert_match(/, response="[0-9a-f]{32}"/, auth)   # response= is its own well-formed field
+  end
+
   def test_digest_sess_auth
     c = HTTPClient.new
     c.set_auth("http://localhost:#{serverport}/", 'admin', 'admin')