diff --git a/fs/operations.go b/fs/operations.go index fd8e35961..679eefa72 100644 --- a/fs/operations.go +++ b/fs/operations.go @@ -616,36 +616,52 @@ func ListDirSorted(fs Fs, includeAll bool, dir string) (entries DirEntries, err if err != nil { return nil, err } + return filterAndSortDir(entries, includeAll, dir, Config.Filter.IncludeObject, Config.Filter.IncludeDirectory) +} - // filter the entries if required - newEntries := entries[:0] // in place filter +// filter (if required) and check the entries, then sort them +func filterAndSortDir(entries DirEntries, includeAll bool, dir string, + IncludeObject func(o Object) bool, + IncludeDirectory func(remote string) bool) (newEntries DirEntries, err error) { + newEntries = entries[:0] // in place filter prefix := "" if dir != "" { prefix = dir + "/" } for _, entry := range entries { ok := true - if !includeAll { - switch x := entry.(type) { - case Object: - // Make sure we don't delete excluded files if not required - if !Config.Filter.IncludeObject(x) { - ok = false - Debugf(x, "Excluded from sync (and deletion)") - } - case Directory: - if !Config.Filter.IncludeDirectory(x.Remote()) { - ok = false - Debugf(x, "Excluded from sync (and deletion)") - } - default: - return nil, errors.Errorf("unknown object type %T", entry) + // check includes and types + switch x := entry.(type) { + case Object: + // Make sure we don't delete excluded files if not required + if !includeAll && !IncludeObject(x) { + ok = false + Debugf(x, "Excluded from sync (and deletion)") } + case Directory: + if !includeAll && !IncludeDirectory(x.Remote()) { + ok = false + Debugf(x, "Excluded from sync (and deletion)") + } + default: + return nil, errors.Errorf("unknown object type %T", entry) } + // check remote name belongs in this directry remote := entry.Remote() - if ok && (!strings.HasPrefix(remote, prefix) || remote == prefix) { + switch { + case !ok: + // ignore + case !strings.HasPrefix(remote, prefix): ok = false - Errorf(entry, "Entry doesn't belong in directory %q - ignoring", dir) + Errorf(entry, "Entry doesn't belong in directory %q (too short) - ignoring", dir) + case remote == prefix: + ok = false + Errorf(entry, "Entry doesn't belong in directory %q (same as directory) - ignoring", dir) + case strings.ContainsRune(remote[len(prefix):], '/'): + ok = false + Errorf(entry, "Entry doesn't belong in directory %q (contains subdir) - ignoring", dir) + default: + // ok } if ok { newEntries = append(newEntries, entry) diff --git a/fs/operations_internal_test.go b/fs/operations_internal_test.go new file mode 100644 index 000000000..e51ea44b4 --- /dev/null +++ b/fs/operations_internal_test.go @@ -0,0 +1,92 @@ +// Internal tests for operations + +package fs + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFilterAndSortIncludeAll(t *testing.T) { + da := newDir("a") + oA := mockObject("A") + db := newDir("b") + oB := mockObject("B") + dc := newDir("c") + oC := mockObject("C") + dd := newDir("d") + oD := mockObject("D") + entries := DirEntries{da, oA, db, oB, dc, oC, dd, oD} + includeObject := func(o Object) bool { + return o != oB + } + includeDirectory := func(remote string) bool { + return remote != "c" + } + // no filter + newEntries, err := filterAndSortDir(entries, true, "", includeObject, includeDirectory) + require.NoError(t, err) + assert.Equal(t, + newEntries, + DirEntries{oA, oB, oC, oD, da, db, dc, dd}, + ) + // filter + newEntries, err = filterAndSortDir(entries, false, "", includeObject, includeDirectory) + require.NoError(t, err) + assert.Equal(t, + newEntries, + DirEntries{oA, oC, oD, da, db, dd}, + ) +} + +func TestFilterAndSortCheckDir(t *testing.T) { + // Check the different kinds of error when listing "dir" + da := newDir("dir/") + oA := mockObject("diR/a") + db := newDir("dir/b") + oB := mockObject("dir/B/sub") + dc := newDir("dir/c") + oC := mockObject("dir/C") + dd := newDir("dir/d") + oD := mockObject("dir/D") + entries := DirEntries{da, oA, db, oB, dc, oC, dd, oD} + newEntries, err := filterAndSortDir(entries, true, "dir", nil, nil) + require.NoError(t, err) + assert.Equal(t, + newEntries, + DirEntries{oC, oD, db, dc, dd}, + ) +} + +func TestFilterAndSortCheckDirRoot(t *testing.T) { + // Check the different kinds of error when listing the root "" + da := newDir("") + oA := mockObject("A") + db := newDir("b") + oB := mockObject("B/sub") + dc := newDir("c") + oC := mockObject("C") + dd := newDir("d") + oD := mockObject("D") + entries := DirEntries{da, oA, db, oB, dc, oC, dd, oD} + newEntries, err := filterAndSortDir(entries, true, "", nil, nil) + require.NoError(t, err) + assert.Equal(t, + newEntries, + DirEntries{oA, oC, oD, db, dc, dd}, + ) +} + +func TestFilterAndSortUnknown(t *testing.T) { + // Check that an unknown entry produces an error + da := newDir("") + oA := mockObject("A") + ub := unknownDirEntry("b") + oB := mockObject("B/sub") + entries := DirEntries{da, oA, ub, oB} + newEntries, err := filterAndSortDir(entries, true, "", nil, nil) + assert.Error(t, err, "error") + assert.Nil(t, newEntries) +} diff --git a/fs/walk_test.go b/fs/walk_test.go index 3c934d2d6..12778e63a 100644 --- a/fs/walk_test.go +++ b/fs/walk_test.go @@ -54,6 +54,13 @@ func (o mockObject) Update(in io.Reader, src ObjectInfo, options ...OpenOption) } func (o mockObject) Remove() error { return errNotImpl } +type unknownDirEntry string + +func (o unknownDirEntry) String() string { return string(o) } +func (o unknownDirEntry) Remote() string { return string(o) } +func (o unknownDirEntry) ModTime() (t time.Time) { return t } +func (o unknownDirEntry) Size() int64 { return 0 } + func newListDirs(t *testing.T, f Fs, includeAll bool, results listResults, walkErrors errorMap, finalError error) *listDirs { return &listDirs{ t: t,