fix: copy-object with replace metadata-directive

In copy-object, if the source and destination are the same then
X-Amz-Metadata-Directive must be set to "REPLACE" in order to use
this api call to update the metadata of the object in place.

The default X-Amz-Metadata-Directive is "COPY" if not specified.
"COPY" is only valid if source and destination are not the same
object.

When "REPLACE" selected, metadata does not have to differ for the
call to be successful. The "REPLACE" always sets the incoming
metadata (even if empty or the same as the source).

Fixes #734
This commit is contained in:
Ben McClelland
2024-08-13 10:52:47 -07:00
parent 42f554b0d6
commit a36d974942
5 changed files with 82 additions and 41 deletions
+32 -41
View File
@@ -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)
+18
View File
@@ -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 {
+6
View File
@@ -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: {
+2
View File
@@ -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,
+24
View File
@@ -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 {