From 932f1c9da779a1504060548ab010d2221a8d3a9a Mon Sep 17 00:00:00 2001 From: niksis02 Date: Mon, 13 Oct 2025 20:39:57 +0400 Subject: [PATCH] fix: sets crc64nvme as defualt checksum for complete mp action Fixes #1547 When no checksum is specified during multipart upload initialization, the complete multipart upload request should default to **CRC64NVME FULL_OBJECT**. The checksum will not be stored in the final object metadata, as it is used solely for data integrity verification. Note that although CRC64NVME is composable, it is calculated using the standard hash reader, since the part checksums are missing and the final checksum calculation is instead based directly on the parts data. --- backend/posix/posix.go | 29 +++++++++++++++++++++++++---- tests/integration/group-tests.go | 2 +- tests/integration/tests.go | 15 +++++++-------- tests/integration/utils.go | 2 +- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 3d4765e..f7c0b37 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1470,7 +1470,11 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return res, "", fmt.Errorf("get mp checksums: %w", err) } - var checksumAlgorithm types.ChecksumAlgorithm + + // The checksum algorithm should default to CRC64NVME + // just for data integrity. It isn't going to be saved + // in the final object metadata + checksumAlgorithm := types.ChecksumAlgorithmCrc64nvme if checksums.Algorithm != "" { checksumAlgorithm = checksums.Algorithm } @@ -1485,6 +1489,15 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C return res, "", s3err.GetChecksumTypeMismatchOnMpErr(checksumType) } + // The checksum type should default to FULL_OBJECT(crc64nvme) + checksumType := checksums.Type + if checksums.Type == "" { + // do not modify checksums.Type to further not save the checksum + // in the final object. As if no checksum has been specified on mp + // creation, the final object shouldn't contain the checksum metadata + checksumType = types.ChecksumTypeFullObject + } + // check all parts ok last := len(parts) - 1 var totalsize int64 @@ -1551,7 +1564,7 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C var hashRdr *utils.HashReader var compositeChecksumRdr *utils.CompositeChecksumReader - switch checksums.Type { + switch checksumType { case types.ChecksumTypeFullObject: if !composableCRC { hashRdr, err = utils.NewHashReader(nil, "", utils.HashType(strings.ToLower(string(checksumAlgorithm)))) @@ -1591,7 +1604,7 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C } var rdr io.Reader = pf - switch checksums.Type { + switch checksumType { case types.ChecksumTypeFullObject: if composableCRC { if i == 0 { @@ -1778,6 +1791,14 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C if err != nil { return res, "", fmt.Errorf("store object checksum: %w", err) } + } else { + // in this case no checksum has been specified on mp creation + // and the complete request checksum is defaulted to crc64nvem + // simply calculated the sum to further retrun in the response + if hashRdr != nil { + sum := hashRdr.Sum() + crc64nvme = &sum + } } // load and set retention @@ -1820,7 +1841,7 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C ChecksumSHA1: sha1, ChecksumSHA256: sha256, ChecksumCRC64NVME: crc64nvme, - ChecksumType: &checksums.Type, + ChecksumType: &checksumType, }, versionID, nil } diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 988d9c2..afde517 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -217,7 +217,7 @@ func TestGetObject(ts *TestState) { ts.Run(GetObject_zero_len_with_range) ts.Run(GetObject_dir_with_range) ts.Run(GetObject_invalid_parent) - ts.Run(GetObject_large_object) + ts.Sync(GetObject_large_object) ts.Run(GetObject_conditional_reads) //TODO: remove the condition after implementing checksums in azure if !ts.conf.azureTests { diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 0180f40..2054375 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -12970,7 +12970,7 @@ func CompleteMultipartUpload_should_ignore_the_final_checksum(s *S3Conf) error { MultipartUpload: &types.CompletedMultipartUpload{ Parts: cParts, }, - ChecksumSHA1: getPtr("Kq5sNclPz7QV2+lfQIuc6R7oRu0="), // should ignore this + ChecksumCRC64NVME: getPtr("vqf3hRLTlJw="), // should ignore this }) cancel() if err != nil { @@ -12993,9 +12993,10 @@ func CompleteMultipartUpload_should_ignore_the_final_checksum(s *S3Conf) error { return fmt.Errorf("expected nil sha256 checksum, insted got %v", *res.ChecksumSHA256) } - if res.ChecksumCRC64NVME != nil { - return fmt.Errorf("expected nil crc64nvme checksum, insted got %v", - *res.ChecksumSHA256) + // If no checksum is specified on mp creation, it should default + // to crc64nvme + if res.ChecksumCRC64NVME == nil { + return fmt.Errorf("expected non nil crc64nvme checksum") } return nil @@ -24659,9 +24660,8 @@ func Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object(s *S3Conf) MultipartUpload: &types.CompletedMultipartUpload{ Parts: []types.CompletedPart{ { - ETag: part.ETag, - PartNumber: part.PartNumber, - ChecksumCRC64NVME: part.ChecksumCRC64NVME, + ETag: part.ETag, + PartNumber: part.PartNumber, }, }, }, @@ -24679,7 +24679,6 @@ func Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object(s *S3Conf) Size: &dataLen, VersionId: res.VersionId, StorageClass: types.ObjectVersionStorageClassStandard, - ChecksumType: res.ChecksumType, } result := []types.ObjectVersion{version, v} diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 308f855..0010c09 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -518,7 +518,7 @@ func putObjectWithData(lgth int64, input *s3.PutObjectInput, client *s3.Client) input.Body = r } - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + ctx, cancel := context.WithTimeout(context.Background(), longTimeout) res, err := client.PutObject(ctx, input) cancel() if err != nil {