-
Notifications
You must be signed in to change notification settings - Fork 354
Fix: multipart upload #1529
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
base: master
Are you sure you want to change the base?
Fix: multipart upload #1529
Conversation
|
Mint PR pulling from here - minio/mint@2b140ce - resolves the problem for the |
|
@balamurugana Even if you remove this line from the test - You can try also with If you have a strong preference, I can change to this: headers.extend({k: v for k, v in checksum_headers.items()
if k == "x-amz-sdk-checksum-algorithm"})Fine either way. |
|
I also tested with local |
|
The bug is <?xml version="1.0"?>
<CompleteMultipartUpload xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
<Part>
<ChecksumCRC32C>GUlcdQ==</ChecksumCRC32C>
<PartNumber>1</PartNumber>
<ETag>6cbee1d380620e11b4f8002ec1990529</ETag>
</Part>
<Part>
<ChecksumCRC32C>dIGSKw==</ChecksumCRC32C>
<PartNumber>2</PartNumber>
<ETag>261f9336675060e97255ef9f0b386aba</ETag>
</Part>
</CompleteMultipartUpload> |
That's just a different approach/fix to the same problem. |
|
Indeed, for community edition, it works, but not for EOS. |
|
Regardless, the bug in |
|
@rraulinio We need to fix the bug not to remove checksum support. |
|
@balamurugana Correct. So we either have this approach that I've just commited, in which we only allow the algo header, but we don't send wrongfully the hash of the first part anymore. Or the approach you suggested, they both solve the problem (I did not test your approach, but I take it will work). |
|
Both fixes are now implemented. Everything passes, so problem solved. 💪 |
|
@balamurugana is working on a bigger PR that might supersede this one, leaving it open for now just in case. |
Relates to: minio/mint#399.
Remove checksum headers from
CreateMultipartUpload.Problem
Multipart uploads fail with
InvalidParterror:Root Cause
Incorrectly existing checksum headers (recently added) to both
CreateMultipartUploadandUploadPartrequests. This causes ETag mismatch because:Why it fails: When checksum headers are present on
CreateMultipartUpload, the S3 server establishes expectations for how the final object's ETag should be calculated. When the same checksum headers are then sent with eachUploadPart, the server calculates part ETags differently than expected from the initial multipart setup. DuringCompleteMultipartUpload, the server validates that uploaded part ETags match what was declared, but they don't match due to the conflicting checksum information, resulting inInvalidParterror.Per S3 spec: Checksum headers should only be on individual
UploadPartrequests (where actual data is uploaded), not onCreateMultipartUpload(which only initializes the upload with metadata like storage class and encryption).Solution
Remove
headers.extend(checksum_headers)beforeCreateMultipartUploadcall. Checksum headers should only be sent withUploadPartrequests.LE: Solution extended to filtering the headers in
CreateMultipartUpload. On top of that, adding the missing checksum fields inCompleteMultipartUpload.