mirror of
https://github.com/versity/versitygw.git
synced 2026-04-19 12:15:02 +00:00
Merge pull request #1834 from versity/ben/copy-source-decoding
fix: CopyObject with URL-encoded special chars
This commit is contained in:
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
150
backend/common_test.go
Normal file
150
backend/common_test.go
Normal file
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user