fix: properly encode URI paths that contain non-standard characters (#658)

ianbotsf · web-flow · commit db64cc65f385 · 2022-06-08T10:00:26.000-07:00
diff --git a/.changes/cc2cb6a3-fc49-4c83-9ca8-448e2f8131d6.json b/.changes/cc2cb6a3-fc49-4c83-9ca8-448e2f8131d6.json
@@ -0,0 +1,8 @@
+{
+    "id": "cc2cb6a3-fc49-4c83-9ca8-448e2f8131d6",
+    "type": "bugfix",
+    "description": "Fix bug in URI encoding during signing when dealing with special characters like '<', '>', and '/'",
+    "issues": [
+        "awslabs/smithy-kotlin#657"
+    ]
+}
diff --git a/runtime/auth/aws-signing-default/common/src/aws/smithy/kotlin/runtime/auth/awssigning/Canonicalizer.kt b/runtime/auth/aws-signing-default/common/src/aws/smithy/kotlin/runtime/auth/awssigning/Canonicalizer.kt
@@ -13,13 +13,11 @@ import aws.smithy.kotlin.runtime.http.UrlBuilder
 import aws.smithy.kotlin.runtime.http.request.HttpRequest
 import aws.smithy.kotlin.runtime.http.request.HttpRequestBuilder
 import aws.smithy.kotlin.runtime.http.request.toBuilder
+import aws.smithy.kotlin.runtime.http.util.encodeLabel
 import aws.smithy.kotlin.runtime.io.SdkByteReadChannel
 import aws.smithy.kotlin.runtime.time.TimestampFormat
 import aws.smithy.kotlin.runtime.util.*
-import aws.smithy.kotlin.runtime.util.text.encodeUrlPath
-import aws.smithy.kotlin.runtime.util.text.normalizePathSegments
-import aws.smithy.kotlin.runtime.util.text.urlEncodeComponent
-import aws.smithy.kotlin.runtime.util.text.urlReencodeComponent
+import aws.smithy.kotlin.runtime.util.text.*
 
 /**
  * The data for a canonical request.
@@ -171,11 +169,10 @@ private const val STREAM_CHUNK_BYTES = 16384 // 16KB
  * @param config The signing configuration to use
  * @return The canonicalized path
  */
-private fun UrlBuilder.canonicalPath(config: AwsSigningConfig): String {
-    val raw = path.trim()
-    val normalized = if (config.normalizeUriPath) raw.normalizePathSegments() else raw
-    val preEncoded = normalized.encodeUrlPath()
-    return if (config.useDoubleUriEncode) preEncoded.encodeUrlPath() else preEncoded
+internal fun UrlBuilder.canonicalPath(config: AwsSigningConfig): String {
+    val segmentTransform = if (config.useDoubleUriEncode) { s: String -> s.encodeLabel() } else null
+    val pathTransform = if (config.normalizeUriPath) String::normalizePathSegments else String::transformPathSegments
+    return pathTransform(path.trim(), segmentTransform)
 }
 
 /**
diff --git a/runtime/auth/aws-signing-default/common/test/aws/smithy/kotlin/runtime/auth/awssigning/DefaultCanonicalizerTest.kt b/runtime/auth/aws-signing-default/common/test/aws/smithy/kotlin/runtime/auth/awssigning/DefaultCanonicalizerTest.kt
@@ -6,10 +6,7 @@ package aws.smithy.kotlin.runtime.auth.awssigning
 
 import aws.smithy.kotlin.runtime.auth.awscredentials.Credentials
 import aws.smithy.kotlin.runtime.auth.awssigning.tests.testCredentialsProvider
-import aws.smithy.kotlin.runtime.http.Headers
-import aws.smithy.kotlin.runtime.http.HttpBody
-import aws.smithy.kotlin.runtime.http.HttpMethod
-import aws.smithy.kotlin.runtime.http.parameters
+import aws.smithy.kotlin.runtime.http.*
 import aws.smithy.kotlin.runtime.http.request.*
 import aws.smithy.kotlin.runtime.time.Instant
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -80,4 +77,21 @@ class DefaultCanonicalizerTest {
         }.entries()
         assertEquals(expectedHeaders, actual.request.headers.entries())
     }
+
+    // Targeted test for proper URI path escaping. See https://github.com/awslabs/smithy-kotlin/issues/657
+    @Test
+    fun testEscapablePath() {
+        val uri = UrlBuilder()
+        uri.path = "/2013-04-01/healthcheck/foo%3Cbar%3Ebaz%3C%2Fbar%3E"
+
+        val config = AwsSigningConfig {
+            normalizeUriPath = true
+            useDoubleUriEncode = true
+            region = "the-moon"
+            service = "landing-pad"
+            credentialsProvider = testCredentialsProvider
+        }
+
+        assertEquals("/2013-04-01/healthcheck/foo%253Cbar%253Ebaz%253C%252Fbar%253E", uri.canonicalPath(config))
+    }
 }
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/context.json b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/context.json
@@ -0,0 +1,12 @@
+{
+    "credentials": {
+        "access_key_id": "AKIDEXAMPLE",
+        "secret_access_key": "wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY"
+    },
+    "expiration_in_seconds": 3600,
+    "normalize": true,
+    "region": "us-east-1",
+    "service": "service",
+    "sign_body": false,
+    "timestamp": "2015-08-30T12:36:00Z"
+}
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/header-canonical-request.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/header-canonical-request.txt
@@ -0,0 +1,8 @@
+GET
+/foo/bar/baz%253Cqux%253Aquux
+
+host:example.amazonaws.com
+x-amz-date:20150830T123600Z
+
+host;x-amz-date
+e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/header-signature.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/header-signature.txt
@@ -0,0 +1 @@
+64ee2b4fb5e80890c58f8ccc1221231f3e3a556ba0f9c485d32bb2742a74e8ac
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/header-signed-request.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/header-signed-request.txt
@@ -0,0 +1,5 @@
+GET /foo/bar/baz%3Cqux%3Aquux HTTP/1.1
+Host:example.amazonaws.com
+X-Amz-Date:20150830T123600Z
+Authorization:AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20150830/us-east-1/service/aws4_request, SignedHeaders=host;x-amz-date, Signature=64ee2b4fb5e80890c58f8ccc1221231f3e3a556ba0f9c485d32bb2742a74e8ac
+
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/header-string-to-sign.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/header-string-to-sign.txt
@@ -0,0 +1,4 @@
+AWS4-HMAC-SHA256
+20150830T123600Z
+20150830/us-east-1/service/aws4_request
+4eb677b0ad9b3c4265bdb9eb11e09772e7ba1a11ef323d6f89800e59186ae790
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/query-canonical-request.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/query-canonical-request.txt
@@ -0,0 +1,7 @@
+GET
+/foo/bar/baz%253Cqux%253Aquux
+X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIDEXAMPLE%2F20150830%2Fus-east-1%2Fservice%2Faws4_request&X-Amz-Date=20150830T123600Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host
+host:example.amazonaws.com
+
+host
+e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/query-signature.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/query-signature.txt
@@ -0,0 +1 @@
+d1f120edc32fec5ee3797a82306977229dca6d3614e3c530897b87db9442533c
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/query-signed-request.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/query-signed-request.txt
@@ -0,0 +1,3 @@
+GET /foo/bar/baz%3Cqux%3Aquux?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIDEXAMPLE%2F20150830%2Fus-east-1%2Fservice%2Faws4_request&X-Amz-Date=20150830T123600Z&X-Amz-SignedHeaders=host&X-Amz-Expires=3600&X-Amz-Signature=d1f120edc32fec5ee3797a82306977229dca6d3614e3c530897b87db9442533c HTTP/1.1
+Host:example.amazonaws.com
+
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/query-string-to-sign.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/query-string-to-sign.txt
@@ -0,0 +1,4 @@
+AWS4-HMAC-SHA256
+20150830T123600Z
+20150830/us-east-1/service/aws4_request
+432c53bc4618c2ea1798092f0738bfe041a79adf83a6be151d7f8401c020f05a
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/request.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-encoded/request.txt
@@ -0,0 +1,2 @@
+GET /foo/bar/baz%3Cqux%3Aquux HTTP/1.1
+Host:example.amazonaws.com
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/context.json b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/context.json
@@ -0,0 +1,13 @@
+{
+    "credentials": {
+        "access_key_id": "AKIDEXAMPLE",
+        "secret_access_key": "wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY"
+    },
+    "double_uri_encode": false,
+    "expiration_in_seconds": 3600,
+    "normalize": true,
+    "region": "us-east-1",
+    "service": "service",
+    "sign_body": false,
+    "timestamp": "2015-08-30T12:36:00Z"
+}
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/header-canonical-request.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/header-canonical-request.txt
@@ -0,0 +1,8 @@
+GET
+/foo/bar/baz%3Cqux%3Aquux
+
+host:example.amazonaws.com
+x-amz-date:20150830T123600Z
+
+host;x-amz-date
+e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/header-signature.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/header-signature.txt
@@ -0,0 +1 @@
+dad1caab96f91a79dbba9fa5b6381af14f5d1be97a92e0dc2210d64cf26ef9d0
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/header-signed-request.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/header-signed-request.txt
@@ -0,0 +1,5 @@
+GET /foo/bar/baz%3Cqux%3Aquux HTTP/1.1
+Host:example.amazonaws.com
+X-Amz-Date:20150830T123600Z
+Authorization:AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20150830/us-east-1/service/aws4_request, SignedHeaders=host;x-amz-date, Signature=dad1caab96f91a79dbba9fa5b6381af14f5d1be97a92e0dc2210d64cf26ef9d0
+
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/header-string-to-sign.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/header-string-to-sign.txt
@@ -0,0 +1,4 @@
+AWS4-HMAC-SHA256
+20150830T123600Z
+20150830/us-east-1/service/aws4_request
+c529bb38b7ecb54ea9bf9702394cda745302d53494d7b9e8894f2758ebbbb7ce
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/query-canonical-request.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/query-canonical-request.txt
@@ -0,0 +1,7 @@
+GET
+/foo/bar/baz%3Cqux%3Aquux
+X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIDEXAMPLE%2F20150830%2Fus-east-1%2Fservice%2Faws4_request&X-Amz-Date=20150830T123600Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host
+host:example.amazonaws.com
+
+host
+e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/query-signature.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/query-signature.txt
@@ -0,0 +1 @@
+28d98975551533fb83f0f934855275c437e6e8cc4df41c64cb5353356a179476
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/query-signed-request.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/query-signed-request.txt
@@ -0,0 +1,3 @@
+GET /foo/bar/baz%3Cqux%3Aquux?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIDEXAMPLE%2F20150830%2Fus-east-1%2Fservice%2Faws4_request&X-Amz-Date=20150830T123600Z&X-Amz-SignedHeaders=host&X-Amz-Expires=3600&X-Amz-Signature=28d98975551533fb83f0f934855275c437e6e8cc4df41c64cb5353356a179476 HTTP/1.1
+Host:example.amazonaws.com
+
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/query-string-to-sign.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/query-string-to-sign.txt
@@ -0,0 +1,4 @@
+AWS4-HMAC-SHA256
+20150830T123600Z
+20150830/us-east-1/service/aws4_request
+734d998e11e89bde6c41c8d8dd9de70b387e05d9dab6c66604182e0f4ed81ded
diff --git a/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/request.txt b/runtime/auth/aws-signing-tests/common/resources/aws-signing-test-suite/v4/get-percent-single-encoded/request.txt
@@ -0,0 +1,2 @@
+GET /foo/bar/baz%3Cqux%3Aquux HTTP/1.1
+Host:example.amazonaws.com
diff --git a/runtime/auth/aws-signing-tests/jvm/src/aws/smithy/kotlin/runtime/auth/awssigning/tests/SigningSuiteTestBaseJVM.kt b/runtime/auth/aws-signing-tests/jvm/src/aws/smithy/kotlin/runtime/auth/awssigning/tests/SigningSuiteTestBaseJVM.kt
@@ -338,6 +338,10 @@ actual abstract class SigningSuiteTestBase : HasSigner {
             config.normalizeUriPath = it
         }
 
+        json["double_uri_encode"]?.jsonPrimitive?.boolean?.let {
+            config.useDoubleUriEncode = it
+        }
+
         val isoDate = json["timestamp"]?.jsonPrimitive?.content ?: DEFAULT_SIGNING_ISO_DATE
         config.signingDate = Instant.fromIso8601(isoDate)
 
diff --git a/runtime/utils/common/src/aws/smithy/kotlin/runtime/util/text/Text.kt b/runtime/utils/common/src/aws/smithy/kotlin/runtime/util/text/Text.kt
@@ -105,7 +105,7 @@ fun String.encodeUrlPath(validDelimiters: Set<Char>, checkPercentEncoded: Boolea
  * * All other segments are unmodified
  */
 @InternalApi
-public fun String.normalizePathSegments(): String {
+public fun String.normalizePathSegments(segmentTransform: ((String) -> String)?): String {
     val segments = split("/").filter(String::isNotEmpty)
     var skip = 0
     val normalizedSegments = buildList {
@@ -124,9 +124,14 @@ public fun String.normalizePathSegments(): String {
         separator = "/",
         prefix = "/",
         postfix = if (normalizedSegments.isNotEmpty() && endsWith("/")) "/" else "",
+        transform = segmentTransform,
     )
 }
 
+@InternalApi
+public fun String.transformPathSegments(segmentTransform: ((String) -> String)?): String =
+    split("/").joinToString(separator = "/", transform = segmentTransform)
+
 private const val upperHex: String = "0123456789ABCDEF"
 private val upperHexSet = upperHex.toSet()
 
diff --git a/runtime/utils/common/test/aws/smithy/kotlin/runtime/util/text/TextTest.kt b/runtime/utils/common/test/aws/smithy/kotlin/runtime/util/text/TextTest.kt
@@ -72,32 +72,32 @@ class TextTest {
     @Test
     fun testNormalizePathSegments() {
         fun assertNormalize(unnormalized: String, expected: String) {
-            val actual = unnormalized.normalizePathSegments()
+            val actual = unnormalized.normalizePathSegments(String::uppercase)
             assertEquals(expected, actual, "Unexpected normalization for `$unnormalized`")
         }
 
         val tests = mapOf(
             "" to "/",
             "/" to "/",
-            "foo" to "/foo",
-            "/foo" to "/foo",
-            "foo/" to "/foo/",
-            "/foo/" to "/foo/",
-            "/a/b/c" to "/a/b/c",
-            "/a/b/../c" to "/a/c",
-            "/a/./c" to "/a/c",
+            "foo" to "/FOO",
+            "/foo" to "/FOO",
+            "foo/" to "/FOO/",
+            "/foo/" to "/FOO/",
+            "/a/b/c" to "/A/B/C",
+            "/a/b/../c" to "/A/C",
+            "/a/./c" to "/A/C",
             "/./" to "/",
-            "/a/b/./../c" to "/a/c",
-            "/a/b/c/d/../e/../../f/../../../g" to "/g",
-            "//a//b//c//" to "/a/b/c/",
+            "/a/b/./../c" to "/A/C",
+            "/a/b/c/d/../e/../../f/../../../g" to "/G",
+            "//a//b//c//" to "/A/B/C/",
         )
         tests.forEach { (unnormalized, expected) -> assertNormalize(unnormalized, expected) }
     }
 
     @Test
     fun testNormalizePathSegmentsError() {
         assertFailsWith(IllegalArgumentException::class) {
-            "/a/b/../../..".normalizePathSegments()
+            "/a/b/../../..".normalizePathSegments { it }
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+64ee2b4fb5e80890c58f8ccc1221231f3e3a556ba0f9c485d32bb2742a74e8ac`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+d1f120edc32fec5ee3797a82306977229dca6d3614e3c530897b87db9442533c`