From ffbbe772ff95613c8043d4d5aaf4ea6c8bcf111e Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 19 Dec 2023 17:57:56 +0100 Subject: [PATCH 01/11] Added generic filtering function for PathList --- list.go | 14 ++++++++------ list_test.go | 6 ++++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/list.go b/list.go index 7892fa0..9646e6d 100644 --- a/list.go +++ b/list.go @@ -91,10 +91,12 @@ func (p *PathList) FilterOutHiddenFiles() { p.FilterOutPrefix(".") } -func (p *PathList) filter(filter func(*Path) bool) { +// Filter will remove all the elements of the list that do not match +// the specified acceptor function +func (p *PathList) Filter(accpetorFunc func(*Path) bool) { res := (*p)[:0] for _, path := range *p { - if filter(path) { + if accpetorFunc(path) { res = append(res, path) } } @@ -106,7 +108,7 @@ func (p *PathList) FilterOutPrefix(prefixes ...string) { filterFunction := func(path *Path) bool { return !path.HasPrefix(prefixes...) } - p.filter(filterFunction) + p.Filter(filterFunction) } // FilterPrefix remove all entries not having one of the specified prefixes @@ -114,7 +116,7 @@ func (p *PathList) FilterPrefix(prefixes ...string) { filterFunction := func(path *Path) bool { return path.HasPrefix(prefixes...) } - p.filter(filterFunction) + p.Filter(filterFunction) } // FilterOutSuffix remove all entries having one of the specified suffixes @@ -122,7 +124,7 @@ func (p *PathList) FilterOutSuffix(suffixies ...string) { filterFunction := func(path *Path) bool { return !path.HasSuffix(suffixies...) } - p.filter(filterFunction) + p.Filter(filterFunction) } // FilterSuffix remove all entries not having one of the specified suffixes @@ -130,7 +132,7 @@ func (p *PathList) FilterSuffix(suffixies ...string) { filterFunction := func(path *Path) bool { return path.HasSuffix(suffixies...) } - p.filter(filterFunction) + p.Filter(filterFunction) } // Add adds a Path to the PathList diff --git a/list_test.go b/list_test.go index c7d9354..eaafc82 100644 --- a/list_test.go +++ b/list_test.go @@ -160,4 +160,10 @@ func TestListFilters(t *testing.T) { l16 := list.Clone() l16.FilterPrefix() require.Equal(t, "[]", fmt.Sprintf("%s", l16)) + + l17 := list.Clone() + l17.Filter(func(p *Path) bool { + return p.Base() == "bbbb" + }) + require.Equal(t, "[bbbb aaaa/bbbb]", fmt.Sprintf("%s", l17)) } From 25a33cedabb463a5deb8cd2dfe2d245c9125e23a Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 19 Dec 2023 18:06:57 +0100 Subject: [PATCH 02/11] Apply suggestions from code review Co-authored-by: Alessio Perugini --- list.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/list.go b/list.go index 9646e6d..4ddecc1 100644 --- a/list.go +++ b/list.go @@ -93,10 +93,10 @@ func (p *PathList) FilterOutHiddenFiles() { // Filter will remove all the elements of the list that do not match // the specified acceptor function -func (p *PathList) Filter(accpetorFunc func(*Path) bool) { +func (p *PathList) Filter(acceptorFunc func(*Path) bool) { res := (*p)[:0] for _, path := range *p { - if accpetorFunc(path) { + if acceptorFunc(path) { res = append(res, path) } } From ad09f55a7cd8c604166a2df21993c6302f5b2651 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 11 Jan 2024 19:05:45 +0100 Subject: [PATCH 03/11] Removed deprecated call to io/ioutil --- paths.go | 5 ++--- readdir.go | 8 ++++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/paths.go b/paths.go index 7fb644d..c42ca31 100644 --- a/paths.go +++ b/paths.go @@ -32,7 +32,6 @@ package paths import ( "fmt" "io" - "io/ioutil" "os" "path/filepath" "strings" @@ -418,14 +417,14 @@ func (p *Path) Chtimes(atime, mtime time.Time) error { // ReadFile reads the file named by filename and returns the contents func (p *Path) ReadFile() ([]byte, error) { - return ioutil.ReadFile(p.path) + return os.ReadFile(p.path) } // WriteFile writes data to a file named by filename. If the file // does not exist, WriteFile creates it otherwise WriteFile truncates // it before writing. func (p *Path) WriteFile(data []byte) error { - return ioutil.WriteFile(p.path, data, os.FileMode(0644)) + return os.WriteFile(p.path, data, os.FileMode(0644)) } // WriteToTempFile writes data to a newly generated temporary file. diff --git a/readdir.go b/readdir.go index 53341b9..ca5b760 100644 --- a/readdir.go +++ b/readdir.go @@ -30,7 +30,7 @@ package paths import ( - "io/ioutil" + "os" "strings" ) @@ -41,7 +41,7 @@ type ReadDirFilter func(file *Path) bool // ReadDir returns a PathList containing the content of the directory // pointed by the current Path. The resulting list is filtered by the given filters chained. func (p *Path) ReadDir(filters ...ReadDirFilter) (PathList, error) { - infos, err := ioutil.ReadDir(p.path) + infos, err := os.ReadDir(p.path) if err != nil { return nil, err } @@ -69,7 +69,7 @@ func (p *Path) ReadDir(filters ...ReadDirFilter) (PathList, error) { // ReadDirRecursive returns a PathList containing the content of the directory // and its subdirectories pointed by the current Path func (p *Path) ReadDirRecursive() (PathList, error) { - infos, err := ioutil.ReadDir(p.path) + infos, err := os.ReadDir(p.path) if err != nil { return nil, err } @@ -101,7 +101,7 @@ func (p *Path) ReadDirRecursive() (PathList, error) { // - `filters` are the filters that are checked to determine if the entry should be // added to the resulting PathList func (p *Path) ReadDirRecursiveFiltered(recursionFilter ReadDirFilter, filters ...ReadDirFilter) (PathList, error) { - infos, err := ioutil.ReadDir(p.path) + infos, err := os.ReadDir(p.path) if err != nil { return nil, err } From 9c9432258b1c1099aa6e826fa95ea1fe90cc7ca7 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 11 Jan 2024 19:09:32 +0100 Subject: [PATCH 04/11] Simplified ReadDirRecursive method --- readdir.go | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/readdir.go b/readdir.go index ca5b760..575ad6c 100644 --- a/readdir.go +++ b/readdir.go @@ -69,27 +69,7 @@ func (p *Path) ReadDir(filters ...ReadDirFilter) (PathList, error) { // ReadDirRecursive returns a PathList containing the content of the directory // and its subdirectories pointed by the current Path func (p *Path) ReadDirRecursive() (PathList, error) { - infos, err := os.ReadDir(p.path) - if err != nil { - return nil, err - } - paths := PathList{} - for _, info := range infos { - path := p.Join(info.Name()) - paths.Add(path) - - if isDir, err := path.IsDirCheck(); err != nil { - return nil, err - } else if isDir { - subPaths, err := path.ReadDirRecursive() - if err != nil { - return nil, err - } - paths.AddAll(subPaths) - } - - } - return paths, nil + return p.ReadDirRecursiveFiltered(nil) } // ReadDirRecursiveFiltered returns a PathList containing the content of the directory From 5647e3716f52ccfdbf7bbbc9a37f5aee620e9f0e Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 11 Jan 2024 19:14:38 +0100 Subject: [PATCH 05/11] Made ReadDirRecursiveFiltered recurse internally This prepares for the next change that requires to keep a shared state in the recursive calls. This change also avoids the need to pass `recursionFilter` and `filters` back into the recursive call. --- readdir.go | 54 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/readdir.go b/readdir.go index 575ad6c..0fe526b 100644 --- a/readdir.go +++ b/readdir.go @@ -81,41 +81,47 @@ func (p *Path) ReadDirRecursive() (PathList, error) { // - `filters` are the filters that are checked to determine if the entry should be // added to the resulting PathList func (p *Path) ReadDirRecursiveFiltered(recursionFilter ReadDirFilter, filters ...ReadDirFilter) (PathList, error) { - infos, err := os.ReadDir(p.path) - if err != nil { - return nil, err - } + var search func(*Path) (PathList, error) - accept := func(p *Path) bool { - for _, filter := range filters { - if !filter(p) { - return false + search = func(currPath *Path) (PathList, error) { + infos, err := os.ReadDir(currPath.path) + if err != nil { + return nil, err + } + + accept := func(p *Path) bool { + for _, filter := range filters { + if !filter(p) { + return false + } } + return true } - return true - } - paths := PathList{} - for _, info := range infos { - path := p.Join(info.Name()) + paths := PathList{} + for _, info := range infos { + path := currPath.Join(info.Name()) - if accept(path) { - paths.Add(path) - } + if accept(path) { + paths.Add(path) + } - if recursionFilter == nil || recursionFilter(path) { - if isDir, err := path.IsDirCheck(); err != nil { - return nil, err - } else if isDir { - subPaths, err := path.ReadDirRecursiveFiltered(recursionFilter, filters...) - if err != nil { + if recursionFilter == nil || recursionFilter(path) { + if isDir, err := path.IsDirCheck(); err != nil { return nil, err + } else if isDir { + subPaths, err := search(path) + if err != nil { + return nil, err + } + paths.AddAll(subPaths) } - paths.AddAll(subPaths) } } + return paths, nil } - return paths, nil + + return search(p) } // FilterDirectories is a ReadDirFilter that accepts only directories From b49819ca94d4c27fb9e39d9e36fb5b75db3bfa5d Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 11 Jan 2024 19:29:41 +0100 Subject: [PATCH 06/11] Added loop detection to ReadDirRecursive* methods --- readdir.go | 9 +++ readdir_test.go | 69 ++++++++++++++++++++++ testdata/loops/loop_1/dir1/loop | 1 + testdata/loops/loop_2/dir1/loop2 | 1 + testdata/loops/loop_2/dir2/loop1 | 1 + testdata/loops/loop_3/dir1/loop2 | 1 + testdata/loops/loop_3/dir2/dir3/loop2 | 1 + testdata/loops/loop_4/dir1/dir2/loop2 | 1 + testdata/loops/loop_4/dir1/dir3/dir4/loop1 | 1 + testdata/loops/regular_1/dir1/file1 | 0 testdata/loops/regular_1/dir2 | 1 + testdata/loops/regular_2/dir1/file1 | 0 testdata/loops/regular_2/dir2/dir1 | 1 + testdata/loops/regular_2/dir2/file2 | 0 14 files changed, 87 insertions(+) create mode 120000 testdata/loops/loop_1/dir1/loop create mode 120000 testdata/loops/loop_2/dir1/loop2 create mode 120000 testdata/loops/loop_2/dir2/loop1 create mode 120000 testdata/loops/loop_3/dir1/loop2 create mode 120000 testdata/loops/loop_3/dir2/dir3/loop2 create mode 120000 testdata/loops/loop_4/dir1/dir2/loop2 create mode 120000 testdata/loops/loop_4/dir1/dir3/dir4/loop1 create mode 100644 testdata/loops/regular_1/dir1/file1 create mode 120000 testdata/loops/regular_1/dir2 create mode 100644 testdata/loops/regular_2/dir1/file1 create mode 120000 testdata/loops/regular_2/dir2/dir1 create mode 100644 testdata/loops/regular_2/dir2/file2 diff --git a/readdir.go b/readdir.go index 0fe526b..42f5d73 100644 --- a/readdir.go +++ b/readdir.go @@ -30,6 +30,7 @@ package paths import ( + "errors" "os" "strings" ) @@ -83,7 +84,15 @@ func (p *Path) ReadDirRecursive() (PathList, error) { func (p *Path) ReadDirRecursiveFiltered(recursionFilter ReadDirFilter, filters ...ReadDirFilter) (PathList, error) { var search func(*Path) (PathList, error) + explored := map[string]bool{} search = func(currPath *Path) (PathList, error) { + canonical := currPath.Canonical().path + if explored[canonical] { + return nil, errors.New("directories symlink loop detected") + } + explored[canonical] = true + defer delete(explored, canonical) + infos, err := os.ReadDir(currPath.path) if err != nil { return nil, err diff --git a/readdir_test.go b/readdir_test.go index d0ec927..c22d542 100644 --- a/readdir_test.go +++ b/readdir_test.go @@ -33,6 +33,7 @@ import ( "fmt" "os" "testing" + "time" "github.com/stretchr/testify/require" ) @@ -245,3 +246,71 @@ func TestReadDirRecursiveFiltered(t *testing.T) { pathEqualsTo(t, "testdata/fileset/test.txt", l[7]) pathEqualsTo(t, "testdata/fileset/test.txt.gz", l[8]) } + +func TestReadDirRecursiveLoopDetection(t *testing.T) { + loopsPath := New("testdata", "loops") + unbuondedReaddir := func(testdir string) (PathList, error) { + // This is required to unbound the recursion, otherwise it will stop + // when the paths becomes too long due to the symlink loop: this is not + // what we want, we are looking for an early detection of the loop. + skipBrokenLinks := func(p *Path) bool { + _, err := p.Stat() + return err == nil + } + + var files PathList + var err error + done := make(chan bool) + go func() { + files, err = loopsPath.Join(testdir).ReadDirRecursiveFiltered( + skipBrokenLinks, + ) + done <- true + }() + require.Eventually( + t, + func() bool { + select { + case <-done: + return true + default: + return false + } + }, + 5*time.Second, + 10*time.Millisecond, + "Infinite symlink loop while loading sketch", + ) + return files, err + } + + for _, dir := range []string{"loop_1", "loop_2", "loop_3", "loop_4"} { + l, err := unbuondedReaddir(dir) + require.EqualError(t, err, "directories symlink loop detected", "loop not detected in %s", dir) + require.Nil(t, l) + } + + { + l, err := unbuondedReaddir("regular_1") + require.NoError(t, err) + require.Len(t, l, 4) + l.Sort() + pathEqualsTo(t, "testdata/loops/regular_1/dir1", l[0]) + pathEqualsTo(t, "testdata/loops/regular_1/dir1/file1", l[1]) + pathEqualsTo(t, "testdata/loops/regular_1/dir2", l[2]) + pathEqualsTo(t, "testdata/loops/regular_1/dir2/file1", l[3]) + } + + { + l, err := unbuondedReaddir("regular_2") + require.NoError(t, err) + require.Len(t, l, 6) + l.Sort() + pathEqualsTo(t, "testdata/loops/regular_2/dir1", l[0]) + pathEqualsTo(t, "testdata/loops/regular_2/dir1/file1", l[1]) + pathEqualsTo(t, "testdata/loops/regular_2/dir2", l[2]) + pathEqualsTo(t, "testdata/loops/regular_2/dir2/dir1", l[3]) + pathEqualsTo(t, "testdata/loops/regular_2/dir2/dir1/file1", l[4]) + pathEqualsTo(t, "testdata/loops/regular_2/dir2/file2", l[5]) + } +} diff --git a/testdata/loops/loop_1/dir1/loop b/testdata/loops/loop_1/dir1/loop new file mode 120000 index 0000000..c9f3ab1 --- /dev/null +++ b/testdata/loops/loop_1/dir1/loop @@ -0,0 +1 @@ +../dir1 \ No newline at end of file diff --git a/testdata/loops/loop_2/dir1/loop2 b/testdata/loops/loop_2/dir1/loop2 new file mode 120000 index 0000000..d014eb4 --- /dev/null +++ b/testdata/loops/loop_2/dir1/loop2 @@ -0,0 +1 @@ +../dir2 \ No newline at end of file diff --git a/testdata/loops/loop_2/dir2/loop1 b/testdata/loops/loop_2/dir2/loop1 new file mode 120000 index 0000000..c9f3ab1 --- /dev/null +++ b/testdata/loops/loop_2/dir2/loop1 @@ -0,0 +1 @@ +../dir1 \ No newline at end of file diff --git a/testdata/loops/loop_3/dir1/loop2 b/testdata/loops/loop_3/dir1/loop2 new file mode 120000 index 0000000..d014eb4 --- /dev/null +++ b/testdata/loops/loop_3/dir1/loop2 @@ -0,0 +1 @@ +../dir2 \ No newline at end of file diff --git a/testdata/loops/loop_3/dir2/dir3/loop2 b/testdata/loops/loop_3/dir2/dir3/loop2 new file mode 120000 index 0000000..85babfd --- /dev/null +++ b/testdata/loops/loop_3/dir2/dir3/loop2 @@ -0,0 +1 @@ +../../dir1/ \ No newline at end of file diff --git a/testdata/loops/loop_4/dir1/dir2/loop2 b/testdata/loops/loop_4/dir1/dir2/loop2 new file mode 120000 index 0000000..3fd50ca --- /dev/null +++ b/testdata/loops/loop_4/dir1/dir2/loop2 @@ -0,0 +1 @@ +../dir3 \ No newline at end of file diff --git a/testdata/loops/loop_4/dir1/dir3/dir4/loop1 b/testdata/loops/loop_4/dir1/dir3/dir4/loop1 new file mode 120000 index 0000000..4f388a6 --- /dev/null +++ b/testdata/loops/loop_4/dir1/dir3/dir4/loop1 @@ -0,0 +1 @@ +../../../dir1 \ No newline at end of file diff --git a/testdata/loops/regular_1/dir1/file1 b/testdata/loops/regular_1/dir1/file1 new file mode 100644 index 0000000..e69de29 diff --git a/testdata/loops/regular_1/dir2 b/testdata/loops/regular_1/dir2 new file mode 120000 index 0000000..df490f8 --- /dev/null +++ b/testdata/loops/regular_1/dir2 @@ -0,0 +1 @@ +dir1 \ No newline at end of file diff --git a/testdata/loops/regular_2/dir1/file1 b/testdata/loops/regular_2/dir1/file1 new file mode 100644 index 0000000..e69de29 diff --git a/testdata/loops/regular_2/dir2/dir1 b/testdata/loops/regular_2/dir2/dir1 new file mode 120000 index 0000000..c9f3ab1 --- /dev/null +++ b/testdata/loops/regular_2/dir2/dir1 @@ -0,0 +1 @@ +../dir1 \ No newline at end of file diff --git a/testdata/loops/regular_2/dir2/file2 b/testdata/loops/regular_2/dir2/file2 new file mode 100644 index 0000000..e69de29 From f6afd8bc617a65065ac05e21054481d9805ee02a Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 11 Jan 2024 19:02:37 +0100 Subject: [PATCH 07/11] Added Lstat function --- paths.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/paths.go b/paths.go index c42ca31..a9b8bc0 100644 --- a/paths.go +++ b/paths.go @@ -32,6 +32,7 @@ package paths import ( "fmt" "io" + "io/fs" "os" "path/filepath" "strings" @@ -69,10 +70,18 @@ func NewFromFile(file *os.File) *Path { // Stat returns a FileInfo describing the named file. The result is // cached internally for next queries. To ensure that the cached // FileInfo entry is updated just call Stat again. -func (p *Path) Stat() (os.FileInfo, error) { +func (p *Path) Stat() (fs.FileInfo, error) { return os.Stat(p.path) } +// Lstat returns a FileInfo describing the named file. If the file is +// a symbolic link, the returned FileInfo describes the symbolic link. +// Lstat makes no attempt to follow the link. If there is an error, it +// will be of type *PathError. +func (p *Path) Lstat() (fs.FileInfo, error) { + return os.Lstat(p.path) +} + // Clone create a copy of the Path object func (p *Path) Clone() *Path { return New(p.path) From 4aa0f1c08a7e62b5a3849dd899de49b6b76c60e1 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 12 Jan 2024 15:15:51 +0100 Subject: [PATCH 08/11] In ReadDirRecursive do not try to recurse into broken symlinks Instead return the symlink as a file in the file list --- readdir.go | 4 +--- readdir_test.go | 12 +----------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/readdir.go b/readdir.go index 42f5d73..985fed5 100644 --- a/readdir.go +++ b/readdir.go @@ -116,9 +116,7 @@ func (p *Path) ReadDirRecursiveFiltered(recursionFilter ReadDirFilter, filters . } if recursionFilter == nil || recursionFilter(path) { - if isDir, err := path.IsDirCheck(); err != nil { - return nil, err - } else if isDir { + if path.IsDir() { subPaths, err := search(path) if err != nil { return nil, err diff --git a/readdir_test.go b/readdir_test.go index c22d542..5d51e49 100644 --- a/readdir_test.go +++ b/readdir_test.go @@ -250,21 +250,11 @@ func TestReadDirRecursiveFiltered(t *testing.T) { func TestReadDirRecursiveLoopDetection(t *testing.T) { loopsPath := New("testdata", "loops") unbuondedReaddir := func(testdir string) (PathList, error) { - // This is required to unbound the recursion, otherwise it will stop - // when the paths becomes too long due to the symlink loop: this is not - // what we want, we are looking for an early detection of the loop. - skipBrokenLinks := func(p *Path) bool { - _, err := p.Stat() - return err == nil - } - var files PathList var err error done := make(chan bool) go func() { - files, err = loopsPath.Join(testdir).ReadDirRecursiveFiltered( - skipBrokenLinks, - ) + files, err = loopsPath.Join(testdir).ReadDirRecursive() done <- true }() require.Eventually( From 99a3246f835ae6db39374bcb17822b329a9a4885 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 12 Jan 2024 15:21:03 +0100 Subject: [PATCH 09/11] Added test --- readdir_test.go | 14 ++++++++++++++ testdata/loops/regular_3/dir1/file1 | 0 testdata/loops/regular_3/dir2/dir1 | 1 + testdata/loops/regular_3/dir2/file2 | 0 testdata/loops/regular_3/link | 1 + 5 files changed, 16 insertions(+) create mode 100644 testdata/loops/regular_3/dir1/file1 create mode 120000 testdata/loops/regular_3/dir2/dir1 create mode 100644 testdata/loops/regular_3/dir2/file2 create mode 120000 testdata/loops/regular_3/link diff --git a/readdir_test.go b/readdir_test.go index 5d51e49..7455746 100644 --- a/readdir_test.go +++ b/readdir_test.go @@ -303,4 +303,18 @@ func TestReadDirRecursiveLoopDetection(t *testing.T) { pathEqualsTo(t, "testdata/loops/regular_2/dir2/dir1/file1", l[4]) pathEqualsTo(t, "testdata/loops/regular_2/dir2/file2", l[5]) } + + { + l, err := unbuondedReaddir("regular_3") + require.NoError(t, err) + require.Len(t, l, 7) + l.Sort() + pathEqualsTo(t, "testdata/loops/regular_3/dir1", l[0]) + pathEqualsTo(t, "testdata/loops/regular_3/dir1/file1", l[1]) + pathEqualsTo(t, "testdata/loops/regular_3/dir2", l[2]) + pathEqualsTo(t, "testdata/loops/regular_3/dir2/dir1", l[3]) + pathEqualsTo(t, "testdata/loops/regular_3/dir2/dir1/file1", l[4]) + pathEqualsTo(t, "testdata/loops/regular_3/dir2/file2", l[5]) + pathEqualsTo(t, "testdata/loops/regular_3/link", l[6]) // broken symlink is reported in files + } } diff --git a/testdata/loops/regular_3/dir1/file1 b/testdata/loops/regular_3/dir1/file1 new file mode 100644 index 0000000..e69de29 diff --git a/testdata/loops/regular_3/dir2/dir1 b/testdata/loops/regular_3/dir2/dir1 new file mode 120000 index 0000000..c9f3ab1 --- /dev/null +++ b/testdata/loops/regular_3/dir2/dir1 @@ -0,0 +1 @@ +../dir1 \ No newline at end of file diff --git a/testdata/loops/regular_3/dir2/file2 b/testdata/loops/regular_3/dir2/file2 new file mode 100644 index 0000000..e69de29 diff --git a/testdata/loops/regular_3/link b/testdata/loops/regular_3/link new file mode 120000 index 0000000..86a410d --- /dev/null +++ b/testdata/loops/regular_3/link @@ -0,0 +1 @@ +broken \ No newline at end of file From 8366036b57772b582438c903e7d2dc40a9e77656 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 15 Jan 2024 10:00:02 +0100 Subject: [PATCH 10/11] Add Chmod and expand a test --- paths.go | 7 ++++++ readdir_test.go | 23 +++++++++++++++++++ .../dir1/file1 | 0 .../regular_4_with_permission_error/link | 1 + 4 files changed, 31 insertions(+) create mode 100644 testdata/loops/regular_4_with_permission_error/dir1/file1 create mode 120000 testdata/loops/regular_4_with_permission_error/link diff --git a/paths.go b/paths.go index a9b8bc0..a734d17 100644 --- a/paths.go +++ b/paths.go @@ -418,6 +418,13 @@ func (p *Path) CopyDirTo(dst *Path) error { return nil } +// Chmod changes the mode of the named file to mode. If the file is a +// symbolic link, it changes the mode of the link's target. If there +// is an error, it will be of type *os.PathError. +func (p *Path) Chmod(mode fs.FileMode) error { + return os.Chmod(p.path, mode) +} + // Chtimes changes the access and modification times of the named file, // similar to the Unix utime() or utimes() functions. func (p *Path) Chtimes(atime, mtime time.Time) error { diff --git a/readdir_test.go b/readdir_test.go index 7455746..ae25ede 100644 --- a/readdir_test.go +++ b/readdir_test.go @@ -31,7 +31,9 @@ package paths import ( "fmt" + "io/fs" "os" + "runtime" "testing" "time" @@ -317,4 +319,25 @@ func TestReadDirRecursiveLoopDetection(t *testing.T) { pathEqualsTo(t, "testdata/loops/regular_3/dir2/file2", l[5]) pathEqualsTo(t, "testdata/loops/regular_3/link", l[6]) // broken symlink is reported in files } + + if runtime.GOOS != "windows" { + dir1 := loopsPath.Join("regular_4_with_permission_error", "dir1") + + l, err := unbuondedReaddir("regular_4_with_permission_error") + require.NoError(t, err) + require.NotEmpty(t, l) + + dir1Stat, err := dir1.Stat() + require.NoError(t, err) + err = dir1.Chmod(fs.FileMode(0)) // Enforce permission error + require.NoError(t, err) + t.Cleanup(func() { + // Restore normal permission after the test + dir1.Chmod(dir1Stat.Mode()) + }) + + l, err = unbuondedReaddir("regular_4_with_permission_error") + require.Error(t, err) + require.Nil(t, l) + } } diff --git a/testdata/loops/regular_4_with_permission_error/dir1/file1 b/testdata/loops/regular_4_with_permission_error/dir1/file1 new file mode 100644 index 0000000..e69de29 diff --git a/testdata/loops/regular_4_with_permission_error/link b/testdata/loops/regular_4_with_permission_error/link new file mode 120000 index 0000000..86a410d --- /dev/null +++ b/testdata/loops/regular_4_with_permission_error/link @@ -0,0 +1 @@ +broken \ No newline at end of file From 075c5c8cba06ee4674fc5ca4dffc3d2c3058a5b3 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 15 Jan 2024 10:10:56 +0100 Subject: [PATCH 11/11] Fixed FilterOutDirs hadnling of broken symlinks --- list.go | 4 +- paths_test.go | 56 +++++++++++++------ testdata/broken_symlink/dir_1/broken_link | 1 + testdata/broken_symlink/dir_1/file2 | 0 testdata/broken_symlink/dir_1/linked_dir | 1 + testdata/broken_symlink/dir_1/linked_file | 1 + testdata/broken_symlink/dir_1/real_dir/file1 | 0 .../regular_4_with_permission_error/dir2/dir1 | 1 + .../dir2/file2 | 0 9 files changed, 46 insertions(+), 18 deletions(-) create mode 120000 testdata/broken_symlink/dir_1/broken_link create mode 100644 testdata/broken_symlink/dir_1/file2 create mode 120000 testdata/broken_symlink/dir_1/linked_dir create mode 120000 testdata/broken_symlink/dir_1/linked_file create mode 100644 testdata/broken_symlink/dir_1/real_dir/file1 create mode 120000 testdata/loops/regular_4_with_permission_error/dir2/dir1 create mode 100644 testdata/loops/regular_4_with_permission_error/dir2/file2 diff --git a/list.go b/list.go index 4ddecc1..fbab13c 100644 --- a/list.go +++ b/list.go @@ -78,7 +78,7 @@ func (p *PathList) FilterDirs() { func (p *PathList) FilterOutDirs() { res := (*p)[:0] for _, path := range *p { - if path.IsNotDir() { + if !path.IsDir() { res = append(res, path) } } @@ -96,7 +96,7 @@ func (p *PathList) FilterOutHiddenFiles() { func (p *PathList) Filter(acceptorFunc func(*Path) bool) { res := (*p)[:0] for _, path := range *p { - if acceptorFunc(path) { + if acceptorFunc(path) { res = append(res, path) } } diff --git a/paths_test.go b/paths_test.go index b495273..27fde62 100644 --- a/paths_test.go +++ b/paths_test.go @@ -284,25 +284,49 @@ func TestFilterDirs(t *testing.T) { } func TestFilterOutDirs(t *testing.T) { - testPath := New("testdata", "fileset") + { + testPath := New("testdata", "fileset") - list, err := testPath.ReadDir() - require.NoError(t, err) - require.Len(t, list, 6) + list, err := testPath.ReadDir() + require.NoError(t, err) + require.Len(t, list, 6) + + pathEqualsTo(t, "testdata/fileset/anotherFile", list[0]) + pathEqualsTo(t, "testdata/fileset/file", list[1]) + pathEqualsTo(t, "testdata/fileset/folder", list[2]) + pathEqualsTo(t, "testdata/fileset/symlinktofolder", list[3]) + pathEqualsTo(t, "testdata/fileset/test.txt", list[4]) + pathEqualsTo(t, "testdata/fileset/test.txt.gz", list[5]) + + list.FilterOutDirs() + require.Len(t, list, 4) + pathEqualsTo(t, "testdata/fileset/anotherFile", list[0]) + pathEqualsTo(t, "testdata/fileset/file", list[1]) + pathEqualsTo(t, "testdata/fileset/test.txt", list[2]) + pathEqualsTo(t, "testdata/fileset/test.txt.gz", list[3]) + } - pathEqualsTo(t, "testdata/fileset/anotherFile", list[0]) - pathEqualsTo(t, "testdata/fileset/file", list[1]) - pathEqualsTo(t, "testdata/fileset/folder", list[2]) - pathEqualsTo(t, "testdata/fileset/symlinktofolder", list[3]) - pathEqualsTo(t, "testdata/fileset/test.txt", list[4]) - pathEqualsTo(t, "testdata/fileset/test.txt.gz", list[5]) + { + list, err := New("testdata", "broken_symlink", "dir_1").ReadDirRecursive() + require.NoError(t, err) - list.FilterOutDirs() - require.Len(t, list, 4) - pathEqualsTo(t, "testdata/fileset/anotherFile", list[0]) - pathEqualsTo(t, "testdata/fileset/file", list[1]) - pathEqualsTo(t, "testdata/fileset/test.txt", list[2]) - pathEqualsTo(t, "testdata/fileset/test.txt.gz", list[3]) + require.Len(t, list, 7) + pathEqualsTo(t, "testdata/broken_symlink/dir_1/broken_link", list[0]) + pathEqualsTo(t, "testdata/broken_symlink/dir_1/file2", list[1]) + pathEqualsTo(t, "testdata/broken_symlink/dir_1/linked_dir", list[2]) + pathEqualsTo(t, "testdata/broken_symlink/dir_1/linked_dir/file1", list[3]) + pathEqualsTo(t, "testdata/broken_symlink/dir_1/linked_file", list[4]) + pathEqualsTo(t, "testdata/broken_symlink/dir_1/real_dir", list[5]) + pathEqualsTo(t, "testdata/broken_symlink/dir_1/real_dir/file1", list[6]) + + list.FilterOutDirs() + require.Len(t, list, 5) + pathEqualsTo(t, "testdata/broken_symlink/dir_1/broken_link", list[0]) + pathEqualsTo(t, "testdata/broken_symlink/dir_1/file2", list[1]) + pathEqualsTo(t, "testdata/broken_symlink/dir_1/linked_dir/file1", list[2]) + pathEqualsTo(t, "testdata/broken_symlink/dir_1/linked_file", list[3]) + pathEqualsTo(t, "testdata/broken_symlink/dir_1/real_dir/file1", list[4]) + } } func TestEquivalentPaths(t *testing.T) { diff --git a/testdata/broken_symlink/dir_1/broken_link b/testdata/broken_symlink/dir_1/broken_link new file mode 120000 index 0000000..86a410d --- /dev/null +++ b/testdata/broken_symlink/dir_1/broken_link @@ -0,0 +1 @@ +broken \ No newline at end of file diff --git a/testdata/broken_symlink/dir_1/file2 b/testdata/broken_symlink/dir_1/file2 new file mode 100644 index 0000000..e69de29 diff --git a/testdata/broken_symlink/dir_1/linked_dir b/testdata/broken_symlink/dir_1/linked_dir new file mode 120000 index 0000000..4b01904 --- /dev/null +++ b/testdata/broken_symlink/dir_1/linked_dir @@ -0,0 +1 @@ +real_dir \ No newline at end of file diff --git a/testdata/broken_symlink/dir_1/linked_file b/testdata/broken_symlink/dir_1/linked_file new file mode 120000 index 0000000..30d67d4 --- /dev/null +++ b/testdata/broken_symlink/dir_1/linked_file @@ -0,0 +1 @@ +file2 \ No newline at end of file diff --git a/testdata/broken_symlink/dir_1/real_dir/file1 b/testdata/broken_symlink/dir_1/real_dir/file1 new file mode 100644 index 0000000..e69de29 diff --git a/testdata/loops/regular_4_with_permission_error/dir2/dir1 b/testdata/loops/regular_4_with_permission_error/dir2/dir1 new file mode 120000 index 0000000..c9f3ab1 --- /dev/null +++ b/testdata/loops/regular_4_with_permission_error/dir2/dir1 @@ -0,0 +1 @@ +../dir1 \ No newline at end of file diff --git a/testdata/loops/regular_4_with_permission_error/dir2/file2 b/testdata/loops/regular_4_with_permission_error/dir2/file2 new file mode 100644 index 0000000..e69de29