From 746eac73efaaf60dfcd56f30ade924f64faaaeb1 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 23 Sep 2026 16:33:57 +0100 Subject: [PATCH] serve s3: fix --etag-hash auto crashing or using the wrong hash with --auth-proxy With --etag-hash auto the hash for the ETags was chosen once from the remote serve s3 was started with. With --auth-proxy there is no such remote so serve s3 crashed on startup, and when started by the rc it was the wrong remote, so users whose backends lacked that hash got no ETags. The hash is now chosen from the backend of the user making each request. --- cmd/serve/s3/backend.go | 4 ++-- cmd/serve/s3/backend_test.go | 41 ++++++++++++++++++++++++++++++++++++ cmd/serve/s3/list.go | 7 +++--- cmd/serve/s3/serve_s3.md | 2 ++ cmd/serve/s3/server.go | 17 +++++++++++---- 5 files changed, 62 insertions(+), 9 deletions(-) diff --git a/cmd/serve/s3/backend.go b/cmd/serve/s3/backend.go index 3bcb10a01..e1c23e3bc 100644 --- a/cmd/serve/s3/backend.go +++ b/cmd/serve/s3/backend.go @@ -163,7 +163,7 @@ func (b *s3Backend) HeadObject(ctx context.Context, bucketName, objectName strin // to hashing the VFS cache when the backing object is not available yet. entry := node.DirEntry() size := node.Size() - hash := getFileHashByte(node, b.s.etagHashType) + hash := getFileHashByte(node, b.s.etagHash(_vfs)) mimeType := fs.MimeTypeFromName(objectName) if fobj, ok := entry.(fs.Object); ok { @@ -220,7 +220,7 @@ func (b *s3Backend) GetObject(ctx context.Context, bucketName, objectName string file := node.(*vfs.File) size := node.Size() - hash := getFileHashByte(node, b.s.etagHashType) + hash := getFileHashByte(node, b.s.etagHash(_vfs)) in, err := file.Open(os.O_RDONLY) if err != nil { diff --git a/cmd/serve/s3/backend_test.go b/cmd/serve/s3/backend_test.go index 574bbc56c..679506a26 100644 --- a/cmd/serve/s3/backend_test.go +++ b/cmd/serve/s3/backend_test.go @@ -2,6 +2,7 @@ package s3 import ( "context" + "crypto/md5" "io" "os" "path/filepath" @@ -9,10 +10,14 @@ import ( "testing" "github.com/rclone/gofakes3" + _ "github.com/rclone/rclone/backend/crypt" _ "github.com/rclone/rclone/backend/local" "github.com/rclone/rclone/cmd/serve/proxy" "github.com/rclone/rclone/fs" + "github.com/rclone/rclone/fs/config/obscure" + "github.com/rclone/rclone/fs/hash" "github.com/rclone/rclone/fstest" + "github.com/rclone/rclone/vfs" "github.com/rclone/rclone/vfs/vfscommon" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -180,3 +185,39 @@ func TestBucketDirPath(t *testing.T) { } } } + +// TestEtagHashAuto checks that --etag-hash auto uses the best hash of +// the backend of each auth proxy user rather than of the remote the +// server was started with, which may have none or not exist at all. +func TestEtagHashAuto(t *testing.T) { + fstest.Initialise() + ctx := context.Background() + opt := Opt + opt.EtagHash = "auto" + opt.HTTP.ListenAddr = []string{endpoint} + proxyOpt := proxy.Opt + proxyOpt.AuthProxy = "/path/to/auth/proxy" + + fCrypt, err := fs.NewFs(ctx, ":crypt,remote="+t.TempDir()+",password="+obscure.MustObscure("password")+":") + require.NoError(t, err) + require.Equal(t, hash.None, fCrypt.Hashes().GetOne(), "crypt has no hashes") + for _, f := range []fs.Fs{nil, fCrypt} { + w, err := newServer(ctx, f, &opt, &vfscommon.Opt, &proxyOpt) + require.NoError(t, err) + t.Cleanup(func() { _ = w.Shutdown() }) + + fUser, err := fs.NewFs(ctx, t.TempDir()) + require.NoError(t, err) + userVFS := vfs.New(ctx, fUser, &vfscommon.Opt) + t.Cleanup(userVFS.Shutdown) + ctx := context.WithValue(ctx, ctxKeyID, userVFS) + require.NoError(t, w.backend.CreateBucket(ctx, "bucket")) + _, err = w.backend.PutObject(ctx, "bucket", "object", nil, strings.NewReader("data"), 4) + require.NoError(t, err) + + obj, err := w.backend.HeadObject(ctx, "bucket", "object") + require.NoError(t, err) + want := md5.Sum([]byte("data")) + assert.Equal(t, want[:], obj.Hash) + } +} diff --git a/cmd/serve/s3/list.go b/cmd/serve/s3/list.go index 3f7a91572..6b5e79e95 100644 --- a/cmd/serve/s3/list.go +++ b/cmd/serve/s3/list.go @@ -8,6 +8,7 @@ import ( "github.com/rclone/gofakes3" "github.com/rclone/rclone/fs" + "github.com/rclone/rclone/fs/hash" "github.com/rclone/rclone/vfs" ) @@ -27,8 +28,8 @@ var errPageFull = errors.New("listing page full") // so the cost of a page is proportional to the keys it returns rather than to // the size of the subtree below the prefix. type lister struct { - b *s3Backend vfs *vfs.VFS + hashType hash.Type // hash used for ETags bucket string marker string // keys at or before this one have already been returned hasMarker bool @@ -49,8 +50,8 @@ func newLister(b *s3Backend, _vfs *vfs.VFS, bucket string, page gofakes3.ListBuc max = 1000 } return &lister{ - b: b, vfs: _vfs, + hashType: b.s.etagHash(_vfs), bucket: bucket, marker: page.Marker, hasMarker: page.HasMarker, @@ -114,7 +115,7 @@ func (l *lister) addObject(key string, entry vfs.Node) error { l.response.Add(&gofakes3.Content{ Key: key, LastModified: gofakes3.NewContentTime(entry.ModTime()), - ETag: getFileHash(entry, l.b.s.etagHashType), + ETag: getFileHash(entry, l.hashType), Size: entry.Size(), StorageClass: gofakes3.StorageStandard, }) diff --git a/cmd/serve/s3/serve_s3.md b/cmd/serve/s3/serve_s3.md index 495efb80a..6bfe3aaba 100644 --- a/cmd/serve/s3/serve_s3.md +++ b/cmd/serve/s3/serve_s3.md @@ -53,6 +53,8 @@ part of the hostname (such as mybucket.local) Use `--etag-hash` if you want to change the hash uses for the `ETag`. Note that using anything other than `MD5` (the default) is likely to cause problems for S3 clients which rely on the Etag being the MD5. +Use `--etag-hash auto` to use the best hash the backend supports - with +`--auth-proxy` that of each user's backend. ### Quickstart diff --git a/cmd/serve/s3/server.go b/cmd/serve/s3/server.go index eedf65c59..c94e414bc 100644 --- a/cmd/serve/s3/server.go +++ b/cmd/serve/s3/server.go @@ -37,7 +37,7 @@ type Server struct { backend *s3Backend handler http.Handler ctx context.Context // for global config - etagHashType hash.Type + etagHashType hash.Type // hash for ETags unless opt.EtagHash is "auto" } // Make a new S3 Server to serve the remote @@ -58,9 +58,7 @@ func newServer(ctx context.Context, f fs.Fs, opt *Options, vfsOpt *vfscommon.Opt } }() - if w.opt.EtagHash == "auto" { - w.etagHashType = f.Hashes().GetOne() - } else if w.opt.EtagHash != "" { + if w.opt.EtagHash != "" && w.opt.EtagHash != "auto" { err := w.etagHashType.Set(w.opt.EtagHash) if err != nil { return nil, err @@ -142,6 +140,17 @@ func (w *Server) getVFS(ctx context.Context) (VFS *vfs.VFS, err error) { return VFS, nil } +// etagHash returns the hash to use for the ETags of objects in _vfs. +// +// With --etag-hash auto this depends on the backend, which may be +// different for each user of an auth proxy. +func (w *Server) etagHash(_vfs *vfs.VFS) hash.Type { + if w.opt.EtagHash == "auto" { + return _vfs.Fs().Hashes().GetOne() + } + return w.etagHashType +} + // auth authenticates the request via the auth proxy. // // The proxy maps the access key ID to a VFS and a secret access key