Skip to content

Fix(s3)/1560 fix copy object#1670

Open
kapoorp99 wants to merge 6 commits into
floci-io:mainfrom
kapoorp99:fix(s3)/1560-fix-copy-object
Open

Fix(s3)/1560 fix copy object#1670
kapoorp99 wants to merge 6 commits into
floci-io:mainfrom
kapoorp99:fix(s3)/1560-fix-copy-object

Conversation

@kapoorp99

Copy link
Copy Markdown
Contributor

Summary

Fixes the issue where x-amz-checksum-algorithm overrides were ignored during CopyObject and multipart upload completion.

Previously, CopyObject would unconditionally carry over the source object's checksum, and completing a multipart upload would default to computing a CRC64NVME checksum, ignoring the explicit algorithm requested during initialization.
This PR captures the x-amz-checksum-algorithm header during multipart initiation and CopyObject operations, and ensures the specified algorithm is used to re-calculate the final checksum when saving the object.

Closes #1560

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

AWS Compatibility

When uploading a standard or multipart object, AWS S3 allows the user to specify an overriding x-amz-checksum-algorithm. Furthermore, a CopyObject operation allows passing x-amz-checksum-algorithm to override the copied object's default checksum type.
The incorrect behavior in Floci was that it was defaulting to CRC64NVME for multipart uploads regardless of the requested algorithm, and completely ignoring the algorithm override header during CopyObject (preserving the source object's checksum instead of recalculating it with the new algorithm).

Checklist

  • ./mvnw test passes locally
  • New or updated integration test added
  • Commit messages follow Conventional Commits

@greptile-apps

greptile-apps Bot commented Jun 30, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes two bugs: CopyObject now respects the x-amz-checksum-algorithm request header instead of blindly copying the source object's checksum, and CompleteMultipartUpload now uses the algorithm captured during InitiateMultipartUpload instead of defaulting to CRC64NVME.

  • Adds a getChecksumAlgorithm() helper that checks x-amz-checksum-algorithm first and falls back to x-amz-sdk-checksum-algorithm, wiring it through CopyObject, PutObject, InitiateMultipartUpload, and UploadPart.
  • Stores checksumAlgorithm on MultipartUpload at initiation, propagates it into buildChecksum for individual parts and for the final CompleteMultipartUpload call.
  • For CopyObject with an algorithm override, correctly nulls out both the copied checksum and the parts list before calling storeObject, ensuring the result gets a FULL_OBJECT checksum rather than COMPOSITE.

Confidence Score: 5/5

Safe to merge — the changes are narrowly scoped to checksum algorithm propagation and do not touch any auth, storage, or data-integrity paths beyond what is intended.

The core logic is correct: CopyObject now nulls out both the source checksum and the source parts list before delegating to storeObject, so the FULL_OBJECT type is computed from scratch with the requested algorithm. The multipart path stores the algorithm at initiation and threads it through UploadPart and CompleteMultipartUpload. The validateAndNormalizeChecksumAlgorithm guard rejects unknown algorithms with a 400 before they can silently fall through to the CRC64NVME default. New integration tests confirm both the algorithm-override and the no-override-preserves-source-checksum cases.

No files require special attention. One gap worth noting for future work: there is no integration test that copies a multipart-uploaded source object with an x-amz-checksum-algorithm override. The production code handles it correctly — the parts=null guard is in place — but an explicit test would lock this in.

Important Files Changed

Filename Overview
src/main/java/io/github/hectorvent/floci/services/s3/S3Service.java Core logic changes: new validateAndNormalizeChecksumAlgorithm(), algorithm propagation through multipart and copy paths, correct nulling of source parts/checksum when overriding algorithm in CopyObject.
src/main/java/io/github/hectorvent/floci/services/s3/S3Controller.java New getChecksumAlgorithm() helper consolidates x-amz-checksum-algorithm / x-amz-sdk-checksum-algorithm lookup, wired into PutObject, InitiateMultipartUpload, UploadPart, and CopyObject.
src/main/java/io/github/hectorvent/floci/services/s3/model/MultipartUpload.java Added checksumAlgorithm field with getter/setter to carry the initiation-time algorithm through the multipart upload lifecycle.
src/main/java/io/github/hectorvent/floci/services/s3/model/CopyObjectOptions.java Added checksumAlgorithm field with getter/withChecksumAlgorithm() fluent setter for passing the override algorithm into the copy path.
src/test/java/io/github/hectorvent/floci/services/s3/S3IntegrationTest.java Extended copyObject test to verify SHA256 override; added copyObjectWithoutChecksumOverride and copyObjectPreservesNonDefaultChecksum tests; renumbered all @order annotations to be consecutive.
src/test/java/io/github/hectorvent/floci/services/s3/S3MultipartIntegrationTest.java Updated multipart test to initiate with x-amz-checksum-algorithm: SHA256 and assert the completed object carries ChecksumSHA256 instead of the former CRC64NVME default.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client
    participant S3Controller
    participant S3Service

    Note over Client,S3Service: CopyObject with x-amz-checksum-algorithm override
    Client->>S3Controller: "PUT /{dest} + x-amz-checksum-algorithm: SHA256"
    S3Controller->>S3Controller: getChecksumAlgorithm() → "SHA256"
    S3Controller->>S3Service: copyObject(src, dest, options.withChecksumAlgorithm("SHA256"))
    S3Service->>S3Service: validateAndNormalizeChecksumAlgorithm("SHA256") → "SHA256"
    S3Service->>S3Service: "effectiveChecksum = null"
    S3Service->>S3Service: "parts = null"
    S3Service->>S3Service: buildChecksum(data, null, false, "SHA256") → FULL_OBJECT SHA256
    S3Service-->>Client: 200 OK

    Note over Client,S3Service: Multipart Upload with algorithm override
    Client->>S3Controller: POST /?uploads + x-amz-checksum-algorithm: SHA256
    S3Controller->>S3Service: "initiateMultipartUpload(..., checksumAlgorithm="SHA256")"
    S3Service->>S3Service: upload.setChecksumAlgorithm("SHA256")
    S3Service-->>Client: uploadId

    Client->>S3Controller: "PUT /?partNumber=1&uploadId=X"
    S3Controller->>S3Service: uploadPart(...)
    S3Service->>S3Service: buildChecksum(partData, [part], true, upload.getChecksumAlgorithm())
    S3Service-->>Client: ETag

    Client->>S3Controller: "POST /?uploadId=X (complete)"
    S3Controller->>S3Service: completeMultipartUpload(...)
    S3Service->>S3Service: buildChecksum(allData, parts, true, upload.getChecksumAlgorithm())
    S3Service-->>Client: 200 OK
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Client
    participant S3Controller
    participant S3Service

    Note over Client,S3Service: CopyObject with x-amz-checksum-algorithm override
    Client->>S3Controller: "PUT /{dest} + x-amz-checksum-algorithm: SHA256"
    S3Controller->>S3Controller: getChecksumAlgorithm() → "SHA256"
    S3Controller->>S3Service: copyObject(src, dest, options.withChecksumAlgorithm("SHA256"))
    S3Service->>S3Service: validateAndNormalizeChecksumAlgorithm("SHA256") → "SHA256"
    S3Service->>S3Service: "effectiveChecksum = null"
    S3Service->>S3Service: "parts = null"
    S3Service->>S3Service: buildChecksum(data, null, false, "SHA256") → FULL_OBJECT SHA256
    S3Service-->>Client: 200 OK

    Note over Client,S3Service: Multipart Upload with algorithm override
    Client->>S3Controller: POST /?uploads + x-amz-checksum-algorithm: SHA256
    S3Controller->>S3Service: "initiateMultipartUpload(..., checksumAlgorithm="SHA256")"
    S3Service->>S3Service: upload.setChecksumAlgorithm("SHA256")
    S3Service-->>Client: uploadId

    Client->>S3Controller: "PUT /?partNumber=1&uploadId=X"
    S3Controller->>S3Service: uploadPart(...)
    S3Service->>S3Service: buildChecksum(partData, [part], true, upload.getChecksumAlgorithm())
    S3Service-->>Client: ETag

    Client->>S3Controller: "POST /?uploadId=X (complete)"
    S3Controller->>S3Service: completeMultipartUpload(...)
    S3Service->>S3Service: buildChecksum(allData, parts, true, upload.getChecksumAlgorithm())
    S3Service-->>Client: 200 OK
Loading

Reviews (4): Last reviewed commit: "fix(s3): handle integration tests" | Re-trigger Greptile

Comment thread src/main/java/io/github/hectorvent/floci/services/s3/S3Service.java
@hectorvent hectorvent added bug Something isn't working s3 Amazon Simple Storage Service (S3) labels Jul 1, 2026
Copilot AI review requested due to automatic review settings July 1, 2026 14:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes S3 checksum-algorithm handling so x-amz-checksum-algorithm overrides are respected for CopyObject and multipart upload completion, aligning Floci behavior more closely with AWS S3 checksum semantics (issue #1560).

Changes:

  • Capture x-amz-checksum-algorithm during multipart initiation and persist it on the MultipartUpload so part/final checksums can be computed using the requested algorithm.
  • Plumb checksum-algorithm through CopyObject options and trigger checksum recalculation when an override is provided.
  • Update S3 integration tests to assert SHA256 checksums are returned for these flows.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/main/java/io/github/hectorvent/floci/services/s3/S3Controller.java Wires request headers into multipart initiation and CopyObject options for checksum-algorithm overrides.
src/main/java/io/github/hectorvent/floci/services/s3/S3Service.java Stores multipart checksum algorithm and uses it when computing part/final checksums; recalculates checksum on CopyObject override.
src/main/java/io/github/hectorvent/floci/services/s3/model/MultipartUpload.java Adds persisted checksumAlgorithm field for multipart uploads.
src/main/java/io/github/hectorvent/floci/services/s3/model/CopyObjectOptions.java Adds checksumAlgorithm option for CopyObject.
src/test/java/io/github/hectorvent/floci/services/s3/S3MultipartIntegrationTest.java Initiates multipart with checksum algorithm and expects ChecksumSHA256 in object attributes.
src/test/java/io/github/hectorvent/floci/services/s3/S3IntegrationTest.java Adds checksum override to CopyObject test and validates checksum presence via headers and GetObjectAttributes.

Comment thread src/main/java/io/github/hectorvent/floci/services/s3/S3Controller.java Outdated
Comment thread src/main/java/io/github/hectorvent/floci/services/s3/S3Controller.java Outdated
Comment on lines +1236 to +1237
upload.setAcl(acl);
upload.setChecksumAlgorithm(checksumAlgorithm);
Comment on lines +2443 to +2447
S3Checksum effectiveChecksum = source.getChecksum();
String copyChecksumAlgorithm = effectiveOptions.getChecksumAlgorithm();
if (copyChecksumAlgorithm != null) {
effectiveChecksum = null;
}
.header("x-amz-metadata-directive", "REPLACE")
.header("x-amz-meta-owner", "team-b")
.header("x-amz-storage-class", "GLACIER")
.header("x-amz-checksum-algorithm", "SHA256")
.header("x-amz-storage-class", equalTo("GLACIER"))
.header("x-amz-checksum-sha256", notNullValue())
.body(equalTo("Hello World from S3!"));

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comment thread src/main/java/io/github/hectorvent/floci/services/s3/S3Service.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working s3 Amazon Simple Storage Service (S3)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] S3: CopyObject does not preserve checksum attributes from source object

3 participants