Fix query string decoding

CHANGELOG.next.toml

@@ -9,4 +9,10 @@
 # message = "Fix typos in module documentation for generated crates"
 # references = ["smithy-rs#920"]
 # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
-# author = "rcoh"
+# author = "rcoh"
+
+[[smithy-rs]]
+message = "Fix severe bug where a router fails to deserialize percent-encoded query strings, reporting no operation match when there could be one. If your Smithy model uses an operation with a request URI spec containing [query string literals](https://smithy.io/2.0/spec/http-bindings.html#query-string-literals), you are affected. This fix was released in `aws-smithy-http-server` v0.53.1."
+references = ["smithy-rs#2201"]
+meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server"}
+author = "david-perez"

rust-runtime/aws-smithy-http-server/src/routing/request_spec.rs

@@ -181,13 +181,18 @@ impl RequestSpec {
 
         match req.uri().query() {
             Some(query) => {
-                // We can't use `HashMap<&str, &str>` because a query string key can appear more
+                // We can't use `HashMap<Cow<str>, Cow<str>>` because a query string key can appear more
                 // than once e.g. `/?foo=bar&foo=baz`. We _could_ use a multiset e.g. the `hashbag`
                 // crate.
-                let res = serde_urlencoded::from_str::<Vec<(&str, &str)>>(query);
+                // We must deserialize into `Cow<str>`s because `serde_urlencoded` might need to
+                // return an owned allocated `String` if it has to percent-decode a slice of the query string.
+                let res = serde_urlencoded::from_str::<Vec<(Cow<str>, Cow<str>)>>(query);
 
                 match res {
-                    Err(_) => Match::No,
+                    Err(error) => {
+                        tracing::debug!(query, %error, "failed to deserialize query string");
+                        Match::No
+                    }
                     Ok(query_map) => {
                         for query_segment in self.uri_spec.path_and_query.query_segments.0.iter() {
                             match query_segment {
@@ -367,6 +372,17 @@ mod tests {
         );
     }
 
+    #[test]
+    fn encoded_query_string() {
+        let request_spec =
+            RequestSpec::from_parts(Method::DELETE, Vec::new(), vec![QuerySegment::Key("foo".to_owned())]);
+
+        assert_eq!(
+            Match::Yes,
+            request_spec.matches(&req(&Method::DELETE, "/?foo=hello%20world", None))
+        );
+    }
+
     fn ab_spec() -> RequestSpec {
         RequestSpec::from_parts(
             Method::GET,