diff --git a/cmd/object-copy-metadata_test.go b/cmd/object-copy-metadata_test.go index 399f192ee..d3f81b774 100644 --- a/cmd/object-copy-metadata_test.go +++ b/cmd/object-copy-metadata_test.go @@ -202,3 +202,122 @@ func testAPICopyObjectSSECKeyRotationKeepsCompressionState(obj ObjectLayer, inst instanceType, response.Code, response.Body.Len(), len(data), response.Body.String()) } } + +// TestAPICopyObjectMetadataOnlyNullVersion covers the copy whose source is a +// null version on a bucket that gained versioning after the object was written. +// The object layer cannot reference such a version, so it rewrites the data and +// the recorded compression metadata has to describe the rewritten bytes. +func TestAPICopyObjectMetadataOnlyNullVersion(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: testAPICopyObjectMetadataOnlyNullVersion, + endpoints: []string{"CopyObject", "PutObject", "GetObject"}, + }) +} + +func testAPICopyObjectMetadataOnlyNullVersion(obj ObjectLayer, instanceType, bucketName string, + apiRouter http.Handler, credentials auth.Credentials, t *testing.T, +) { + restoreCompression := setCopyChecksumCompression(true) + compressionRestored := false + defer func() { + if !compressionRestored { + restoreCompression() + } + }() + + data := bytes.Repeat([]byte("null-version-metadata-copy-"), 64*1024) + want := mustChecksum(t, hash.ChecksumCRC32, data) + object := "copy-metadata/null-version.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 { + t.Fatal(err) + } + if !before.IsCompressed() || before.VersionID != "" { + t.Fatalf("%s: invalid null-version precondition: compressed=%v versionID=%q", + instanceType, before.IsCompressed(), before.VersionID) + } + + // Versioning is enabled after the write, so the object keeps a null version. + if _, err := globalBucketMetadataSys.Update(t.Context(), bucketName, + bucketVersioningConfig, enabledBucketVersioningConfig); err != nil { + t.Fatalf("%s: unable to enable versioning: %v", instanceType, err) + } + if !globalBucketVersioningSys.PrefixEnabled(bucketName, object) { + t.Fatalf("%s: versioning did not become enabled", instanceType) + } + + // Without compression the rewritten destination stores plaintext. + restoreCompression() + compressionRestored = true + + 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()) + } + + after := assertCopyChecksum(t, obj, bucketName, object, hash.ChecksumCRC32, data, false, nil) + if after.VersionID == "" { + t.Fatalf("%s: versioned copy did not create a new version", instanceType) + } + if got := readCopyChecksumObject(t, obj, bucketName, object, ObjectOptions{}); !bytes.Equal(got, data) { + t.Fatalf("%s: copied object body differs: got %d bytes, want %d", instanceType, len(got), len(data)) + } +} + +func TestCopyRewritesObjectData(t *testing.T) { + tests := []struct { + name string + metadataOnly bool + srcOpts ObjectOptions + dstOpts ObjectOptions + want bool + }{ + { + name: "data copy always rewrites", + want: true, + }, + { + name: "unversioned in-place metadata update", + metadataOnly: true, + }, + { + name: "addressed version updated in place", + metadataOnly: true, + srcOpts: ObjectOptions{VersionID: "v1"}, + dstOpts: ObjectOptions{VersionID: "v1"}, + }, + { + name: "versioned self referential version", + metadataOnly: true, + srcOpts: ObjectOptions{VersionID: "v1"}, + dstOpts: ObjectOptions{Versioned: true}, + }, + { + name: "versioned null source version cannot be referenced", + metadataOnly: true, + dstOpts: ObjectOptions{Versioned: true}, + want: true, + }, + { + name: "suspended destination with an addressed source version", + metadataOnly: true, + srcOpts: ObjectOptions{VersionID: "v1"}, + dstOpts: ObjectOptions{VersionSuspended: true, VersionID: nullVersionID}, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := copyRewritesObjectData(tt.metadataOnly, tt.srcOpts, tt.dstOpts); got != tt.want { + t.Fatalf("copyRewritesObjectData() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 649136227..d25e99b5c 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -1179,6 +1179,31 @@ func isRemoteCallRequired(ctx context.Context, bucket string, objAPI ObjectLayer return false } +// copyRewritesObjectData reports whether the object layer stores new object data +// for this copy instead of updating metadata in place or adding a +// self-referential version. It mirrors the metadata-only decision taken by +// erasureServerPools.CopyObject and erasureSets.CopyObject. CopyObjectHandler +// has to predict that decision because the compression metadata it records must +// describe whichever bytes are finally stored. metadataOnly already excludes +// legacy sources, which the object layer always rewrites. +func copyRewritesObjectData(metadataOnly bool, srcOpts, dstOpts ObjectOptions) bool { + if !metadataOnly { + return true + } + switch { + case dstOpts.VersionID != "" && srcOpts.VersionID == dstOpts.VersionID: + // In-place update of the addressed version. + return false + case !dstOpts.Versioned && srcOpts.VersionID == "": + // In-place update of an unversioned object. + return false + case dstOpts.Versioned && srcOpts.VersionID != dstOpts.VersionID: + // A new version referencing the existing data. + return false + } + return true +} + // CopyObjectHandler - Copy Object // ---------- // This implementation of the PUT operation adds an object to a bucket @@ -1708,8 +1733,20 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re srcInfo.UserDefined[ReservedMetadataPrefixLower+ReplicationStatus] = dsc.PendingStatus() srcInfo.UserDefined[ReservedMetadataPrefixLower+ReplicationTimestamp] = UTCNow().Format(time.RFC3339Nano) } - // Compression metadata must describe data that is actually rewritten. - if !srcInfo.metadataOnly || srcInfo.Legacy || dstOpts.WantServerSideChecksumType.IsSet() { + // srcInfo.metadataOnly is still cleared below for legacy sources and for + // server-side checksum recomputation; both of those rewrite the object data. + metadataOnly := srcInfo.metadataOnly && !srcInfo.Legacy && !dstOpts.WantServerSideChecksumType.IsSet() + + // Name the source version explicitly so a metadata-only copy into a + // versioned bucket adds a self-referential version instead of rewriting the + // object data. A null source version cannot be referenced this way. + copySrcOpts := srcOpts + if metadataOnly && dstOpts.Versioned && copySrcOpts.VersionID == "" { + copySrcOpts.VersionID = srcInfo.VersionID + } + + // Compression metadata must describe the bytes that are actually stored. + if copyRewritesObjectData(metadataOnly, copySrcOpts, dstOpts) { if isDstCompressed { maps.Copy(srcInfo.UserDefined, compressMetadata) } else { @@ -1808,11 +1845,6 @@ 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, copySrcOpts, dstOpts)