Enc bug fix

[amaline]

Properly encode the url before sending it to S3.

[rickheil]

Thanks @amaline for the patch! I'm testing this and have not run into any bugs yet.

[rickheil]

This definitely does fix the problem with spaces, however a similar if not identical bug seems to be happening with a test URL in my suite with an ampersand (%26) in it.

[anomalizer]

@rickheil could you please make a new pull request with a test case that demonstrates the problem? I can quickly provide the fix if you can give me the test case

[rickheil]

@anomalizer I just run it through the URLs that are in my bucket, some of which have special characters - I'll try to get it into a test in C on Monday/Tuesday.

[amaline]

Not sure how to setup, run and add to the test suite.

[wk8]

@amaline thanks for this patch, but it wasn't quite working for me (not escaping @ symbols for example).

Created another PR at https://github.com/anomalizer/ngx_aws_auth/pull/40 that I think does what we want, with tests, would you mind checking if it works for you? (same for @rickheil )

[rickheil]

Thank you, I can test tomorrow and let you know! I appreciate the patch.

Sent from my iPhone

On Jan 30, 2017, at 19:20, Jean Rougé notifications@github.com wrote:

@amaline thanks for this patch, but it wasn't quite working for me (not escaping @ symbols for example).

Created another PR at #40 that I think does what we want, with tests, would you mind checking if it works for you? (same for @rickheil )

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[rickheil]

Tested and is working with all the special chars in my URL test suite. Thank you @wk8!

[wk8]

@rickheil that's good to hear, thanks for checking :) if you don't mind sharing some or all of the URLs you used to test (or maybe just the set of special chars they contain?), happy to add them to the test suite to make sure it doesn't break again.

[rickheil]

The URLs are for an internal tool, but they mostly contain space, &, and the occasional @.

From: Jean Rougé notifications@github.com Reply-To: anomalizer/ngx_aws_auth reply@reply.github.com Date: Tuesday, January 31, 2017 at 12:34 PM To: anomalizer/ngx_aws_auth ngx_aws_auth@noreply.github.com Cc: Rick Heil rickheil@partnersandsimons.com, Mention mention@noreply.github.com Subject: Re: [anomalizer/ngx_aws_auth] Enc bug fix (#38)

@rickheil that's good to hear, thanks for checking :) if you don't mind sharing some or all of the URLs you used to test (or maybe just the set of special chars they contain?), happy to add them to the test suite to make sure it doesn't break again.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[wk8]

@rickheil thanks, added a space in the test https://github.com/anomalizer/ngx_aws_auth/pull/40/commits/d2425a78f13319009160b9ed1148ccf9b519fe7b

[anomalizer]

40 is merged. Hope this addresses this issue. Thanks again @wk8 @rickheil @amaline

[wk8]

Thanks @anomalizer !

[anomalizer]

@amaline , can you close this PR if PR-40 solves the problem?

[amaline]

OK, sorry I haven’t found the cycles to test recently.

From: Arvind Jayaprakash [mailto:notifications@github.com] Sent: Thursday, February 16, 2017 9:02 AM To: anomalizer/ngx_aws_auth Cc: Maline, Alan J.; Mention Subject: Re: [anomalizer/ngx_aws_auth] Enc bug fix (#38)

@amalinehttps://github.com/amaline , can you close this PR if PR-40 solves the problem?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/anomalizer/ngx_aws_auth/pull/38#issuecomment-280337686, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AFOWUWWMwac1Z5N3szyRsWE-G6-zHvwMks5rdFbKgaJpZM4LoxZM.

[anomalizer]

Closing this as it has been superseded by #40