diff --git a/tools/filesystem/internal/s3blob/s3/copy_object_test.go b/tools/filesystem/internal/s3blob/s3/copy_object_test.go index 968f818e..04fdb4ad 100644 --- a/tools/filesystem/internal/s3blob/s3/copy_object_test.go +++ b/tools/filesystem/internal/s3blob/s3/copy_object_test.go @@ -17,7 +17,7 @@ func TestS3CopyObject(t *testing.T) { httpClient := tests.NewClient( &tests.RequestStub{ Method: http.MethodPut, - URL: "http://test_bucket.example.com/@dst_test", + URL: "http://test_bucket.example.com/%40dst_test", Match: func(req *http.Request) bool { return tests.ExpectHeaders(req.Header, map[string]string{ "test_header": "test", diff --git a/tools/filesystem/internal/s3blob/s3/s3.go b/tools/filesystem/internal/s3blob/s3/s3.go index 3a60b358..3165888d 100644 --- a/tools/filesystem/internal/s3blob/s3/s3.go +++ b/tools/filesystem/internal/s3blob/s3/s3.go @@ -62,6 +62,13 @@ type S3 struct { } // URL constructs an S3 request URL based on the current configuration. +// +// Note that the path will be URL escaped based on the AWS [UriEncode rules] +// for broader compatibility with some providers that expect the same +// path format as the one in the canonical signed header +// (see also https://github.com/pocketbase/pocketbase/issues/7153). +// +// [UriEncode rules]: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_sigv-create-signed-request.html func (s3 *S3) URL(path string) string { scheme := "https" endpoint := strings.TrimRight(s3.Endpoint, "/") @@ -72,6 +79,23 @@ func (s3 *S3) URL(path string) string { scheme = "http" } + // to prevent double escaping we first parse/unescape it + parsed, err := url.Parse(path) + if err != nil { + // truly rare case, keep the path as it is + } else { + path = escapePath(parsed.Path) + + // the rest is usually not expected to be part of the S3 path but it is kept to avoid surprises + // (it will be further escaped if necessery by the Go HTTP client) + if parsed.RawQuery != "" { + path += "?" + parsed.RawQuery + } + if parsed.RawFragment != "" { + path += "#" + parsed.RawFragment + } + } + path = strings.TrimLeft(path, "/") if s3.UsePathStyle { diff --git a/tools/filesystem/internal/s3blob/s3/s3_test.go b/tools/filesystem/internal/s3blob/s3/s3_test.go index c2d76592..edb1cc48 100644 --- a/tools/filesystem/internal/s3blob/s3/s3_test.go +++ b/tools/filesystem/internal/s3blob/s3/s3_test.go @@ -13,6 +13,12 @@ import ( func TestS3URL(t *testing.T) { t.Parallel() + path := "/test_key/a/b c@d?a=@1&b=!2#@a b c" + + // note: query params and fragments are kept as it is + // since they are later escaped if necessery by the Go HTTP client + expectedPath := "/test_key/a/b%20c%40d?a=@1&b=!2#@a b c" + scenarios := []struct { name string s3Client *s3.S3 @@ -27,7 +33,7 @@ func TestS3URL(t *testing.T) { AccessKey: "123", SecretKey: "abc", }, - "https://test_bucket.example.com/test_key/a/b/c?q=1", + "https://test_bucket.example.com" + expectedPath, }, { "with https schema", @@ -38,7 +44,7 @@ func TestS3URL(t *testing.T) { AccessKey: "123", SecretKey: "abc", }, - "https://test_bucket.example.com/test_key/a/b/c?q=1", + "https://test_bucket.example.com" + expectedPath, }, { "with http schema", @@ -49,7 +55,7 @@ func TestS3URL(t *testing.T) { AccessKey: "123", SecretKey: "abc", }, - "http://test_bucket.example.com/test_key/a/b/c?q=1", + "http://test_bucket.example.com" + expectedPath, }, { "path style addressing (non-explicit schema)", @@ -61,7 +67,7 @@ func TestS3URL(t *testing.T) { SecretKey: "abc", UsePathStyle: true, }, - "https://example.com/test_bucket/test_key/a/b/c?q=1", + "https://example.com/test_bucket" + expectedPath, }, { "path style addressing (explicit schema)", @@ -73,13 +79,13 @@ func TestS3URL(t *testing.T) { SecretKey: "abc", UsePathStyle: true, }, - "http://example.com/test_bucket/test_key/a/b/c?q=1", + "http://example.com/test_bucket" + expectedPath, }, } for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { - result := s.s3Client.URL("/test_key/a/b/c?q=1") + result := s.s3Client.URL(path) if result != s.expected { t.Fatalf("Expected URL\n%s\ngot\n%s", s.expected, result) } @@ -158,7 +164,7 @@ func TestS3SignAndSend(t *testing.T) { }, { "minimal with special characters", - "/ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.~!@#$^&*()=/@sub?a=1&@b=@2", + "/ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789 -_.~!@&*():=$()?a=1&@b=@2#@a b c", func(req *http.Request) { req.Header.Set("x-amz-date", "20250102T150405Z") }, @@ -170,11 +176,11 @@ func TestS3SignAndSend(t *testing.T) { SecretKey: "def", Client: tests.NewClient(&tests.RequestStub{ Method: http.MethodGet, - URL: "https://test_bucket.example.com/ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.~!@#$%5E&*()=/@sub?a=1&@b=@2", + URL: "https://test_bucket.example.com/ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789%20-_.~%21%40%26%2A%28%29%3A%3D%24%28%29?a=1&@b=@2#@a%20b%20c", Response: testResponse(), Match: func(req *http.Request) bool { return tests.ExpectHeaders(req.Header, map[string]string{ - "Authorization": "AWS4-HMAC-SHA256 Credential=456/20250102/test_region/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=e0001982deef1652704f74503203e77d83d4c88369421f9fca644d96f2a62a3c", + "Authorization": "AWS4-HMAC-SHA256 Credential=456/20250102/test_region/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=9458a033554f52913801b3de16f54409b36ed25c6da3aed14e64439500e2c5e1", "Host": "test_bucket.example.com", "X-Amz-Content-Sha256": "UNSIGNED-PAYLOAD", "X-Amz-Date": "20250102T150405Z",