fix: correct signing for query parameters containing '+' and '%' (#654)

Author: ianbotsf

.changes/922b32ef-f194-4e5f-9ff2-9eca98246755.json

@@ -0,0 +1,8 @@
+{
+    "id": "922b32ef-f194-4e5f-9ff2-9eca98246755",
+    "type": "bugfix",
+    "description": "Fix bugs with signing for query parameters containing '+' and '%'",
+    "issues": [
+        "awslabs/aws-sdk-kotlin#619"
+    ]
+}

runtime/utils/common/src/aws/smithy/kotlin/runtime/util/text/Text.kt

@@ -170,37 +170,40 @@ public fun String.splitAsQueryString(): Map<String, List<String>> {
  * Decode a URL's query string, resolving percent-encoding (e.g., "%3B" → ";").
  */
 @InternalApi
-public fun String.urlDecodeComponent(): String {
+public fun String.urlDecodeComponent(formUrlDecode: Boolean = false): String {
     val orig = this
     return buildString(orig.length) {
         var byteBuffer: ByteArray? = null // Do not initialize unless needed
         var i = 0
         var c: Char
         while (i < orig.length) {
             c = orig[i]
-            when (c) {
-                '+' -> {
+            when {
+                c == '+' && formUrlDecode -> {
                     append(' ')
                     i++
                 }
 
-                '%' -> {
+                c == '%' -> {
                     if (byteBuffer == null) {
                         byteBuffer = ByteArray((orig.length - i) / 3) // Max remaining percent-encoded bytes
                     }
 
                     var byteCount = 0
                     while ((i + 2) < orig.length && c == '%') {
-                        val byte = orig.substring(i + 1, i + 3).toInt(radix = 16).toByte()
+                        val byte = orig.substring(i + 1, i + 3).toIntOrNull(radix = 16)?.toByte() ?: break
                         byteBuffer[byteCount++] = byte
 
                         i += 3
                         if (i < orig.length) c = orig[i]
                     }
 
-                    require(i == orig.length || c != '%') { "Incomplete escape pattern at end of string" }
-
                     append(byteBuffer.decodeToString(endIndex = byteCount))
+
+                    if (i != orig.length && c == '%') {
+                        append(c)
+                        i++
+                    }
                 }
 
                 else -> {
@@ -213,5 +216,5 @@ public fun String.urlDecodeComponent(): String {
 }
 
 @InternalApi
-public fun String.urlReencodeComponent(formUrlEncode: Boolean = false): String =
-    urlDecodeComponent().urlEncodeComponent(formUrlEncode)
+public fun String.urlReencodeComponent(formUrlDecode: Boolean = false, formUrlEncode: Boolean = false): String =
+    urlDecodeComponent(formUrlDecode).urlEncodeComponent(formUrlEncode)

runtime/utils/common/test/aws/smithy/kotlin/runtime/util/text/TextTest.kt

@@ -148,8 +148,22 @@ class TextTest {
 
     @Test
     fun decodeUrlComponent() {
+        val component = "a%3Bb+c%7Ed%20e%2Bf+g%3D%E1%88%B4"
+        val expected = "a;b+c~d e+f+g=ሴ"
+        assertEquals(expected, component.urlDecodeComponent())
+    }
+
+    @Test
+    fun decodeUrlComponentWithFormUrl() {
         val component = "a%3Bb+c%7Ed%20e%2Bf+g%3D%E1%88%B4"
         val expected = "a;b c~d e+f g=ሴ"
+        assertEquals(expected, component.urlDecodeComponent(true))
+    }
+
+    @Test
+    fun decodeUrlComponentInvalidSequence() {
+        val component = "%20%&'%%%f"
+        val expected = " %&'%%%f" // Only the %20 was valid
         assertEquals(expected, component.urlDecodeComponent())
     }