From 494d18b25fca26766fa72ec407baeee9c4eb8a4e Mon Sep 17 00:00:00 2001 From: david-perez Date: Thu, 12 Jan 2023 10:47:21 +0100 Subject: [PATCH 1/4] Fix query string decoding `serde_urlencoded` can't decode into `Vec<(&str, &str)` query string slices that need percent decoding. --- .../src/routing/request_spec.rs | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) 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, From f2e4d1d65426a8f9d5aecfeb75ffd73bd4871ced Mon Sep 17 00:00:00 2001 From: david-perez Date: Thu, 12 Jan 2023 11:07:27 +0100 Subject: [PATCH 2/4] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4623373ece..ba1c75845d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ January 11th, 2023 - 🐛 (client, [smithy-rs#2150](https://github.com/awslabs/smithy-rs/issues/2150)) Fix bug where string default values were not supported for endpoint parameters - 🐛 (all, [smithy-rs#2170](https://github.com/awslabs/smithy-rs/issues/2170), [aws-sdk-rust#706](https://github.com/awslabs/aws-sdk-rust/issues/706)) Remove the webpki-roots feature from `hyper-rustls` - 🐛 (server, [smithy-rs#2054](https://github.com/awslabs/smithy-rs/issues/2054)) Servers can generate a unique request ID and use it in their handlers. +- 🐛 (server, [smithy-rs#2201](https://github.com/awslabs/smithy-rs/issues/2201)) 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; v0.53.0 has been yanked. December 12th, 2022 From 3afa389a83d49444709239bad851ec9dbae7c442 Mon Sep 17 00:00:00 2001 From: david-perez Date: Thu, 12 Jan 2023 11:38:57 +0100 Subject: [PATCH 3/4] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba1c75845d..3b5dd1ef77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ January 11th, 2023 - 🐛 (client, [smithy-rs#2150](https://github.com/awslabs/smithy-rs/issues/2150)) Fix bug where string default values were not supported for endpoint parameters - 🐛 (all, [smithy-rs#2170](https://github.com/awslabs/smithy-rs/issues/2170), [aws-sdk-rust#706](https://github.com/awslabs/aws-sdk-rust/issues/706)) Remove the webpki-roots feature from `hyper-rustls` - 🐛 (server, [smithy-rs#2054](https://github.com/awslabs/smithy-rs/issues/2054)) Servers can generate a unique request ID and use it in their handlers. -- 🐛 (server, [smithy-rs#2201](https://github.com/awslabs/smithy-rs/issues/2201)) 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; v0.53.0 has been yanked. +- 🐛 (server, [smithy-rs#2201](https://github.com/awslabs/smithy-rs/issues/2201)) 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. December 12th, 2022 From a9b3ad2c834edfa3543bb884110dcd364e675a91 Mon Sep 17 00:00:00 2001 From: david-perez Date: Thu, 12 Jan 2023 12:11:40 +0100 Subject: [PATCH 4/4] Move to CHANGELOG.next.toml --- CHANGELOG.md | 1 - CHANGELOG.next.toml | 8 +++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b5dd1ef77..4623373ece 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,6 @@ January 11th, 2023 - 🐛 (client, [smithy-rs#2150](https://github.com/awslabs/smithy-rs/issues/2150)) Fix bug where string default values were not supported for endpoint parameters - 🐛 (all, [smithy-rs#2170](https://github.com/awslabs/smithy-rs/issues/2170), [aws-sdk-rust#706](https://github.com/awslabs/aws-sdk-rust/issues/706)) Remove the webpki-roots feature from `hyper-rustls` - 🐛 (server, [smithy-rs#2054](https://github.com/awslabs/smithy-rs/issues/2054)) Servers can generate a unique request ID and use it in their handlers. -- 🐛 (server, [smithy-rs#2201](https://github.com/awslabs/smithy-rs/issues/2201)) 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. December 12th, 2022 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"