From 77aa4366b549a8ef17910fe1c6767820961ea06b Mon Sep 17 00:00:00 2001
From: Ben McClelland <ben.mcclelland@versity.com>
Date: Thu, 22 Aug 2024 10:06:38 -0700
Subject: [PATCH] fix: unescape copy source before handing to backend

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
---
 s3api/controllers/base.go  | 40 ++++++++++++++++++++++++++++++++++++--
 tests/integration/tests.go |  2 +-
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go
index cfda9724..8804c3a1 100644
--- a/s3api/controllers/base.go
+++ b/s3api/controllers/base.go
@@ -22,6 +22,7 @@ import (
 	"io"
 	"log"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 	"time"
@@ -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 {
@@ -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,
@@ -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,
diff --git a/tests/integration/tests.go b/tests/integration/tests.go
index 0f5d7614..7b87b521 100644
--- a/tests/integration/tests.go
+++ b/tests/integration/tests.go
@@ -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 {