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

Added x-amz-checksum-sha256 header for Object Lock support #1393

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AlexisPPLIN
Copy link

This is a follow-up of PR #1353 which is inactive since Dec 5, 2023

As asked I moved the sha256 digest to base64 string into a separate function into S3/Crypto.py

Fixes #1177

@fviard
Copy link
Contributor

fviard commented Sep 10, 2024

Sorry for the long delay of my reply.
As you can see with the failed tests, you forgot the import for base64 in crypto.
In fact, as it is done for encodestring, you can directly import the function that you want to use.

@AlexisPPLIN
Copy link
Author

Hi !
No worries for the delay

I fixed my import like you advised me to.
I also rebased my PR on master.

# Extract digest from sha256 hash
sha256_hash_digest = sha256_hash.digest()
# Then convert it to base64
sha256_hash_digest_b64 = b64encode(sha256_hash_digest).decode()
Copy link
Contributor

Choose a reason for hiding this comment

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

In this module encodestring (which imported above as an alias of encodebytes when available) is the function used for base64 encoding.
I think you can use:

sha256_hash_digest_b64 = encodestring(sha256_hash_digest).strip().decode()

to align with other usecases.

# Provide the checksum with the request. This is important for buckets that have
# Object Lock enabled.

headers['x-amz-checksum-sha256'] = sha256_hash_to_base64(sha256_hash)
Copy link
Contributor

@snosratiershad snosratiershad Jan 19, 2025

Choose a reason for hiding this comment

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

As someone could use s3cmd as library this line can override the header potentially comes with request argument's headers attribute. I think it's better to check if it currently has values:

if not headers.get("x-amz-checksum-sha256"):
    headers['x-amz-checksum-sha256'] = sha256_hash_to_base64(sha256_hash)

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.

Missing Content-MD5 http header for put request on object with bucket defined Lock
3 participants