Skip to content

Fix: support arbitrary pre-calculated checksum values on s3#3332

Merged
Madrigal merged 3 commits intomainfrom
fix-support-unknown-checksum-s3
Feb 26, 2026
Merged

Fix: support arbitrary pre-calculated checksum values on s3#3332
Madrigal merged 3 commits intomainfrom
fix-support-unknown-checksum-s3

Conversation

@Madrigal
Copy link
Copy Markdown
Collaborator

@Madrigal Madrigal commented Feb 25, 2026

Previously, we were checking which algorithm was used before checking if there was any x-amz-checksum-... value setup on the input request. This is contrary to the expected behavior, where if someone sets x-amz-checksum-whatever, we should just use that value and pass it along to the service.

This PR moves the logic after we check for existing x-amz-checksum-... header

Copy link
Copy Markdown
Contributor

@lucix-aws lucix-aws left a comment

Choose a reason for hiding this comment

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

Do we need to also check that the pre-provided checksum matches the algorithm set? Probably not?

@Madrigal Madrigal marked this pull request as ready for review February 26, 2026 15:43
@Madrigal Madrigal requested a review from a team as a code owner February 26, 2026 15:44
@Madrigal
Copy link
Copy Markdown
Collaborator Author

Do we need to also check that the pre-provided checksum matches the algorithm set? Probably not?

I wondered the same. For a known algorithm, we could check that the algorithm matches the checksum header and do a SDK side error before sending the request. Currently we just send the values as set by the user, which I believe more closely respects what the user wants, even if that's incorrect.

I'll poke around, but we can always follow up on this. Let me know if you feel strongly about it

@lucix-aws
Copy link
Copy Markdown
Contributor

consider it nonblocking

@Madrigal Madrigal merged commit 51e7bd8 into main Feb 26, 2026
21 checks passed
@Madrigal Madrigal deleted the fix-support-unknown-checksum-s3 branch February 26, 2026 16:15
@Madrigal
Copy link
Copy Markdown
Collaborator Author

For closure, discussed internally with Java, we agree that we'll be sending both values and not doing validation on the SDK

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.

2 participants