Fix label & query URI encoding (#953)

* Fix label & query URI encoding

https://github.com/awslabs/aws-sdk-rust/issues/331 demonstrated that we were failing to properly encode characters for URI path components and query components in several situation. This:
- Fixes the specific bugs
- Adds proptests (run locally with 16K cases) to verify that this is the complete set.
- Adds an S3-specific protocol test that targets this issue

* Make the test a bit stronger

* Update changelog

CHANGELOG.next.toml

@@ -129,3 +129,15 @@ message = "The features `aws-hyper/rustls` and `aws-hyper/native-tls` have been
 meta = { "breaking" = true, "tada" = false, "bug" = false }
 references = ["smithy-rs#947"]
 author = "rcoh"
+
+[[aws-sdk-rust]]
+message = "Fixed a bug where certain characters caused a panic during URI encoding."
+meta = { "breaking" = false, "tada" = false, "bug" = true }
+references = ["smithy-rs#953", "aws-sdk-rust#331"]
+author = "rcoh"
+
+[[smithy-rs]]
+message = "Fixed a bug where certain characters caused a panic during URI encoding."
+meta = { "breaking" = false, "tada" = false, "bug" = true }
+references = ["smithy-rs#953", "aws-sdk-rust#331"]
+author = "rcoh"

aws/sdk/aws-models/s3-tests.smithy

@@ -24,6 +24,7 @@ apply NotFound @httpResponseTests([
     }
 ])
 
+
 apply GetBucketLocation @httpResponseTests([
     {
         id: "GetBucketLocation",
@@ -183,3 +184,18 @@ apply PutObject @httpRequestTests([
         }
     }
 ])
+
+apply HeadObject @httpRequestTests([
+    {
+        id: "HeadObjectUriEncoding",
+        documentation: "https://github.com/awslabs/aws-sdk-rust/issues/331",
+
+        method: "HEAD",
+        protocol: "aws.protocols#restXml",
+        uri: "/test-bucket/%3C%3E%20%60%3F%F0%9F%90%B1",
+        params: {
+            Bucket: "test-bucket",
+            Key: "<> `?🐱",
+        }
+    }
+])

rust-runtime/aws-smithy-http/proptest-regressions/label.txt

@@ -0,0 +1,10 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc dfac816a3fc3fff8523f1a1707da0065b72fc3c0d70fce001627a8e2e7ee5e0e # shrinks to s = ">"
+cc 22bce3cd581f5f5a55e6ba18b1fb027481a496f6b35fee6dc4ef84659b99ddca # shrinks to s = "`"
+cc be619cccfee48e3bf642cf0f82e98e00dceccbe10963fbaf3a622a68a55a3227 # shrinks to s = "?\""
+cc 3e0b2e6f64642d7c58e5d2fe9223f75238a874bd8c3812dcb3ecc721d9aa0243 # shrinks to s = " "

rust-runtime/aws-smithy-http/proptest-regressions/query.txt

@@ -0,0 +1,9 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc b8ff8401495a7e4b4604f4438d8fc6b0ba63a58ddf58273ddcb3bb511e5cf91a # shrinks to s = "<"
+cc 59ee40f6a097f80254a91d0ee7d6cde97a353f7ccdf83eddd1d437781019431f # shrinks to s = "\""
+cc 65e6e5f9082c6cbebf599af889721d30d8ee2388f2f7be372520aa86526c8379 # shrinks to s = ">"

rust-runtime/aws-smithy-http/src/label.rs

@@ -6,15 +6,15 @@
 //! Formatting values as Smithy
 //! [httpLabel](https://awslabs.github.io/smithy/1.0/spec/core/http-traits.html#httplabel-trait)
 
-use crate::urlencode::BASE_SET;
+use crate::urlencode::LABEL_SET;
 use aws_smithy_types::date_time::{DateTimeFormatError, Format};
 use aws_smithy_types::DateTime;
 use percent_encoding::AsciiSet;
 
-const GREEDY: &AsciiSet = &BASE_SET.remove(b'/');
+const GREEDY: &AsciiSet = &LABEL_SET.remove(b'/');
 
 pub fn fmt_string<T: AsRef<str>>(t: T, greedy: bool) -> String {
-    let uri_set = if greedy { GREEDY } else { BASE_SET };
+    let uri_set = if greedy { GREEDY } else { LABEL_SET };
     percent_encoding::utf8_percent_encode(t.as_ref(), uri_set).to_string()
 }
 
@@ -25,10 +25,20 @@ pub fn fmt_timestamp(t: &DateTime, format: Format) -> Result<String, DateTimeFor
 #[cfg(test)]
 mod test {
     use crate::label::fmt_string;
+    use http::Uri;
+    use proptest::proptest;
 
     #[test]
     fn greedy_params() {
         assert_eq!(fmt_string("a/b", false), "a%2Fb");
         assert_eq!(fmt_string("a/b", true), "a/b");
     }
+
+    proptest! {
+        #[test]
+        fn test_encode_request(s: String) {
+            let _: Uri = format!("http://host.example.com/{}", fmt_string(&s, false)).parse().expect("all strings should be encoded properly");
+            let _: Uri = format!("http://host.example.com/{}", fmt_string(&s, true)).parse().expect("all strings should be encoded properly");
+        }
+    }
 }

rust-runtime/aws-smithy-http/src/query.rs

@@ -59,6 +59,8 @@ impl<'a> Writer<'a> {
 #[cfg(test)]
 mod test {
     use crate::query::{fmt_string, Writer};
+    use http::Uri;
+    use proptest::proptest;
 
     #[test]
     fn url_encode() {
@@ -79,4 +81,11 @@ mod test {
         writer.push_kv("b", "c");
         assert_eq!(out, "?a&b=c");
     }
+
+    proptest! {
+        #[test]
+        fn test_encode_request(s: String) {
+            let _: Uri = format!("http://host.example.com/?{}", fmt_string(&s)).parse().expect("all strings should be encoded properly");
+        }
+    }
 }

rust-runtime/aws-smithy-http/src/urlencode.rs

@@ -30,7 +30,12 @@ pub const BASE_SET: &AsciiSet = &CONTROLS
     .add(b'+')
     .add(b';')
     .add(b'=')
-    .add(b'%');
+    .add(b'%')
+    .add(b'<')
+    .add(b'>')
+    .add(b'"');
+
+pub const LABEL_SET: &AsciiSet = &BASE_SET.add(b'`').add(b'?').add(b' ');
 
 #[cfg(test)]
 mod test {