Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix query string decoding #2201

Merged
merged 4 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 19 additions & 3 deletions rust-runtime/aws-smithy-http-server/src/routing/request_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down