From aa29742be23c061fb9ada78fae80d26a7d2caa40 Mon Sep 17 00:00:00 2001 From: Artur Neumann Date: Fri, 23 Jun 2023 09:52:50 +0545 Subject: [PATCH] serve s3: fix file name encoding using s3 serve with mc client using the mc (minio) client file encoding were wrong see Mikubill/gofakes3#2 for details --- cmd/serve/s3/s3.go | 8 +- cmd/serve/s3/s3_test.go | 157 ++++++++++++++++++++++++++++++---------- go.mod | 6 +- go.sum | 15 ++++ 4 files changed, 143 insertions(+), 43 deletions(-) diff --git a/cmd/serve/s3/s3.go b/cmd/serve/s3/s3.go index 394f3101a..305b46dd7 100644 --- a/cmd/serve/s3/s3.go +++ b/cmd/serve/s3/s3.go @@ -31,10 +31,10 @@ func init() { flagSet := Command.Flags() httplib.AddHTTPFlagsPrefix(flagSet, flagPrefix, &Opt.HTTP) vfsflags.AddFlags(flagSet) - flags.BoolVarP(flagSet, &Opt.pathBucketMode, "force-path-style", "", Opt.pathBucketMode, "If true use path style access if false use virtual hosted style (default true)") - flags.StringVarP(flagSet, &Opt.hashName, "etag-hash", "", Opt.hashName, "Which hash to use for the ETag, or auto or blank for off") - flags.StringArrayVarP(flagSet, &Opt.authPair, "s3-authkey", "", Opt.authPair, "Set key pair for v4 authorization, split by comma") - flags.BoolVarP(flagSet, &Opt.noCleanup, "no-cleanup", "", Opt.noCleanup, "Not to cleanup empty folder after object is deleted") + flags.BoolVarP(flagSet, &Opt.pathBucketMode, "force-path-style", "", Opt.pathBucketMode, "If true use path style access if false use virtual hosted style (default true)", "") + flags.StringVarP(flagSet, &Opt.hashName, "etag-hash", "", Opt.hashName, "Which hash to use for the ETag, or auto or blank for off", "") + flags.StringArrayVarP(flagSet, &Opt.authPair, "s3-authkey", "", Opt.authPair, "Set key pair for v4 authorization, split by comma", "") + flags.BoolVarP(flagSet, &Opt.noCleanup, "no-cleanup", "", Opt.noCleanup, "Not to cleanup empty folder after object is deleted", "") } // Command definition for cobra diff --git a/cmd/serve/s3/s3_test.go b/cmd/serve/s3/s3_test.go index b9374795a..dd5200299 100644 --- a/cmd/serve/s3/s3_test.go +++ b/cmd/serve/s3/s3_test.go @@ -4,16 +4,24 @@ package s3 import ( + "bytes" "context" "encoding/hex" "fmt" + "io" "math/rand" + "net/url" "os" "os/exec" + "path" "strings" "testing" "time" + "github.com/minio/minio-go/v7" + "github.com/minio/minio-go/v7/pkg/credentials" + "github.com/rclone/rclone/fs/object" + _ "github.com/rclone/rclone/backend/local" "github.com/rclone/rclone/cmd/serve/servetest" "github.com/rclone/rclone/fs" @@ -29,43 +37,27 @@ const ( endpoint = "localhost:0" ) -// TestS3 runs the s3 server then runs the unit tests for the -// s3 remote against it. -func TestS3(t *testing.T) { - // Configure and start the server - start := func(f fs.Fs) (configmap.Simple, func()) { - keyid := RandString(16) - keysec := RandString(16) - serveropt := &Options{ - HTTP: httplib.DefaultCfg(), - pathBucketMode: true, - hashName: "", - hashType: hash.None, - authPair: []string{fmt.Sprintf("%s,%s", keyid, keysec)}, - } - - serveropt.HTTP.ListenAddr = []string{endpoint} - w, err := newServer(context.Background(), f, serveropt) - router := w.Router() - assert.NoError(t, err) - - w.Bind(router) - w.Serve() - testURL := w.Server.URLs()[0] - // Config for the backend we'll use to connect to the server - config := configmap.Simple{ - "type": "s3", - "provider": "Rclone", - "endpoint": testURL, - "list_url_encode": "true", - "access_key_id": keyid, - "secret_access_key": keysec, - } - - return config, func() {} +// Configure and serve the server +func serveS3(f fs.Fs) (testURL string, keyid string, keysec string) { + keyid = RandString(16) + keysec = RandString(16) + serveropt := &Options{ + HTTP: httplib.DefaultCfg(), + pathBucketMode: true, + hashName: "", + hashType: hash.None, + authPair: []string{fmt.Sprintf("%s,%s", keyid, keysec)}, } - Run(t, "s3", start) + serveropt.HTTP.ListenAddr = []string{endpoint} + w, _ := newServer(context.Background(), f, serveropt) + router := w.Router() + + w.Bind(router) + w.Serve() + testURL = w.Server.URLs()[0] + + return } func RandString(n int) string { @@ -79,7 +71,28 @@ func RandString(n int) string { return hex.EncodeToString(b)[:n] } -func Run(t *testing.T, name string, start servetest.StartFn) { +// TestS3 runs the s3 server then runs the unit tests for the +// s3 remote against it. +func TestS3(t *testing.T) { + start := func(f fs.Fs) (configmap.Simple, func()) { + testURL, keyid, keysec := serveS3(f) + // Config for the backend we'll use to connect to the server + config := configmap.Simple{ + "type": "s3", + "provider": "Rclone", + "endpoint": testURL, + "list_url_encode": "true", + "access_key_id": keyid, + "secret_access_key": keysec, + } + + return config, func() {} + } + + RunS3UnitTests(t, "s3", start) +} + +func RunS3UnitTests(t *testing.T, name string, start servetest.StartFn) { fstest.Initialise() ci := fs.GetConfig(context.Background()) ci.DisableFeatures = append(ci.DisableFeatures, "Metadata") @@ -105,7 +118,7 @@ func Run(t *testing.T, name string, start servetest.StartFn) { require.NoError(t, os.Chdir(cwd)) }() - // Run the backend tests with an on the fly remote + // RunS3UnitTests the backend tests with an on the fly remote args := []string{"test"} if testing.Verbose() { args = append(args, "-v") @@ -126,11 +139,79 @@ func Run(t *testing.T, name string, start servetest.StartFn) { cmd.Env = append(cmd.Env, prefix+strings.ToUpper(k)+"="+v) } - // Run the test + // RunS3UnitTests the test out, err := cmd.CombinedOutput() if len(out) != 0 { t.Logf("\n----------\n%s----------\n", string(out)) } assert.NoError(t, err, "Running "+name+" integration tests") +} + +// tests using the minio client +func TestEncodingWithMinioClient(t *testing.T) { + cases := []struct { + description string + bucket string + path string + filename string + expected string + }{ + { + description: "weird file in bucket root", + bucket: "mybucket", + path: "", + filename: " file with w€r^d ch@r \\#~+§4%&'. txt ", + }, + { + description: "weird file inside a weird folder", + bucket: "mybucket", + path: "ä#/नेपाल&/?/", + filename: " file with w€r^d ch@r \\#~+§4%&'. txt ", + }, + } + + for _, tt := range cases { + t.Run(tt.description, func(t *testing.T) { + fstest.Initialise() + f, _, clean, err := fstest.RandomRemote() + assert.NoError(t, err) + defer clean() + err = f.Mkdir(context.Background(), path.Join(tt.bucket, tt.path)) + assert.NoError(t, err) + + buf := bytes.NewBufferString("contents") + uploadHash := hash.NewMultiHasher() + in := io.TeeReader(buf, uploadHash) + + obji := object.NewStaticObjectInfo( + path.Join(tt.bucket, tt.path, tt.filename), + time.Now(), + int64(buf.Len()), + true, + nil, + nil, + ) + _, err = f.Put(context.Background(), in, obji) + assert.NoError(t, err) + + endpoint, keyid, keysec := serveS3(f) + testURL, _ := url.Parse(endpoint) + minioClient, err := minio.New(testURL.Host, &minio.Options{ + Creds: credentials.NewStaticV4(keyid, keysec, ""), + Secure: false, + }) + assert.NoError(t, err) + + buckets, err := minioClient.ListBuckets(context.Background()) + assert.NoError(t, err) + assert.Equal(t, buckets[0].Name, tt.bucket) + objects := minioClient.ListObjects(context.Background(), tt.bucket, minio.ListObjectsOptions{ + Recursive: true, + }) + for object := range objects { + assert.Equal(t, path.Join(tt.path, tt.filename), object.Key) + } + }) + } } diff --git a/go.mod b/go.mod index bbf98d60b..8f8a06c02 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.2.0 github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358 github.com/Max-Sum/base32768 v0.0.0-20230304063302-18e6ce5945fd - github.com/Mikubill/gofakes3 v0.0.3-0.20221030004050-725f2cf2bf5e + github.com/Mikubill/gofakes3 v0.0.3-0.20230622102024-284c0f988700 github.com/Unknwon/goconfig v1.0.0 github.com/a8m/tree v0.0.0-20230208161321-36ae24ddad15 github.com/aalpar/deheap v0.0.0-20210914013432-0cc84d79dec3 @@ -43,6 +43,7 @@ require ( github.com/koofr/go-koofrclient v0.0.0-20221207135200-cbd7fc9ad6a6 github.com/mattn/go-colorable v0.1.13 github.com/mattn/go-runewidth v0.0.15 + github.com/minio/minio-go/v7 v7.0.57 github.com/mitchellh/go-homedir v1.1.0 github.com/moby/sys/mountinfo v0.6.2 github.com/ncw/go-acd v0.0.0-20201019170801-fe55f33415b1 @@ -103,6 +104,7 @@ require ( github.com/cpuguy83/go-md2man/v2 v2.0.3 // indirect github.com/cronokirby/saferith v0.33.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect + github.com/dustin/go-humanize v1.0.1 // indirect github.com/emersion/go-message v0.17.0 // indirect github.com/emersion/go-textwrapper v0.0.0-20200911093747-65d896831594 // indirect github.com/emersion/go-vcard v0.0.0-20230815062825-8fda7d206ec9 // indirect @@ -147,6 +149,7 @@ require ( github.com/prometheus/procfs v0.12.0 // indirect github.com/rasky/go-xdr v0.0.0-20170124162913-1a41d1a06c93 // indirect github.com/relvacode/iso8601 v1.3.0 // indirect + github.com/rs/xid v1.5.0 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/ryszard/goskiplist v0.0.0-20150312221310-2dfbae5fcf46 // indirect github.com/shabbyrobe/gocovmerge v0.0.0-20190829150210-3e036491d500 // indirect @@ -168,6 +171,7 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20231016165738-49dd2c1f3d0b // indirect google.golang.org/grpc v1.59.0 // indirect google.golang.org/protobuf v1.31.0 // indirect + gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect storj.io/common v0.0.0-20231027080355-b4cb1b0d728e // indirect storj.io/drpc v0.0.33 // indirect diff --git a/go.sum b/go.sum index 8a101619a..e9328f329 100644 --- a/go.sum +++ b/go.sum @@ -156,6 +156,8 @@ github.com/dropbox/dropbox-sdk-go-unofficial/v6 v6.0.5/go.mod h1:rSS3kM9XMzSQ6pw github.com/dsnet/try v0.0.3 h1:ptR59SsrcFUYbT/FhAbKTV6iLkeD6O18qfIWRml2fqI= github.com/dustin/go-humanize v0.0.0-20180421182945-02af3965c54e/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= +github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= +github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= github.com/emersion/go-message v0.17.0 h1:NIdSKHiVUx4qKqdd0HyJFD41cW8iFguM2XJnRZWQH04= github.com/emersion/go-message v0.17.0/go.mod h1:/9Bazlb1jwUNB0npYYBsdJ2EMOiiyN3m5UVHbY7GoNw= github.com/emersion/go-textwrapper v0.0.0-20200911093747-65d896831594 h1:IbFBtwoTQyw0fIM5xv1HF+Y+3ZijDR839WMulgxCcUY= @@ -259,6 +261,7 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= +github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= github.com/google/martian/v3 v3.0.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0= github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc= @@ -336,6 +339,7 @@ github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfC github.com/josephspurrier/goversioninfo v1.4.0 h1:Puhl12NSHUSALHSuzYwPYQkqa2E1+7SrtAPJorKK0C8= github.com/josephspurrier/goversioninfo v1.4.0/go.mod h1:JWzv5rKQr+MmW+LvM412ToT/IkYDZjaclF2pKDss8IY= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= +github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU= github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/XSXhF0NWZEnDohbsk= github.com/jtolds/gls v4.2.1+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU= @@ -389,14 +393,23 @@ github.com/mattn/go-runewidth v0.0.15/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 h1:jWpvCLoY8Z/e3VKvlsiIGKtc+UG6U5vzxaoagmhXfyg= github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0/go.mod h1:QUyp042oQthUoa9bqDv0ER0wrtXnBruoNd7aNjkbP+k= github.com/miekg/dns v1.1.42/go.mod h1:+evo5L0630/F6ca/Z9+GAqzhjGyn8/c+TBaOyfEl0V4= +github.com/minio/md5-simd v1.1.2 h1:Gdi1DZK69+ZVMoNHRXJyNcxrMA4dSxoYHZSQbirFg34= +github.com/minio/md5-simd v1.1.2/go.mod h1:MzdKDxYpY2BT9XQFocsiZf/NKVtR7nkE4RoEpN+20RM= github.com/minio/minio-go/v6 v6.0.46/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tBvN2PaZg7Gg= +github.com/minio/minio-go/v7 v7.0.57 h1:xsFiOiWjpC1XAGbFEUOzj1/gMXGz7ljfxifwcb/5YXU= +github.com/minio/minio-go/v7 v7.0.57/go.mod h1:NUDy4A4oXPq1l2yK6LTSvCEzAMeIcoz9lcj5dbzSrRE= github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM= +github.com/minio/sha256-simd v1.0.1 h1:6kaan5IFmwTNynnKKpDHe6FWHohJOHhCPchzK49dzMM= +github.com/minio/sha256-simd v1.0.1/go.mod h1:Pz6AKMiUdngCLpeTL/RJY1M9rUuPMYujV5xJjtbRSN8= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/moby/sys/mountinfo v0.6.2 h1:BzJjoreD5BMFNmD9Rus6gdd1pLuecOFPt8wC+Vygl78= github.com/moby/sys/mountinfo v0.6.2/go.mod h1:IJb6JQeOklcdMU9F5xQ8ZALD+CUr5VlGpwtX+VE0rpI= +github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= +github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9Gz0M= +github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= github.com/mschoch/smat v0.0.0-20160514031455-90eadee771ae/go.mod h1:qAyveg+e4CE+eKJXWVjKXM4ck2QobLqTDytGJbLLhJg= github.com/ncw/go-acd v0.0.0-20201019170801-fe55f33415b1 h1:nAjWYc03awJAjsozNehdGZsm5LP7AhLOvjgbS8zN1tk= github.com/ncw/go-acd v0.0.0-20201019170801-fe55f33415b1/go.mod h1:MLIrzg7gp/kzVBxRE1olT7CWYMCklcUWU+ekoxOD9x0= @@ -913,6 +926,8 @@ gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntN gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/ini.v1 v1.42.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= +gopkg.in/ini.v1 v1.67.0 h1:Dgnx+6+nfE+IfzjUEISNeydPJh9AXNNsWbGP9KzCsOA= +gopkg.in/ini.v1 v1.67.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=