From 20940f0b4661cba30ab377598d2afa226bdc9e1b Mon Sep 17 00:00:00 2001 From: Christophe Vu-Brugier Date: Wed, 28 Aug 2024 11:19:39 +0200 Subject: [PATCH] fix: handle "x-amz-copy-source" header starting with '/' in s3api The "x-amz-copy-source" header may start with '/' as observed with WinSCP. However, '/' is also the separator between the bucket and the object path in "x-amz-copy-source". Consider the following code in VerifyObjectCopyAccess(): srcBucket, srcObject, found := strings.Cut(copySource, "/") If `copySource` starts with '/', then `srcBucket` is set to an empty string. Later, an error is returned because bucket "" does not exist. This issue was fixed in the Posix and Azure backends by the following commit: * 5e484f2 fix: Fixed CopySource parsing to handle the values starting with '/' in CopyObject action in posix and azure backends. But the issue was not fixed in `VerifyObjectCopyAccess`. This commit sanitizes "x-amz-copy-source" right after the header is extracted in `s3api/controllers/base.go`. This ensures that the `CopySource` argument passed to the backend functions UploadPartCopy() and CopyObject() does not start with '/'. Since the backends no longer need to strip away any leading '/' in `CopySource`, the parts of commit 5e484f2 modifying the Posix and Azure backends are reverted. Fixes issue #773. Signed-off-by: Christophe Vu-Brugier --- backend/azure/azure.go | 9 ++------- backend/posix/posix.go | 8 +------- s3api/controllers/base.go | 3 +++ 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 8d408e3f..b5716ebb 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -692,12 +692,7 @@ func (az *Azure) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3 return nil, err } - cpSrc := *input.CopySource - if cpSrc[0] == '/' { - cpSrc = cpSrc[1:] - } - - if strings.Join([]string{*input.Bucket, *input.Key}, "/") == cpSrc && isMetaSame(mdmap, input.Metadata) { + if strings.Join([]string{*input.Bucket, *input.Key}, "/") == *input.CopySource && isMetaSame(mdmap, input.Metadata) { return nil, s3err.GetAPIError(s3err.ErrInvalidCopyDest) } @@ -711,7 +706,7 @@ func (az *Azure) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3 return nil, err } - resp, err := bclient.CopyFromURL(ctx, az.serviceURL+"/"+cpSrc, &blob.CopyFromURLOptions{ + resp, err := bclient.CopyFromURL(ctx, az.serviceURL+"/"+*input.CopySource, &blob.CopyFromURLOptions{ BlobTags: tags, Metadata: parseMetadata(input.Metadata), }) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 850de7c2..11aaef85 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1983,13 +1983,7 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3. if input.ExpectedBucketOwner == nil { return nil, s3err.GetAPIError(s3err.ErrInvalidRequest) } - - cpSrc := *input.CopySource - if cpSrc[0] == '/' { - cpSrc = cpSrc[1:] - } - - srcBucket, srcObject, ok := strings.Cut(cpSrc, "/") + srcBucket, srcObject, ok := strings.Cut(*input.CopySource, "/") if !ok { return nil, s3err.GetAPIError(s3err.ErrInvalidCopySource) } diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 4fd6fa6e..9415aab9 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -1535,6 +1535,9 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { // Copy source headers copySource := ctx.Get("X-Amz-Copy-Source") + if len(copySource) > 0 && copySource[0] == '/' { + copySource = copySource[1:] + } copySrcIfMatch := ctx.Get("X-Amz-Copy-Source-If-Match") copySrcIfNoneMatch := ctx.Get("X-Amz-Copy-Source-If-None-Match") copySrcModifSince := ctx.Get("X-Amz-Copy-Source-If-Modified-Since")