Merge pull request #735 from versity/ben/copy_object_meta

fix: copy-object with replace metadata-directive
This commit is contained in:
Ben McClelland
2024-08-13 14:16:42 -07:00
committed by GitHub
7 changed files with 85 additions and 44 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)
+1 -1
View File
@@ -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
+19 -1
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 {
@@ -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,
+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 {
+1 -1
View File
@@ -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))
}
}