sync: overlap check is now filter-sensitive

Previously, the overlap check was based on simple prefix checks of the source and destination paths. Now it actually checks whether the destination is excluded via any filter rule or a "--exclude-if-present"-file.

fs/operations/operations.go

@@ -796,6 +796,47 @@ func Overlapping(fdst, fsrc fs.Info) bool {
 	return strings.HasPrefix(fdstRoot, fsrcRoot) || strings.HasPrefix(fsrcRoot, fdstRoot)
 }
 
+// OverlappingFilterCheck returns true if fdst and fsrc point to the same
+// underlying Fs and they overlap without fdst being excluded by any filter rule.
+func OverlappingFilterCheck(ctx context.Context, fdst fs.Fs, fsrc fs.Fs) bool {
+	if !SameConfig(fdst, fsrc) {
+		return false
+	}
+	fdstRoot := fixRoot(fdst)
+	fsrcRoot := fixRoot(fsrc)
+	if strings.HasPrefix(fdstRoot, fsrcRoot) {
+		fdstRelative := fdstRoot[len(fsrcRoot):]
+		return filterCheckR(ctx, fdstRelative, 0, fsrc)
+	}
+	return strings.HasPrefix(fsrcRoot, fdstRoot)
+}
+
+// filterCheckR checks if fdst would be included in the sync
+func filterCheckR(ctx context.Context, fdstRelative string, pos int, fsrc fs.Fs) bool {
+	include := true
+	fi := filter.GetConfig(ctx)
+	includeDirectory := fi.IncludeDirectory(ctx, fsrc)
+	dirs := strings.SplitAfterN(fdstRelative, "/", pos+2)
+	newPath := ""
+	for i := 0; i <= pos; i++ {
+		newPath += dirs[i]
+	}
+	if !strings.HasSuffix(newPath, "/") {
+		newPath += "/"
+	}
+	if strings.HasPrefix(fdstRelative, newPath) {
+		include, _ = includeDirectory(newPath)
+		if include {
+			if newPath == fdstRelative {
+				return true
+			}
+			pos++
+			include = filterCheckR(ctx, fdstRelative, pos, fsrc)
+		}
+	}
+	return include
+}
+
 // SameDir returns true if fdst and fsrc point to the same
 // underlying Fs and they are the same directory.
 func SameDir(fdst, fsrc fs.Info) bool {

fs/operations/operations_test.go

@@ -1253,6 +1253,93 @@ func TestOverlapping(t *testing.T) {
 	}
 }
 
+// testFs is for unit testing fs.Fs
+type testFs struct {
+	testFsInfo
+}
+
+func (i *testFs) List(ctx context.Context, dir string) (entries fs.DirEntries, err error) {
+	return nil, nil
+}
+
+func (i *testFs) NewObject(ctx context.Context, remote string) (fs.Object, error) { return nil, nil }
+
+func (i *testFs) Put(ctx context.Context, in io.Reader, src fs.ObjectInfo, options ...fs.OpenOption) (fs.Object, error) {
+	return nil, nil
+}
+
+func (i *testFs) Mkdir(ctx context.Context, dir string) error { return nil }
+
+func (i *testFs) Rmdir(ctx context.Context, dir string) error { return nil }
+
+// copied from TestOverlapping because the behavior of OverlappingFilterCheck should be identical to Overlapping
+// when no filters are set
+func TestOverlappingFilterCheckWithoutFilter(t *testing.T) {
+	ctx := context.Background()
+	src := &testFs{testFsInfo{name: "name", root: "root"}}
+	slash := string(os.PathSeparator) // native path separator
+	for _, test := range []struct {
+		name     string
+		root     string
+		expected bool
+	}{
+		{"name", "root", true},
+		{"namey", "root", false},
+		{"name", "rooty", false},
+		{"namey", "rooty", false},
+		{"name", "roo", false},
+		{"name", "root/toot", true},
+		{"name", "root/toot/", true},
+		{"name", "root" + slash + "toot", true},
+		{"name", "root" + slash + "toot" + slash, true},
+		{"name", "", true},
+		{"name", "/", true},
+	} {
+		dst := &testFs{testFsInfo{name: test.name, root: test.root}}
+		what := fmt.Sprintf("(%q,%q) vs (%q,%q)", src.name, src.root, dst.name, dst.root)
+		actual := operations.OverlappingFilterCheck(ctx, src, dst)
+		assert.Equal(t, test.expected, actual, what)
+		actual = operations.OverlappingFilterCheck(ctx, dst, src)
+		assert.Equal(t, test.expected, actual, what)
+	}
+}
+
+func TestOverlappingFilterCheckWithFilter(t *testing.T) {
+	ctx := context.Background()
+	fi, err := filter.NewFilter(nil)
+	require.NoError(t, err)
+	require.NoError(t, fi.Add(false, "*/exclude/"))
+	fi.Opt.ExcludeFile = ".ignore"
+	ctx = filter.ReplaceConfig(ctx, fi)
+
+	src := &testFs{testFsInfo{name: "name", root: "root"}}
+	slash := string(os.PathSeparator) // native path separator
+	for _, test := range []struct {
+		name     string
+		root     string
+		expected bool
+	}{
+		{"name", "root", true},
+		{"name", "root/", true},
+		{"name", "root" + slash, true},
+		{"name", "root/exclude", false},
+		{"name", "root/exclude/", false},
+		{"name", "root" + slash + "exclude", false},
+		{"name", "root" + slash + "exclude" + slash, false},
+		{"name", "root/.ignore", false},
+		{"name", "root" + slash + ".ignore", false},
+		{"namey", "root/include", false},
+		{"namey", "root/include/", false},
+		{"namey", "root" + slash + "include", false},
+		{"namey", "root" + slash + "include" + slash, false},
+	} {
+		dst := &testFs{testFsInfo{name: test.name, root: test.root}}
+		what := fmt.Sprintf("(%q,%q) vs (%q,%q)", src.name, src.root, dst.name, dst.root)
+		actual := operations.OverlappingFilterCheck(ctx, dst, src)
+		assert.Equal(t, test.expected, actual, what)
+	}
+}
+
 func TestListFormat(t *testing.T) {
 	item0 := &operations.ListJSONItem{
 		Path:      "a",

fs/sync/sync.go

@@ -97,7 +97,7 @@ func (strategy trackRenamesStrategy) leaf() bool {
 }
 
 func newSyncCopyMove(ctx context.Context, fdst, fsrc fs.Fs, deleteMode fs.DeleteMode, DoMove bool, deleteEmptySrcDirs bool, copyEmptySrcDirs bool) (*syncCopyMove, error) {
-	if (deleteMode != fs.DeleteModeOff || DoMove) && operations.Overlapping(fdst, fsrc) {
+	if (deleteMode != fs.DeleteModeOff || DoMove) && operations.OverlappingFilterCheck(ctx, fdst, fsrc) {
 		return nil, fserrors.FatalError(fs.ErrorOverlapping)
 	}
 	ci := fs.GetConfig(ctx)

fs/sync/sync_test.go

@@ -1442,6 +1442,61 @@ func TestSyncOverlap(t *testing.T) {
 	checkErr(Sync(ctx, FremoteSync, FremoteSync, false))
 }
 
+// Test a sync with filtered overlap
+func TestSyncOverlapWithFilter(t *testing.T) {
+	ctx := context.Background()
+	r := fstest.NewRun(t)
+	defer r.Finalise()
+
+	fi, err := filter.NewFilter(nil)
+	require.NoError(t, err)
+	require.NoError(t, fi.Add(false, "/rclone-sync-test/"))
+	require.NoError(t, fi.Add(false, "*/layer2/"))
+	fi.Opt.ExcludeFile = ".ignore"
+	ctx = filter.ReplaceConfig(ctx, fi)
+
+	subRemoteName := r.FremoteName + "/rclone-sync-test"
+	FremoteSync, err := fs.NewFs(ctx, subRemoteName)
+	require.NoError(t, FremoteSync.Mkdir(ctx, ""))
+	require.NoError(t, err)
+
+	subRemoteName2 := r.FremoteName + "/rclone-sync-test-include/layer2"
+	FremoteSync2, err := fs.NewFs(ctx, subRemoteName2)
+	require.NoError(t, FremoteSync2.Mkdir(ctx, ""))
+	require.NoError(t, err)
+
+	subRemoteName3 := r.FremoteName + "/rclone-sync-test-ignore-file"
+	FremoteSync3, err := fs.NewFs(ctx, subRemoteName3)
+	require.NoError(t, FremoteSync3.Mkdir(ctx, ""))
+	require.NoError(t, err)
+	r.WriteObject(context.Background(), "/rclone-sync-test-ignore-file/.ignore", "-", t1)
+
+	checkErr := func(err error) {
+		require.Error(t, err)
+		assert.True(t, fserrors.IsFatalError(err))
+		assert.Equal(t, fs.ErrorOverlapping.Error(), err.Error())
+	}
+
+	checkNoErr := func(err error) {
+		require.NoError(t, err)
+	}
+
+	checkNoErr(Sync(ctx, FremoteSync, r.Fremote, false))
+	checkErr(Sync(ctx, r.Fremote, FremoteSync, false))
+	checkErr(Sync(ctx, r.Fremote, r.Fremote, false))
+	checkErr(Sync(ctx, FremoteSync, FremoteSync, false))
+
+	checkNoErr(Sync(ctx, FremoteSync2, r.Fremote, false))
+	checkErr(Sync(ctx, r.Fremote, FremoteSync2, false))
+	checkErr(Sync(ctx, r.Fremote, r.Fremote, false))
+	checkErr(Sync(ctx, FremoteSync2, FremoteSync2, false))
+
+	checkNoErr(Sync(ctx, FremoteSync3, r.Fremote, false))
+	checkErr(Sync(ctx, r.Fremote, FremoteSync3, false))
+	checkErr(Sync(ctx, r.Fremote, r.Fremote, false))
+	checkErr(Sync(ctx, FremoteSync3, FremoteSync3, false))
+}
+
 // Test with CompareDest set
 func TestSyncCompareDest(t *testing.T) {
 	ctx := context.Background()