Skip to content

Commit

Permalink
Add test for httpLabel/httpQuery escaping
Browse files Browse the repository at this point in the history
Smithy recently amended the HTTP label spec to clarify the set of characters that must be escaped. But, due to
a peculiarty of the spec, it Smithy ommitted the `%` character from this set which definitely needs to be escaped.

This diff updates the spec and adds exhaustive test cases for both. These tests have been run on and pass the Rust SDK protocol
test suite. (They also went the other way, I had 2 bugs caused by copy-paste errors converting the Smithy set to the percent
encoding set used by the Rust SDK).

Exhaustive tests are important here, because I suspect other SDKs may have similar copy-paste errors, furthermore, several
of these characters, namely: `:()!,` may not be escaped by a "out of the box" protocol coder but these are known to cause
problems when sent to AWS services.
  • Loading branch information
rcoh authored and mtdowling committed May 17, 2021
1 parent d5c208b commit 7ec2b66
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 3 deletions.
6 changes: 3 additions & 3 deletions docs/source/1.0/spec/core/http-traits.rst
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ structure with the ``httpLabel`` trait MUST have a corresponding
``1985-04-12T23%3A20%3A50.52Z``). The :ref:`timestampFormat-trait`
MAY be used to use a custom serialization format.
- Reserved characters defined in :rfc:`section 2.2 of RFC3986 <3986#section-2.2>`
MUST be percent-encoded_ (that is, ``:/?#[]@!$&'()*+,;=``).
and the `%` itself MUST be percent-encoded_ (that is, ``:/?#[]@!$&'()*+,;=%``).
- However, if the label is greedy, then "/" MUST NOT be percent-encoded
because greedy labels are meant to span multiple path segments.

Expand Down Expand Up @@ -894,8 +894,8 @@ request:

* "&" is used to separate query string parameter key-value pairs.
* "=" is used to separate query string parameter names from values.
* Reserved characters in keys and values as defined in :rfc:`section 2.2 of RFC3986 <3986#section-2.2>`
MUST be percent-encoded_ (that is, ``:/?#[]@!$&'()*+,;=``).
* Reserved characters in keys and values as defined in :rfc:`section 2.2 of RFC3986 <3986#section-2.2>` and `%`
MUST be percent-encoded_ (that is, ``:/?#[]@!$&'()*+,;=%``).
* ``boolean`` values are serialized as ``true`` or ``false``.
* ``timestamp`` values are serialized as an :rfc:`3339`
``date-time`` string by default (for example, ``1985-04-12T23:20:50.52Z``,
Expand Down
18 changes: 18 additions & 0 deletions smithy-aws-protocol-tests/model/restJson1/http-labels.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,24 @@ apply HttpRequestWithLabels @httpRequestTests([
timestamp: 1576540098
}
},
{
id: "RestJsonHttpRequestLabelEscaping",
documentation: "Sends a GET request that uses URI label bindings",
protocol: restJson1,
method: "GET",
uri: "/HttpRequestWithLabels/%25%3A%2F%3F%23%5B%5D%40%21%24%26%27%28%29%2A%2B%2C%3B%3D%F0%9F%98%B9/1/2/3/4.1/5.1/true/2019-12-16T23%3A48%3A18Z",
body: "",
params: {
string: "%:/?#[]@!$&'()*+,;=😹",
short: 1,
integer: 2,
long: 3,
float: 4.1,
double: 5.1,
boolean: true,
timestamp: 1576540098
}
},
])

structure HttpRequestWithLabelsInput {
Expand Down
14 changes: 14 additions & 0 deletions smithy-aws-protocol-tests/model/restJson1/http-query.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,20 @@ apply AllQueryStringTypes @httpRequestTests([
"QueryParamsStringKeyB": "Bar",
},
}
},
{
id: "RestJsonQueryStringEscaping",
documentation: "Handles escaping all required characters in the query string.",
protocol: restJson1,
method: "GET",
uri: "/AllQueryStringTypesInput",
body: "",
queryParams: [
"String=%25%3A%2F%3F%23%5B%5D%40%21%24%26%27%28%29%2A%2B%2C%3B%3D%F0%9F%98%B9",
],
params: {
queryString: "%:/?#[]@!$&'()*+,;=😹"
}
}
])

Expand Down

0 comments on commit 7ec2b66

Please sign in to comment.