diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 2143506f..25c30b70 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -805,20 +805,6 @@ func (p *Posix) loadUserMetaData(bucket, object string, m map[string]string) (st return contentType, contentEncoding } -func compareUserMetadata(meta1, meta2 map[string]string) bool { - if len(meta1) != len(meta2) { - return false - } - - for key, val := range meta1 { - if meta2[key] != val { - return false - } - } - - return true -} - func isValidMeta(val string) bool { if strings.HasPrefix(val, metaHdr) { return true @@ -2012,42 +1998,47 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3. return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } - meta := make(map[string]string) - p.loadUserMetaData(srcBucket, srcObject, meta) + mdmap := make(map[string]string) + p.loadUserMetaData(srcBucket, srcObject, mdmap) + + var etag string dstObjdPath := filepath.Join(dstBucket, dstObject) if dstObjdPath == objPath { - if compareUserMetadata(meta, input.Metadata) { + if input.MetadataDirective == types.MetadataDirectiveCopy { return &s3.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidCopyDest) - } else { - for key := range meta { - err := p.meta.DeleteAttribute(dstBucket, dstObject, key) - if err != nil { - return nil, fmt.Errorf("delete user metadata: %w", err) - } + } + + for key := range mdmap { + err := p.meta.DeleteAttribute(dstBucket, dstObject, key) + if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { + return nil, fmt.Errorf("delete user metadata: %w", err) } - for k, v := range input.Metadata { - err := p.meta.StoreAttribute(dstBucket, dstObject, - fmt.Sprintf("%v.%v", metaHdr, k), []byte(v)) - if err != nil { - return nil, fmt.Errorf("set user attr %q: %w", k, err) - } + } + for k, v := range input.Metadata { + err := p.meta.StoreAttribute(dstBucket, dstObject, + fmt.Sprintf("%v.%v", metaHdr, k), []byte(v)) + if err != nil { + return nil, fmt.Errorf("set user attr %q: %w", k, err) } } - } - contentLength := fi.Size() + b, _ := p.meta.RetrieveAttribute(dstBucket, dstObject, etagkey) + etag = string(b) + } else { + contentLength := fi.Size() - etag, err := p.PutObject(ctx, - &s3.PutObjectInput{ - Bucket: &dstBucket, - Key: &dstObject, - Body: f, - ContentLength: &contentLength, - Metadata: meta, - }) - if err != nil { - return nil, err + etag, err = p.PutObject(ctx, + &s3.PutObjectInput{ + Bucket: &dstBucket, + Key: &dstObject, + Body: f, + ContentLength: &contentLength, + Metadata: input.Metadata, + }) + if err != nil { + return nil, err + } } fi, err = os.Stat(dstObjdPath) diff --git a/backend/s3proxy/s3.go b/backend/s3proxy/s3.go index f2e2cdf7..9a9533c1 100644 --- a/backend/s3proxy/s3.go +++ b/backend/s3proxy/s3.go @@ -654,7 +654,7 @@ func (s *S3Proxy) ChangeBucketOwner(ctx context.Context, bucket string, acl []by return err } defer resp.Body.Close() - return fmt.Errorf(string(body)) + return fmt.Errorf("%v", string(body)) } return nil diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index b90f2a70..cfda9724 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -1533,6 +1533,7 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { copySrcModifSince := ctx.Get("X-Amz-Copy-Source-If-Modified-Since") copySrcUnmodifSince := ctx.Get("X-Amz-Copy-Source-If-Unmodified-Since") copySrcRange := ctx.Get("X-Amz-Copy-Source-Range") + directive := ctx.Get("X-Amz-Metadata-Directive") // Permission headers acl := ctx.Get("X-Amz-Acl") @@ -2054,6 +2055,22 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { metadata := utils.GetUserMetaData(&ctx.Request().Header) + if directive != "" && directive != "COPY" && directive != "REPLACE" { + return SendXMLResponse(ctx, nil, + s3err.GetAPIError(s3err.ErrInvalidMetadataDirective), + &MetaOpts{ + Logger: c.logger, + MetricsMng: c.mm, + Action: metrics.ActionCopyObject, + BucketOwner: parsedAcl.Owner, + }) + } + + metaDirective := types.MetadataDirectiveCopy + if directive == "REPLACE" { + metaDirective = types.MetadataDirectiveReplace + } + res, err := c.be.CopyObject(ctx.Context(), &s3.CopyObjectInput{ Bucket: &bucket, @@ -2065,6 +2082,7 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { CopySourceIfUnmodifiedSince: umtime, ExpectedBucketOwner: &acct.Access, Metadata: metadata, + MetadataDirective: metaDirective, StorageClass: types.StorageClass(storageClass), }) if err == nil { @@ -2783,7 +2801,7 @@ func (c S3ApiController) CreateActions(ctx *fiber.Ctx) error { if ctx.Request().URI().QueryArgs().Has("restore") { var restoreRequest types.RestoreRequest if err := xml.Unmarshal(ctx.Body(), &restoreRequest); err != nil { - if !errors.Is(io.EOF, err) { + if !errors.Is(err, io.EOF) { return SendResponse(ctx, s3err.GetAPIError(s3err.ErrMalformedXML), &MetaOpts{ Logger: c.logger, diff --git a/s3err/s3err.go b/s3err/s3err.go index 9b4b2629..8433ca94 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -130,6 +130,7 @@ const ( ErrMalformedACL ErrUnexpectedContent ErrMissingSecurityHeader + ErrInvalidMetadataDirective // Non-AWS errors ErrExistingObjectIsDirectory @@ -514,6 +515,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "Your request was missing a required header", HTTPStatusCode: http.StatusNotFound, }, + ErrInvalidMetadataDirective: { + Code: "InvalidArgument", + Description: "Unknown metadata directive.", + HTTPStatusCode: http.StatusBadRequest, + }, // non aws errors ErrExistingObjectIsDirectory: { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index f0692d4a..f8ee0853 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -199,6 +199,7 @@ func TestCopyObject(s *S3Conf) { CopyObject_non_existing_dst_bucket(s) CopyObject_not_owned_source_bucket(s) CopyObject_copy_to_itself(s) + CopyObject_copy_to_itself_invalid_directive(s) CopyObject_to_itself_with_new_metadata(s) CopyObject_CopySource_starting_with_slash(s) CopyObject_non_existing_dir_object(s) @@ -620,6 +621,7 @@ func GetIntTests() IntTests { "CopyObject_non_existing_dst_bucket": CopyObject_non_existing_dst_bucket, "CopyObject_not_owned_source_bucket": CopyObject_not_owned_source_bucket, "CopyObject_copy_to_itself": CopyObject_copy_to_itself, + "CopyObject_copy_to_itself_invalid_directive": CopyObject_copy_to_itself_invalid_directive, "CopyObject_to_itself_with_new_metadata": CopyObject_to_itself_with_new_metadata, "CopyObject_CopySource_starting_with_slash": CopyObject_CopySource_starting_with_slash, "CopyObject_non_existing_dir_object": CopyObject_non_existing_dir_object, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index a4c1ab9d..0f5d7614 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -4268,6 +4268,29 @@ func CopyObject_copy_to_itself(s *S3Conf) error { }) } +func CopyObject_copy_to_itself_invalid_directive(s *S3Conf) error { + testName := "CopyObject_copy_to_itself_invalid_directive" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-obj" + err := putObjects(s3client, []string{obj}, bucket) + if err != nil { + return err + } + 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)), + MetadataDirective: types.MetadataDirective("invalid"), + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidMetadataDirective)); err != nil { + return err + } + return nil + }) +} + func CopyObject_to_itself_with_new_metadata(s *S3Conf) error { testName := "CopyObject_to_itself_with_new_metadata" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -4284,6 +4307,7 @@ func CopyObject_to_itself_with_new_metadata(s *S3Conf) error { Metadata: map[string]string{ "Hello": "World", }, + MetadataDirective: types.MetadataDirectiveReplace, }) cancel() if err != nil { diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 785ee88b..a4d3d9d1 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -613,7 +613,7 @@ func changeBucketsOwner(s *S3Conf, buckets []string, owner string) error { return err } if !strings.Contains(string(out), "Bucket owner has been updated successfully") { - return fmt.Errorf(string(out)) + return fmt.Errorf("%v", string(out)) } }