From d204d2e2383ac511d6641e47603a2c6ed5996290 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Sun, 20 Sep 2026 15:35:08 +0400 Subject: [PATCH 1/2] fix: stop posix copying data from a delete marker `CopyObject` and `UploadPartCopy` never looked at the source's `delete-marker` attribute. A delete marker leaves the data file in place at the object path and only flags it, so a copy whose source resolved to a marker opened that file and succeeded, handing back the data of the version the marker had deleted. AWS rejects such a copy: `NoSuchKey` when the marker is the current version of the key, and `InvalidRequest` when the marker is named by version id, the latter regardless of whether it is the latest version. Versions the marker hides stay copyable by version id. Both copy paths now run the resolved source through `checkCopySourceDeleteMarker` once the entry has been validated, returning `NoSuchKey` for an unqualified source and the new `ErrCopySourceDeleteMarker` for one carrying a version id. --- backend/posix/posix.go | 24 +++++ s3err/s3err.go | 6 ++ tests/integration/group-tests.go | 4 + tests/integration/utils.go | 22 +++++ tests/integration/versioning.go | 151 +++++++++++++++++++++++++++++++ 5 files changed, 207 insertions(+) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index a93ad686..b3c7eaaf 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1618,6 +1618,24 @@ func (p *Posix) isObjDeleteMarker(bucket, object string) (bool, error) { return true, nil } +// checkCopySourceDeleteMarker rejects a copy whose source resolves to a +// delete marker: the key has no current version when the marker is the +// latest, and a marker named by version id holds no data to copy. +func (p *Posix) checkCopySourceDeleteMarker(bucket, object, versionId string) error { + isDel, err := p.isObjDeleteMarker(bucket, object) + if err != nil { + return err + } + if !isDel { + return nil + } + if versionId != "" { + return s3err.GetAPIError(s3err.ErrCopySourceDeleteMarker) + } + + return s3err.GetAPIError(s3err.ErrNoSuchKey) +} + // Converts the file to object version. Finds all the object versions, // delete markers from the versioning directory and returns func (p *Posix) fileToObjVersions(bucket string) backend.GetVersionsFunc { @@ -4034,6 +4052,9 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput) if strings.HasSuffix(srcObject, "/") != fi.IsDir() { return s3response.CopyPartResult{}, s3err.GetAPIError(s3err.ErrNoSuchKey) } + if err := p.checkCopySourceDeleteMarker(srcBucket, srcObject, srcVersionId); err != nil { + return s3response.CopyPartResult{}, err + } // a directory object holds no data srcSize := fi.Size() if fi.IsDir() { @@ -6237,6 +6258,9 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput if !strings.HasSuffix(srcObject, "/") && fi.IsDir() { return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrNoSuchKey) } + if err := p.checkCopySourceDeleteMarker(srcBucket, srcObject, srcVersionId); err != nil { + return s3response.CopyObjectOutput{}, err + } // a directory object holds no data srcSize := fi.Size() var srcBody io.Reader = f diff --git a/s3err/s3err.go b/s3err/s3err.go index 5a02d440..6262d77e 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -131,6 +131,7 @@ const ( ErrMissingDateHeader ErrGetUploadsWithKey ErrVersionsWithKey + ErrCopySourceDeleteMarker ErrInvalidRequest ErrAuthNotSetup ErrNotImplemented @@ -453,6 +454,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "There is no such thing as the ?versions sub-resource for a key", HTTPStatusCode: http.StatusBadRequest, }, + ErrCopySourceDeleteMarker: { + Code: "InvalidRequest", + Description: "The source of a copy request may not specifically refer to a delete marker by version id.", + HTTPStatusCode: http.StatusBadRequest, + }, ErrInvalidRequest: { Code: "InvalidRequest", Description: "Invalid Request.", diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index c9961db4..cbed13f6 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -1957,6 +1957,7 @@ func TestVersioning(ts *TestState) { ts.Run(Versioning_CopyObject_success) ts.Run(Versioning_CopyObject_non_existing_version_id) ts.Run(Versioning_CopyObject_from_an_object_version) + ts.Run(Versioning_CopyObject_from_a_delete_marker) if !ts.conf.windowsTests { ts.Run(Versioning_CopyObject_special_chars) } @@ -2026,6 +2027,7 @@ func TestVersioning(ts *TestState) { ts.Run(Versioning_UploadPartCopy_encoded_versionid_separator_invalid_versionId) ts.Run(Versioning_UploadPartCopy_non_existing_versionId) ts.Run(Versioning_UploadPartCopy_from_an_object_version) + ts.Run(Versioning_UploadPartCopy_from_a_delete_marker) // Object lock configuration ts.Run(Versioning_object_lock_not_enabled_on_bucket_creation) ts.Run(Versioning_Enable_object_lock) @@ -3494,6 +3496,7 @@ func GetIntTests() IntTests { "Versioning_CopyObject_success": Versioning_CopyObject_success, "Versioning_CopyObject_non_existing_version_id": Versioning_CopyObject_non_existing_version_id, "Versioning_CopyObject_from_an_object_version": Versioning_CopyObject_from_an_object_version, + "Versioning_CopyObject_from_a_delete_marker": Versioning_CopyObject_from_a_delete_marker, "Versioning_CopyObject_special_chars": Versioning_CopyObject_special_chars, "Versioning_HeadObject_invalid_versionId": Versioning_HeadObject_invalid_versionId, "Versioning_HeadObject_non_existing_object_version": Versioning_HeadObject_non_existing_object_version, @@ -3550,6 +3553,7 @@ func GetIntTests() IntTests { "Versioning_UploadPartCopy_encoded_versionid_separator_invalid_versionId": Versioning_UploadPartCopy_encoded_versionid_separator_invalid_versionId, "Versioning_UploadPartCopy_non_existing_versionId": Versioning_UploadPartCopy_non_existing_versionId, "Versioning_UploadPartCopy_from_an_object_version": Versioning_UploadPartCopy_from_an_object_version, + "Versioning_UploadPartCopy_from_a_delete_marker": Versioning_UploadPartCopy_from_a_delete_marker, "Versioning_object_lock_not_enabled_on_bucket_creation": Versioning_object_lock_not_enabled_on_bucket_creation, "Versioning_Enable_object_lock": Versioning_Enable_object_lock, "Versioning_status_switch_to_suspended_with_object_lock": Versioning_status_switch_to_suspended_with_object_lock, diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 7f5e2cbb..f849a966 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -2375,6 +2375,28 @@ func createObjVersions(client *s3.Client, bucket, object string, count int, opts return versions, nil } +// createDeleteMarker deletes object without a version id, making the +// resulting delete marker the current version, and returns its version id. +func createDeleteMarker(client *s3.Client, bucket, object string) (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: &bucket, + Key: &object, + }) + cancel() + if err != nil { + return "", err + } + if out.DeleteMarker == nil || !*out.DeleteMarker { + return "", fmt.Errorf("expected a delete marker to be created for %v", object) + } + if getString(out.VersionId) == "" { + return "", fmt.Errorf("expected non empty delete marker versionId for %v", object) + } + + return *out.VersionId, nil +} + // objDataLen returns the data length to upload for key: a directory // object can't hold data func objDataLen(key string, lgth int64) int64 { diff --git a/tests/integration/versioning.go b/tests/integration/versioning.go index 277846f8..45f2491a 100644 --- a/tests/integration/versioning.go +++ b/tests/integration/versioning.go @@ -670,6 +670,78 @@ func Versioning_CopyObject_from_an_object_version(s *S3Conf) error { }, withVersioning(types.BucketVersioningStatusEnabled)) } +// A copy source that resolves to a delete marker is rejected: the key has no +// current version when the marker is the latest, and naming the marker by +// version id is an invalid request. Versions the marker hides stay copyable. +func Versioning_CopyObject_from_a_delete_marker(s *S3Conf) error { + testName := "Versioning_CopyObject_from_a_delete_marker" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + dstBucket, dstObj := getBucketName(), "dst-obj" + if err := setup(s, dstBucket); err != nil { + return err + } + + err := forEachKey([]string{"my-obj", "my-dir/"}, func(srcObj string) error { + srcObjVersions, err := createObjVersions(s3client, bucket, srcObj, 1) + if err != nil { + return err + } + + delMarker, err := createDeleteMarker(s3client, bucket, srcObj) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.CopyObject(ctx, &s3.CopyObjectInput{ + Bucket: &dstBucket, + Key: &dstObj, + CopySource: getPtr(fmt.Sprintf("%v/%v", bucket, srcObj)), + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchKey)); err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.CopyObject(ctx, &s3.CopyObjectInput{ + Bucket: &dstBucket, + Key: &dstObj, + CopySource: getPtr(fmt.Sprintf("%v/%v?versionId=%v", + bucket, srcObj, delMarker)), + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrCopySourceDeleteMarker)); err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.CopyObject(ctx, &s3.CopyObjectInput{ + Bucket: &dstBucket, + Key: &dstObj, + CopySource: getPtr(fmt.Sprintf("%v/%v?versionId=%v", + bucket, srcObj, getString(srcObjVersions[0].VersionId))), + }) + cancel() + if err != nil { + return err + } + + if getString(out.CopySourceVersionId) != getString(srcObjVersions[0].VersionId) { + return fmt.Errorf("expected the copy-source-version-id to be %v, instead got %v", + getString(srcObjVersions[0].VersionId), getString(out.CopySourceVersionId)) + } + + return nil + }) + if err != nil { + return err + } + + return teardown(s, dstBucket) + }, withVersioning(types.BucketVersioningStatusEnabled)) +} + func Versioning_CopyObject_special_chars(s *S3Conf) error { testName := "Versioning_CopyObject_special_chars" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -3009,6 +3081,85 @@ func Versioning_UploadPartCopy_from_an_object_version(s *S3Conf) error { }, withVersioning(types.BucketVersioningStatusEnabled)) } +// A copy source that resolves to a delete marker is rejected: the key has no +// current version when the marker is the latest, and naming the marker by +// version id is an invalid request. Versions the marker hides stay copyable. +func Versioning_UploadPartCopy_from_a_delete_marker(s *S3Conf) error { + testName := "Versioning_UploadPartCopy_from_a_delete_marker" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + return forEachKey([]string{"my-obj", "my-dir/"}, func(srcObj string) error { + dstBucket, dstObj := getBucketName(), "dst-obj" + if err := setup(s, dstBucket); err != nil { + return err + } + + srcObjVersions, err := createObjVersions(s3client, bucket, srcObj, 1) + if err != nil { + return err + } + + delMarker, err := createDeleteMarker(s3client, bucket, srcObj) + if err != nil { + return err + } + + mp, err := createMp(s3client, dstBucket, dstObj) + if err != nil { + return err + } + + partNumber := int32(1) + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.UploadPartCopy(ctx, &s3.UploadPartCopyInput{ + Bucket: &dstBucket, + Key: &dstObj, + UploadId: mp.UploadId, + PartNumber: &partNumber, + CopySource: getPtr(fmt.Sprintf("%v/%v", bucket, srcObj)), + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchKey)); err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.UploadPartCopy(ctx, &s3.UploadPartCopyInput{ + Bucket: &dstBucket, + Key: &dstObj, + UploadId: mp.UploadId, + PartNumber: &partNumber, + CopySource: getPtr(fmt.Sprintf("%v/%v?versionId=%v", + bucket, srcObj, delMarker)), + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrCopySourceDeleteMarker)); err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.UploadPartCopy(ctx, &s3.UploadPartCopyInput{ + Bucket: &dstBucket, + Key: &dstObj, + UploadId: mp.UploadId, + PartNumber: &partNumber, + CopySource: getPtr(fmt.Sprintf("%v/%v?versionId=%v", + bucket, srcObj, getString(srcObjVersions[0].VersionId))), + }) + cancel() + if err != nil { + return err + } + + if getString(out.CopySourceVersionId) != getString(srcObjVersions[0].VersionId) { + return fmt.Errorf("expected the copy-source-version-id to be %v, instead got %v", + getString(srcObjVersions[0].VersionId), getString(out.CopySourceVersionId)) + } + + return teardown(s, dstBucket) + }) + }, withVersioning(types.BucketVersioningStatusEnabled)) +} + func Versioning_Enable_object_lock(s *S3Conf) error { testName := "Versioning_Enable_object_lock" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { From ba02c766abf09da68b014366bdc5686211f6101c Mon Sep 17 00:00:00 2001 From: niksis02 Date: Sun, 20 Sep 2026 16:07:40 +0400 Subject: [PATCH 2/2] fix: stop posix rewriting the current version on a copy to itself `CopyObject` treated a destination path equal to the source path as an in-place metadata rewrite, regardless of bucket versioning. On a versioned bucket a self copy therefore edited the current version and returned its existing version id instead of creating a new one. This also defeated object lock: `CheckObjectAccess` skips the retention and legal hold checks for overwrites on version-enabled buckets because an overwrite is expected to create a new version, so a `COMPLIANCE` retained or legal held version had its metadata replaced underneath it. A self copy is now rewritten in place only when the bucket is unversioned, and otherwise goes through the regular copy path, which creates a new version and leaves the one it replaces untouched. The rejection of a self copy that replaces nothing moved out of the in-place branch and is now applied only when the copy source carries no version id. Naming a version explicitly makes the request a regular copy, which AWS accepts even with the `COPY` metadata directive, while versitygw answered `InvalidRequest`. Source tagging for a `COPY` tagging directive is now read before `PutObject` writes the destination rather than after. A self copy replaces the source object's attributes, so the later read returned nothing and the tags were dropped from the new version. --- backend/posix/posix.go | 53 +++-- backend/posix/posix_io_helpers.go | 19 ++ backend/posix/posix_io_helpers_test.go | 79 +++++++ tests/integration/group-tests.go | 6 + tests/integration/versioning.go | 275 +++++++++++++++++++++++++ 5 files changed, 416 insertions(+), 16 deletions(-) create mode 100644 backend/posix/posix_io_helpers_test.go diff --git a/backend/posix/posix.go b/backend/posix/posix.go index b3c7eaaf..165a67f9 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -6310,11 +6310,18 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput var chType types.ChecksumType dstObjdPath := joinPathWithTrailer(p.BucketPath(dstBucket), dstObject) - if dstObjdPath == objPath { - if input.MetadataDirective == types.MetadataDirectiveCopy { - return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidCopyDest) - } + // A copy of an object onto itself is rejected unless it replaces the + // object metadata. Naming a source version makes it a regular copy. + selfCopy := dstObjdPath == objPath + if selfCopy && srcVersionId == "" && + input.MetadataDirective == types.MetadataDirectiveCopy { + return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidCopyDest) + } + // In a versioned bucket a self copy creates a new version like any other + // write, so only unversioned buckets are rewritten in place. + versioned := p.versioningEnabled() && vStatus != "" + if selfCopy && !versioned { // Delete the object metadata err = p.meta.DeleteAttribute(dstBucket, dstObject, metadataHdr) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { @@ -6452,6 +6459,14 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput checksums.Algorithm = input.ChecksumAlgorithm } + // A self copy publishes the new version over the source path, which + // on Windows can't be renamed over while the source is still open. + // PutObject reads the body before publishing, so the handle is + // released as soon as the data has been staged. + if selfCopy { + srcBody = &closeOnEOFReader{r: srcBody, c: f} + } + putObjectInput := s3response.PutObjectInput{ Bucket: &dstBucket, Key: &dstObject, @@ -6488,23 +6503,29 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput putObjectInput.Tagging = input.Tagging } - res, err := p.PutObject(withCtxNoSlot(ctx), putObjectInput) - if err != nil { - return s3response.CopyObjectOutput{}, err - } - - // copy the source object tagging after the destination object - // creation, if tagging directive is "COPY" + // read the source tagging before the destination is written, as a + // self copy replaces the source object's metadata + var srcTagging []byte + var hasSrcTagging bool if input.TaggingDirective == types.TaggingDirectiveCopy { tagging, err := p.meta.RetrieveAttribute(nil, srcBucket, srcObject, tagHdr) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return s3response.CopyObjectOutput{}, fmt.Errorf("get source object tagging: %w", err) } - if err == nil { - err := p.meta.StoreAttribute(nil, dstBucket, dstObject, tagHdr, tagging) - if err != nil { - return s3response.CopyObjectOutput{}, fmt.Errorf("set destination object tagging: %w", err) - } + srcTagging, hasSrcTagging = tagging, err == nil + } + + res, err := p.PutObject(withCtxNoSlot(ctx), putObjectInput) + if err != nil { + return s3response.CopyObjectOutput{}, err + } + + // the source tagging is stored after the destination object creation, + // if tagging directive is "COPY" + if hasSrcTagging { + err := p.meta.StoreAttribute(nil, dstBucket, dstObject, tagHdr, srcTagging) + if err != nil { + return s3response.CopyObjectOutput{}, fmt.Errorf("set destination object tagging: %w", err) } } diff --git a/backend/posix/posix_io_helpers.go b/backend/posix/posix_io_helpers.go index 62eed318..76d8fd70 100644 --- a/backend/posix/posix_io_helpers.go +++ b/backend/posix/posix_io_helpers.go @@ -16,6 +16,7 @@ package posix import ( "bufio" + "errors" "io" "log" "sync" @@ -45,6 +46,24 @@ func (b *bufferedReadCloser) Close() error { return b.c.Close() } +// closeOnEOFReader closes c once r is drained, for readers whose source has +// to be released before the caller is done with the reader. +type closeOnEOFReader struct { + r io.Reader + c io.Closer + closed bool +} + +func (e *closeOnEOFReader) Read(p []byte) (int, error) { + n, err := e.r.Read(p) + if errors.Is(err, io.EOF) && !e.closed { + e.closed = true + e.c.Close() + } + + return n, err +} + var odirectUnsupportedWarnByOp sync.Map func warnODirectUnsupportedOnce(op string, err error) { diff --git a/backend/posix/posix_io_helpers_test.go b/backend/posix/posix_io_helpers_test.go new file mode 100644 index 00000000..5beb962f --- /dev/null +++ b/backend/posix/posix_io_helpers_test.go @@ -0,0 +1,79 @@ +// Copyright 2026 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package posix + +import ( + "bytes" + "io" + "strings" + "testing" +) + +type countingCloser struct { + count int +} + +func (c *countingCloser) Close() error { + c.count++ + return nil +} + +func TestCloseOnEOFReader(t *testing.T) { + c := &countingCloser{} + r := &closeOnEOFReader{r: strings.NewReader("hello"), c: c} + + buf := make([]byte, 2) + n, err := r.Read(buf) + if err != nil { + t.Fatalf("read: %v", err) + } + if n != 2 { + t.Fatalf("expected 2 bytes, got %v", n) + } + if c.count != 0 { + t.Fatalf("expected the source to stay open before EOF, closed %v times", c.count) + } + + data, err := io.ReadAll(r) + if err != nil { + t.Fatalf("read all: %v", err) + } + if !bytes.Equal(data, []byte("llo")) { + t.Fatalf("expected the remaining data to be llo, got %s", data) + } + if c.count != 1 { + t.Fatalf("expected the source to be closed once at EOF, closed %v times", c.count) + } + + // reads past EOF don't close the source again + if _, err := r.Read(buf); err != io.EOF { + t.Fatalf("expected io.EOF, got %v", err) + } + if c.count != 1 { + t.Fatalf("expected the source to be closed once, closed %v times", c.count) + } +} + +func TestCloseOnEOFReaderEmptySource(t *testing.T) { + c := &countingCloser{} + r := &closeOnEOFReader{r: strings.NewReader(""), c: c} + + if _, err := io.ReadAll(r); err != nil { + t.Fatalf("read all: %v", err) + } + if c.count != 1 { + t.Fatalf("expected the source to be closed once at EOF, closed %v times", c.count) + } +} diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index cbed13f6..6f5d8c79 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -1958,6 +1958,8 @@ func TestVersioning(ts *TestState) { ts.Run(Versioning_CopyObject_non_existing_version_id) ts.Run(Versioning_CopyObject_from_an_object_version) ts.Run(Versioning_CopyObject_from_a_delete_marker) + ts.Run(Versioning_CopyObject_to_itself) + ts.Run(Versioning_CopyObject_to_itself_from_the_current_version) if !ts.conf.windowsTests { ts.Run(Versioning_CopyObject_special_chars) } @@ -2056,6 +2058,7 @@ func TestVersioning(ts *TestState) { ts.Run(Versioning_WORM_delete_marker_locked_object_compliance_retention) ts.Run(Versioning_WORM_PutObject_overwrite_locked_object) ts.Run(Versioning_WORM_CopyObject_overwrite_locked_object) + ts.Run(Versioning_WORM_CopyObject_to_itself_locked_object) ts.Run(Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object) if !ts.conf.windowsTests { ts.Run(Versioning_WORM_remove_delete_marker_under_bucket_default_retention) @@ -3497,6 +3500,8 @@ func GetIntTests() IntTests { "Versioning_CopyObject_non_existing_version_id": Versioning_CopyObject_non_existing_version_id, "Versioning_CopyObject_from_an_object_version": Versioning_CopyObject_from_an_object_version, "Versioning_CopyObject_from_a_delete_marker": Versioning_CopyObject_from_a_delete_marker, + "Versioning_CopyObject_to_itself": Versioning_CopyObject_to_itself, + "Versioning_CopyObject_to_itself_from_the_current_version": Versioning_CopyObject_to_itself_from_the_current_version, "Versioning_CopyObject_special_chars": Versioning_CopyObject_special_chars, "Versioning_HeadObject_invalid_versionId": Versioning_HeadObject_invalid_versionId, "Versioning_HeadObject_non_existing_object_version": Versioning_HeadObject_non_existing_object_version, @@ -3578,6 +3583,7 @@ func GetIntTests() IntTests { "Versioning_WORM_delete_marker_locked_object_compliance_retention": Versioning_WORM_delete_marker_locked_object_compliance_retention, "Versioning_WORM_PutObject_overwrite_locked_object": Versioning_WORM_PutObject_overwrite_locked_object, "Versioning_WORM_CopyObject_overwrite_locked_object": Versioning_WORM_CopyObject_overwrite_locked_object, + "Versioning_WORM_CopyObject_to_itself_locked_object": Versioning_WORM_CopyObject_to_itself_locked_object, "Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object": Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object, "Versioning_WORM_remove_delete_marker_under_bucket_default_retention": Versioning_WORM_remove_delete_marker_under_bucket_default_retention, "Versioning_WORM_trailing_slash_counterpart": Versioning_WORM_trailing_slash_counterpart, diff --git a/tests/integration/versioning.go b/tests/integration/versioning.go index 45f2491a..8e71ecd5 100644 --- a/tests/integration/versioning.go +++ b/tests/integration/versioning.go @@ -742,6 +742,182 @@ func Versioning_CopyObject_from_a_delete_marker(s *S3Conf) error { }, withVersioning(types.BucketVersioningStatusEnabled)) } +// A copy of an object onto itself in a versioned bucket is an ordinary +// write: it creates a new version and leaves the one it replaces untouched. +// Without a metadata directive there is nothing to replace, so it's rejected. +func Versioning_CopyObject_to_itself(s *S3Conf) error { + testName := "Versioning_CopyObject_to_itself" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + return forEachKey([]string{"my-obj", "my-dir/"}, func(obj string) error { + // directory objects always carry the directory content-type + srcContentType, dstContentType := "text/plain", "application/json" + if strings.HasSuffix(obj, "/") { + srcContentType, dstContentType = directoryContentType, directoryContentType + } + + srcMeta := map[string]string{"key": "value"} + r, err := putObjectWithData(objDataLen(obj, 1234), &s3.PutObjectInput{ + Bucket: &bucket, + Key: &obj, + ContentType: getPtr("text/plain"), + Metadata: srcMeta, + }, s3client) + if err != nil { + return err + } + + srcVersionId := getString(r.res.VersionId) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.CopyObject(ctx, &s3.CopyObjectInput{ + Bucket: &bucket, + Key: &obj, + CopySource: getPtr(fmt.Sprintf("%v/%v", bucket, obj)), + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidCopyDest)); err != nil { + return err + } + + dstMeta := map[string]string{"new-key": "new-value"} + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.CopyObject(ctx, &s3.CopyObjectInput{ + Bucket: &bucket, + Key: &obj, + CopySource: getPtr(fmt.Sprintf("%v/%v", bucket, obj)), + MetadataDirective: types.MetadataDirectiveReplace, + ContentType: getPtr("application/json"), + Metadata: dstMeta, + }) + cancel() + if err != nil { + return err + } + + dstVersionId := getString(out.VersionId) + if dstVersionId == "" { + return fmt.Errorf("expected non empty versionId") + } + if dstVersionId == srcVersionId { + return fmt.Errorf("expected a new versionId, instead got %v", dstVersionId) + } + + // the replaced version keeps its own metadata + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &obj, + VersionId: &srcVersionId, + }) + cancel() + if err != nil { + return err + } + + if getString(res.ContentType) != srcContentType { + return fmt.Errorf("expected the source version content-type to be %v, instead got %v", + srcContentType, getString(res.ContentType)) + } + if !areMapsSame(res.Metadata, srcMeta) { + return fmt.Errorf("expected the source version metadata to be %v, instead got %v", + srcMeta, res.Metadata) + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + res, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &obj, + }) + cancel() + if err != nil { + return err + } + + if getString(res.VersionId) != dstVersionId { + return fmt.Errorf("expected the current versionId to be %v, instead got %v", + dstVersionId, getString(res.VersionId)) + } + if getString(res.ContentType) != dstContentType { + return fmt.Errorf("expected the new version content-type to be %v, instead got %v", + dstContentType, getString(res.ContentType)) + } + if !areMapsSame(res.Metadata, dstMeta) { + return fmt.Errorf("expected the new version metadata to be %v, instead got %v", + dstMeta, res.Metadata) + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + vRes, err := s3client.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{ + Bucket: &bucket, + Prefix: &obj, + }) + cancel() + if err != nil { + return err + } + + if len(vRes.Versions) != 2 { + return fmt.Errorf("expected 2 object versions, instead got %v", len(vRes.Versions)) + } + + return nil + }) + }, withVersioning(types.BucketVersioningStatusEnabled)) +} + +// Naming the current version in the copy source makes a copy onto the same +// key a regular copy, so it is accepted even without a metadata directive. +func Versioning_CopyObject_to_itself_from_the_current_version(s *S3Conf) error { + testName := "Versioning_CopyObject_to_itself_from_the_current_version" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + return forEachKey([]string{"my-obj", "my-dir/"}, func(obj string) error { + versions, err := createObjVersions(s3client, bucket, obj, 1) + if err != nil { + return err + } + + srcVersionId := getString(versions[0].VersionId) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.CopyObject(ctx, &s3.CopyObjectInput{ + Bucket: &bucket, + Key: &obj, + CopySource: getPtr(fmt.Sprintf("%v/%v?versionId=%v", bucket, obj, srcVersionId)), + }) + cancel() + if err != nil { + return err + } + + if getString(out.CopySourceVersionId) != srcVersionId { + return fmt.Errorf("expected the copy-source-version-id to be %v, instead got %v", + srcVersionId, getString(out.CopySourceVersionId)) + } + if getString(out.VersionId) == srcVersionId { + return fmt.Errorf("expected a new versionId, instead got %v", getString(out.VersionId)) + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &obj, + VersionId: &srcVersionId, + }) + cancel() + if err != nil { + return err + } + + if getString(res.VersionId) != srcVersionId { + return fmt.Errorf("expected the source version to remain, instead got %v", + getString(res.VersionId)) + } + + return nil + }) + }, withVersioning(types.BucketVersioningStatusEnabled)) +} + func Versioning_CopyObject_special_chars(s *S3Conf) error { testName := "Versioning_CopyObject_special_chars" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -4250,6 +4426,105 @@ func Versioning_WORM_CopyObject_overwrite_locked_object(s *S3Conf) error { }, withLock()) } +// A copy of a locked object onto itself creates a new version, leaving the +// locked one and its legal hold in place. +func Versioning_WORM_CopyObject_to_itself_locked_object(s *S3Conf) error { + testName := "Versioning_WORM_CopyObject_to_itself_locked_object" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + return forEachKey([]string{"my-obj", "my-dir/"}, func(obj string) error { + versions, err := createObjVersions(s3client, bucket, obj, 1) + if err != nil { + return err + } + + v := versions[0] + v.IsLatest = getPtr(false) + lockedVersionId := getString(v.VersionId) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.PutObjectLegalHold(ctx, &s3.PutObjectLegalHoldInput{ + Bucket: &bucket, + Key: &obj, + LegalHold: &types.ObjectLockLegalHold{ + Status: types.ObjectLockLegalHoldStatusOn, + }, + }) + cancel() + if err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + copyResult, err := s3client.CopyObject(ctx, &s3.CopyObjectInput{ + Bucket: &bucket, + Key: &obj, + CopySource: getPtr(fmt.Sprintf("%v/%v", bucket, obj)), + MetadataDirective: types.MetadataDirectiveReplace, + ContentType: getPtr("application/json"), + }) + cancel() + if err != nil { + return err + } + + if getString(copyResult.VersionId) == lockedVersionId { + return fmt.Errorf("expected a new versionId, instead got %v", + getString(copyResult.VersionId)) + } + + version := types.ObjectVersion{ + ETag: copyResult.CopyObjectResult.ETag, + IsLatest: getPtr(true), + Key: &obj, + Size: v.Size, + VersionId: copyResult.VersionId, + StorageClass: types.ObjectVersionStorageClassStandard, + ChecksumType: copyResult.CopyObjectResult.ChecksumType, + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{ + Bucket: &bucket, + Prefix: &obj, + }) + cancel() + if err != nil { + return err + } + + if !compareVersions([]types.ObjectVersion{version, v}, out.Versions) { + return fmt.Errorf("expected the object versions to be %v, instead got %v", + []types.ObjectVersion{version, v}, out.Versions) + } + + // the legal hold stays on the version it was set on + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + lhRes, err := s3client.GetObjectLegalHold(ctx, &s3.GetObjectLegalHoldInput{ + Bucket: &bucket, + Key: &obj, + VersionId: &lockedVersionId, + }) + cancel() + if err != nil { + return err + } + + if lhRes.LegalHold.Status != types.ObjectLockLegalHoldStatusOn { + return fmt.Errorf("expected the legal hold status to be %v, instead got %v", + types.ObjectLockLegalHoldStatusOn, lhRes.LegalHold.Status) + } + + return cleanupLockedObjects(s3client, bucket, []objToDelete{ + { + key: obj, + versionId: lockedVersionId, + removeOnlyLeglHold: true, + }, + }) + }) + }, withLock()) +} + func Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object(s *S3Conf) error { testName := "Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {