Skip to content

S3: fix Checksum behavior for GetObject with PartNumber #12842

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

Merged
merged 1 commit into from
Jul 8, 2025

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Jul 7, 2025

Motivation

As reported by #12840, we did not return the right Checksum when calling GetObject with PartNumber when the multipart upload was created with a COMPOSITE checksum type. It should have returned the part checksum. But if the objects were made of several parts, it would have returned no checksum at all. If the object only had one part, it would have returned the object checksum, which was wrong.

This PR fixes the behavior. As a side-note, it seems S3 has quite special cases around checksums, and since they implemented COMPOSITE & FULL_OBJECT, the codebase became quite a bit more complex 😕

Changes

  • if a Multipart Upload is created with ChecksumType=COMPOSITE, properly return the part checksum when requested with GetObject and HeadObject
  • add a few tests covering multiple cases:
    • when the multipart upload has only one part, both with COMPOSITE and FULL_OBJECT
      • COMPOSITE returns the part checksum and not the composite checksum of the object
      • FULL_OBJECT returns the part checksum which is equal to the object checksum
    • when the multipart upload has multiple parts, both with COMPOSITE and FULL_OBJECT
      • COMPOSITE returns the part checksum
      • FULL_OBJECT does not return the part checksum, even though the list_parts call earlier showed the part had a checksum

fixes #12840

@bentsku bentsku added this to the 4.7 milestone Jul 7, 2025
@bentsku bentsku self-assigned this Jul 7, 2025
@bentsku bentsku added aws:s3 Amazon Simple Storage Service semver: patch Non-breaking changes which can be included in patch releases labels Jul 7, 2025
Copy link

github-actions bot commented Jul 7, 2025

S3 Image Test Results (AMD64 / ARM64)

    2 files  ±0    2 suites  ±0   8m 49s ⏱️ -1s
  514 tests +2  464 ✅ +2   50 💤 ±0  0 ❌ ±0 
1 028 runs  +4  928 ✅ +4  100 💤 ±0  0 ❌ ±0 

Results for commit 548a460. ± Comparison against base commit 655d4b2.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jul 7, 2025

Test Results - Preflight, Unit

21 854 tests  ±0   20 197 ✅ ±0   6m 36s ⏱️ +10s
     1 suites ±0    1 657 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 548a460. ± Comparison against base commit 655d4b2.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jul 7, 2025

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 17s ⏱️ +11s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 548a460. ± Comparison against base commit 655d4b2.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jul 7, 2025

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   1h 26m 21s ⏱️
2 278 tests 1 663 ✅ 615 💤 0 ❌
2 284 runs  1 663 ✅ 621 💤 0 ❌

Results for commit 548a460.

♻️ This comment has been updated with latest results.

@bentsku bentsku requested a review from k-a-il July 8, 2025 10:45
@bentsku bentsku marked this pull request as ready for review July 8, 2025 10:45
@bentsku bentsku force-pushed the fix-s3-get-obj-part-checksum branch from 100014e to 548a460 Compare July 8, 2025 10:46
Copy link

github-actions bot commented Jul 8, 2025

LocalStack Community integration with Pro

    2 files      2 suites   1h 3m 32s ⏱️
2 254 tests 1 636 ✅ 618 💤 0 ❌
2 256 runs  1 636 ✅ 620 💤 0 ❌

Results for commit 548a460.

Copy link
Contributor

@k-a-il k-a-il left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Interesting use-case 🤔

@bentsku bentsku merged commit e2a8b9e into master Jul 8, 2025
46 checks passed
@bentsku bentsku deleted the fix-s3-get-obj-part-checksum branch July 8, 2025 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:s3 Amazon Simple Storage Service semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: S3 GetObjectCommand with part (from multipart-part upload) returns metadata of object
2 participants