ex-aws
Signing URLs with query params appears to be broken in PR#660
Environment
- Elixir & Erlang versions: Elixir 1.11.2 on Erlang/OTP 23 [erts-11.1.5]
- ExAws 2.1.6
- hackney 1.17.0
Current behavior
I noticed this when running the ex_aws_s3 test suite which now fails at https://github.com/ex-aws/ex_aws_s3/blob/2482ca3827f67ff50f238e07a567b7b769833c7f/test/lib/s3_test.exs#L371
Prior to PR#660 the path was being uri_encoded, which meant that query string components were being added to the path. Now that the path no longer is encoded, it is possible to get URLs with two query strings due to:
path <> "?" <> uri.query
followed by
"#{uri.scheme}://#{uri.authority}#{path}?#{query_for_url}&X-Amz-Signature=#{signature}"
I'm not entirely sure what the correct solution ought to be here: should a query string passed in with the URL be added to the path as a URI-encoded suffix, as the test in ex_aws_3 expects? Or should it be added to the query string along-side the items such as X-Amz-Signature?
I'm happy to do a PR for this if someone knows which is the correct approach for AWS.
My problem at the moment is that I only just took over this project and one of the main things Ben told me as he disappeared into the sunset was "by the way, the URL signing is all over the place and every time someone provides a PR to fix it, it breaks it for someone else". Looks like he wasn't joking. My plan at the moment is to actually get some decent tests up and running to exercise all this stuff so that whatever changes get made in the future can be done so with some confidence that we're not breaking behaviour others are relying on. The framework for the test stuff is about 90% done, I just need to do the remaining 90%. Like you, right now I don't have my head around this stuff quite enough to know what's the "right" solution here, so I'm more than happy to listen to anyone else who has opinions.
Re: tests -> that's a great idea. There seem to be more tests that exercise this code in the ex_aws_s3 repo (which is how I found this issue: by running its test suite!), so maybe that could be a starting point worth looking at.
I located what appears to be the official documentation on URL signing: https://docs.aws.amazon.com/general/latest/gr/sigv4_signing.html
Not hard to see how tricky this can be to implement. It is well documented, but using natural language and with loooots of steps and details. Fair enough ....
... and to answer my original question, it appears that the original query string should be included in the final URL, and they must appear in the 'canonical' order (sorted by key, then value, with values URI encoded): https://docs.aws.amazon.com/general/latest/gr/sigv4-add-signature-to-request.html
p.s. Thanks for stepping up to help out with this set of libraries. Not a small task, but one that as a new user of these libraries I greatly appreciate. I will try to be a useful OSS citizen and push fixes and whatnot as I can ... cheers!