Skip to content

Commit 05df6e7

Browse files
VonngVonng
committed
fix: preserve transform state on metadata-only copies
Signed-off-by: Feng Ruohang <rh@vonng.com>
1 parent c0e7159 commit 05df6e7

2 files changed

Lines changed: 228 additions & 5 deletions

File tree

‎cmd/object-copy-metadata_test.go‎

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
// Copyright (c) 2015-2026 MinIO, Inc.
2+
//
3+
// This file is part of MinIO Object Storage stack
4+
//
5+
// This program is free software: you can redistribute it and/or modify
6+
// it under the terms of the GNU Affero General Public License as published by
7+
// the Free Software Foundation, either version 3 of the License, or
8+
// (at your option) any later version.
9+
//
10+
// This program is distributed in the hope that it will be useful
11+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
// GNU Affero General Public License for more details.
14+
//
15+
// You should have received a copy of the GNU Affero General Public License
16+
// along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
18+
package cmd
19+
20+
import (
21+
"bytes"
22+
"crypto/md5"
23+
"encoding/base64"
24+
"net/http"
25+
"net/http/httptest"
26+
"testing"
27+
28+
"github.com/minio/minio/internal/auth"
29+
"github.com/minio/minio/internal/hash"
30+
xhttp "github.com/minio/minio/internal/http"
31+
)
32+
33+
func TestAPICopyObjectMetadataOnlyCompression(t *testing.T) {
34+
defer DetectTestLeak(t)()
35+
for _, versioned := range []bool{false, true} {
36+
name := "unversioned"
37+
if versioned {
38+
name = "versioned"
39+
}
40+
t.Run(name, func(t *testing.T) {
41+
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
42+
t: t,
43+
objAPITest: testAPICopyObjectMetadataOnlyCompression,
44+
endpoints: []string{"CopyObject", "PutObject", "GetObject"},
45+
makeBucketOptions: MakeBucketOptions{VersioningEnabled: versioned},
46+
})
47+
})
48+
}
49+
}
50+
51+
func testAPICopyObjectMetadataOnlyCompression(obj ObjectLayer, instanceType, bucketName string,
52+
apiRouter http.Handler, credentials auth.Credentials, t *testing.T,
53+
) {
54+
data := bytes.Repeat([]byte("metadata-only-copy-plaintext-"), 64*1024)
55+
want := mustChecksum(t, hash.ChecksumCRC32, data)
56+
object := "copy-metadata/existing-checksum.txt"
57+
putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, data,
58+
map[string]string{xhttp.AmzChecksumCRC32: want})
59+
before, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{})
60+
if err != nil || before.IsCompressed() {
61+
t.Fatalf("%s: invalid metadata-copy precondition: compressed=%v size=%d err=%v",
62+
instanceType, before.IsCompressed(), before.Size, err)
63+
}
64+
65+
restoreCompression := setCopyChecksumCompression(true)
66+
compressionRestored := false
67+
defer func() {
68+
if !compressionRestored {
69+
restoreCompression()
70+
}
71+
}()
72+
73+
rec := copyChecksumRequest(t, apiRouter, credentials, bucketName, object, object,
74+
map[string]string{xhttp.AmzMetadataDirective: "REPLACE"})
75+
if rec.Code != http.StatusOK {
76+
t.Fatalf("%s: metadata-only CopyObject failed: %d %s", instanceType, rec.Code, rec.Body.String())
77+
}
78+
assertCopyChecksum(t, obj, bucketName, object, hash.ChecksumCRC32, data, false, nil)
79+
if got := readCopyChecksumObject(t, obj, bucketName, object, ObjectOptions{}); !bytes.Equal(got, data) {
80+
prefix := got
81+
if len(prefix) > 100 {
82+
prefix = prefix[:100]
83+
}
84+
t.Fatalf("%s: metadata-only CopyObject body differs: got %d bytes, want %d, prefix %q",
85+
instanceType, len(got), len(data), prefix)
86+
}
87+
afterMetadataCopy, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{})
88+
if err != nil {
89+
t.Fatal(err)
90+
}
91+
if before.VersionID != "" && afterMetadataCopy.VersionID == before.VersionID {
92+
t.Fatalf("%s: versioned metadata-only copy did not create a new version", instanceType)
93+
}
94+
95+
destination := "copy-metadata/rewritten.txt"
96+
rec = copyChecksumRequest(t, apiRouter, credentials, bucketName, object, destination, nil)
97+
if rec.Code != http.StatusOK {
98+
t.Fatalf("%s: data-rewriting CopyObject failed: %d %s", instanceType, rec.Code, rec.Body.String())
99+
}
100+
assertCopyChecksum(t, obj, bucketName, destination, hash.ChecksumCRC32, data, true, nil)
101+
102+
compressedObject := "copy-metadata/preserve-compressed.txt"
103+
putCopyChecksumSource(t, apiRouter, credentials, bucketName, compressedObject, data,
104+
map[string]string{xhttp.AmzChecksumCRC32: want})
105+
assertCopyChecksum(t, obj, bucketName, compressedObject, hash.ChecksumCRC32, data, true, nil)
106+
107+
restoreCompression()
108+
compressionRestored = true
109+
rec = copyChecksumRequest(t, apiRouter, credentials, bucketName, compressedObject, compressedObject,
110+
map[string]string{xhttp.AmzMetadataDirective: "REPLACE"})
111+
if rec.Code != http.StatusOK {
112+
t.Fatalf("%s: compressed metadata-only CopyObject failed: %d %s", instanceType, rec.Code, rec.Body.String())
113+
}
114+
assertCopyChecksum(t, obj, bucketName, compressedObject, hash.ChecksumCRC32, data, true, nil)
115+
if got := readCopyChecksumObject(t, obj, bucketName, compressedObject, ObjectOptions{}); !bytes.Equal(got, data) {
116+
t.Fatalf("%s: compressed metadata-only CopyObject body differs", instanceType)
117+
}
118+
}
119+
120+
func TestAPICopyObjectSSECKeyRotationKeepsCompressionState(t *testing.T) {
121+
defer DetectTestLeak(t)()
122+
for _, versioned := range []bool{false, true} {
123+
name := "unversioned"
124+
if versioned {
125+
name = "versioned"
126+
}
127+
t.Run(name, func(t *testing.T) {
128+
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
129+
t: t,
130+
objAPITest: testAPICopyObjectSSECKeyRotationKeepsCompressionState,
131+
endpoints: []string{"CopyObject", "PutObject", "GetObject"},
132+
makeBucketOptions: MakeBucketOptions{VersioningEnabled: versioned},
133+
})
134+
})
135+
}
136+
}
137+
138+
func testAPICopyObjectSSECKeyRotationKeepsCompressionState(obj ObjectLayer, instanceType, bucketName string,
139+
apiRouter http.Handler, credentials auth.Credentials, t *testing.T,
140+
) {
141+
previousTLS := globalIsTLS
142+
globalIsTLS = true
143+
defer func() { globalIsTLS = previousTLS }()
144+
145+
data := bytes.Repeat([]byte("key-rotation-plaintext-"), 64*1024)
146+
object := "copy-metadata/key-rotation.txt"
147+
oldKey := bytes.Repeat([]byte{0x11}, 32)
148+
oldMD5 := md5.Sum(oldKey)
149+
newKey := bytes.Repeat([]byte{0x22}, 32)
150+
newMD5 := md5.Sum(newKey)
151+
152+
putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, data, map[string]string{
153+
xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES,
154+
xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(oldKey),
155+
xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(oldMD5[:]),
156+
})
157+
before, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{})
158+
if err != nil || before.IsCompressed() {
159+
t.Fatalf("%s: invalid key-rotation precondition: compressed=%v err=%v", instanceType, before.IsCompressed(), err)
160+
}
161+
162+
restoreCompression := setCopyChecksumCompression(true)
163+
defer restoreCompression()
164+
rec := copyChecksumRequest(t, apiRouter, credentials, bucketName, object, object, map[string]string{
165+
xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES,
166+
xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(newKey),
167+
xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(newMD5[:]),
168+
xhttp.AmzServerSideEncryptionCopyCustomerAlgorithm: xhttp.AmzEncryptionAES,
169+
xhttp.AmzServerSideEncryptionCopyCustomerKey: base64.StdEncoding.EncodeToString(oldKey),
170+
xhttp.AmzServerSideEncryptionCopyCustomerKeyMD5: base64.StdEncoding.EncodeToString(oldMD5[:]),
171+
})
172+
if rec.Code != http.StatusOK {
173+
t.Fatalf("%s: key rotation failed: %d %s", instanceType, rec.Code, rec.Body.String())
174+
}
175+
after, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{})
176+
if err != nil {
177+
t.Fatal(err)
178+
}
179+
if after.IsCompressed() {
180+
t.Fatalf("%s: metadata-only key rotation stamped compression metadata", instanceType)
181+
}
182+
if before.VersionID != "" && after.VersionID == before.VersionID {
183+
t.Fatalf("%s: versioned key rotation did not create a new version", instanceType)
184+
}
185+
186+
getHeaders := map[string]string{
187+
xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES,
188+
xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(newKey),
189+
xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(newMD5[:]),
190+
}
191+
req, err := newTestSignedRequestV4(http.MethodGet, getGetObjectURL("", bucketName, object),
192+
0, nil, credentials.AccessKey, credentials.SecretKey, getHeaders)
193+
if err != nil {
194+
t.Fatalf("failed to build GetObject request: %v", err)
195+
}
196+
response := httptest.NewRecorder()
197+
apiRouter.ServeHTTP(response, req)
198+
if response.Code != http.StatusOK || !bytes.Equal(response.Body.Bytes(), data) {
199+
t.Fatalf("%s: post-rotation GetObject returned %d with %d bytes, want 200 with %d bytes: %s",
200+
instanceType, response.Code, response.Body.Len(), len(data), response.Body.String())
201+
}
202+
}

‎cmd/object-handlers.go‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,6 +1357,15 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
13571357
} // no changes in storage-class expected so its a metadataonly operation.
13581358

13591359
var reader io.Reader = gr
1360+
sourceCompressMetadata := make(map[string]string, 2)
1361+
for _, key := range []string{
1362+
ReservedMetadataPrefix + "compression",
1363+
ReservedMetadataPrefix + "actual-size",
1364+
} {
1365+
if value, ok := srcInfo.UserDefined[key]; ok {
1366+
sourceCompressMetadata[key] = value
1367+
}
1368+
}
13601369

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

13881397
reader = etag.NewReader(ctx, reader, nil, nil)
13891398
} else {
1390-
delete(srcInfo.UserDefined, ReservedMetadataPrefix+"compression")
1391-
delete(srcInfo.UserDefined, ReservedMetadataPrefix+"actual-size")
13921399
reader = gr
13931400
}
13941401

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

16931709
// We need to preserve the encryption headers set in EncryptRequest,
16941710
// 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
17781794

17791795
copyObjectFn := objectAPI.CopyObject
17801796

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

0 commit comments

Comments
 (0)