Skip to content

Commit

Permalink
fix: copy object missing prefix on metadata delete
Browse files Browse the repository at this point in the history
When using the REPLACE directive, we were incorrectly removing the
old metadata on the object due to missing the metadata prefix on
the key.  Fix this to remove the correct metadata before setting
new metadata.

Fixes #787
  • Loading branch information
benmcclelland committed Sep 8, 2024
1 parent 66a7879 commit ccd4166
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 10 deletions.
5 changes: 3 additions & 2 deletions backend/posix/posix.go
Original file line number Diff line number Diff line change
Expand Up @@ -2038,8 +2038,9 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3.
return &s3.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidCopyDest)
}

for key := range mdmap {
err := p.meta.DeleteAttribute(dstBucket, dstObject, key)
for k := range mdmap {
err := p.meta.DeleteAttribute(dstBucket, dstObject,
fmt.Sprintf("%v.%v", metaHdr, k))
if err != nil && !errors.Is(err, meta.ErrNoSuchKey) {
return nil, fmt.Errorf("delete user metadata: %w", err)
}
Expand Down
60 changes: 54 additions & 6 deletions tests/integration/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -4567,6 +4567,11 @@ func CopyObject_copy_to_itself_invalid_directive(s *S3Conf) error {

func CopyObject_to_itself_with_new_metadata(s *S3Conf) error {
testName := "CopyObject_to_itself_with_new_metadata"

meta := map[string]string{
"Hello": "World",
}

return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
err := putObjects(s3client, []string{obj}, bucket)
Expand All @@ -4575,19 +4580,62 @@ func CopyObject_to_itself_with_new_metadata(s *S3Conf) error {
}
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)),
Metadata: map[string]string{
"Hello": "World",
},
Bucket: &bucket,
Key: &obj,
CopySource: getPtr(fmt.Sprintf("%v/%v", bucket, obj)),
Metadata: meta,
MetadataDirective: types.MetadataDirectiveReplace,
})
cancel()
if err != nil {
return err
}

ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
resp, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
if err != nil {
return err
}

if !areMapsSame(resp.Metadata, meta) {
return fmt.Errorf("expected uploaded object metadata to be %v, instead got %v", meta, resp.Metadata)
}

// verify updating metadata has correct meta
meta = map[string]string{
"New": "Metadata",
}
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)),
Metadata: meta,
MetadataDirective: types.MetadataDirectiveReplace,
})
cancel()
if err != nil {
return err
}

ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
resp, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
if err != nil {
return err
}

if !areMapsSame(resp.Metadata, meta) {
return fmt.Errorf("expected uploaded object metadata to be %v, instead got %v", meta, resp.Metadata)
}

return nil
})
}
Expand Down
7 changes: 5 additions & 2 deletions tests/integration/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,12 +428,15 @@ func getPtr(str string) *string {
return &str
}

// mp1 needs to be the response from the server
// mp2 needs to be the expected values
// The keys from the server are always converted to lowercase
func areMapsSame(mp1, mp2 map[string]string) bool {
if len(mp1) != len(mp2) {
return false
}
for key, val := range mp1 {
if mp2[key] != val {
for key, val := range mp2 {
if mp1[strings.ToLower(key)] != val {
return false
}
}
Expand Down

0 comments on commit ccd4166

Please sign in to comment.