Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve broken symlink and other error handling #13

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Improve broken symlink and other error handling #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fa31eeb
Ignore DoesNotExist and ErrPermission in recursive ReadDir
matthijskooijman Sep 8, 2021
753b6dd
Add Path.Lstat method
matthijskooijman Sep 8, 2021
fed0409
Ensure that CopyDirTo really ignores symlinks
matthijskooijman Sep 8, 2021
429170e
Fix FilterOutDirs on broken symlinks
matthijskooijman Sep 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions _testdata/brokensymlink
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
nonexistent
2 changes: 1 addition & 1 deletion list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
35 changes: 26 additions & 9 deletions paths.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ func (p *Path) Stat() (os.FileInfo, error) {
return os.Stat(p.path)
}

// Lstat returns a FileInfo like stat, but when the path points to a
// symlink, returns information about the symlink itself instead of the
// target like Stat().
func (p *Path) Lstat() (os.FileInfo, error) {
return os.Lstat(p.path)
}

// Clone create a copy of the Path object
func (p *Path) Clone() *Path {
return New(p.path)
Expand Down Expand Up @@ -334,7 +341,17 @@ func (p *Path) ReadDirRecursive() (PathList, error) {
paths.Add(path)

if isDir, err := path.IsDirCheck(); err != nil {
return nil, err
// Do not report not exists (broken symlink) or
// permission errors, since then the entry
// *does* exist, so let our caller figure out
// whether these files are relevant and this is
// actually a problem. We cannot ignore *all*
// errors, since other errors during recursion
// (i.e. infinite recursion due to looping
// symlinks) should still be returned by us.
if !errors.Is(err, os.ErrNotExist) && !errors.Is(err, os.ErrPermission) {
return nil, err
}
} else if isDir {
subPaths, err := path.ReadDirRecursive()
if err != nil {
Expand Down Expand Up @@ -418,25 +435,25 @@ func (p *Path) CopyDirTo(dst *Path) error {
}

for _, srcPath := range srcFiles {
srcPathInfo, err := srcPath.Stat()
srcPathInfo, err := srcPath.Lstat()
if err != nil {
return fmt.Errorf("getting stat info for %s: %s", srcPath, err)
}
dstPath := dst.Join(srcPath.Base())

// Skip symlinks.
if srcPathInfo.Mode()&os.ModeSymlink != 0 {
// TODO
continue
}

dstPath := dst.Join(srcPath.Base())
if srcPathInfo.IsDir() {
if err := srcPath.CopyDirTo(dstPath); err != nil {
return fmt.Errorf("copying %s to %s: %s", srcPath, dstPath, err)
}
continue
}

// Skip symlinks.
if srcPathInfo.Mode()&os.ModeSymlink != 0 {
// TODO
continue
}

if err := srcPath.CopyTo(dstPath); err != nil {
return fmt.Errorf("copying %s to %s: %s", srcPath, dstPath, err)
}
Expand Down
85 changes: 53 additions & 32 deletions paths_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,19 @@ func TestPath(t *testing.T) {
require.False(t, anotherFilePath.Exist())
require.True(t, anotherFilePath.NotExist())

brokenFilePath := testPath.Join("brokensymlink")
pathEqualsTo(t, "_testdata/brokensymlink", brokenFilePath)
isDir, err = brokenFilePath.IsDirCheck()
require.False(t, isDir)
require.Error(t, err)
require.False(t, brokenFilePath.IsDir())
require.False(t, brokenFilePath.IsNotDir())
exist, err = brokenFilePath.ExistCheck()
require.False(t, exist)
require.NoError(t, err)
require.False(t, brokenFilePath.Exist())
require.True(t, brokenFilePath.NotExist())

list, err := folderPath.ReadDir()
require.NoError(t, err)
require.Len(t, list, 4)
Expand Down Expand Up @@ -230,6 +243,10 @@ func TestCopyDir(t *testing.T) {
require.False(t, isdir)
require.NoError(t, err)

exist, err = tmp.Join("dest", "symlinktofolder").ExistCheck()
require.False(t, exist)
require.NoError(t, err)

err = src.CopyDirTo(tmp.Join("dest"))
require.Error(t, err, "copying dir to already existing")

Expand Down Expand Up @@ -260,24 +277,25 @@ func TestReadDirRecursive(t *testing.T) {

list, err := testPath.ReadDirRecursive()
require.NoError(t, err)
require.Len(t, list, 16)
require.Len(t, list, 17)

pathEqualsTo(t, "_testdata/anotherFile", list[0])
pathEqualsTo(t, "_testdata/file", list[1])
pathEqualsTo(t, "_testdata/folder", list[2])
pathEqualsTo(t, "_testdata/folder/.hidden", list[3])
pathEqualsTo(t, "_testdata/folder/file2", list[4])
pathEqualsTo(t, "_testdata/folder/file3", list[5])
pathEqualsTo(t, "_testdata/folder/subfolder", list[6])
pathEqualsTo(t, "_testdata/folder/subfolder/file4", list[7])
pathEqualsTo(t, "_testdata/symlinktofolder", list[8])
pathEqualsTo(t, "_testdata/symlinktofolder/.hidden", list[9])
pathEqualsTo(t, "_testdata/symlinktofolder/file2", list[10])
pathEqualsTo(t, "_testdata/symlinktofolder/file3", list[11])
pathEqualsTo(t, "_testdata/symlinktofolder/subfolder", list[12])
pathEqualsTo(t, "_testdata/symlinktofolder/subfolder/file4", list[13])
pathEqualsTo(t, "_testdata/test.txt", list[14])
pathEqualsTo(t, "_testdata/test.txt.gz", list[15])
pathEqualsTo(t, "_testdata/brokensymlink", list[1])
pathEqualsTo(t, "_testdata/file", list[2])
pathEqualsTo(t, "_testdata/folder", list[3])
pathEqualsTo(t, "_testdata/folder/.hidden", list[4])
pathEqualsTo(t, "_testdata/folder/file2", list[5])
pathEqualsTo(t, "_testdata/folder/file3", list[6])
pathEqualsTo(t, "_testdata/folder/subfolder", list[7])
pathEqualsTo(t, "_testdata/folder/subfolder/file4", list[8])
pathEqualsTo(t, "_testdata/symlinktofolder", list[9])
pathEqualsTo(t, "_testdata/symlinktofolder/.hidden", list[10])
pathEqualsTo(t, "_testdata/symlinktofolder/file2", list[11])
pathEqualsTo(t, "_testdata/symlinktofolder/file3", list[12])
pathEqualsTo(t, "_testdata/symlinktofolder/subfolder", list[13])
pathEqualsTo(t, "_testdata/symlinktofolder/subfolder/file4", list[14])
pathEqualsTo(t, "_testdata/test.txt", list[15])
pathEqualsTo(t, "_testdata/test.txt.gz", list[16])

// Test symlink loop
tmp, err := MkTempDir("", "")
Expand All @@ -298,14 +316,15 @@ func TestFilterDirs(t *testing.T) {

list, err := testPath.ReadDir()
require.NoError(t, err)
require.Len(t, list, 6)
require.Len(t, list, 7)

pathEqualsTo(t, "_testdata/anotherFile", list[0])
pathEqualsTo(t, "_testdata/file", list[1])
pathEqualsTo(t, "_testdata/folder", list[2])
pathEqualsTo(t, "_testdata/symlinktofolder", list[3])
pathEqualsTo(t, "_testdata/test.txt", list[4])
pathEqualsTo(t, "_testdata/test.txt.gz", list[5])
pathEqualsTo(t, "_testdata/brokensymlink", list[1])
pathEqualsTo(t, "_testdata/file", list[2])
pathEqualsTo(t, "_testdata/folder", list[3])
pathEqualsTo(t, "_testdata/symlinktofolder", list[4])
pathEqualsTo(t, "_testdata/test.txt", list[5])
pathEqualsTo(t, "_testdata/test.txt.gz", list[6])

list.FilterDirs()
require.Len(t, list, 2)
Expand All @@ -318,21 +337,23 @@ func TestFilterOutDirs(t *testing.T) {

list, err := testPath.ReadDir()
require.NoError(t, err)
require.Len(t, list, 6)
require.Len(t, list, 7)

pathEqualsTo(t, "_testdata/anotherFile", list[0])
pathEqualsTo(t, "_testdata/file", list[1])
pathEqualsTo(t, "_testdata/folder", list[2])
pathEqualsTo(t, "_testdata/symlinktofolder", list[3])
pathEqualsTo(t, "_testdata/test.txt", list[4])
pathEqualsTo(t, "_testdata/test.txt.gz", list[5])
pathEqualsTo(t, "_testdata/brokensymlink", list[1])
pathEqualsTo(t, "_testdata/file", list[2])
pathEqualsTo(t, "_testdata/folder", list[3])
pathEqualsTo(t, "_testdata/symlinktofolder", list[4])
pathEqualsTo(t, "_testdata/test.txt", list[5])
pathEqualsTo(t, "_testdata/test.txt.gz", list[6])

list.FilterOutDirs()
require.Len(t, list, 4)
require.Len(t, list, 5)
pathEqualsTo(t, "_testdata/anotherFile", list[0])
pathEqualsTo(t, "_testdata/file", list[1])
pathEqualsTo(t, "_testdata/test.txt", list[2])
pathEqualsTo(t, "_testdata/test.txt.gz", list[3])
pathEqualsTo(t, "_testdata/brokensymlink", list[1])
pathEqualsTo(t, "_testdata/file", list[2])
pathEqualsTo(t, "_testdata/test.txt", list[3])
pathEqualsTo(t, "_testdata/test.txt.gz", list[4])
}

func TestEquivalentPaths(t *testing.T) {
Expand Down