From d2df00a409eacc50a52999b6b6f65be878908f4f Mon Sep 17 00:00:00 2001
From: jonaustin09 <jonaustin460@gmail.com>
Date: Wed, 2 Oct 2024 11:40:25 -0400
Subject: [PATCH] fix: Fixed CopyObject copy-source parsing to handle object
 names with special characters

---
 backend/common.go                | 16 +++++-----
 s3err/s3err.go                   |  2 +-
 tests/integration/group-tests.go |  2 ++
 tests/integration/tests.go       | 53 ++++++++++++++++++++++++++++++++
 4 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/backend/common.go b/backend/common.go
index 2f82f4f8..a3a54a9b 100644
--- a/backend/common.go
+++ b/backend/common.go
@@ -116,15 +116,13 @@ func ParseCopySource(copySourceHeader string) (string, string, string, error) {
 		copySourceHeader = copySourceHeader[1:]
 	}
 
-	cSplitted := strings.Split(copySourceHeader, "?")
-	copySource := cSplitted[0]
-	var versionId string
-	if len(cSplitted) > 1 {
-		versionIdParts := strings.Split(cSplitted[1], "=")
-		if len(versionIdParts) != 2 || versionIdParts[0] != "versionId" {
-			return "", "", "", s3err.GetAPIError(s3err.ErrInvalidRequest)
-		}
-		versionId = versionIdParts[1]
+	var copySource, versionId string
+	i := strings.LastIndex(copySourceHeader, "?versionId=")
+	if i == -1 {
+		copySource = copySourceHeader
+	} else {
+		copySource = copySourceHeader[:i]
+		versionId = copySourceHeader[i+11:]
 	}
 
 	srcBucket, srcObject, ok := strings.Cut(copySource, "/")
diff --git a/s3err/s3err.go b/s3err/s3err.go
index dac0e295..57050316 100644
--- a/s3err/s3err.go
+++ b/s3err/s3err.go
@@ -356,7 +356,7 @@ var errorCodeResponse = map[ErrorCode]APIError{
 	},
 	ErrInvalidAccessKeyID: {
 		Code:           "InvalidAccessKeyId",
-		Description:    "The access key ID you provided does not exist in our records.",
+		Description:    "The AWS Access Key Id you provided does not exist in our records.",
 		HTTPStatusCode: http.StatusForbidden,
 	},
 	ErrRequestNotReadyYet: {
diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go
index a46582aa..7ac59886 100644
--- a/tests/integration/group-tests.go
+++ b/tests/integration/group-tests.go
@@ -528,6 +528,7 @@ func TestVersioning(s *S3Conf) {
 	Versioning_CopyObject_success(s)
 	Versioning_CopyObject_non_existing_version_id(s)
 	Versioning_CopyObject_from_an_object_version(s)
+	Versioning_CopyObject_special_chars(s)
 	// HeadObject action
 	Versioning_HeadObject_invalid_versionId(s)
 	Versioning_HeadObject_success(s)
@@ -895,6 +896,7 @@ func GetIntTests() IntTests {
 		"Versioning_CopyObject_success":                                       Versioning_CopyObject_success,
 		"Versioning_CopyObject_non_existing_version_id":                       Versioning_CopyObject_non_existing_version_id,
 		"Versioning_CopyObject_from_an_object_version":                        Versioning_CopyObject_from_an_object_version,
+		"Versioning_CopyObject_special_chars":                                 Versioning_CopyObject_special_chars,
 		"Versioning_HeadObject_invalid_versionId":                             Versioning_HeadObject_invalid_versionId,
 		"Versioning_HeadObject_success":                                       Versioning_HeadObject_success,
 		"Versioning_HeadObject_delete_marker":                                 Versioning_HeadObject_delete_marker,
diff --git a/tests/integration/tests.go b/tests/integration/tests.go
index de44a565..497abc3a 100644
--- a/tests/integration/tests.go
+++ b/tests/integration/tests.go
@@ -10673,6 +10673,59 @@ func Versioning_CopyObject_from_an_object_version(s *S3Conf) error {
 	}, withVersioning())
 }
 
+func Versioning_CopyObject_special_chars(s *S3Conf) error {
+	testName := "Versioning_CopyObject_special_chars"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		srcObj, dstBucket, dstObj := "foo?bar", getBucketName(), "bar&foo"
+		err := setup(s, dstBucket)
+		if err != nil {
+			return err
+		}
+
+		srcObjVersions, err := createObjVersions(s3client, bucket, srcObj, 1)
+		if err != nil {
+			return err
+		}
+
+		srcObjVersionId := *srcObjVersions[0].VersionId
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		res, err := s3client.CopyObject(ctx, &s3.CopyObjectInput{
+			Bucket:     &bucket,
+			Key:        &dstObj,
+			CopySource: getPtr(fmt.Sprintf("%v/%v?versionId=%v", bucket, srcObj, srcObjVersionId)),
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		if res.VersionId == nil || *res.VersionId == "" {
+			return fmt.Errorf("expected non empty versionId")
+		}
+		if *res.CopySourceVersionId != srcObjVersionId {
+			return fmt.Errorf("expected the SourceVersionId to be %v, instead got %v", srcObjVersionId, *res.CopySourceVersionId)
+		}
+
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		out, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{
+			Bucket:    &bucket,
+			Key:       &dstObj,
+			VersionId: res.VersionId,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		if *out.VersionId != *res.VersionId {
+			return fmt.Errorf("expected the copied object versionId to be %v, instead got %v", *res.VersionId, *out.VersionId)
+		}
+
+		return nil
+	}, withVersioning())
+}
+
 func Versioning_HeadObject_invalid_versionId(s *S3Conf) error {
 	testName := "Versioning_HeadObject_invalid_versionId"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {