From 3ccf222acb9697c38cb0c28ddc0e43c5343b2e58 Mon Sep 17 00:00:00 2001 From: Nick Date: Wed, 1 Jun 2022 19:24:54 +0200 Subject: [PATCH] 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 | 41 +++++++++++++++ fs/operations/operations_test.go | 87 ++++++++++++++++++++++++++++++++ fs/sync/sync.go | 2 +- fs/sync/sync_test.go | 55 ++++++++++++++++++++ 4 files changed, 184 insertions(+), 1 deletion(-) diff --git a/fs/operations/operations.go b/fs/operations/operations.go index 2807657e4..c34cdf97d 100644 --- a/fs/operations/operations.go +++ b/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 { diff --git a/fs/operations/operations_test.go b/fs/operations/operations_test.go index a47ad0ae2..363d88f59 100644 --- a/fs/operations/operations_test.go +++ b/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", diff --git a/fs/sync/sync.go b/fs/sync/sync.go index 6e71af499..f9844ea16 100644 --- a/fs/sync/sync.go +++ b/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) diff --git a/fs/sync/sync_test.go b/fs/sync/sync_test.go index 0520a2323..bd0772bd6 100644 --- a/fs/sync/sync_test.go +++ b/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()