diff options
author | Bryan C. Mills <bcmills@google.com> | 2020-02-28 15:03:54 -0500 |
---|---|---|
committer | Bryan C. Mills <bcmills@google.com> | 2020-02-28 21:56:35 +0000 |
commit | 618126b9895db7f29a861caa4e330d149858ff56 (patch) | |
tree | 14b22fa6de658827549b6c2e8d6f17bf2d102207 | |
parent | 964fac3ee74fe4df5423dad18f78322d88aae84a (diff) | |
download | go-618126b9895db7f29a861caa4e330d149858ff56.tar.gz go-618126b9895db7f29a861caa4e330d149858ff56.zip |
cmd/go: avoid matching wildcards rooted outside of available modules
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>
-rw-r--r-- | src/cmd/go/internal/get/get.go | 5 | ||||
-rw-r--r-- | src/cmd/go/internal/modload/load.go | 60 | ||||
-rw-r--r-- | src/cmd/go/internal/search/search.go | 64 | ||||
-rw-r--r-- | src/cmd/go/testdata/script/mod_list_std.txt | 10 |
4 files changed, 90 insertions, 49 deletions
diff --git a/src/cmd/go/internal/get/get.go b/src/cmd/go/internal/get/get.go index b048eafa74..f7b2fa96e8 100644 --- a/src/cmd/go/internal/get/get.go +++ b/src/cmd/go/internal/get/get.go @@ -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) } diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index 32841d96cb..6ea7d8c69b 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -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) diff --git a/src/cmd/go/internal/search/search.go b/src/cmd/go/internal/search/search.go index 69d0e2d16f..b588c3e467 100644 --- a/src/cmd/go/internal/search/search.go +++ b/src/cmd/go/internal/search/search.go @@ -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() } diff --git a/src/cmd/go/testdata/script/mod_list_std.txt b/src/cmd/go/testdata/script/mod_list_std.txt index 8552aebf42..76a3b00d1c 100644 --- a/src/cmd/go/testdata/script/mod_list_std.txt +++ b/src/cmd/go/testdata/script/mod_list_std.txt @@ -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. |