diff --git a/cmd/serve/s3/backend.go b/cmd/serve/s3/backend.go index 9fc9a0fc1..81605ce28 100644 --- a/cmd/serve/s3/backend.go +++ b/cmd/serve/s3/backend.go @@ -3,8 +3,10 @@ package s3 import ( "context" + "crypto/md5" "encoding/hex" "errors" + "hash" "io" "maps" "math" @@ -289,7 +291,7 @@ func (b *s3Backend) ListBucket(ctx context.Context, bucket string, prefix *gofak return response, nil } - err = b.listPage(_vfs, bucket, prefix.Prefix, prefix.Delimiter, page, response) + err = b.listPage(ctx, _vfs, bucket, prefix.Prefix, prefix.Delimiter, page, response) if err == gofakes3.ErrNoSuchKey { // AWS just returns an empty list response = gofakes3.NewObjectList() @@ -613,6 +615,15 @@ func (b *s3Backend) PutObject( removeMadeDirs(objectDir, madeDir, _vfs) } + // With MD5 ETags gofakes3 returns the MD5 of the data, which is + // kept with the metadata if the remote can't give it, so HEAD + // returns it too. + var md5Hasher hash.Hash + if b.s.etagHash(_vfs) == fshash.MD5 && !fsys.Hashes().Contains(fshash.MD5) { + md5Hasher = md5.New() + input = io.TeeReader(input, md5Hasher) + } + f, err := _vfs.Create(tmpFp) if err != nil { cleanup() @@ -655,7 +666,11 @@ func (b *s3Backend) PutObject( return result, err } - if err := b.tenant(ctx).storeMeta(_vfs, fp, meta); err != nil { + om := objectMeta{meta: meta} + if md5Hasher != nil { + om.hash = md5Hasher.Sum(nil) + } + if err := b.tenant(ctx).storeObjectMeta(_vfs, fp, om); err != nil { return result, err } @@ -847,7 +862,12 @@ func (b *s3Backend) CopyObject(ctx context.Context, srcBucket, srcKey, dstBucket // Directory markers have no metadata to replace return result, nil } - return result, b.tenant(ctx).storeMeta(_vfs, fp, meta) + // The data is unchanged, so its stored ETag and part sizes + // still apply + t := b.tenant(ctx) + om, _ := t.getMeta(ctx, fp, cStat) + om.meta = meta + return result, t.storeObjectMeta(_vfs, fp, om) } c, err := b.GetObject(ctx, srcBucket, srcKey, nil) diff --git a/cmd/serve/s3/list.go b/cmd/serve/s3/list.go index 0bc3ba7ba..1fc393f5a 100644 --- a/cmd/serve/s3/list.go +++ b/cmd/serve/s3/list.go @@ -1,6 +1,7 @@ package s3 import ( + "context" "errors" "path" "slices" @@ -29,7 +30,8 @@ var errPageFull = errors.New("listing page full") // the size of the subtree below the prefix. type lister struct { vfs *vfs.VFS - hashType hash.Type // hash used for ETags + hashType hash.Type // hash used for ETags + etag func(fp string, entry vfs.Node) []byte // returns the ETag of the object at fp bucket string marker string // keys at or before this one have already been returned hasMarker bool @@ -49,7 +51,7 @@ type lister struct { // newLister makes a lister which fills response with at most one page of the // listing of bucket, as described by page. -func newLister(b *s3Backend, _vfs *vfs.VFS, bucket string, page gofakes3.ListBucketPage, response *gofakes3.ObjectList) *lister { +func newLister(ctx context.Context, b *s3Backend, _vfs *vfs.VFS, bucket string, page gofakes3.ListBucketPage, response *gofakes3.ObjectList) *lister { max := int(page.MaxKeys) if max <= 0 { // A missing or zero max-keys means the client didn't ask for a @@ -64,6 +66,9 @@ func newLister(b *s3Backend, _vfs *vfs.VFS, bucket string, page gofakes3.ListBuc hasMarker: page.HasMarker, max: max, response: response, + etag: func(fp string, entry vfs.Node) []byte { + return b.objectHash(ctx, _vfs, fp, entry) + }, } } @@ -122,7 +127,7 @@ func (l *lister) addObject(key string, entry vfs.Node) error { l.response.Add(&gofakes3.Content{ Key: key, LastModified: gofakes3.NewContentTime(entry.ModTime()), - ETag: quoteETag(getFileHashByte(entry, l.hashType)), + ETag: quoteETag(l.etag(path.Join(l.bucket, key), entry)), Size: entry.Size(), StorageClass: gofakes3.StorageStandard, }) @@ -325,8 +330,8 @@ func (l *lister) list(fdPath, name string, addPrefix bool) error { // // It returns gofakes3.ErrNoSuchKey if the directory the prefix is in // doesn't exist. -func (b *s3Backend) listPage(_vfs *vfs.VFS, bucket, prefix, delimiter string, page gofakes3.ListBucketPage, response *gofakes3.ObjectList) error { - l := newLister(b, _vfs, bucket, page, response) +func (b *s3Backend) listPage(ctx context.Context, _vfs *vfs.VFS, bucket, prefix, delimiter string, page gofakes3.ListBucketPage, response *gofakes3.ObjectList) error { + l := newLister(ctx, b, _vfs, bucket, page, response) // The prefix is split into the directory to list and the start of // the names in it to list. fdPath, name := "", prefix diff --git a/cmd/serve/s3/list_test.go b/cmd/serve/s3/list_test.go index 7a08e20f1..47d9711d1 100644 --- a/cmd/serve/s3/list_test.go +++ b/cmd/serve/s3/list_test.go @@ -170,7 +170,7 @@ func countDirs(t *testing.T, b *s3Backend, page gofakes3.ListBucketPage) (dirsRe _vfs, err := b.s.getVFS(context.Background()) require.NoError(t, err) list = gofakes3.NewObjectList() - l := newLister(b, _vfs, "bucket", page, list) + l := newLister(context.Background(), b, _vfs, "bucket", page, list) err = l.list("", "", false) if err != nil && !errors.Is(err, errPageFull) { require.NoError(t, err) diff --git a/cmd/serve/s3/multipart_test.go b/cmd/serve/s3/multipart_test.go index 8fc710b1c..b1f0626ac 100644 --- a/cmd/serve/s3/multipart_test.go +++ b/cmd/serve/s3/multipart_test.go @@ -26,6 +26,7 @@ import ( _ "github.com/rclone/rclone/backend/memory" "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/fs/object" "github.com/rclone/rclone/fs/operations" @@ -2039,3 +2040,66 @@ func TestETagHashConsistent(t *testing.T) { } } } + +// TestETagMD5WithoutRemoteMD5 checks that with MD5 ETags on a remote +// which can't give the MD5 of objects, the ETags returned when objects +// are made are the same as HEAD and listings return for them, and are +// kept when an object is copied onto itself. +func TestETagMD5WithoutRemoteMD5(t *testing.T) { + backing := ":crypt,remote='" + t.TempDir() + "',password=" + obscure.MustObscure("password") + ":" + core, f, bucket := newMultipartTestServerBacking(t, backing, false) + require.False(t, f.Hashes().Contains(hash.MD5)) + ctx := context.Background() + md5Hex := func(data []byte) string { + sum := md5.Sum(data) + return hex.EncodeToString(sum[:]) + } + etags := map[string]string{} + + data := []byte("put object") + info, err := core.PutObject(ctx, bucket, "put.txt", bytes.NewReader(data), int64(len(data)), "", "", minio.PutObjectOptions{}) + require.NoError(t, err) + assert.Equal(t, md5Hex(data), info.ETag, "put") + etags["put.txt"] = info.ETag + + uploadID, err := core.NewMultipartUpload(ctx, bucket, "multipart.bin", minio.PutObjectOptions{}) + require.NoError(t, err) + var all []byte + var parts []minio.CompletePart + for i, size := range []int{100 * 1024, 30 * 1024} { + data := []byte(random.String(size)) + all = append(all, data...) + p, err := core.PutObjectPart(ctx, bucket, "multipart.bin", uploadID, i+1, bytes.NewReader(data), int64(size), minio.PutObjectPartOptions{}) + require.NoError(t, err) + parts = append(parts, minio.CompletePart{PartNumber: i + 1, ETag: p.ETag}) + } + complete, err := core.CompleteMultipartUpload(ctx, bucket, "multipart.bin", uploadID, parts, minio.PutObjectOptions{}) + require.NoError(t, err) + assert.Equal(t, md5Hex(all), complete.ETag, "multipart") + etags["multipart.bin"] = complete.ETag + + // Replacing the metadata of an object leaves its data and so its + // ETag unchanged + _, err = core.CopyObject(ctx, bucket, "multipart.bin", bucket, "multipart.bin", map[string]string{ + "x-amz-metadata-directive": "REPLACE", + "x-amz-meta-potato": "jersey royal", + }, minio.CopySrcOptions{}, minio.PutObjectOptions{}) + require.NoError(t, err) + + for key, etag := range etags { + stat, err := core.StatObject(ctx, bucket, key, minio.StatObjectOptions{}) + require.NoError(t, err, key) + assert.Equal(t, etag, stat.ETag, "HEAD %s", key) + } + stat, err := core.StatObject(ctx, bucket, "multipart.bin", minio.StatObjectOptions{PartNumber: 2}) + require.NoError(t, err) + assert.Equal(t, int64(30*1024), stat.Size, "part 2 size after copy") + + listed := map[string]string{} + result, err := core.ListObjects(bucket, "", "", "", 1000) + require.NoError(t, err) + for _, obj := range result.Contents { + listed[obj.Key] = strings.Trim(obj.ETag, `"`) + } + assert.Equal(t, etags, listed) +} diff --git a/cmd/serve/s3/serve_s3.md b/cmd/serve/s3/serve_s3.md index 201615351..e486d4b3f 100644 --- a/cmd/serve/s3/serve_s3.md +++ b/cmd/serve/s3/serve_s3.md @@ -68,8 +68,8 @@ Use `--etag-hash auto` to use the best hash the backend supports - with The `ETag` of an object made by a multipart upload is its hash, like any other object, rather than the `-N` suffixed `ETag` AWS S3 gives these. If the remote doesn't have the MD5 of objects, `HeadObject` -only returns the MD5 of these while the object's metadata is kept (see -below). +and listings only return the MD5 of objects uploaded through `serve +s3` while the object's metadata is kept (see below). ### Quickstart