-
Notifications
You must be signed in to change notification settings - Fork 151
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
fix: correctly encode bytes for V2 signature #382
Conversation
V2 signature was passing a string to the sign_bytes function instead of bytes. This works fine for most credentials (since their sign_bytes implementations accept strings) but not for impersonated credentials. V4 signature encodes the string before calling sign_bytes, so I do the same here. We should also look into clarifying the contract for the sign_bytes interface in the auth library. Fixes googleapis#373
@@ -77,7 +77,7 @@ def get_signed_query_params_v2(credentials, expiration, string_to_sign): | |||
signed payload. | |||
""" | |||
ensure_signed_credentials(credentials) | |||
signature_bytes = credentials.sign_bytes(string_to_sign) | |||
signature_bytes = credentials.sign_bytes(string_to_sign.encode("ascii")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See https://github.com/googleapis/python-storage/blob/master/google/cloud/storage/_signing.py#L628 for what we do in V4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change LGTM.
Opnion: Issue here is that we have multiple sign versions and we focused on v4 more recently.
* fix: correctly encode bytes for V2 signature V2 signature was passing a string to the sign_bytes function instead of bytes. This works fine for most credentials (since their sign_bytes implementations accept strings) but not for impersonated credentials. V4 signature encodes the string before calling sign_bytes, so I do the same here. We should also look into clarifying the contract for the sign_bytes interface in the auth library. Fixes googleapis#373 * fix py2 failure
* fix: correctly encode bytes for V2 signature V2 signature was passing a string to the sign_bytes function instead of bytes. This works fine for most credentials (since their sign_bytes implementations accept strings) but not for impersonated credentials. V4 signature encodes the string before calling sign_bytes, so I do the same here. We should also look into clarifying the contract for the sign_bytes interface in the auth library. Fixes googleapis#373 * fix py2 failure
V2 signature was passing a string to the sign_bytes function
instead of bytes. This works fine for most credentials (since
their sign_bytes implementations accept strings) but not for
impersonated credentials. V4 signature encodes the string before
calling sign_bytes, so I do the same here.
We should also look into clarifying the contract for the
sign_bytes interface in the auth library.
Fixes #373