Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Skip the ETag header check in responce while using SSE-C encrpytion of S3 #2368

Merged
merged 2 commits into from
Jul 22, 2014

Conversation

KennF
Copy link

@KennF KennF commented Jul 7, 2014

AWS S3 provides the new feature - server side encrpytion with customer keys. I can add three headers - "x-amz-server-side​-encryption​-customer-algorithm,x-amz-server-side​-encryption​-customer-key,x-amz-server-side​-encryption​-customer-key-MD5" to use the Key.set_contents_from_filename and upload the file. However, the ETag check in response stops the operation.

According to the AWS document, this header takes no effect while using the new SSE-C encryption. So this fix is to skip this check. "x-amz-server-side​-encryption​-customer-algorithm" is one header in the response, which I use to determine if it is SSE-C encryption.

@rectalogic
Copy link
Contributor

+1

@mdellavo
Copy link

mdellavo commented Jul 7, 2014

Can't wait to use this

@khomenko
Copy link

khomenko commented Jul 8, 2014

+1 - this would make our lives much easier

@KennF
Copy link
Author

KennF commented Jul 11, 2014

Does any one know how to accelerate the merge of this pull request? Any suggestion? Thanks!

raise provider.storage_data_error(
'ETag from S3 did not match computed MD5. '
'%s vs. %s' % (self.etag, self.md5))
# If you use customer-provided encryption keys, the ETag value that Amazon S3 returns in the response will not be the MD5 of the object.
Copy link
Member

Choose a reason for hiding this comment

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

Line length (PEP8, should be 80 chars max)

@danielgtaylor danielgtaylor self-assigned this Jul 11, 2014
@danielgtaylor
Copy link
Member

Thanks for this pull request. It looks great so far, but would need to have at least a unit test added to cover this new functionality. If someone can add a test then I'll merge it in. 👍

@KennF
Copy link
Author

KennF commented Jul 14, 2014

@danielgtaylor I've added one test for this pull request and fixed the style issue. Would you please have a look and merge it if it is Ok. Thanks!

@KennF
Copy link
Author

KennF commented Jul 22, 2014

Can any one give me the guidance to do anything else to have this pull request merged? Thanks!

@danielgtaylor
Copy link
Member

Thanks for adding the test! 👍

danielgtaylor added a commit that referenced this pull request Jul 22, 2014
Skip the ETag header check in responce while using SSE-C encrpytion of S3. Fixes #2368.
@danielgtaylor danielgtaylor merged commit 907fc6d into boto:develop Jul 22, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants