fix: preserve transform state on metadata-only copies

Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-09-05 18:16:16 +03:00 · 2026-08-24 20:32:11 +08:00
parent c0e7159771
commit 05df6e70d7
2 changed files with 228 additions and 5 deletions
@@ -0,0 +1,202 @@
+// 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 cmd
+
+import (
+	"bytes"
+	"crypto/md5"
+	"encoding/base64"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/minio/minio/internal/auth"
+	"github.com/minio/minio/internal/hash"
+	xhttp "github.com/minio/minio/internal/http"
+)
+
+func TestAPICopyObjectMetadataOnlyCompression(t *testing.T) {
+	defer DetectTestLeak(t)()
+	for _, versioned := range []bool{false, true} {
+		name := "unversioned"
+		if versioned {
+			name = "versioned"
+		}
+		t.Run(name, func(t *testing.T) {
+			ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
+				t:                 t,
+				objAPITest:        testAPICopyObjectMetadataOnlyCompression,
+				endpoints:         []string{"CopyObject", "PutObject", "GetObject"},
+				makeBucketOptions: MakeBucketOptions{VersioningEnabled: versioned},
+			})
+		})
+	}
+}
+
+func testAPICopyObjectMetadataOnlyCompression(obj ObjectLayer, instanceType, bucketName string,
+	apiRouter http.Handler, credentials auth.Credentials, t *testing.T,
+) {
+	data := bytes.Repeat([]byte("metadata-only-copy-plaintext-"), 64*1024)
+	want := mustChecksum(t, hash.ChecksumCRC32, data)
+	object := "copy-metadata/existing-checksum.txt"
+	putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, data,
+		map[string]string{xhttp.AmzChecksumCRC32: want})
+	before, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{})
+	if err != nil || before.IsCompressed() {
+		t.Fatalf("%s: invalid metadata-copy precondition: compressed=%v size=%d err=%v",
+			instanceType, before.IsCompressed(), before.Size, err)
+	}
+
+	restoreCompression := setCopyChecksumCompression(true)
+	compressionRestored := false
+	defer func() {
+		if !compressionRestored {
+			restoreCompression()
+		}
+	}()
+
+	rec := copyChecksumRequest(t, apiRouter, credentials, bucketName, object, object,
+		map[string]string{xhttp.AmzMetadataDirective: "REPLACE"})
+	if rec.Code != http.StatusOK {
+		t.Fatalf("%s: metadata-only CopyObject failed: %d %s", instanceType, rec.Code, rec.Body.String())
+	}
+	assertCopyChecksum(t, obj, bucketName, object, hash.ChecksumCRC32, data, false, nil)
+	if got := readCopyChecksumObject(t, obj, bucketName, object, ObjectOptions{}); !bytes.Equal(got, data) {
+		prefix := got
+		if len(prefix) > 100 {
+			prefix = prefix[:100]
+		}
+		t.Fatalf("%s: metadata-only CopyObject body differs: got %d bytes, want %d, prefix %q",
+			instanceType, len(got), len(data), prefix)
+	}
+	afterMetadataCopy, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if before.VersionID != "" && afterMetadataCopy.VersionID == before.VersionID {
+		t.Fatalf("%s: versioned metadata-only copy did not create a new version", instanceType)
+	}
+
+	destination := "copy-metadata/rewritten.txt"
+	rec = copyChecksumRequest(t, apiRouter, credentials, bucketName, object, destination, nil)
+	if rec.Code != http.StatusOK {
+		t.Fatalf("%s: data-rewriting CopyObject failed: %d %s", instanceType, rec.Code, rec.Body.String())
+	}
+	assertCopyChecksum(t, obj, bucketName, destination, hash.ChecksumCRC32, data, true, nil)
+
+	compressedObject := "copy-metadata/preserve-compressed.txt"
+	putCopyChecksumSource(t, apiRouter, credentials, bucketName, compressedObject, data,
+		map[string]string{xhttp.AmzChecksumCRC32: want})
+	assertCopyChecksum(t, obj, bucketName, compressedObject, hash.ChecksumCRC32, data, true, nil)
+
+	restoreCompression()
+	compressionRestored = true
+	rec = copyChecksumRequest(t, apiRouter, credentials, bucketName, compressedObject, compressedObject,
+		map[string]string{xhttp.AmzMetadataDirective: "REPLACE"})
+	if rec.Code != http.StatusOK {
+		t.Fatalf("%s: compressed metadata-only CopyObject failed: %d %s", instanceType, rec.Code, rec.Body.String())
+	}
+	assertCopyChecksum(t, obj, bucketName, compressedObject, hash.ChecksumCRC32, data, true, nil)
+	if got := readCopyChecksumObject(t, obj, bucketName, compressedObject, ObjectOptions{}); !bytes.Equal(got, data) {
+		t.Fatalf("%s: compressed metadata-only CopyObject body differs", instanceType)
+	}
+}
+
+func TestAPICopyObjectSSECKeyRotationKeepsCompressionState(t *testing.T) {
+	defer DetectTestLeak(t)()
+	for _, versioned := range []bool{false, true} {
+		name := "unversioned"
+		if versioned {
+			name = "versioned"
+		}
+		t.Run(name, func(t *testing.T) {
+			ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
+				t:                 t,
+				objAPITest:        testAPICopyObjectSSECKeyRotationKeepsCompressionState,
+				endpoints:         []string{"CopyObject", "PutObject", "GetObject"},
+				makeBucketOptions: MakeBucketOptions{VersioningEnabled: versioned},
+			})
+		})
+	}
+}
+
+func testAPICopyObjectSSECKeyRotationKeepsCompressionState(obj ObjectLayer, instanceType, bucketName string,
+	apiRouter http.Handler, credentials auth.Credentials, t *testing.T,
+) {
+	previousTLS := globalIsTLS
+	globalIsTLS = true
+	defer func() { globalIsTLS = previousTLS }()
+
+	data := bytes.Repeat([]byte("key-rotation-plaintext-"), 64*1024)
+	object := "copy-metadata/key-rotation.txt"
+	oldKey := bytes.Repeat([]byte{0x11}, 32)
+	oldMD5 := md5.Sum(oldKey)
+	newKey := bytes.Repeat([]byte{0x22}, 32)
+	newMD5 := md5.Sum(newKey)
+
+	putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, data, map[string]string{
+		xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES,
+		xhttp.AmzServerSideEncryptionCustomerKey:       base64.StdEncoding.EncodeToString(oldKey),
+		xhttp.AmzServerSideEncryptionCustomerKeyMD5:    base64.StdEncoding.EncodeToString(oldMD5[:]),
+	})
+	before, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{})
+	if err != nil || before.IsCompressed() {
+		t.Fatalf("%s: invalid key-rotation precondition: compressed=%v err=%v", instanceType, before.IsCompressed(), err)
+	}
+
+	restoreCompression := setCopyChecksumCompression(true)
+	defer restoreCompression()
+	rec := copyChecksumRequest(t, apiRouter, credentials, bucketName, object, object, map[string]string{
+		xhttp.AmzServerSideEncryptionCustomerAlgorithm:     xhttp.AmzEncryptionAES,
+		xhttp.AmzServerSideEncryptionCustomerKey:           base64.StdEncoding.EncodeToString(newKey),
+		xhttp.AmzServerSideEncryptionCustomerKeyMD5:        base64.StdEncoding.EncodeToString(newMD5[:]),
+		xhttp.AmzServerSideEncryptionCopyCustomerAlgorithm: xhttp.AmzEncryptionAES,
+		xhttp.AmzServerSideEncryptionCopyCustomerKey:       base64.StdEncoding.EncodeToString(oldKey),
+		xhttp.AmzServerSideEncryptionCopyCustomerKeyMD5:    base64.StdEncoding.EncodeToString(oldMD5[:]),
+	})
+	if rec.Code != http.StatusOK {
+		t.Fatalf("%s: key rotation failed: %d %s", instanceType, rec.Code, rec.Body.String())
+	}
+	after, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if after.IsCompressed() {
+		t.Fatalf("%s: metadata-only key rotation stamped compression metadata", instanceType)
+	}
+	if before.VersionID != "" && after.VersionID == before.VersionID {
+		t.Fatalf("%s: versioned key rotation did not create a new version", instanceType)
+	}
+
+	getHeaders := map[string]string{
+		xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES,
+		xhttp.AmzServerSideEncryptionCustomerKey:       base64.StdEncoding.EncodeToString(newKey),
+		xhttp.AmzServerSideEncryptionCustomerKeyMD5:    base64.StdEncoding.EncodeToString(newMD5[:]),
+	}
+	req, err := newTestSignedRequestV4(http.MethodGet, getGetObjectURL("", bucketName, object),
+		0, nil, credentials.AccessKey, credentials.SecretKey, getHeaders)
+	if err != nil {
+		t.Fatalf("failed to build GetObject request: %v", err)
+	}
+	response := httptest.NewRecorder()
+	apiRouter.ServeHTTP(response, req)
+	if response.Code != http.StatusOK || !bytes.Equal(response.Body.Bytes(), data) {
+		t.Fatalf("%s: post-rotation GetObject returned %d with %d bytes, want 200 with %d bytes: %s",
+			instanceType, response.Code, response.Body.Len(), len(data), response.Body.String())
+	}
+}
@@ -1357,6 +1357,15 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
 	} // no changes in storage-class expected so its a metadataonly operation.

 	var reader io.Reader = gr
+	sourceCompressMetadata := make(map[string]string, 2)
+	for _, key := range []string{
+		ReservedMetadataPrefix + "compression",
+		ReservedMetadataPrefix + "actual-size",
+	} {
+		if value, ok := srcInfo.UserDefined[key]; ok {
+			sourceCompressMetadata[key] = value
+		}
+	}

 	// Set the actual size to the compressed/decrypted size if encrypted.
 	actualSize, err := srcInfo.GetActualSize()
@@ -1387,8 +1396,6 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re

 		reader = etag.NewReader(ctx, reader, nil, nil)
 	} else {
-		delete(srcInfo.UserDefined, ReservedMetadataPrefix+"compression")
-		delete(srcInfo.UserDefined, ReservedMetadataPrefix+"actual-size")
 		reader = gr
 	}

@@ -1687,8 +1694,17 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
 		srcInfo.UserDefined[ReservedMetadataPrefixLower+ReplicationStatus] = dsc.PendingStatus()
 		srcInfo.UserDefined[ReservedMetadataPrefixLower+ReplicationTimestamp] = UTCNow().Format(time.RFC3339Nano)
 	}
-	// Store the preserved compression metadata.
-	maps.Copy(srcInfo.UserDefined, compressMetadata)
+	// Compression metadata must describe data that is actually rewritten.
+	if !srcInfo.metadataOnly || srcInfo.Legacy || dstOpts.WantServerSideChecksumType.IsSet() {
+		if isDstCompressed {
+			maps.Copy(srcInfo.UserDefined, compressMetadata)
+		} else {
+			delete(srcInfo.UserDefined, ReservedMetadataPrefix+"compression")
+			delete(srcInfo.UserDefined, ReservedMetadataPrefix+"actual-size")
+		}
+	} else {
+		maps.Copy(srcInfo.UserDefined, sourceCompressMetadata)
+	}

 	// We need to preserve the encryption headers set in EncryptRequest,
 	// so we do not want to override them, copy them instead.
@@ -1778,9 +1794,14 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re

 		copyObjectFn := objectAPI.CopyObject

+		copySrcOpts := srcOpts
+		if srcInfo.metadataOnly && dstOpts.Versioned && copySrcOpts.VersionID == "" {
+			copySrcOpts.VersionID = srcInfo.VersionID
+		}
+
 		// Copy source object to destination, if source and destination
 		// object is same then only metadata is updated.
-		objInfo, err = copyObjectFn(ctx, srcBucket, srcObject, dstBucket, dstObject, srcInfo, srcOpts, dstOpts)
+		objInfo, err = copyObjectFn(ctx, srcBucket, srcObject, dstBucket, dstObject, srcInfo, copySrcOpts, dstOpts)
 		if err != nil {
 			writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
 			return