From 84c650818ebb0cf96672d3f8a695cfc086ada495 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Thu, 14 Feb 2019 12:06:26 +0000
Subject: [PATCH] sync: don't allow syncs on overlapping remotes - fixes #2932

---
 fs/fs.go                    |  2 +-
 fs/operations/operations.go |  2 +-
 fs/sync/sync.go             | 10 +++-------
 fs/sync/sync_test.go        | 25 +++++++++++++++++++++++--
 4 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/fs/fs.go b/fs/fs.go
index 25ada9701..0b1cd5b2a 100644
--- a/fs/fs.go
+++ b/fs/fs.go
@@ -61,7 +61,7 @@ var (
 	ErrorNotAFile                    = errors.New("is a not a regular file")
 	ErrorNotDeleting                 = errors.New("not deleting files as there were IO errors")
 	ErrorNotDeletingDirs             = errors.New("not deleting directories as there were IO errors")
-	ErrorCantMoveOverlapping         = errors.New("can't move files on overlapping remotes")
+	ErrorOverlapping                 = errors.New("can't sync or move files on overlapping remotes")
 	ErrorDirectoryNotEmpty           = errors.New("directory not empty")
 	ErrorImmutableModified           = errors.New("immutable file modified")
 	ErrorPermissionDenied            = errors.New("permission denied")
diff --git a/fs/operations/operations.go b/fs/operations/operations.go
index a90732cc3..b97fade44 100644
--- a/fs/operations/operations.go
+++ b/fs/operations/operations.go
@@ -532,7 +532,7 @@ func SameConfig(fdst, fsrc fs.Info) bool {
 
 // Same returns true if fdst and fsrc point to the same underlying Fs
 func Same(fdst, fsrc fs.Info) bool {
-	return SameConfig(fdst, fsrc) && fdst.Root() == fsrc.Root()
+	return SameConfig(fdst, fsrc) && strings.Trim(fdst.Root(), "/") == strings.Trim(fsrc.Root(), "/")
 }
 
 // Overlapping returns true if fdst and fsrc point to the same
diff --git a/fs/sync/sync.go b/fs/sync/sync.go
index 15ee2fd56..c11a01428 100644
--- a/fs/sync/sync.go
+++ b/fs/sync/sync.go
@@ -64,6 +64,9 @@ type syncCopyMove struct {
 }
 
 func newSyncCopyMove(fdst, fsrc fs.Fs, deleteMode fs.DeleteMode, DoMove bool, deleteEmptySrcDirs bool) (*syncCopyMove, error) {
+	if (deleteMode != fs.DeleteModeOff || DoMove) && operations.Overlapping(fdst, fsrc) {
+		return nil, fserrors.FatalError(fs.ErrorOverlapping)
+	}
 	s := &syncCopyMove{
 		fdst:               fdst,
 		fsrc:               fsrc,
@@ -920,13 +923,6 @@ func MoveDir(fdst, fsrc fs.Fs, deleteEmptySrcDirs bool) error {
 		}
 	}
 
-	// The two remotes mustn't overlap if we didn't do server side move
-	if operations.Overlapping(fdst, fsrc) {
-		err := fs.ErrorCantMoveOverlapping
-		fs.Errorf(fdst, "%v", err)
-		return err
-	}
-
 	// Otherwise move the files one by one
 	return moveDir(fdst, fsrc, deleteEmptySrcDirs)
 }
diff --git a/fs/sync/sync_test.go b/fs/sync/sync_test.go
index f98988387..f2e2a533c 100644
--- a/fs/sync/sync_test.go
+++ b/fs/sync/sync_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/ncw/rclone/fs"
 	"github.com/ncw/rclone/fs/accounting"
 	"github.com/ncw/rclone/fs/filter"
+	"github.com/ncw/rclone/fs/fserrors"
 	"github.com/ncw/rclone/fs/hash"
 	"github.com/ncw/rclone/fs/operations"
 	"github.com/ncw/rclone/fstest"
@@ -1102,7 +1103,7 @@ func TestServerSideMoveOverlap(t *testing.T) {
 
 	// Subdir move with no filters should return ErrorCantMoveOverlapping
 	err = MoveDir(FremoteMove, r.Fremote, false)
-	assert.EqualError(t, err, fs.ErrorCantMoveOverlapping.Error())
+	assert.EqualError(t, err, fs.ErrorOverlapping.Error())
 
 	// Now try with a filter which should also fail with ErrorCantMoveOverlapping
 	filter.Active.Opt.MinSize = 40
@@ -1110,7 +1111,27 @@ func TestServerSideMoveOverlap(t *testing.T) {
 		filter.Active.Opt.MinSize = -1
 	}()
 	err = MoveDir(FremoteMove, r.Fremote, false)
-	assert.EqualError(t, err, fs.ErrorCantMoveOverlapping.Error())
+	assert.Equal(t, err, fs.ErrorOverlapping)
+}
+
+// Test a sync with overlap
+func TestSyncOverlap(t *testing.T) {
+	r := fstest.NewRun(t)
+	defer r.Finalise()
+
+	subRemoteName := r.FremoteName + "/rclone-sync-test"
+	FremoteSync, err := fs.NewFs(subRemoteName)
+	require.NoError(t, err)
+
+	checkErr := func(err error) {
+		assert.True(t, fserrors.IsFatalError(err))
+		assert.Equal(t, err.Error(), fs.ErrorOverlapping.Error())
+	}
+
+	checkErr(Sync(FremoteSync, r.Fremote))
+	checkErr(Sync(r.Fremote, FremoteSync))
+	checkErr(Sync(r.Fremote, r.Fremote))
+	checkErr(Sync(FremoteSync, FremoteSync))
 }
 
 // Test with BackupDir set