Make ListDirSorted check for subdirectories and write test

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
+// filter (if required) and check the entries, then sort them
-	newEntries := entries[:0] // in place filter
+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 {
+		// check includes and types
-			switch x := entry.(type) {
+		switch x := entry.(type) {
-			case Object:
+		case Object:
-				// Make sure we don't delete excluded files if not required
+			// Make sure we don't delete excluded files if not required
-				if !Config.Filter.IncludeObject(x) {
+			if !includeAll && !IncludeObject(x) {
-					ok = false
+				ok = false
-					Debugf(x, "Excluded from sync (and deletion)")
+				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)
 			}
+		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)

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)
+}

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,