diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 7846c1f..25e2371 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -25,6 +25,7 @@ import ( "fmt" "io" "math" + "net/url" "os" "path/filepath" "sort" @@ -950,7 +951,7 @@ func (az *Azure) CopyObject(ctx context.Context, input s3response.CopyObjectInpu } } - if strings.Join([]string{*input.Bucket, *input.Key}, "/") == *input.CopySource { + if srcBucket == *input.Bucket && srcObj == *input.Key { if input.MetadataDirective != types.MetadataDirectiveReplace { return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidCopyDest) } @@ -1858,7 +1859,10 @@ func (az *Azure) getContainerURL(cntr string) string { } func (az *Azure) getBlobURL(cntr, blb string) string { - return fmt.Sprintf("%v/%v", az.getContainerURL(cntr), blb) + // URL-encode the blob name to handle special characters like {, }, #, spaces, etc. + // Use PathEscape to encode the blob name while preserving forward slashes + encodedBlob := url.PathEscape(blb) + return fmt.Sprintf("%v/%v", az.getContainerURL(cntr), encodedBlob) } func (az *Azure) getBlobClient(cntr, blb string) (*blob.Client, error) { diff --git a/backend/common.go b/backend/common.go index 44bd0d9..95846d9 100644 --- a/backend/common.go +++ b/backend/common.go @@ -247,6 +247,17 @@ func ParseCopySource(copySourceHeader string) (string, string, string, error) { return "", "", "", s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket) } + var err error + // URL-decode the bucket and object names to handle special characters + srcBucket, err = url.QueryUnescape(srcBucket) + if err != nil { + return "", "", "", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding) + } + srcObject, err = url.QueryUnescape(srcObject) + if err != nil { + return "", "", "", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding) + } + return srcBucket, srcObject, versionId, nil } diff --git a/backend/common_test.go b/backend/common_test.go new file mode 100644 index 0000000..c448103 --- /dev/null +++ b/backend/common_test.go @@ -0,0 +1,150 @@ +// 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 backend + +import ( + "errors" + "testing" + + "github.com/versity/versitygw/s3err" +) + +func TestParseCopySource(t *testing.T) { + tests := []struct { + name string + copySourceHeader string + wantBucket string + wantObject string + wantVersionId string + wantErr bool + wantErrCode s3err.ErrorCode + }{ + { + name: "simple path", + copySourceHeader: "mybucket/myobject", + wantBucket: "mybucket", + wantObject: "myobject", + wantVersionId: "", + wantErr: false, + }, + { + name: "path with leading slash", + copySourceHeader: "/mybucket/myobject", + wantBucket: "mybucket", + wantObject: "myobject", + wantVersionId: "", + wantErr: false, + }, + { + name: "path with versionId", + copySourceHeader: "mybucket/myobject?versionId=abc123", + wantBucket: "mybucket", + wantObject: "myobject", + wantVersionId: "abc123", + wantErr: false, + }, + { + name: "URL-encoded curly braces", + copySourceHeader: "mybucket/myfolder/%7Be14c392b-09ad-4188-85f4-b779af00fb88%7D/testfile", + wantBucket: "mybucket", + wantObject: "myfolder/{e14c392b-09ad-4188-85f4-b779af00fb88}/testfile", + wantVersionId: "", + wantErr: false, + }, + { + name: "URL-encoded space", + copySourceHeader: "mybucket/my%20object", + wantBucket: "mybucket", + wantObject: "my object", + wantVersionId: "", + wantErr: false, + }, + { + name: "URL-encoded special chars", + copySourceHeader: "mybucket/obj%23%24%25%26", + wantBucket: "mybucket", + wantObject: "obj#$%&", + wantVersionId: "", + wantErr: false, + }, + { + name: "URL-encoded path with versionId", + copySourceHeader: "mybucket/my%20folder/my%20object?versionId=xyz789", + wantBucket: "mybucket", + wantObject: "my folder/my object", + wantVersionId: "xyz789", + wantErr: false, + }, + { + name: "invalid URL encoding - incomplete escape", + copySourceHeader: "mybucket/object%", + wantBucket: "", + wantObject: "", + wantVersionId: "", + wantErr: true, + wantErrCode: s3err.ErrInvalidCopySourceEncoding, + }, + { + name: "invalid URL encoding - invalid hex", + copySourceHeader: "mybucket/object%ZZ", + wantBucket: "", + wantObject: "", + wantVersionId: "", + wantErr: true, + wantErrCode: s3err.ErrInvalidCopySourceEncoding, + }, + { + name: "missing object", + copySourceHeader: "mybucket", + wantBucket: "", + wantObject: "", + wantVersionId: "", + wantErr: true, + wantErrCode: s3err.ErrInvalidCopySourceBucket, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotBucket, gotObject, gotVersionId, err := ParseCopySource(tt.copySourceHeader) + + if tt.wantErr { + if err == nil { + t.Errorf("ParseCopySource() error = nil, wantErr %v", tt.wantErr) + return + } + if !errors.Is(err, s3err.GetAPIError(tt.wantErrCode)) { + t.Errorf("ParseCopySource() error = %v, want error code %v", err, tt.wantErrCode) + } + return + } + + if err != nil { + t.Errorf("ParseCopySource() unexpected error = %v", err) + return + } + + if gotBucket != tt.wantBucket { + t.Errorf("ParseCopySource() gotBucket = %v, want %v", gotBucket, tt.wantBucket) + } + if gotObject != tt.wantObject { + t.Errorf("ParseCopySource() gotObject = %v, want %v", gotObject, tt.wantObject) + } + if gotVersionId != tt.wantVersionId { + t.Errorf("ParseCopySource() gotVersionId = %v, want %v", gotVersionId, tt.wantVersionId) + } + }) + } +} diff --git a/tests/integration/CopyObject.go b/tests/integration/CopyObject.go index ca764a4..9952708 100644 --- a/tests/integration/CopyObject.go +++ b/tests/integration/CopyObject.go @@ -1498,6 +1498,96 @@ func CopyObject_success(s *S3Conf) error { }) } +func CopyObject_with_special_characters(s *S3Conf) error { + testName := "CopyObject_with_special_characters" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + // Test copying objects with special characters that need URL encoding + // like curly braces, spaces, and other reserved characters + testCases := []struct { + name string + srcKey string + dstKey string + }{ + { + name: "curly braces", + srcKey: "myfolder/{e14c392b-09ad-4188-85f4-b779af00fb88}/testfile", + dstKey: "myfolder/{e14c392b-09ad-4188-85f4-b779af00fb88}/copy", + }, + { + name: "multiple special chars", + srcKey: "folder/{id}/file #1", + dstKey: "folder/{id}/file #1 copy", + }, + { + name: "ampersand and hash", + srcKey: "file&with#special", + dstKey: "file&with#special-copy", + }, + { + name: "spaces and brackets", + srcKey: "my file [2024].txt", + dstKey: "my file [2024] copy.txt", + }, + } + + for _, tc := range testCases { + // Create source object + dataLength := int64(100) + r, err := putObjectWithData(dataLength, &s3.PutObjectInput{ + Bucket: &bucket, + Key: &tc.srcKey, + }, s3client) + if err != nil { + return fmt.Errorf("%s: failed to put source object: %w", tc.name, err) + } + + // Copy the object + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.CopyObject(ctx, &s3.CopyObjectInput{ + Bucket: &bucket, + Key: &tc.dstKey, + CopySource: getPtr(fmt.Sprintf("%v/%v", bucket, tc.srcKey)), + }) + cancel() + if err != nil { + return fmt.Errorf("%s: failed to copy object: %w", tc.name, err) + } + + // Verify the copied object + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.GetObject(ctx, &s3.GetObjectInput{ + Bucket: &bucket, + Key: &tc.dstKey, + }) + cancel() + if err != nil { + return fmt.Errorf("%s: failed to get copied object: %w", tc.name, err) + } + + if out.ContentLength == nil { + return fmt.Errorf("%s: expected content-length to be set, instead got nil", tc.name) + } + if *out.ContentLength != dataLength { + return fmt.Errorf("%s: expected content-length %v, instead got %v", + tc.name, dataLength, *out.ContentLength) + } + + bdy, err := io.ReadAll(out.Body) + if err != nil { + return fmt.Errorf("%s: failed to read body: %w", tc.name, err) + } + out.Body.Close() + + outCsum := sha256.Sum256(bdy) + if outCsum != r.csum { + return fmt.Errorf("%s: invalid object data, checksum mismatch", tc.name) + } + } + + return nil + }) +} + func CopyObject_object_acl_not_supported(s *S3Conf) error { testName := "CopyObject_object_acl_not_supported" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 942671c..5e9dd12 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -347,6 +347,7 @@ func TestCopyObject(ts *TestState) { // azure doesn't support these metadata characters ts.Run(CopyObject_with_metadata) } + ts.Run(CopyObject_with_special_characters) ts.Run(CopyObject_success) } @@ -1404,6 +1405,7 @@ func GetIntTests() IntTests { "CopyObject_should_copy_the_existing_checksum": CopyObject_should_copy_the_existing_checksum, "CopyObject_should_replace_the_existing_checksum": CopyObject_should_replace_the_existing_checksum, "CopyObject_to_itself_by_replacing_the_checksum": CopyObject_to_itself_by_replacing_the_checksum, + "CopyObject_with_special_characters": CopyObject_with_special_characters, "CopyObject_success": CopyObject_success, "PutObjectTagging_non_existing_object": PutObjectTagging_non_existing_object, "PutObjectTagging_long_tags": PutObjectTagging_long_tags,