Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

URL encode the uri part of the stringToSign #526

Closed
wants to merge 1 commit into from

Conversation

rafou
Copy link

@rafou rafou commented Sep 3, 2019

Fix #513

Tested locally. Now we are able to verify signature coming from Python boto with non ascii character (such a "é").

@allan-simon
Copy link
Contributor

@jamhall is this PR missing something to be merged ? (sorry to pushy :/ )

@kherock
Copy link
Collaborator

kherock commented Sep 24, 2019

@allan-simon no worries - I should also probably advertise myself as the active maintainer of this project somewhere more visible.

Anyway, apologies for the short attention span on this project lately. I'd like to see a test case on this before merging. I'll try to spend some time today adding one in addition to addressing the other open PRs.

@allan-simon
Copy link
Contributor

@rafou should be able to get one, maybe not as a "code test" but may be as a "try this at home" test ? (would it be enough for you ?)

@allan-simon
Copy link
Contributor

@kherock up , if we give you repeatable step to reproduce the issue, would it be enough for you to merge this PR ? as we're hitting once again a wall , if we can do anything to help you , let us know :)

@kherock kherock self-assigned this Jun 2, 2020
@kherock
Copy link
Collaborator

kherock commented Jun 2, 2020

@rafou sorry for the unnecessarily long delay on such a simple fix. I'll be including this in the next release which will should be out sometime this week!

@kherock
Copy link
Collaborator

kherock commented Jun 7, 2020

v2 signatures actually had much bigger problems which I've addressed in #630. For this particular issue, AWS actually uses RFC 3986's reserved char list, so encodeURIComponent isn't enough by itself.

@kherock kherock closed this in #630 Jun 7, 2020
@rafou
Copy link
Author

rafou commented Jun 8, 2020

Thanks @kherock for the explanation and for taking a look at the problem !

Cheers

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.

SignatureDoesNotMatch error when signing a request containing unicode characters
3 participants