operations: fix overlapping check on case insensitive file systems

Before this change, the overlapping check could erroneously give this
error on case insensitive file systems:

    Failed to sync: destination and parameter to --backup-dir mustn't overlap

The code was fixed and re-worked to be simpler and more reliable.

See: https://forum.rclone.org/t/backup-dir-cannot-be-in-root-even-when-excluded/39844/

fs/operations/operations.go

@@ -826,17 +826,19 @@ func Same(fdst, fsrc fs.Info) bool {
 	return SameConfig(fdst, fsrc) && strings.Trim(fdst.Root(), "/") == strings.Trim(fsrc.Root(), "/")
 }
 
-// fixRoot returns the Root with a trailing / if not empty. It is
-// aware of case insensitive filesystems.
-func fixRoot(f fs.Info) string {
-	s := strings.Trim(filepath.ToSlash(f.Root()), "/")
+// fixRoot returns the Root with a trailing / if not empty.
+//
+// It returns a case folded version for case insensitive file systems
+func fixRoot(f fs.Info) (s string, folded string) {
+	s = strings.Trim(filepath.ToSlash(f.Root()), "/")
 	if s != "" {
 		s += "/"
 	}
+	folded = s
 	if f.Features().CaseInsensitive {
-		s = strings.ToLower(s)
+		folded = strings.ToLower(s)
 	}
-	return s
+	return s, folded
 }
 
 // OverlappingFilterCheck returns true if fdst and fsrc point to the same
@@ -845,37 +847,28 @@ 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) {
+	fdstRoot, fdstRootFolded := fixRoot(fdst)
+	fsrcRoot, fsrcRootFolded := fixRoot(fsrc)
+	if fdstRootFolded == fsrcRootFolded {
+		return true
+	} else if strings.HasPrefix(fdstRootFolded, fsrcRootFolded) {
 		fdstRelative := fdstRoot[len(fsrcRoot):]
-		return filterCheckR(ctx, fdstRelative, 0, fsrc)
+		return filterCheck(ctx, fsrc, fdstRelative)
+	} else if strings.HasPrefix(fsrcRootFolded, fdstRootFolded) {
+		fsrcRelative := fsrcRoot[len(fdstRoot):]
+		return filterCheck(ctx, fdst, fsrcRelative)
 	}
-	return strings.HasPrefix(fsrcRoot, fdstRoot)
+	return false
 }
 
-// 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
+// filterCheck checks if dir is included in f
+func filterCheck(ctx context.Context, f fs.Fs, dir string) bool {
 	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)
-		}
+	includeDirectory := fi.IncludeDirectory(ctx, f)
+	include, err := includeDirectory(dir)
+	if err != nil {
+		fs.Errorf(f, "Failed to discover whether directory is included: %v", err)
+		return true
 	}
 	return include
 }
@@ -886,9 +879,9 @@ func SameDir(fdst, fsrc fs.Info) bool {
 	if !SameConfig(fdst, fsrc) {
 		return false
 	}
-	fdstRoot := fixRoot(fdst)
-	fsrcRoot := fixRoot(fsrc)
-	return fdstRoot == fsrcRoot
+	_, fdstRootFolded := fixRoot(fdst)
+	_, fsrcRootFolded := fixRoot(fsrc)
+	return fdstRootFolded == fsrcRootFolded
 }
 
 // Retry runs fn up to maxTries times if it returns a retriable error

fs/operations/operations_test.go

@@ -1418,11 +1418,13 @@ 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 = []string{".ignore"}
+	require.NoError(t, fi.Add(false, "/exclude/"))
+	require.NoError(t, fi.Add(false, "/Exclude2/"))
+	require.NoError(t, fi.Add(true, "*"))
 	ctx = filter.ReplaceConfig(ctx, fi)
 
 	src := &testFs{testFsInfo{name: "name", root: "root"}}
+	src.features.CaseInsensitive = true
 	slash := string(os.PathSeparator) // native path separator
 	for _, test := range []struct {
 		name     string
@@ -1430,25 +1432,32 @@ func TestOverlappingFilterCheckWithFilter(t *testing.T) {
 		expected bool
 	}{
 		{"name", "root", true},
+		{"name", "ROOT", true}, // case insensitive is set
 		{"name", "/root", true},
 		{"name", "root/", true},
 		{"name", "root" + slash, true},
 		{"name", "root/exclude", false},
+		{"name", "root/Exclude2", false},
+		{"name", "root/include", true},
 		{"name", "root/exclude/", false},
+		{"name", "root/Exclude2/", false},
+		{"name", "root/exclude/sub", false},
+		{"name", "root/Exclude2/sub", 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}}
+		dst.features.CaseInsensitive = true
 		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)
+		actual = operations.OverlappingFilterCheck(ctx, src, dst)
+		assert.Equal(t, test.expected, actual, what)
 	}
 }
 

fs/sync/sync_test.go

@@ -1423,7 +1423,7 @@ func TestSyncOverlapWithFilter(t *testing.T) {
 	require.NoError(t, fi.Add(false, "/rclone-sync-test/"))
 	require.NoError(t, fi.Add(false, "*/layer2/"))
 	fi.Opt.ExcludeFile = []string{".ignore"}
-	ctx = filter.ReplaceConfig(ctx, fi)
+	filterCtx := filter.ReplaceConfig(ctx, fi)
 
 	subRemoteName := r.FremoteName + "/rclone-sync-test"
 	FremoteSync, err := fs.NewFs(ctx, subRemoteName)
@@ -1453,19 +1453,27 @@ func TestSyncOverlapWithFilter(t *testing.T) {
 	}
 
 	accounting.GlobalStats().ResetCounters()
-	checkNoErr(Sync(ctx, FremoteSync, r.Fremote, false))
+	checkNoErr(Sync(filterCtx, FremoteSync, r.Fremote, false))
+	checkErr(Sync(ctx, FremoteSync, r.Fremote, false))
+	checkNoErr(Sync(filterCtx, r.Fremote, FremoteSync, false))
 	checkErr(Sync(ctx, r.Fremote, FremoteSync, false))
+	checkErr(Sync(filterCtx, r.Fremote, r.Fremote, false))
 	checkErr(Sync(ctx, r.Fremote, r.Fremote, false))
+	checkErr(Sync(filterCtx, FremoteSync, FremoteSync, false))
 	checkErr(Sync(ctx, FremoteSync, FremoteSync, false))
 
-	checkNoErr(Sync(ctx, FremoteSync2, r.Fremote, false))
+	checkNoErr(Sync(filterCtx, FremoteSync2, r.Fremote, false))
+	checkErr(Sync(ctx, FremoteSync2, r.Fremote, false))
+	checkNoErr(Sync(filterCtx, r.Fremote, FremoteSync2, false))
 	checkErr(Sync(ctx, r.Fremote, FremoteSync2, false))
-	checkErr(Sync(ctx, r.Fremote, r.Fremote, false))
+	checkErr(Sync(filterCtx, FremoteSync2, FremoteSync2, false))
 	checkErr(Sync(ctx, FremoteSync2, FremoteSync2, false))
 
-	checkNoErr(Sync(ctx, FremoteSync3, r.Fremote, false))
+	checkNoErr(Sync(filterCtx, FremoteSync3, r.Fremote, false))
+	checkErr(Sync(ctx, FremoteSync3, r.Fremote, false))
+	checkNoErr(Sync(filterCtx, r.Fremote, FremoteSync3, false))
 	checkErr(Sync(ctx, r.Fremote, FremoteSync3, false))
-	checkErr(Sync(ctx, r.Fremote, r.Fremote, false))
+	checkErr(Sync(filterCtx, FremoteSync3, FremoteSync3, false))
 	checkErr(Sync(ctx, FremoteSync3, FremoteSync3, false))
 }