URL encode non-alpha querystring parameters

[varju]

Fixes #138

[okigan]

@varju seems github added manual approval before commit checks are run for new commuters -- I've approved that for you.

Thanks for PR -- have a look at 3.6 build above (and I think mac builds will run once that is resolved).

[varju]

Thanks for PR -- have a look at 3.6 build above

Darn. I included examples in that test of all the character classes that AWS says should not be URL encoded. It looks like Python 3.6's URL encoder is encoding the ~ character.

Given that 3.6 hits end of life at the end of this month, is there any chance you'd consider dropping support for it? The alternative would be hand-rolling a URL encoder similar to what the python-aws-sig project did, and that feels a bit heavy.

[okigan]

Hmm, interesting - I am not opposed to that (weird python lib is making this kinds of changes). Let me relook at currently tested versions, then this can rebased onto that. Thanks!

…

On Dec 4, 2021, at 10:44 AM, Alex Varju ***@***.***> wrote:  Thanks for PR -- have a look at 3.6 build above Darn. I included examples in that test of all the character classes that AWS says should not be URL encoded. It looks like Python 3.6's URL encoder is encoding the ~ character. Given that 3.6 hits end of life at the end of this month, is there any chance you'd consider dropping support for it? The alternative would be hand-rolling a URL encoder similar to what the python-aws-sig project did, and that feels a bit heavy. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[varju]

Just reading some docs some more, I think the encoding change happened in 3.7. The urllib.parse.quote docs say:

Changed in version 3.7: Moved from RFC 2396 to RFC 3986 for quoting URL strings. “~” is now included in the set of unreserved characters.

That said, I think we can implement AWS-compatible encoding in a way that doesn't require quite as much code as that link I shared earlier, and with support for Python 3.6.

What do you think of the latest push?

[okigan]

All checks passed - nice!, I’ll re-review when I get home later today.

[okigan]

looks good to me

[okigan]

@varju please confirm all is resolved. Thanks

[varju]

Yes, everything is working great now! Thanks for the quick response.

[kibiz0r]

What if the caller has already percent-encoded the query string?

(Spoiler: Our scripts are doing just that.)

In our case, we can simply stop doing the percent-encoding on our end, which is actually a win for us. But just FYI, this might introduce a headache for people that don't have that option. Browsers seem to only percent-encode pasted URLs "if necessary". Might be good to take a similar approach here.