]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: traverse module-root symlinks in Walk calls
authorBryan C. Mills <bcmills@google.com>
Fri, 13 Jan 2023 18:59:19 +0000 (13:59 -0500)
committerGopher Robot <gobot@golang.org>
Tue, 31 Jan 2023 20:33:05 +0000 (20:33 +0000)
fsys.Walk, like filepath.Walk, avoids traversing symlinks. Also like
filepath.Walk, it follows a symlink at the root if the root path ends
in a file separator (consistent with POSIX pathname resolution¹).

If the user's working directory is within a repository stored in
(and symlinked to) a different filesystem path, we want to follow the
symlink instead of treating the module as completely empty.

¹https://pubs.opengroup.org/onlinepubs/9699919799.2013edition/basedefs/V1_chap04.html#tag_04_12

Fixes #50807.
Updates #57754.

Change-Id: Idaf6168dfffafe879e05b4ded5fda287fcd3eeec
Reviewed-on: https://go-review.googlesource.com/c/go/+/463179
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>

src/cmd/go/internal/fsys/fsys.go
src/cmd/go/internal/fsys/fsys_test.go
src/cmd/go/internal/modindex/scan.go
src/cmd/go/internal/modload/search.go
src/cmd/go/internal/search/search.go
src/cmd/go/internal/workcmd/use.go
src/cmd/go/testdata/script/list_goroot_symlink.txt
src/cmd/go/testdata/script/list_symlink_dotdotdot.txt [new file with mode: 0644]

index 454574a592e5a9142e530e82fdb8484890fa9dee..57a8c2c352d4792c796000095f4b61f780846ad3 100644 (file)
@@ -295,6 +295,10 @@ func parentIsOverlayFile(name string) (string, bool) {
 // return an error.
 var errNotDir = errors.New("not a directory")
 
+func nonFileInOverlayError(overlayPath string) error {
+       return fmt.Errorf("replacement path %q is a directory, not a file", overlayPath)
+}
+
 // readDir reads a dir on disk, returning an error that is errNotDir if the dir is not a directory.
 // Unfortunately, the error returned by os.ReadDir if dir is not a directory
 // can vary depending on the OS (Linux, Mac, Windows return ENOTDIR; BSD returns EINVAL).
@@ -354,18 +358,21 @@ func ReadDir(dir string) ([]fs.FileInfo, error) {
                case to.isDeleted():
                        delete(files, name)
                default:
-                       // This is a regular file.
-                       f, err := os.Lstat(to.actualFilePath)
+                       // To keep the data model simple, if the overlay contains a symlink we
+                       // always stat through it (using Stat, not Lstat). That way we don't need
+                       // to worry about the interaction between Lstat and directories: if a
+                       // symlink in the overlay points to a directory, we reject it like an
+                       // ordinary directory.
+                       fi, err := os.Stat(to.actualFilePath)
                        if err != nil {
                                files[name] = missingFile(name)
                                continue
-                       } else if f.IsDir() {
-                               return nil, fmt.Errorf("for overlay of %q to %q: overlay Replace entries can't point to directories",
-                                       filepath.Join(dir, name), to.actualFilePath)
+                       } else if fi.IsDir() {
+                               return nil, &fs.PathError{Op: "Stat", Path: filepath.Join(dir, name), Err: nonFileInOverlayError(to.actualFilePath)}
                        }
                        // Add a fileinfo for the overlaid file, so that it has
                        // the original file's name, but the overlaid file's metadata.
-                       files[name] = fakeFile{name, f}
+                       files[name] = fakeFile{name, fi}
                }
        }
        sortedFiles := diskfis[:0]
@@ -541,14 +548,22 @@ func overlayStat(path string, osStat func(string) (fs.FileInfo, error), opName s
 
        switch {
        case node.isDeleted():
-               return nil, &fs.PathError{Op: "lstat", Path: cpath, Err: fs.ErrNotExist}
+               return nil, &fs.PathError{Op: opName, Path: cpath, Err: fs.ErrNotExist}
        case node.isDir():
                return fakeDir(filepath.Base(path)), nil
        default:
-               fi, err := osStat(node.actualFilePath)
+               // To keep the data model simple, if the overlay contains a symlink we
+               // always stat through it (using Stat, not Lstat). That way we don't need to
+               // worry about the interaction between Lstat and directories: if a symlink
+               // in the overlay points to a directory, we reject it like an ordinary
+               // directory.
+               fi, err := os.Stat(node.actualFilePath)
                if err != nil {
                        return nil, err
                }
+               if fi.IsDir() {
+                       return nil, &fs.PathError{Op: opName, Path: cpath, Err: nonFileInOverlayError(node.actualFilePath)}
+               }
                return fakeFile{name: filepath.Base(path), real: fi}, nil
        }
 }
index b441e19afefcaf3c29efd1e9d784d27bc3bb0637..2ab2bb2fbaecca8d6f1bbb17678badbc44b3f9bc 100644 (file)
@@ -827,7 +827,7 @@ func TestWalkSymlink(t *testing.T) {
        testenv.MustHaveSymlink(t)
 
        initOverlay(t, `{
-       "Replace": {"overlay_symlink": "symlink"}
+       "Replace": {"overlay_symlink/file": "symlink/file"}
 }
 -- dir/file --`)
 
@@ -841,11 +841,15 @@ func TestWalkSymlink(t *testing.T) {
                dir       string
                wantFiles []string
        }{
-               {"control", "dir", []string{"dir", "dir" + string(filepath.Separator) + "file"}},
+               {"control", "dir", []string{"dir", filepath.Join("dir", "file")}},
                // ensure Walk doesn't walk into the directory pointed to by the symlink
                // (because it's supposed to use Lstat instead of Stat).
                {"symlink_to_dir", "symlink", []string{"symlink"}},
-               {"overlay_to_symlink_to_dir", "overlay_symlink", []string{"overlay_symlink"}},
+               {"overlay_to_symlink_to_dir", "overlay_symlink", []string{"overlay_symlink", filepath.Join("overlay_symlink", "file")}},
+
+               // However, adding filepath.Separator should cause the link to be resolved.
+               {"symlink_with_slash", "symlink" + string(filepath.Separator), []string{"symlink" + string(filepath.Separator), filepath.Join("symlink", "file")}},
+               {"overlay_to_symlink_to_dir", "overlay_symlink" + string(filepath.Separator), []string{"overlay_symlink" + string(filepath.Separator), filepath.Join("overlay_symlink", "file")}},
        }
 
        for _, tc := range testCases {
@@ -853,6 +857,7 @@ func TestWalkSymlink(t *testing.T) {
                        var got []string
 
                        err := Walk(tc.dir, func(path string, info fs.FileInfo, err error) error {
+                               t.Logf("walk %q", path)
                                got = append(got, path)
                                if err != nil {
                                        t.Errorf("walkfn: got non nil err argument: %v, want nil err argument", err)
index dce8e09a23a073a163fdd0a7e618a13d3327e953..60197898a026fe47674b600901f15d080d7237ad 100644 (file)
@@ -23,12 +23,12 @@ import (
 // moduleWalkErr returns filepath.SkipDir if the directory isn't relevant
 // when indexing a module or generating a filehash, ErrNotIndexed,
 // if the module shouldn't be indexed, and nil otherwise.
-func moduleWalkErr(modroot string, path string, info fs.FileInfo, err error) error {
+func moduleWalkErr(root string, path string, info fs.FileInfo, err error) error {
        if err != nil {
                return ErrNotIndexed
        }
        // stop at module boundaries
-       if info.IsDir() && path != modroot {
+       if info.IsDir() && path != root {
                if fi, err := fsys.Stat(filepath.Join(path, "go.mod")); err == nil && !fi.IsDir() {
                        return filepath.SkipDir
                }
@@ -52,18 +52,23 @@ func moduleWalkErr(modroot string, path string, info fs.FileInfo, err error) err
 func indexModule(modroot string) ([]byte, error) {
        fsys.Trace("indexModule", modroot)
        var packages []*rawPackage
-       err := fsys.Walk(modroot, func(path string, info fs.FileInfo, err error) error {
-               if err := moduleWalkErr(modroot, path, info, err); err != nil {
+
+       // If the root itself is a symlink to a directory,
+       // we want to follow it (see https://go.dev/issue/50807).
+       // Add a trailing separator to force that to happen.
+       root := str.WithFilePathSeparator(modroot)
+       err := fsys.Walk(root, func(path string, info fs.FileInfo, err error) error {
+               if err := moduleWalkErr(root, path, info, err); err != nil {
                        return err
                }
 
                if !info.IsDir() {
                        return nil
                }
-               if !str.HasFilePathPrefix(path, modroot) {
+               if !strings.HasPrefix(path, root) {
                        panic(fmt.Errorf("path %v in walk doesn't have modroot %v as prefix", path, modroot))
                }
-               rel := str.TrimFilePathPrefix(path, modroot)
+               rel := path[len(root):]
                packages = append(packages, importRaw(modroot, rel))
                return nil
        })
index 36e05321ae7b73bc4dc6b587208ec7db098d5b56..627f91f09c11804e37342a819d79426c45644c1f 100644 (file)
@@ -23,6 +23,7 @@ import (
        "cmd/go/internal/modindex"
        "cmd/go/internal/par"
        "cmd/go/internal/search"
+       "cmd/go/internal/str"
        "cmd/go/internal/trace"
        "cmd/internal/pkgpattern"
 
@@ -77,7 +78,10 @@ func matchPackages(ctx context.Context, m *search.Match, tags map[string]bool, f
                _, span := trace.StartSpan(ctx, "walkPkgs "+root)
                defer span.Done()
 
-               root = filepath.Clean(root)
+               // If the root itself is a symlink to a directory,
+               // we want to follow it (see https://go.dev/issue/50807).
+               // Add a trailing separator to force that to happen.
+               root = str.WithFilePathSeparator(filepath.Clean(root))
                err := fsys.Walk(root, func(pkgDir string, fi fs.FileInfo, err error) error {
                        if err != nil {
                                m.AddError(err)
@@ -100,9 +104,7 @@ func matchPackages(ctx context.Context, m *search.Match, tags map[string]bool, f
                                }
                        }
 
-                       rel := strings.TrimPrefix(filepath.ToSlash(pkgDir[len(root):]), "/")
-                       name := path.Join(importPathRoot, rel)
-
+                       name := path.Join(importPathRoot, filepath.ToSlash(pkgDir[len(root):]))
                        if !treeCanMatch(name) {
                                want = false
                        }
index fad1acb610c01d2371fa15c3d2b7adc47cdfca90..9f216d57568e551b130f7b1a761469fefbb5f0bb 100644 (file)
@@ -125,11 +125,16 @@ func (m *Match) MatchPackages() {
                if (m.pattern == "std" || m.pattern == "cmd") && src != cfg.GOROOTsrc {
                        continue
                }
+
+               // If the root itself is a symlink to a directory,
+               // we want to follow it (see https://go.dev/issue/50807).
+               // Add a trailing separator to force that to happen.
                src = str.WithFilePathSeparator(filepath.Clean(src))
                root := src
                if m.pattern == "cmd" {
                        root += "cmd" + string(filepath.Separator)
                }
+
                err := fsys.Walk(root, func(path string, fi fs.FileInfo, err error) error {
                        if err != nil {
                                return err // Likely a permission error, which could interfere with matching.
@@ -269,6 +274,10 @@ func (m *Match) MatchDirs(modRoots []string) {
                }
        }
 
+       // If dir is actually a symlink to a directory,
+       // we want to follow it (see https://go.dev/issue/50807).
+       // Add a trailing separator to force that to happen.
+       dir = str.WithFilePathSeparator(dir)
        err := fsys.Walk(dir, func(path string, fi fs.FileInfo, err error) error {
                if err != nil {
                        return err // Likely a permission error, which could interfere with matching.
index a306498d587b3c424e4f85ce09f6e48cfc6f0612..71f38e2e30c788eaa77e3a2ff98f9d26f60e0f35 100644 (file)
@@ -131,7 +131,10 @@ func runUse(ctx context.Context, cmd *base.Command, args []string) {
                }
 
                // Add or remove entries for any subdirectories that still exist.
-               fsys.Walk(useDir, func(path string, info fs.FileInfo, err error) error {
+               // If the root itself is a symlink to a directory,
+               // we want to follow it (see https://go.dev/issue/50807).
+               // Add a trailing separator to force that to happen.
+               fsys.Walk(str.WithFilePathSeparator(useDir), func(path string, info fs.FileInfo, err error) error {
                        if err != nil {
                                return err
                        }
index 989a8c2dd53af642408058fe2ac69dd3a149acfb..40c994306592e7515390c1195100840aaa8ebcbb 100644 (file)
@@ -55,7 +55,6 @@ stdout '^encoding/binary: '$WORK${/}lib${/}goroot${/}src${/}encoding${/}binary'$
 # So we check such a pattern to confirm that it works and reports a path relative
 # to $GOROOT/src (and not the symlink target).
 
-       # BUG(#50807): This should report encoding/binary, not "matched no packages".
 exec $WORK/lib/goroot/bin/go list -f '{{.ImportPath}}: {{.Dir}}' .../binary
-! stdout .
-stderr '^go: warning: "\.\.\./binary" matched no packages$'
+stdout '^encoding/binary: '$WORK${/}lib${/}goroot${/}src${/}encoding${/}binary'$'
+! stderr .
diff --git a/src/cmd/go/testdata/script/list_symlink_dotdotdot.txt b/src/cmd/go/testdata/script/list_symlink_dotdotdot.txt
new file mode 100644 (file)
index 0000000..8df1982
--- /dev/null
@@ -0,0 +1,20 @@
+[!symlink] skip
+
+symlink $WORK/gopath/src/sym -> $WORK/gopath/src/tree
+symlink $WORK/gopath/src/tree/squirrel -> $WORK/gopath/src/dir2 # this symlink should not be followed
+cd sym
+go list ./...
+cmp stdout $WORK/gopath/src/want_list.txt
+-- tree/go.mod --
+module example.com/tree
+
+go 1.20
+-- tree/tree.go --
+package tree
+-- tree/branch/branch.go --
+package branch
+-- dir2/squirrel.go --
+package squirrel
+-- want_list.txt --
+example.com/tree
+example.com/tree/branch