Added loop detection to ReadDirRecursive* methods

Author: cmaglie

Diff for: 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 for: 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 for: testdata/loops/loop_1/dir1/loop

@@ -0,0 +1 @@
+../dir1

Diff for: testdata/loops/loop_2/dir1/loop2

@@ -0,0 +1 @@
+../dir2

Diff for: testdata/loops/loop_2/dir2/loop1

@@ -0,0 +1 @@
+../dir1

Diff for: testdata/loops/loop_3/dir1/loop2

@@ -0,0 +1 @@
+../dir2

Diff for: testdata/loops/loop_3/dir2/dir3/loop2

@@ -0,0 +1 @@
+../../dir1/

Diff for: testdata/loops/loop_4/dir1/dir2/loop2

@@ -0,0 +1 @@
+../dir3

Diff for: testdata/loops/loop_4/dir1/dir3/dir4/loop1

@@ -0,0 +1 @@
+../../../dir1

Diff for: testdata/loops/regular_1/dir1/file1

@@ -0,0 +1 @@
+dir1

Diff for: testdata/loops/regular_1/dir2

@@ -0,0 +1 @@
+../dir1