KTOR-4318, KTOR-7561: Fix digest auth parameters (#4399)

- Fix nc parameter implementation
- Quote params on the client
- Don't quote 'stale' and 'algorithm' params on the server

ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/DigestAuthProvider.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2019 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
+ * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
  */
 
 package io.ktor.client.plugins.auth.providers
@@ -9,6 +9,7 @@ import io.ktor.client.request.*
 import io.ktor.client.statement.*
 import io.ktor.http.*
 import io.ktor.http.auth.*
+import io.ktor.http.auth.HeaderValueEncoding
 import io.ktor.util.*
 import io.ktor.utils.io.*
 import io.ktor.utils.io.charsets.*
@@ -169,20 +170,21 @@ public class DigestAuthProvider(
         val token = makeDigest(tokenSequence.joinToString(":"))
 
         val auth = HttpAuthHeader.Parameterized(
-            AuthScheme.Digest,
-            linkedMapOf<String, String>().apply {
-                realm?.let { this["realm"] = it }
-                serverOpaque?.let { this["opaque"] = it }
-                this["username"] = credentials.username
-                this["nonce"] = nonce
-                this["cnonce"] = clientNonce
-                this["response"] = hex(token)
-                this["uri"] = url.fullPath
+            authScheme = AuthScheme.Digest,
+            parameters = linkedMapOf<String, String>().apply {
+                realm?.let { this["realm"] = it.quote() }
+                serverOpaque?.let { this["opaque"] = it.quote() }
+                this["username"] = credentials.username.quote()
+                this["nonce"] = nonce.quote()
+                this["cnonce"] = clientNonce.quote()
+                this["response"] = hex(token).quote()
+                this["uri"] = url.fullPath.quote()
                 actualQop?.let { this["qop"] = it }
-                this["nc"] = nonceCount.toString()
+                this["nc"] = nonceCount.toString(radix = 16).padStart(length = 8, padChar = '0')
                 @Suppress("DEPRECATION_ERROR")
                 this["algorithm"] = algorithmName
-            }
+            },
+            encoding = HeaderValueEncoding.QUOTED_WHEN_REQUIRED
         )
 
         request.headers {

ktor-client/ktor-client-plugins/ktor-client-auth/common/test/io/ktor/client/plugins/auth/DigestProviderTest.kt

@@ -1,6 +1,6 @@
 /*
-* Copyright 2014-2021 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
-*/
+ * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
+ */
 
 @file:Suppress("DEPRECATION")
 
@@ -21,14 +21,31 @@ class DigestProviderTest {
 
     private val paramValue = "value"
 
-    private val authAllFields =
-        "Digest algorithm=MD5, username=\"username\", realm=\"realm\", nonce=\"nonce\", qop=\"qop\", " +
-            "snonce=\"server-nonce\", cnonce=\"client-nonce\", uri=\"requested-uri\", " +
-            "request=\"client-digest\", message=\"message-digest\", opaque=\"opaque\""
-
-    private val authMissingQopAndOpaque =
-        "Digest algorithm=MD5, username=\"username\", realm=\"realm\", nonce=\"nonce\", snonce=\"server-nonce\", " +
-            "cnonce=\"client-nonce\", uri=\"requested-uri\", request=\"client-digest\", message=\"message-digest\""
+    private val authAllFields = """
+        Digest
+        algorithm=MD5,
+        username="username",
+        realm="realm",
+        nonce="nonce",
+        qop=qop,
+        cnonce="client-nonce",
+        uri="requested-uri",
+        request="client-digest",
+        message="message-digest",
+        opaque="opaque"
+    """.normalize()
+
+    private val authMissingQopAndOpaque = """
+        Digest
+        algorithm=MD5,
+        username="username",
+        realm="realm",
+        nonce="nonce",
+        cnonce="client-nonce",
+        uri="requested-uri",
+        request="client-digest",
+        message="message-digest"
+    """.normalize()
 
     private val digestAuthProvider by lazy {
         DigestAuthProvider({ DigestAuthCredentials("username", "password") }, "realm")
@@ -55,9 +72,9 @@ class DigestProviderTest {
         runIsApplicable(authAllFields)
         val authHeader = addRequestHeaders(authAllFields)
 
-        assertTrue(authHeader.contains("qop=qop"))
-        assertTrue(authHeader.contains("opaque=opaque"))
-        checkStandardFields(authHeader)
+        authHeader.assertParameter("qop", expectedValue = "qop")
+        authHeader.assertParameter("opaque", expectedValue = "opaque".quote())
+        authHeader.checkStandardParameters()
     }
 
     @Test
@@ -72,7 +89,7 @@ class DigestProviderTest {
         providerWithoutRealm.addRequestHeaders(requestBuilder, authHeader)
 
         val resultAuthHeader = requestBuilder.headers[HttpHeaders.Authorization]!!
-        checkStandardFields(resultAuthHeader)
+        resultAuthHeader.checkStandardParameters()
     }
 
     @Test
@@ -93,9 +110,9 @@ class DigestProviderTest {
         runIsApplicable(authMissingQopAndOpaque)
         val authHeader = addRequestHeaders(authMissingQopAndOpaque)
 
-        assertFalse(authHeader.contains("opaque="))
-        assertFalse(authHeader.contains("qop="))
-        checkStandardFields(authHeader)
+        authHeader.assertParameterNotSet("opaque")
+        authHeader.assertParameterNotSet("qop")
+        authHeader.checkStandardParameters()
     }
 
     @Test
@@ -123,12 +140,21 @@ class DigestProviderTest {
         return requestBuilder.headers[HttpHeaders.Authorization]!!
     }
 
-    private fun checkStandardFields(authHeader: String) {
-        assertTrue(authHeader.contains("realm=realm"))
-        assertTrue(authHeader.contains("username=username"))
-        assertTrue(authHeader.contains("nonce=nonce"))
+    private fun String.checkStandardParameters() {
+        assertParameter("realm", expectedValue = "realm".quote())
+        assertParameter("username", expectedValue = "username".quote())
+        assertParameter("nonce", expectedValue = "nonce".quote())
+        assertParameter("nc", expectedValue = "00000001")
+        assertParameter("uri", expectedValue = "/$path?$paramName=$paramValue".quote())
+    }
 
-        val uriPattern = "uri=\"/$path?$paramName=$paramValue\""
-        assertTrue(authHeader.contains(uriPattern))
+    private fun String.assertParameter(name: String, expectedValue: String?) {
+        assertContains(this, "$name=$expectedValue")
     }
+
+    private fun String.assertParameterNotSet(name: String) {
+        assertFalse(this.contains("$name="))
+    }
+
+    private fun String.normalize(): String = trimIndent().replace("\n", " ")
 }

ktor-http/common/src/io/ktor/http/auth/HttpAuthHeader.kt

@@ -1,6 +1,6 @@
 /*
-* Copyright 2014-2021 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
-*/
+ * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
+ */
 
 package io.ktor.http.auth
 
@@ -394,22 +394,22 @@ public sealed class HttpAuthHeader(public val authScheme: String) {
             stale: Boolean? = null,
             algorithm: String = "MD5"
         ): Parameterized = Parameterized(
-            AuthScheme.Digest,
-            linkedMapOf<String, String>().apply {
-                put("realm", realm)
-                put("nonce", nonce)
+            authScheme = AuthScheme.Digest,
+            parameters = linkedMapOf<String, String>().apply {
+                put("realm", realm.quote())
+                put("nonce", nonce.quote())
                 if (domain.isNotEmpty()) {
-                    put("domain", domain.joinToString(" "))
+                    put("domain", domain.joinToString(" ").quote())
                 }
                 if (opaque != null) {
-                    put("opaque", opaque)
+                    put("opaque", opaque.quote())
                 }
                 if (stale != null) {
                     put("stale", stale.toString())
                 }
                 put("algorithm", algorithm)
             },
-            HeaderValueEncoding.QUOTED_ALWAYS
+            encoding = HeaderValueEncoding.QUOTED_WHEN_REQUIRED
         )
     }