From fecce67ac66f889f729b1ae11bebaee4885798c4 Mon Sep 17 00:00:00 2001 From: nielash Date: Mon, 25 Mar 2024 10:20:01 -0400 Subject: [PATCH] memory: fix dst mutating src after server-side copy Before this change, the Memory backend's Copy method created a dst object that referenced the src's objectData by pointer instead of making a copy. While this minimized memory usage, an unintended consequence was that subsequently mutating the src (such as changing the modtime) would inadvertently also mutate the dst, and vice versa. This change fixes the issue and adds a test. --- backend/memory/memory.go | 3 ++- fstest/fstests/fstests.go | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/backend/memory/memory.go b/backend/memory/memory.go index 80fc321f9..11c02d78a 100644 --- a/backend/memory/memory.go +++ b/backend/memory/memory.go @@ -482,7 +482,8 @@ func (f *Fs) Copy(ctx context.Context, src fs.Object, remote string) (fs.Object, if od == nil { return nil, fs.ErrorObjectNotFound } - buckets.updateObjectData(dstBucket, dstPath, od) + odCopy := *od + buckets.updateObjectData(dstBucket, dstPath, &odCopy) return f.NewObject(ctx, remote) } diff --git a/fstest/fstests/fstests.go b/fstest/fstests/fstests.go index 3e3237f1a..72527a1e4 100644 --- a/fstest/fstests/fstests.go +++ b/fstest/fstests/fstests.go @@ -1237,6 +1237,13 @@ func Run(t *testing.T, opt *Opt) { // Check dst lightly - list above has checked ModTime/Hashes assert.Equal(t, file2Copy.Path, dst.Remote()) + // check that mutating dst does not mutate src + err = dst.SetModTime(ctx, fstest.Time("2004-03-03T04:05:06.499999999Z")) + if err != fs.ErrorCantSetModTimeWithoutDelete && err != fs.ErrorCantSetModTime { + assert.NoError(t, err) + assert.False(t, src.ModTime(ctx).Equal(dst.ModTime(ctx)), "mutating dst should not mutate src -- is it Copying by pointer?") + } + // Delete copy err = dst.Remove(ctx) require.NoError(t, err)