spf13 / afero

A FileSystem Abstraction System for Go
Apache License 2.0
5.79k stars 498 forks source link

Fix the infinite recursion with afero.Walk #409

Open erstam opened 7 months ago

erstam commented 7 months ago

This PR fixes https://github.com/spf13/afero/issues/185

Fix the infinite recursion by skipping the current directory in the directory names listing

CLAassistant commented 7 months ago

CLA assistant check
All committers have signed the CLA.

erstam commented 7 months ago

@spf13 Please, who can review this PR ?

bep commented 7 months ago

@erstam this needs a simple test (that does infinite recursion without this patch).

erstam commented 7 months ago

@bep I worked more on this issue and noticed something interesting. In my code where I use afero, I am removing the root folder of the fs instance and this causes the infinite recursion when using the Walk method. Should removing the root be forbidden on all Fs types ? I see in MemMapFs there is a init sync.Once attribute which is used to initialize a "root path" only once. But what if we delete that root path ? It kinda breaks the Fs.

Btw, using Windows here, if ever it matters.

The below test function will reproduce the issue.

func TestWalkRemoveRoot(t *testing.T) {
    defer removeAllTestFiles(t)
    fs := Fss[0]
    rootPath := "."

    err := fs.RemoveAll(rootPath)
    if err != nil {
        t.Error(err)
    }

    testRegistry[fs] = append(testRegistry[fs], rootPath)
    setupTestFiles(t, fs, rootPath)

    walkFn := func(path string, info os.FileInfo, err error) error {
        fmt.Println(path, info.Name(), info.IsDir(), info.Size(), err)
        return err
    }

    err = Walk(fs, rootPath, walkFn)
    if err != nil {
        t.Error(err)
    }
}

Seeing this case implies my proposed fix is just a workaround. The real fix would be to better protect the root path from deletion.

erstam commented 6 months ago

@bep, @spf13 any thoughts on my previous comment ?

erstam commented 5 months ago

Will we see this merged at some point ?