Skip to content

encode_uri/1 should html encode with uppercase to handle special chars such as +#74

Merged
onno-vos-dev merged 1 commit intoaws-beam:masterfrom
onno-vos-dev:encoding-should-be-uppercase-as-base16-does-so
Feb 14, 2022
Merged

encode_uri/1 should html encode with uppercase to handle special chars such as +#74
onno-vos-dev merged 1 commit intoaws-beam:masterfrom
onno-vos-dev:encoding-should-be-uppercase-as-base16-does-so

Conversation

@onno-vos-dev
Copy link
Copy Markdown
Member

@onno-vos-dev onno-vos-dev commented Feb 13, 2022

Description:

encode_uri/1 should html encode with uppercase to handle special chars such as +

  • Preserve old behavior for aws_util:base16/1

This is essentially the same code as can be seen in aws_signature: aws_signature_utils.erl#L44

Certain special characters such as + would still lead to SignatureDoesNotMatch errors even after #73 since the signature would uppercase encoded characters whereas the encoded uri would not. Leading to SignatureDoesNotMatch errors since the signature would be signed with an uppercase html encoded char whereas the url that would be called would have the lowercased version.

1> s3_wrapper:write(Bucket, <<"file(+1)">>, <<>>).
ok
2> s3_wrapper:read(Bucket, <<"file(+1)">>).
{ok,<<>>}
3> s3_wrapper:list_objects(Bucket, <<"file">>).
[<<"file(+1)">>]
4> s3_wrapper:delete(Bucket, <<"file(+1)">>).
ok
5> s3_wrapper:write(Bucket, <<"file%#&(+1)">>, <<>>).
ok
6> s3_wrapper:read(Bucket, <<"file%#&(+1)">>).
{ok,<<>>}
7> s3_wrapper:delete(Bucket, <<"file%#&(+1)">>).
ok

Added a unit test with the most screwed up filename possible:

1> s3_wrapper:write(Bucket, <<"file_!-_.(*)&=;:+ ,?{^}%]>[~<#`|.content">>, <<"content">>).
ok
2> s3_wrapper:read(Bucket, <<"file_!-_.(*)&=;:+ ,?{^}%]>[~<#`|.content">>).
{ok,<<"content">>}
3> s3_wrapper:list_objects(Bucket, <<"file">>).
[<<"file_!-_.(*)&=;:+ ,?{^}%]>[~<#`|.content">>]
4> s3_wrapper:delete(Bucket, <<"file_!-_.(*)&=;:+ ,?{^}%]>[~<#`|.content">>).
ok

@onno-vos-dev
Copy link
Copy Markdown
Member Author

@philss Sorry for the mess but it's really annoying to have all tests towards S3Mock from Adobe pass whereas things end up exploding when moving to my staging environment 😢 Hence, I tested this in staging towards AWS itself and now every bit of weirdness seems to be resolved. 🤞

…s such as +

- Preserve old behavior for aws_util:base16/1
@danfilip
Copy link
Copy Markdown

here's the link to the object key names documentation: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html

@onno-vos-dev onno-vos-dev requested a review from philss February 14, 2022 07:55
@onno-vos-dev onno-vos-dev merged commit 9df0986 into aws-beam:master Feb 14, 2022
@philss
Copy link
Copy Markdown
Contributor

philss commented Feb 14, 2022

@onno-vos-dev no problem, and thanks for the PR! 💜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants