Skip to content

Commit

Permalink
fix: unescape copy source before handing to backend
Browse files Browse the repository at this point in the history
We were handing the URL escaped string to the backend as the
copysource which includes "%<hex>" for spaces and other special
characters. The backend would then interpret this as the source
path. This fixes the copyobject and upload part copy.

Fixes #749
  • Loading branch information
benmcclelland committed Aug 22, 2024
1 parent 2aef5e4 commit 77aa436
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 3 deletions.
40 changes: 38 additions & 2 deletions s3api/controllers/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"io"
"log"
"net/http"
"net/url"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -1733,6 +1734,24 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error {
if ctx.Request().URI().QueryArgs().Has("uploadId") &&
ctx.Request().URI().QueryArgs().Has("partNumber") &&
copySource != "" {

cs := copySource
copySource, err := url.QueryUnescape(copySource)
if err != nil {
if c.debug {
log.Printf("error unescaping copy source %q: %v",
cs, err)
}
return SendXMLResponse(ctx, nil,
s3err.GetAPIError(s3err.ErrInvalidCopySource),
&MetaOpts{
Logger: c.logger,
MetricsMng: c.mm,
Action: metrics.ActionUploadPartCopy,
BucketOwner: parsedAcl.Owner,
})
}

partNumber := int32(ctx.QueryInt("partNumber", -1))
if partNumber < 1 || partNumber > 10000 {
if c.debug {
Expand All @@ -1748,7 +1767,7 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error {
})
}

err := auth.VerifyObjectCopyAccess(ctx.Context(), c.be, copySource,
err = auth.VerifyObjectCopyAccess(ctx.Context(), c.be, copySource,
auth.AccessOptions{
Acl: parsedAcl,
AclPermission: types.PermissionWrite,
Expand Down Expand Up @@ -1994,7 +2013,24 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error {
}

if copySource != "" {
err := auth.VerifyObjectCopyAccess(ctx.Context(), c.be, copySource,
cs := copySource
copySource, err := url.QueryUnescape(copySource)
if err != nil {
if c.debug {
log.Printf("error unescaping copy source %q: %v",
cs, err)
}
return SendXMLResponse(ctx, nil,
s3err.GetAPIError(s3err.ErrInvalidCopySource),
&MetaOpts{
Logger: c.logger,
MetricsMng: c.mm,
Action: metrics.ActionCopyObject,
BucketOwner: parsedAcl.Owner,
})
}

err = auth.VerifyObjectCopyAccess(ctx.Context(), c.be, copySource,
auth.AccessOptions{
Acl: parsedAcl,
AclPermission: types.PermissionWrite,
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -4421,7 +4421,7 @@ func CopyObject_non_existing_dir_object(s *S3Conf) error {
func CopyObject_success(s *S3Conf) error {
testName := "CopyObject_success"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
dataLength, obj := int64(1234567), "my-obj"
dataLength, obj := int64(1234567), "my obj with spaces"
dstBucket := getBucketName()
err := setup(s, dstBucket)
if err != nil {
Expand Down

0 comments on commit 77aa436

Please sign in to comment.