mirror of
https://github.com/pgsty/minio.git
synced 2026-08-08 23:33:30 +03:00
fix: keep the checksum of a zero length multipart object
hash.Checksum.AddPart returned before seeding the accumulator when the part was empty, so a multipart object with no content at all ended up with no checksum instead of the checksum of zero bytes. Completing such an upload failed with XAmzContentChecksumMismatch when the client supplied the correct object checksum, and stored an empty checksum when it did not. Run the type check and the first checksum seeding before the zero size early return. Appending zero bytes still leaves an existing accumulator unchanged, so only the all empty case changes: a zero length part followed by content already merged correctly, because prepending no bytes does not alter a CRC. AddPart has a single production caller, the multipart completion path, and its part checksum type is derived from the upload's own checksum type, so the type check now reached for zero sized parts cannot fire there. Add a table test over CRC32, CRC32C and CRC64NVME covering every position an empty part can take, and an API level zero length full object upload that exercises the persisted AppendTo/ReadCheckSums round trip. Co-authored-by: ChatGPT <noreply@openai.com> Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -397,4 +397,35 @@ func testAPICompleteMultipartFullObjectVariants(obj ObjectLayer, instanceType, b
|
||||
t.Fatalf("%s: expected server computed checksum %q, got %q", instanceType, want, got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("zero-length-object", func(t *testing.T) {
|
||||
// A single empty part is a legal multipart upload: only non-final parts
|
||||
// have a minimum size. The merged checksum must be the checksum of no
|
||||
// bytes, not the empty string.
|
||||
name := "variants/zero-length"
|
||||
empty := []byte{}
|
||||
uploadID := newMultipartUploadHTTP(t, apiRouter, credentials, bucketName, name,
|
||||
typ.String(), xhttp.AmzChecksumTypeFullObject)
|
||||
etags := uploadPartsHTTP(t, apiRouter, credentials, bucketName, name, uploadID, typ, [][]byte{empty})
|
||||
|
||||
rec := completeMultipartUploadHTTP(t, apiRouter, credentials, bucketName, name, uploadID, etags, nil,
|
||||
map[string]string{
|
||||
typ.Key(): mustChecksum(t, typ, empty),
|
||||
xhttp.AmzChecksumType: xhttp.AmzChecksumTypeFullObject,
|
||||
})
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("%s: want 200, got %d %s", instanceType, rec.Code, rec.Body.String())
|
||||
}
|
||||
oi, err := obj.GetObjectInfo(t.Context(), bucketName, name, ObjectOptions{})
|
||||
if err != nil {
|
||||
t.Fatalf("%s: GetObjectInfo failed: %v", instanceType, err)
|
||||
}
|
||||
if oi.Size != 0 {
|
||||
t.Fatalf("%s: expected zero length object, got %d", instanceType, oi.Size)
|
||||
}
|
||||
cs, _ := oi.decryptChecksums(0, nil)
|
||||
if got, want := cs[typ.String()], mustChecksum(t, typ, empty); got != want {
|
||||
t.Fatalf("%s: expected stored checksum %q, got %q", instanceType, want, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -35,18 +35,22 @@ func (c *Checksum) AddPart(other Checksum, size int64) error {
|
||||
if !other.Type.CanMerge() {
|
||||
return fmt.Errorf("checksum type cannot be merged")
|
||||
}
|
||||
if size == 0 {
|
||||
return nil
|
||||
}
|
||||
if !c.Type.Is(other.Type.Base()) {
|
||||
return fmt.Errorf("checksum type does not match got %s and %s", c.Type.String(), other.Type.String())
|
||||
}
|
||||
// If never set, just add first checksum.
|
||||
// This must happen before the zero size check below, otherwise an object
|
||||
// with no content at all never seeds the accumulator and ends up with no
|
||||
// checksum instead of the checksum of zero bytes.
|
||||
if len(c.Raw) == 0 {
|
||||
c.Raw = other.Raw
|
||||
c.Encoded = other.Encoded
|
||||
return nil
|
||||
}
|
||||
// Appending zero bytes leaves the checksum unchanged.
|
||||
if size == 0 {
|
||||
return nil
|
||||
}
|
||||
if !c.Valid() {
|
||||
return fmt.Errorf("invalid base checksum")
|
||||
}
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
// Copyright (c) 2015-2026 MinIO, Inc.
|
||||
//
|
||||
// This file is part of MinIO Object Storage stack
|
||||
//
|
||||
// This program is free software: you can redistribute it and/or modify
|
||||
// it under the terms of the GNU Affero General Public License as published by
|
||||
// the Free Software Foundation, either version 3 of the License, or
|
||||
// (at your option) any later version.
|
||||
//
|
||||
// This program is distributed in the hope that it will be useful
|
||||
// but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
// GNU Affero General Public License for more details.
|
||||
//
|
||||
// You should have received a copy of the GNU Affero General Public License
|
||||
// along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
|
||||
package hash
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestChecksumAddPart checks that merging part checksums reproduces the
|
||||
// checksum of the concatenated content, including when parts are empty.
|
||||
// A zero length first part used to leave the accumulator unseeded, which left
|
||||
// multipart objects with an empty checksum.
|
||||
func TestChecksumAddPart(t *testing.T) {
|
||||
empty := []byte{}
|
||||
a := bytes.Repeat([]byte("a"), 1024)
|
||||
b := bytes.Repeat([]byte("b"), 7)
|
||||
|
||||
layouts := []struct {
|
||||
name string
|
||||
parts [][]byte
|
||||
}{
|
||||
{"single-empty", [][]byte{empty}},
|
||||
{"empty-first", [][]byte{empty, a, b}},
|
||||
{"empty-middle", [][]byte{a, empty, b}},
|
||||
{"empty-last", [][]byte{a, b, empty}},
|
||||
{"all-empty", [][]byte{empty, empty}},
|
||||
{"no-empty", [][]byte{a, b}},
|
||||
{"single", [][]byte{a}},
|
||||
}
|
||||
|
||||
for _, typ := range []ChecksumType{ChecksumCRC32, ChecksumCRC32C, ChecksumCRC64NVME} {
|
||||
for _, l := range layouts {
|
||||
t.Run(typ.String()+"/"+l.name, func(t *testing.T) {
|
||||
var merged Checksum
|
||||
merged.Type = typ | ChecksumMultipart | ChecksumIncludesMultipart
|
||||
|
||||
var full []byte
|
||||
for i, p := range l.parts {
|
||||
part := NewChecksumFromData(typ, p)
|
||||
if part == nil {
|
||||
t.Fatalf("unable to compute part %d checksum", i+1)
|
||||
}
|
||||
if err := merged.AddPart(*part, int64(len(p))); err != nil {
|
||||
t.Fatalf("AddPart(%d): %v", i+1, err)
|
||||
}
|
||||
full = append(full, p...)
|
||||
}
|
||||
|
||||
want := NewChecksumFromData(typ, full)
|
||||
if want == nil {
|
||||
t.Fatal("unable to compute expected checksum")
|
||||
}
|
||||
if merged.Encoded != want.Encoded {
|
||||
t.Fatalf("merged checksum = %q, want %q", merged.Encoded, want.Encoded)
|
||||
}
|
||||
if !merged.Valid() {
|
||||
t.Fatalf("merged checksum is not valid: %+v", merged)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestChecksumAddPartTypeMismatch checks that a mismatched part type is
|
||||
// reported even when the part is empty. The zero size shortcut used to hide it.
|
||||
func TestChecksumAddPartTypeMismatch(t *testing.T) {
|
||||
var merged Checksum
|
||||
merged.Type = ChecksumCRC32 | ChecksumMultipart
|
||||
|
||||
other := NewChecksumFromData(ChecksumCRC32C, []byte("x"))
|
||||
if other == nil {
|
||||
t.Fatal("unable to compute part checksum")
|
||||
}
|
||||
if err := merged.AddPart(*other, 0); err == nil {
|
||||
t.Fatal("expected a type mismatch error for a zero sized part of the wrong type")
|
||||
}
|
||||
if err := merged.AddPart(*other, 1); err == nil {
|
||||
t.Fatal("expected a type mismatch error for a non-empty part of the wrong type")
|
||||
}
|
||||
}
|
||||
|
||||
// TestChecksumAddPartUnmergeable checks that non-CRC types are still refused.
|
||||
func TestChecksumAddPartUnmergeable(t *testing.T) {
|
||||
var merged Checksum
|
||||
merged.Type = ChecksumSHA256 | ChecksumMultipart
|
||||
|
||||
other := NewChecksumFromData(ChecksumSHA256, []byte("x"))
|
||||
if other == nil {
|
||||
t.Fatal("unable to compute part checksum")
|
||||
}
|
||||
if err := merged.AddPart(*other, 1); err == nil {
|
||||
t.Fatal("expected SHA256 to be refused as unmergeable")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user