#221 uri encoding fixes

[castaway]

This mostly involves ensuring we use the newly released version of URI::Template, which means we do not double-encode escaped characters. Also added - URI encoding calls die() if it encounters any of AWS's "avoid" characters.

[pplu]

Before merging this for release: I understand that upgrading to 0.39 (with this fix applied) will make objects uploaded to S3 with older versions not findable (since they were double escaped, and now arent).

I'm thinking we should put a very explicit notice in the Changelog so that people know of this situation, and a bit of guidance of what scenarios will be of problems to them.

[castaway]

Very good point.

It will be possible to find them, but only by using the explicit current name of the object. Eg old name:
Fred Bloggs - stored as Fred%20Bloggs
to find using new release, lookup using:
Fred%20Bloggs

How about something like:

BREAKING CHANGE: Versions of Paws pre 0.39 would incorrectly store object names URI encoded, eg " " is stored as "%20". To retrieve these objects using version 0.39 and above, explicitly ask for the URI encoded name.

?

Might be best to add a line in the POD as well.

[castaway]

Hmm.. testing this.. hang on - new code is not yet in release/0.39, right? - oops, I need to downgrade URI::Template for test..

[castaway]

Ah I'm wrong - we can't fetch the pre-0.39 objects, as now "%" is disallowed as a Key character, and the code dies().

Would it be worth adding a workaround, maybe an environment variable that could be set, to turn off that dying? I've just tested and it seems to work fine, fetching "Fred%20Bloggs" if we remove the code for disallowing % in keys..

[pplu]

S3 was declared unstable, so I wouldn't go through great lengths to re-support broken behavior. If someone depends on it, they should "hack" their way back into the broken state (delete the die?, downgrade URI::Template?). We can inform appropiately as to what has changed, so they can be aware of the situation.

[castaway]

@shadowcat-mst wanted to have a think about it. It occurs to me that people can also use the AWS web front-end to rename the broken ones, I think. At the moment I'm thinking something like:

BREAKING CHANGE:Versions of Paws pre 0.39 would incorrectly store object names URI encoded, eg " " is stored as "%20". These will not be retrievable using Paws 0.39+.

[dheffx]

Left a Data::Dumper print in

[byterock]

I think this has been fixed long ago as 42 does not show the behaviour.

If you are in doupt try this branch all the S3 commands are working there

https://github.com/byterock/aws-sdk-perl/tree/s3ObjectTagging

[dheffx]

I guess I was mistaken by this pull request still being open if it is supposedly fixed in 0.42 release? I don't know.
I cloned both the pplu/aws-sdk-perl branch of release/0.42 and your fork's s3ObjectTagging branch and I still run into URI encoding issues, namely related to the = symbol in S3 object keys. I guess I should open a separate ticket/pull request for that.

i.e. Key = client=TEST/year=2020/month=01/day=02/test.txt'

[byterock]

That would be good

you would help me alot if you ran it against the s3ObjectTagging branch as well.
Though I do not think I have touched the 'Paws::Net::Caller' class in my branch.

Any of the S3 commands that run though the RestXml callers and responders have been fixed up.

Hopefully there will be a relase that will get things a little more stable.

[byterock]

Ok so a doc or bucket key like this

TEST/year=2020/month=01/day=02/test.txt

well have a look and see what happens

not a very good name for a bucket ;) but that is not my call

[dheffx]

This is the recommended way to prefix object keys in order to get out of the box Hive-partitioning in S3.
https://docs.aws.amazon.com/glue/latest/dg/aws-glue-programming-etl-partitions.html

[dheffx]

Adding the = character fixes a double encoding bug we are encountering. Would there be any perceived harm of including it in the regex, or making the regex configurable or overridable? Would a separate ticket be necessary since it is related to URI encoding?

I can probably find some time to work on this to send a fix and tests.

i.e. Key = client=TEST/year=2020/month=01/day=02/test.txt

[castaway]

Hi, sorry, I didn't see these comments - this change hasnt been merged yet, however we're working on it. The issue should be fixed in #265 , there's a test in t/s3/uri_encoding.t that checks = is encoded . Are you saying it shouldn't be? Its on the list here: https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html

See "Characters that might require special handling" section. (Though the "might" makes for great documentation.. either we do or we don't.. !)