From 4aa0f1c08a7e62b5a3849dd899de49b6b76c60e1 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 12 Jan 2024 15:15:51 +0100 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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