From c97ddac6dfb278134025d0f0e6998433d2182845 Mon Sep 17 00:00:00 2001 From: Jack Henschel Date: Sun, 31 Jan 2021 07:31:56 +0200 Subject: [PATCH 1/4] test: Check filenames with special characters work correctly --- simples3_test.go | 107 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/simples3_test.go b/simples3_test.go index 8d35991..f54fc22 100644 --- a/simples3_test.go +++ b/simples3_test.go @@ -23,6 +23,12 @@ func TestS3_FileUpload(t *testing.T) { return } defer testTxt.Close() + // Note: cannot re-use the same file descriptor due to seeking! + testTxtSpecialChars, err := os.Open("testdata/test.txt") + if err != nil { + return + } + defer testTxtSpecialChars.Close() testPng, err := os.Open("testdata/avatar.png") if err != nil { return @@ -76,6 +82,25 @@ func TestS3_FileUpload(t *testing.T) { }, wantErr: false, }, + { + name: "Upload special filename txt", + fields: tConfig{ + AccessKey: os.Getenv("AWS_S3_ACCESS_KEY"), + SecretKey: os.Getenv("AWS_S3_SECRET_KEY"), + Endpoint: os.Getenv("AWS_S3_ENDPOINT"), + Region: os.Getenv("AWS_S3_REGION"), + }, + args: args{ + UploadInput{ + Bucket: os.Getenv("AWS_S3_BUCKET"), + ObjectKey: "xyz/example file%with$special&chars(1)?.txt", + ContentType: "text/plain", + FileName: "example file%with$special&chars(1)?.txt", + Body: testTxtSpecialChars, + }, + }, + wantErr: false, + }, } for _, testcase := range tests { tt := testcase @@ -159,6 +184,23 @@ func TestS3_FileDownload(t *testing.T) { wantErr: false, wantResponse: testPngData, }, + { + name: "txt-special-filename", + fields: tConfig{ + AccessKey: os.Getenv("AWS_S3_ACCESS_KEY"), + SecretKey: os.Getenv("AWS_S3_SECRET_KEY"), + Endpoint: os.Getenv("AWS_S3_ENDPOINT"), + Region: os.Getenv("AWS_S3_REGION"), + }, + args: args{ + u: DownloadInput{ + Bucket: os.Getenv("AWS_S3_BUCKET"), + ObjectKey: "xyz/example file%with$special&chars(1)?.txt", + }, + }, + wantErr: false, + wantResponse: testTxtData, + }, } for _, testcase := range tests { @@ -227,6 +269,22 @@ func TestS3_FileDelete(t *testing.T) { }, wantErr: false, }, + { + name: "Delete special filename txt", + fields: tConfig{ + AccessKey: os.Getenv("AWS_S3_ACCESS_KEY"), + SecretKey: os.Getenv("AWS_S3_SECRET_KEY"), + Endpoint: os.Getenv("AWS_S3_ENDPOINT"), + Region: os.Getenv("AWS_S3_REGION"), + }, + args: args{ + DeleteInput{ + Bucket: os.Getenv("AWS_S3_BUCKET"), + ObjectKey: "xyz/example file%with$special&chars(1)?.txt", + }, + }, + wantErr: false, + }, } for _, testcase := range tests { tt := testcase @@ -294,3 +352,52 @@ func TestCustomEndpoint(t *testing.T) { t.Errorf("S3.SetEndpoint() got = %v", s3.Endpoint) } } + +func TestGetURL(t *testing.T) { + s3 := New("us-east-1", "AccessKey", "SuperSecretKey") + + type args struct { + bucket string + params []string + } + + tests := []struct { + name string + args args + want string + }{ + { + name: "getURL: basic test", + args: args{ + bucket: "xyz", + }, + want: "https://s3.us-east-1.amazonaws.com/xyz", + }, + { + name: "getURL: multiple parameters", + args: args{ + bucket: "xyz", + params: []string{"hello", "world"}, + }, + want: "https://s3.us-east-1.amazonaws.com/xyz/hello/world", + }, + { + name: "getURL: special characters", + args: args{ + bucket: "xyz", + params: []string{"hello, world!", "#!@$%^&*(1).txt"}, + }, + want: "https://s3.us-east-1.amazonaws.com/xyz/hello%2C%20world%21/%23%21%40%24%25%5E%26%2A%281%29.txt", + }, + } + + for _, testcase := range tests { + tt := testcase + t.Run(tt.name, func(t *testing.T) { + url := s3.getURL(tt.args.bucket, tt.args.params...) + if url != tt.want { + t.Errorf("S3.getURL() got = %v, want %v", url, tt.want) + } + }) + } +} From ce1c2504223ea5ef086281dc402547abb01d0a59 Mon Sep 17 00:00:00 2001 From: Jack Henschel Date: Sun, 31 Jan 2021 08:20:23 +0200 Subject: [PATCH 2/4] fix: Modify getURL method to correctly support special characters fixes https://github.com/rhnvrm/simples3/issues/7 --- simples3.go | 65 +++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 58 insertions(+), 7 deletions(-) diff --git a/simples3.go b/simples3.go index 072534c..11ff045 100644 --- a/simples3.go +++ b/simples3.go @@ -7,6 +7,7 @@ import ( "bytes" "crypto/hmac" "crypto/sha256" + "encoding/hex" "encoding/json" "encoding/xml" "errors" @@ -15,8 +16,10 @@ import ( "io/ioutil" "mime/multipart" "net/http" + "regexp" "strings" "time" + "unicode/utf8" ) const ( @@ -159,17 +162,23 @@ func (s3 *S3) getClient() *http.Client { return s3.Client } -func (s3 *S3) getURL(bucket string, args ...string) (uri string) { +// getURL constructs a URL for a given path, with multiple optional +// arguments as individual subfolders, based on the endpoint +// specified in s3 struct. +func (s3 *S3) getURL(path string, args ...string) (uri string) { + if len(args) > 0 { + path += "/" + strings.Join(args, "/") + } + // need to encode special characters in the path part of the URL + encodedPath := encodePath(path) + if len(s3.Endpoint) > 0 { - uri = s3.Endpoint + "/" + bucket + uri = s3.Endpoint + "/" + encodedPath } else { - uri = fmt.Sprintf(s3.URIFormat, s3.Region, bucket) + uri = fmt.Sprintf(s3.URIFormat, s3.Region, encodedPath) } - if len(args) > 0 { - uri = uri + "/" + strings.Join(args, "/") - } - return + return uri } // SetEndpoint can be used to the set a custom endpoint for @@ -391,3 +400,45 @@ func (s3 *S3) FileDelete(u DeleteInput) error { return nil } + +// if object matches reserved string, no need to encode them +var reservedObjectNames = regexp.MustCompile("^[a-zA-Z0-9-_.~/]+$") + +// encodePath encode the strings from UTF-8 byte representations to HTML hex escape sequences +// +// This is necessary since regular url.Parse() and url.Encode() functions do not support UTF-8 +// non english characters cannot be parsed due to the nature in which url.Encode() is written +// +// This function on the other hand is a direct replacement for url.Encode() technique to support +// pretty much every UTF-8 character. +// adapted from https://github.com/minio/minio-go/blob/fe1f3855b146c1b6ce4199740d317e44cf9e85c2/pkg/s3utils/utils.go#L285 +func encodePath(pathName string) string { + if reservedObjectNames.MatchString(pathName) { + return pathName + } + var encodedPathname strings.Builder + for _, s := range pathName { + if 'A' <= s && s <= 'Z' || 'a' <= s && s <= 'z' || '0' <= s && s <= '9' { // §2.3 Unreserved characters (mark) + encodedPathname.WriteRune(s) + continue + } + switch s { + case '-', '_', '.', '~', '/': // §2.3 Unreserved characters (mark) + encodedPathname.WriteRune(s) + continue + default: + len := utf8.RuneLen(s) + if len < 0 { + // if utf8 cannot convert, return the same string as is + return pathName + } + u := make([]byte, len) + utf8.EncodeRune(u, s) + for _, r := range u { + hex := hex.EncodeToString([]byte{r}) + encodedPathname.WriteString("%" + strings.ToUpper(hex)) + } + } + } + return encodedPathname.String() +} From b42d2b1f1e27da37663a9bfdb5552cc10efe50d6 Mon Sep 17 00:00:00 2001 From: Jack Henschel Date: Tue, 2 Feb 2021 08:52:22 +0200 Subject: [PATCH 3/4] docs: Make bucket references in README example consistent --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 78b8f0c..69c1e4c 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ err := s3.FileDelete(simples3.DeleteInput{ // You can also download the file. file, _ := s3.FileDownload(simples3.DownloadInput{ - Bucket: os.Getenv("AWS_S3_BUCKET"), + Bucket: AWSBucket, ObjectKey: "test.txt", }) data, _ := ioutil.ReadAll(file) @@ -63,7 +63,7 @@ file.Close() var time, _ = time.Parse(time.RFC1123, "Fri, 24 May 2013 00:00:00 GMT") url := s.GeneratePresignedURL(PresignedInput{ - Bucket: "examplebucket", + Bucket: AWSBucket, ObjectKey: "test.txt", Method: "GET", Timestamp: time, From bf02e3bacfa9175e363d6bb0fa46eb2b7e92b2a5 Mon Sep 17 00:00:00 2001 From: Jack Henschel Date: Wed, 3 Feb 2021 09:22:05 +0200 Subject: [PATCH 4/4] fix: Ensure trailing slash in CustomEndpoint is handled gracefully --- simples3.go | 5 +++++ simples3_test.go | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/simples3.go b/simples3.go index 11ff045..b875fe3 100644 --- a/simples3.go +++ b/simples3.go @@ -189,6 +189,11 @@ func (s3 *S3) SetEndpoint(uri string) *S3 { if !strings.HasPrefix(uri, "http") { uri = "https://" + uri } + + // make sure there is no trailing slash + if uri[len(uri)-1] == '/' { + uri = uri[:len(uri)-1] + } s3.Endpoint = uri } return s3 diff --git a/simples3_test.go b/simples3_test.go index f54fc22..2b99e71 100644 --- a/simples3_test.go +++ b/simples3_test.go @@ -351,6 +351,12 @@ func TestCustomEndpoint(t *testing.T) { if s3.getURL("bucket3") != "https://example.com/bucket3" { t.Errorf("S3.SetEndpoint() got = %v", s3.Endpoint) } + + // try with trailing slash + s3.SetEndpoint("https://example.com/foobar/") + if s3.getURL("bucket4") != "https://example.com/foobar/bucket4" { + t.Errorf("S3.SetEndpoint() got = %v", s3.Endpoint) + } } func TestGetURL(t *testing.T) {