From 8b8156f7c3cfbe18800c720a5901b5bf19970cc5 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Fri, 8 Dec 2023 14:00:22 +0000
Subject: [PATCH] chunker,compress,crypt,hasher,union: fix rclone move a file
 over itself deleting the file

This fixes the Root() returned by the backend when it has returned
fs.ErrorIsFile.

Before this change it returned a root which included the file path.

Because Root() was wrong this caused the detection of the file being
moved over itself check to fail.

This adds an integration test to check it for all backends.

See: https://forum.rclone.org/t/rclone-move-chunker-dir-file-chunker-dir-deletes-all-file-chunks/43333/
---
 backend/chunker/chunker.go   |  8 ++++++++
 backend/compress/compress.go |  8 ++++++++
 backend/crypt/crypt.go       |  7 +++++++
 backend/hasher/hasher.go     |  7 +++++++
 backend/union/union.go       |  7 +++++++
 fstest/fstests/fstests.go    | 18 ++++++++++++++++++
 6 files changed, 55 insertions(+)

diff --git a/backend/chunker/chunker.go b/backend/chunker/chunker.go
index bb4b49812..baf4656ed 100644
--- a/backend/chunker/chunker.go
+++ b/backend/chunker/chunker.go
@@ -325,6 +325,14 @@ func NewFs(ctx context.Context, name, rpath string, m configmap.Mapper) (fs.Fs,
 		}
 	}
 
+	// Correct root if definitely pointing to a file
+	if err == fs.ErrorIsFile {
+		f.root = path.Dir(f.root)
+		if f.root == "." || f.root == "/" {
+			f.root = ""
+		}
+	}
+
 	// Note 1: the features here are ones we could support, and they are
 	// ANDed with the ones from wrappedFs.
 	// Note 2: features.Fill() points features.PutStream to our PutStream,
diff --git a/backend/compress/compress.go b/backend/compress/compress.go
index f635c5e59..b477512e7 100644
--- a/backend/compress/compress.go
+++ b/backend/compress/compress.go
@@ -14,6 +14,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path"
 	"regexp"
 	"strings"
 	"time"
@@ -172,6 +173,13 @@ func NewFs(ctx context.Context, name, rpath string, m configmap.Mapper) (fs.Fs,
 		opt:  *opt,
 		mode: compressionModeFromName(opt.CompressionMode),
 	}
+	// Correct root if definitely pointing to a file
+	if err == fs.ErrorIsFile {
+		f.root = path.Dir(f.root)
+		if f.root == "." || f.root == "/" {
+			f.root = ""
+		}
+	}
 	// the features here are ones we could support, and they are
 	// ANDed with the ones from wrappedFs
 	f.features = (&fs.Features{
diff --git a/backend/crypt/crypt.go b/backend/crypt/crypt.go
index 281805fd5..54b1335dc 100644
--- a/backend/crypt/crypt.go
+++ b/backend/crypt/crypt.go
@@ -253,6 +253,13 @@ func NewFs(ctx context.Context, name, rpath string, m configmap.Mapper) (fs.Fs,
 		cipher: cipher,
 	}
 	cache.PinUntilFinalized(f.Fs, f)
+	// Correct root if definitely pointing to a file
+	if err == fs.ErrorIsFile {
+		f.root = path.Dir(f.root)
+		if f.root == "." || f.root == "/" {
+			f.root = ""
+		}
+	}
 	// the features here are ones we could support, and they are
 	// ANDed with the ones from wrappedFs
 	f.features = (&fs.Features{
diff --git a/backend/hasher/hasher.go b/backend/hasher/hasher.go
index 3428b4dc2..325f7f619 100644
--- a/backend/hasher/hasher.go
+++ b/backend/hasher/hasher.go
@@ -114,6 +114,13 @@ func NewFs(ctx context.Context, fsname, rpath string, cmap configmap.Mapper) (fs
 		root: rpath,
 		opt:  opt,
 	}
+	// Correct root if definitely pointing to a file
+	if err == fs.ErrorIsFile {
+		f.root = path.Dir(f.root)
+		if f.root == "." || f.root == "/" {
+			f.root = ""
+		}
+	}
 	baseFeatures := baseFs.Features()
 	f.fpTime = baseFs.Precision() != fs.ModTimeNotSupported
 
diff --git a/backend/union/union.go b/backend/union/union.go
index eeaed8f31..5854afc11 100644
--- a/backend/union/union.go
+++ b/backend/union/union.go
@@ -877,6 +877,13 @@ func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (fs.Fs, e
 		opt:       *opt,
 		upstreams: usedUpstreams,
 	}
+	// Correct root if definitely pointing to a file
+	if fserr == fs.ErrorIsFile {
+		f.root = path.Dir(f.root)
+		if f.root == "." || f.root == "/" {
+			f.root = ""
+		}
+	}
 	err = upstream.Prepare(f.upstreams)
 	if err != nil {
 		return nil, err
diff --git a/fstest/fstests/fstests.go b/fstest/fstests/fstests.go
index 7ab03dd60..095a94d23 100644
--- a/fstest/fstests/fstests.go
+++ b/fstest/fstests/fstests.go
@@ -1675,6 +1675,24 @@ func Run(t *testing.T, opt *Opt) {
 				require.NotNil(t, fileRemote)
 				assert.Equal(t, fs.ErrorIsFile, err)
 
+				// Check Fs.Root returns the right thing
+				t.Run("FsRoot", func(t *testing.T) {
+					skipIfNotOk(t)
+					got := fileRemote.Root()
+					remoteDir := path.Dir(remoteName)
+					want := remoteDir
+					colon := strings.LastIndex(want, ":")
+					if colon >= 0 {
+						want = want[colon+1:]
+					}
+					if isLocalRemote {
+						// only check last path element on local
+						require.Equal(t, filepath.Base(remoteDir), filepath.Base(got))
+					} else {
+						require.Equal(t, want, got)
+					}
+				})
+
 				if strings.HasPrefix(remoteName, "TestChunker") && strings.Contains(remoteName, "Nometa") {
 					// TODO fix chunker and remove this bypass
 					t.Logf("Skip listing check -- chunker can't yet handle this tricky case")