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

Don't use MD5 with s3v4 #804

Merged
merged 1 commit into from
Feb 24, 2016
Merged

Conversation

JordonPhillips
Copy link
Contributor

We have a function which avoids md5 when in sigv4, but forgot to include s3v4 even though we attach the function to several s3 commands.

cc @jamesls @kyleknap @rayluo

@@ -150,7 +150,7 @@ def _calculate_md5_from_file(fileobj):
def conditionally_calculate_md5(params, **kwargs):
"""Only add a Content-MD5 when not using sigv4"""
signer = kwargs['request_signer']
if signer.signature_version != 'v4':
if signer.signature_version != 'v4' and signer.signature_version != 's3v4':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about if signer.signature_version not in ['v4, 's3v4']?

@jamesls
Copy link
Member

jamesls commented Feb 23, 2016

:shipit:

@rayluo
Copy link
Contributor

rayluo commented Feb 23, 2016

🚢

@jamesls jamesls added the pr/ready-to-merge This PR is ready to be merged. label Feb 24, 2016
JordonPhillips added a commit that referenced this pull request Feb 24, 2016
@JordonPhillips JordonPhillips merged commit 2847bcd into boto:develop Feb 24, 2016
@JordonPhillips JordonPhillips deleted the s3-md5 branch March 21, 2016 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready-to-merge This PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants