]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: avoid matching wildcards rooted outside of available modules
authorBryan C. Mills <bcmills@google.com>
Fri, 28 Feb 2020 20:03:54 +0000 (15:03 -0500)
committerBryan C. Mills <bcmills@google.com>
Fri, 28 Feb 2020 21:56:35 +0000 (21:56 +0000)
To avoid confusion, also distinguish between packages and dirs in
search.Match results.

No test because this is technically only a performance optimization:
it would be very difficult to write such a test so that it would not
be flaky. (However, tested the change manually.)

Fixes #37521

Change-Id: I17b443699ce6a8f3a63805a7ef0be806f695a4b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/221544
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
src/cmd/go/internal/get/get.go
src/cmd/go/internal/modload/load.go
src/cmd/go/internal/search/search.go
src/cmd/go/testdata/script/mod_list_std.txt

index b048eafa7414442dfadd39a9af28036623adb1fa..f7b2fa96e8d8d268a4a449c05e931130ee95d665 100644 (file)
@@ -286,11 +286,12 @@ func download(arg string, parent *load.Package, stk *load.ImportStack, mode int)
                if wildcardOkay && strings.Contains(arg, "...") {
                        match := search.NewMatch(arg)
                        if match.IsLocal() {
-                               match.MatchPackagesInFS()
+                               match.MatchDirs()
+                               args = match.Dirs
                        } else {
                                match.MatchPackages()
+                               args = match.Pkgs
                        }
-                       args = match.Pkgs
                        for _, err := range match.Errs {
                                base.Errorf("%s", err)
                        }
index 32841d96cb7545402414104bc16d1ac304c829c1..6ea7d8c69b376cf25defd5eab39a2f127c683f02 100644 (file)
@@ -65,24 +65,13 @@ func ImportPaths(patterns []string) []*search.Match {
 // packages. The build tags should typically be imports.Tags() or
 // imports.AnyTags(); a nil map has no special meaning.
 func ImportPathsQuiet(patterns []string, tags map[string]bool) []*search.Match {
-       var fsDirs [][]string
        updateMatches := func(matches []*search.Match, iterating bool) {
-               for i, m := range matches {
+               for _, m := range matches {
                        switch {
                        case m.IsLocal():
                                // Evaluate list of file system directories on first iteration.
-                               if fsDirs == nil {
-                                       fsDirs = make([][]string, len(matches))
-                               }
-                               if fsDirs[i] == nil {
-                                       if m.IsLiteral() {
-                                               fsDirs[i] = []string{m.Pattern()}
-                                       } else {
-                                               m.MatchPackagesInFS()
-                                               // Pull out the matching directories: we are going to resolve them
-                                               // to package paths below.
-                                               fsDirs[i], m.Pkgs = m.Pkgs, nil
-                                       }
+                               if m.Dirs == nil {
+                                       matchLocalDirs(m)
                                }
 
                                // Make a copy of the directory list and translate to import paths.
@@ -91,10 +80,9 @@ func ImportPathsQuiet(patterns []string, tags map[string]bool) []*search.Match {
                                // from not being in the build list to being in it and back as
                                // the exact version of a particular module increases during
                                // the loader iterations.
-                               pkgs := str.StringList(fsDirs[i])
-                               m.Pkgs = pkgs[:0]
-                               for _, pkg := range pkgs {
-                                       pkg, err := resolveLocalPackage(pkg)
+                               m.Pkgs = m.Pkgs[:0]
+                               for _, dir := range m.Dirs {
+                                       pkg, err := resolveLocalPackage(dir)
                                        if err != nil {
                                                if !m.IsLiteral() && (err == errPkgIsBuiltin || err == errPkgIsGorootSrc) {
                                                        continue // Don't include "builtin" or GOROOT/src in wildcard patterns.
@@ -131,7 +119,7 @@ func ImportPathsQuiet(patterns []string, tags map[string]bool) []*search.Match {
                                }
 
                        case m.Pattern() == "std" || m.Pattern() == "cmd":
-                               if len(m.Pkgs) == 0 {
+                               if m.Pkgs == nil {
                                        m.MatchPackages() // Locate the packages within GOROOT/src.
                                }
 
@@ -186,6 +174,34 @@ func checkMultiplePaths() {
        base.ExitIfErrors()
 }
 
+// matchLocalDirs is like m.MatchDirs, but tries to avoid scanning directories
+// outside of the standard library and active modules.
+func matchLocalDirs(m *search.Match) {
+       if !m.IsLocal() {
+               panic(fmt.Sprintf("internal error: resolveLocalDirs on non-local pattern %s", m.Pattern()))
+       }
+
+       if i := strings.Index(m.Pattern(), "..."); i >= 0 {
+               // The pattern is local, but it is a wildcard. Its packages will
+               // only resolve to paths if they are inside of the standard
+               // library, the main module, or some dependency of the main
+               // module. Verify that before we walk the filesystem: a filesystem
+               // walk in a directory like /var or /etc can be very expensive!
+               dir := filepath.Dir(filepath.Clean(m.Pattern()[:i+3]))
+               absDir := dir
+               if !filepath.IsAbs(dir) {
+                       absDir = filepath.Join(base.Cwd, dir)
+               }
+               if search.InDir(absDir, cfg.GOROOTsrc) == "" && search.InDir(absDir, ModRoot()) == "" && pathInModuleCache(absDir) == "" {
+                       m.Dirs = []string{}
+                       m.AddError(fmt.Errorf("directory prefix %s outside available modules", base.ShortPath(absDir)))
+                       return
+               }
+       }
+
+       m.MatchDirs()
+}
+
 // resolveLocalPackage resolves a filesystem path to a package path.
 func resolveLocalPackage(dir string) (string, error) {
        var absDir string
@@ -269,7 +285,11 @@ func resolveLocalPackage(dir string) (string, error) {
        }
 
        if sub := search.InDir(absDir, cfg.GOROOTsrc); sub != "" && sub != "." && !strings.Contains(sub, "@") {
-               return filepath.ToSlash(sub), nil
+               pkg := filepath.ToSlash(sub)
+               if pkg == "builtin" {
+                       return "", errPkgIsBuiltin
+               }
+               return pkg, nil
        }
 
        pkg := pathInModuleCache(absDir)
index 69d0e2d16f1257cef463dbba5a3f042c3092bc79..b588c3e467c672cf276beb9cf71e84528159aa00 100644 (file)
@@ -19,7 +19,8 @@ import (
 // A Match represents the result of matching a single package pattern.
 type Match struct {
        pattern string   // the pattern itself
-       Pkgs    []string // matching packages (dirs or import paths)
+       Dirs    []string // if the pattern is local, directories that potentially contain matching packages
+       Pkgs    []string // matching packages (import paths)
        Errs    []error  // errors matching the patterns to packages, NOT errors loading those packages
 
        // Errs may be non-empty even if len(Pkgs) > 0, indicating that some matching
@@ -84,20 +85,25 @@ func (e *MatchError) Unwrap() error {
        return e.Err
 }
 
-// MatchPackages sets m.Pkgs to contain all the packages that can be found
-// under the $GOPATH directories and $GOROOT matching pattern.
-// The pattern is either "all" (all packages), "std" (standard packages),
-// "cmd" (standard commands), or a path including "...".
+// MatchPackages sets m.Pkgs to a non-nil slice containing all the packages that
+// can be found under the $GOPATH directories and $GOROOT that match the
+// pattern. The pattern must be either "all" (all packages), "std" (standard
+// packages), "cmd" (standard commands), or a path including "...".
 //
-// MatchPackages sets m.Errs to contain any errors encountered while processing
-// the match.
+// If any errors may have caused the set of packages to be incomplete,
+// MatchPackages appends those errors to m.Errs.
 func (m *Match) MatchPackages() {
-       m.Pkgs, m.Errs = nil, nil
+       m.Pkgs = []string{}
        if m.IsLocal() {
                m.AddError(fmt.Errorf("internal error: MatchPackages: %s is not a valid package pattern", m.pattern))
                return
        }
 
+       if m.IsLiteral() {
+               m.Pkgs = []string{m.pattern}
+               return
+       }
+
        match := func(string) bool { return true }
        treeCanMatch := func(string) bool { return true }
        if !m.IsMeta() {
@@ -197,16 +203,22 @@ func SetModRoot(dir string) {
        modRoot = dir
 }
 
-// MatchPackagesInFS is like MatchPackages but is passed a pattern that
-// begins with an absolute path or "./" or "../". On Windows, the pattern may
-// use slash or backslash separators or a mix of both.
+// MatchDirs sets m.Dirs to a non-nil slice containing all directories that
+// potentially match a local pattern. The pattern must begin with an absolute
+// path, or "./", or "../". On Windows, the pattern may use slash or backslash
+// separators or a mix of both.
 //
-// MatchPackagesInFS scans the tree rooted at the directory that contains the
-// first "..." wildcard.
-func (m *Match) MatchPackagesInFS() {
-       m.Pkgs, m.Errs = nil, nil
+// If any errors may have caused the set of directories to be incomplete,
+// MatchDirs appends those errors to m.Errs.
+func (m *Match) MatchDirs() {
+       m.Dirs = []string{}
        if !m.IsLocal() {
-               m.AddError(fmt.Errorf("internal error: MatchPackagesInFS: %s is not a valid filesystem pattern", m.pattern))
+               m.AddError(fmt.Errorf("internal error: MatchDirs: %s is not a valid filesystem pattern", m.pattern))
+               return
+       }
+
+       if m.IsLiteral() {
+               m.Dirs = []string{m.pattern}
                return
        }
 
@@ -301,7 +313,7 @@ func (m *Match) MatchPackagesInFS() {
                        // which is all that Match promises to do.
                        // Ignore the import error.
                }
-               m.Pkgs = append(m.Pkgs, name)
+               m.Dirs = append(m.Dirs, name)
                return nil
        })
        if err != nil {
@@ -416,25 +428,23 @@ func ImportPathsQuiet(patterns []string) []*Match {
        for _, a := range CleanPatterns(patterns) {
                m := NewMatch(a)
                if m.IsLocal() {
-                       if m.IsLiteral() {
-                               m.Pkgs = []string{a}
-                       } else {
-                               m.MatchPackagesInFS()
-                       }
+                       m.MatchDirs()
 
                        // Change the file import path to a regular import path if the package
                        // is in GOPATH or GOROOT. We don't report errors here; LoadImport
                        // (or something similar) will report them later.
-                       for i, dir := range m.Pkgs {
+                       m.Pkgs = make([]string, len(m.Dirs))
+                       for i, dir := range m.Dirs {
+                               absDir := dir
                                if !filepath.IsAbs(dir) {
-                                       dir = filepath.Join(base.Cwd, dir)
+                                       absDir = filepath.Join(base.Cwd, dir)
                                }
-                               if bp, _ := cfg.BuildContext.ImportDir(dir, build.FindOnly); bp.ImportPath != "" && bp.ImportPath != "." {
+                               if bp, _ := cfg.BuildContext.ImportDir(absDir, build.FindOnly); bp.ImportPath != "" && bp.ImportPath != "." {
                                        m.Pkgs[i] = bp.ImportPath
+                               } else {
+                                       m.Pkgs[i] = dir
                                }
                        }
-               } else if m.IsLiteral() {
-                       m.Pkgs = []string{a}
                } else {
                        m.MatchPackages()
                }
index 8552aebf42cd4d3d0bc7d8d89f8a40d823dd65a0..76a3b00d1c9779ae2282be6752c653c9028d2612 100644 (file)
@@ -14,6 +14,16 @@ go list cmd/...
 stdout ^cmd/compile
 ! stdout ^cmd/vendor/golang\.org/x/arch/x86/x86asm
 
+# GOROOT/src/... should list the packages in std as if it were a module
+# dependency: omitting vendored dependencies and stopping at the 'cmd' module
+# boundary.
+
+go list $GOROOT/src/...
+stdout ^bytes$
+! stdout ^builtin$
+! stdout ^cmd/
+! stdout ^vendor/
+
 
 # Within the std module, listing ./... should omit the 'std' prefix:
 # the package paths should be the same via ./... or the 'std' meta-pattern.