From 55ab629f0ff2b306bad6e906ead30e7ab3f48442 Mon Sep 17 00:00:00 2001
From: Rowan Seymour <rowanseymour@gmail.com>
Date: Fri, 26 Jul 2024 16:51:28 -0500
Subject: [PATCH] Fix s3x URL generation to not mangle slashes

---
 s3x/s3_test.go   | 10 +++++-----
 s3x/urls.go      | 11 ++++++++---
 s3x/urls_test.go |  4 ++--
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/s3x/s3_test.go b/s3x/s3_test.go
index d9e339e..7719922 100644
--- a/s3x/s3_test.go
+++ b/s3x/s3_test.go
@@ -25,16 +25,16 @@ func TestService(t *testing.T) {
 	err = svc.Test(ctx, "gocommon-tests")
 	assert.NoError(t, err)
 
-	url, err := svc.PutObject(ctx, "gocommon-tests", "hello world.txt", "text/plain", []byte("hello world"), s3.BucketCannedACLPublicRead)
+	url, err := svc.PutObject(ctx, "gocommon-tests", "1/hello world.txt", "text/plain", []byte("hello world"), s3.BucketCannedACLPublicRead)
 	assert.NoError(t, err)
-	assert.Equal(t, "http://localhost:9000/gocommon-tests/hello%20world.txt", url)
+	assert.Equal(t, "http://localhost:9000/gocommon-tests/1/hello+world.txt", url)
 
-	contentType, body, err := svc.GetObject(ctx, "gocommon-tests", "hello world.txt")
+	contentType, body, err := svc.GetObject(ctx, "gocommon-tests", "1/hello world.txt")
 	assert.NoError(t, err)
 	assert.Equal(t, "text/plain", contentType)
 	assert.Equal(t, []byte("hello world"), body)
 
-	_, err = svc.Client.DeleteObject(&s3.DeleteObjectInput{Bucket: aws.String("gocommon-tests"), Key: aws.String("hello world.txt")})
+	_, err = svc.Client.DeleteObject(&s3.DeleteObjectInput{Bucket: aws.String("gocommon-tests"), Key: aws.String("1/hello world.txt")})
 	assert.NoError(t, err)
 
 	_, err = svc.Client.DeleteBucket(&s3.DeleteBucketInput{Bucket: aws.String("gocommon-tests")})
@@ -45,5 +45,5 @@ func TestService(t *testing.T) {
 
 	aws, err := s3x.NewService("AA1234", "2345263", "us-east-1", "https://s3.amazonaws.com", false)
 	assert.NoError(t, err)
-	assert.Equal(t, "https://gocommon-tests.s3.us-east-1.amazonaws.com/hello%20world.txt", aws.ObjectURL("gocommon-tests", "hello world.txt"))
+	assert.Equal(t, "https://gocommon-tests.s3.us-east-1.amazonaws.com/1/hello+world.txt", aws.ObjectURL("gocommon-tests", "1/hello world.txt"))
 }
diff --git a/s3x/urls.go b/s3x/urls.go
index 033b316..960dedb 100644
--- a/s3x/urls.go
+++ b/s3x/urls.go
@@ -2,7 +2,7 @@ package s3x
 
 import (
 	"fmt"
-	"net/url"
+	"strings"
 )
 
 // ObjectURLer is a function that takes a key and returns the publicly accessible URL for that object
@@ -10,12 +10,17 @@ type ObjectURLer func(string, string) string
 
 func AWSURLer(region string) ObjectURLer {
 	return func(bucket, key string) string {
-		return fmt.Sprintf("https://%s.s3.%s.amazonaws.com/%s", bucket, region, url.PathEscape(key))
+		return fmt.Sprintf("https://%s.s3.%s.amazonaws.com/%s", bucket, region, escape(key))
 	}
 }
 
 func MinioURLer(endpoint string) ObjectURLer {
 	return func(bucket, key string) string {
-		return fmt.Sprintf("%s/%s/%s", endpoint, bucket, url.PathEscape(key))
+		return fmt.Sprintf("%s/%s/%s", endpoint, bucket, escape(key))
 	}
 }
+
+// can't URL escape keys because need to preserve slashes
+func escape(key string) string {
+	return strings.ReplaceAll(key, " ", "+")
+}
diff --git a/s3x/urls_test.go b/s3x/urls_test.go
index 19e95b1..4d2e6ce 100644
--- a/s3x/urls_test.go
+++ b/s3x/urls_test.go
@@ -9,8 +9,8 @@ import (
 
 func TestURLers(t *testing.T) {
 	urler := s3x.AWSURLer("us-east-1")
-	assert.Equal(t, "https://mybucket.s3.us-east-1.amazonaws.com/hello%20world.txt", urler("mybucket", "hello world.txt"))
+	assert.Equal(t, "https://mybucket.s3.us-east-1.amazonaws.com/1/hello+world.txt", urler("mybucket", "1/hello world.txt"))
 
 	urler = s3x.MinioURLer("http://localhost:9000")
-	assert.Equal(t, "http://localhost:9000/mybucket/hello%20world.txt", urler("mybucket", "hello world.txt"))
+	assert.Equal(t, "http://localhost:9000/mybucket/1/hello+world.txt", urler("mybucket", "1/hello world.txt"))
 }