diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index fc4c4c2578..ddb5697d93 100644 --- a/CHANGELOG.next.toml +++ b/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" \ No newline at end of file +# 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" diff --git a/rust-runtime/aws-smithy-http-server/src/routing/request_spec.rs b/rust-runtime/aws-smithy-http-server/src/routing/request_spec.rs index e1f8e4740a..bbabefb019 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/request_spec.rs +++ b/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>` 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::>(query); + // We must deserialize into `Cow`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::, Cow)>>(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,