From f9ef0775db1ecd575b379b81e9a85a090abb07c2 Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Thu, 16 Jul 2020 15:31:57 +0200 Subject: [PATCH] File Browser: Follow symlinks #201 #403 #407 Warning: Following symlinks can make folder lists non-deterministic Signed-off-by: Michael Mayer --- internal/api/folder_test.go | 8 +-- internal/query/folders.go | 2 +- pkg/fs/dirs.go | 42 +++++++++++--- pkg/fs/dirs_test.go | 55 +++++++++++++++++-- pkg/fs/ignore_test.go | 1 + .../directory/subdirectory/bar/helloworld.md | 1 + pkg/fs/testdata/linked/photoprism | 1 + 7 files changed, 91 insertions(+), 19 deletions(-) create mode 100644 pkg/fs/testdata/directory/subdirectory/bar/helloworld.md create mode 120000 pkg/fs/testdata/linked/photoprism diff --git a/internal/api/folder_test.go b/internal/api/folder_test.go index 2a45e0b19..94bc1f665 100644 --- a/internal/api/folder_test.go +++ b/internal/api/folder_test.go @@ -13,7 +13,7 @@ func TestGetFoldersOriginals(t *testing.T) { t.Run("flat", func(t *testing.T) { app, router, conf := NewApiTest() _ = conf.CreateDirectories() - expected, err := fs.Dirs(conf.OriginalsPath(), false) + expected, err := fs.Dirs(conf.OriginalsPath(), false, true) if err != nil { t.Fatal(err) @@ -56,7 +56,7 @@ func TestGetFoldersOriginals(t *testing.T) { t.Run("recursive", func(t *testing.T) { app, router, conf := NewApiTest() _ = conf.CreateDirectories() - expected, err := fs.Dirs(conf.OriginalsPath(), true) + expected, err := fs.Dirs(conf.OriginalsPath(), true, true) if err != nil { t.Fatal(err) @@ -96,7 +96,7 @@ func TestGetFoldersImport(t *testing.T) { t.Run("flat", func(t *testing.T) { app, router, conf := NewApiTest() _ = conf.CreateDirectories() - expected, err := fs.Dirs(conf.ImportPath(), false) + expected, err := fs.Dirs(conf.ImportPath(), false, true) if err != nil { t.Fatal(err) @@ -140,7 +140,7 @@ func TestGetFoldersImport(t *testing.T) { t.Run("recursive", func(t *testing.T) { app, router, conf := NewApiTest() _ = conf.CreateDirectories() - expected, err := fs.Dirs(conf.ImportPath(), true) + expected, err := fs.Dirs(conf.ImportPath(), true, true) if err != nil { t.Fatal(err) diff --git a/internal/query/folders.go b/internal/query/folders.go index c509fab45..fde245576 100644 --- a/internal/query/folders.go +++ b/internal/query/folders.go @@ -9,7 +9,7 @@ import ( // FoldersByPath returns a slice of folders in a given directory incl sub directories in recursive mode. func FoldersByPath(rootName, rootPath, path string, recursive bool) (folders entity.Folders, err error) { - dirs, err := fs.Dirs(filepath.Join(rootPath, path), recursive) + dirs, err := fs.Dirs(filepath.Join(rootPath, path), recursive, true) if err != nil { return folders, err diff --git a/pkg/fs/dirs.go b/pkg/fs/dirs.go index 1da07235a..e0d9ce5bd 100644 --- a/pkg/fs/dirs.go +++ b/pkg/fs/dirs.go @@ -88,25 +88,51 @@ var ImportPaths = []string{ "~/Import", } -func Dirs(root string, recursive bool) (result []string, err error) { +// Dirs returns a slice of directories in a path, optional recursively and with symlinks. +// +// Warning: Following symlinks can make the result non-deterministic and hard to test! +func Dirs(root string, recursive bool, followLinks bool) (result []string, err error) { result = []string{} ignore := NewIgnoreList(".ppignore", true, false) mutex := sync.Mutex{} - err = fastwalk.Walk(root, func(fileName string, info os.FileMode) error { - if info.IsDir() { + symlinks := make(map[string]bool) + symlinksMutex := sync.Mutex{} + + appendResult := func(fileName string) { + fileName = strings.Replace(fileName, root, "", 1) + mutex.Lock() + defer mutex.Unlock() + result = append(result, fileName) + } + + err = fastwalk.Walk(root, func(fileName string, typ os.FileMode) error { + if typ.IsDir() || typ == os.ModeSymlink && followLinks { if ignore.Ignore(fileName) { return filepath.SkipDir } if fileName != root { - mutex.Lock() - fileName = strings.Replace(fileName, root, "", 1) - result = append(result, fileName) - mutex.Unlock() - if !recursive { + appendResult(fileName) + return filepath.SkipDir + } else if typ != os.ModeSymlink { + appendResult(fileName) + + return nil + } else if resolved, err := filepath.EvalSymlinks(fileName); err == nil { + symlinksMutex.Lock() + defer symlinksMutex.Unlock() + + if _, ok := symlinks[resolved]; ok { + return filepath.SkipDir + } else { + symlinks[resolved] = true + appendResult(fileName) + } + + return fastwalk.ErrTraverseLink } } } diff --git a/pkg/fs/dirs_test.go b/pkg/fs/dirs_test.go index 68322dc58..d6f685682 100644 --- a/pkg/fs/dirs_test.go +++ b/pkg/fs/dirs_test.go @@ -9,27 +9,70 @@ import ( func TestDirs(t *testing.T) { t.Run("recursive", func(t *testing.T) { - result, err := Dirs("testdata", true) + result, err := Dirs("testdata", true, true) if err != nil { t.Fatal(err) } - expected := []string{"/directory", "/directory/subdirectory", "/linked"} + assert.Contains(t, result, "/directory") + assert.Contains(t, result, "/directory/subdirectory") + assert.Contains(t, result, "/linked") + assert.Contains(t, result, "/linked/photoprism") + assert.Contains(t, result, "/linked/photoprism/sub") + }) - assert.Equal(t, expected, result) + t.Run("recursive no-symlinks", func(t *testing.T) { + result, err := Dirs("testdata", true, false) + + if err != nil { + t.Fatal(err) + } + + assert.Contains(t, result, "/directory") + assert.Contains(t, result, "/directory/subdirectory") + assert.Contains(t, result, "/linked") }) t.Run("non-recursive", func(t *testing.T) { - result, err := Dirs("testdata", false) + result, err := Dirs("testdata", false, true) if err != nil { t.Fatal(err) } - expected := []string{"/directory", "/linked"} + assert.Contains(t, result, "/directory") + assert.Contains(t, result, "/linked") + }) - assert.Equal(t, expected, result) + t.Run("non-recursive no-symlinks", func(t *testing.T) { + result, err := Dirs("testdata/directory/subdirectory", false, false) + + if err != nil { + t.Fatal(err) + } + assert.Contains(t, result, "/bar") + }) + + t.Run("non-recursive symlinks", func(t *testing.T) { + result, err := Dirs("testdata/linked", false, true) + + if err != nil { + t.Fatal(err) + } + + assert.Contains(t, result, "/photoprism") + assert.Contains(t, result, "/self") + }) + + t.Run("no-result", func(t *testing.T) { + result, err := Dirs("testdata/linked", false, false) + + if err != nil { + t.Fatal(err) + } + + assert.Empty(t, result) }) } diff --git a/pkg/fs/ignore_test.go b/pkg/fs/ignore_test.go index 1320c8808..b1605e3ae 100644 --- a/pkg/fs/ignore_test.go +++ b/pkg/fs/ignore_test.go @@ -94,6 +94,7 @@ func TestIgnoreList_Ignored(t *testing.T) { expectIgnored := []string{ "testdata/directory/bar.txt", "testdata/directory/baz.xml", + "testdata/directory/subdirectory/bar", "testdata/directory/subdirectory/example.txt", "testdata/directory/subdirectory/foo.txt", } diff --git a/pkg/fs/testdata/directory/subdirectory/bar/helloworld.md b/pkg/fs/testdata/directory/subdirectory/bar/helloworld.md new file mode 100644 index 000000000..cc0be1e56 --- /dev/null +++ b/pkg/fs/testdata/directory/subdirectory/bar/helloworld.md @@ -0,0 +1 @@ +# Hello World! diff --git a/pkg/fs/testdata/linked/photoprism b/pkg/fs/testdata/linked/photoprism new file mode 120000 index 000000000..b9fb6acee --- /dev/null +++ b/pkg/fs/testdata/linked/photoprism @@ -0,0 +1 @@ +../.photoprism \ No newline at end of file