diff --git a/src/cmd/go/internal/base/path.go b/src/cmd/go/internal/base/path.go index 96cf1a2f053..5bb7bc3bde6 100644 --- a/src/cmd/go/internal/base/path.go +++ b/src/cmd/go/internal/base/path.go @@ -6,13 +6,12 @@ package base import ( "errors" + "io/fs" "os" "path/filepath" "runtime" "strings" "sync" - - "cmd/go/internal/str" ) // UncachedCwd returns the current working directory. @@ -35,44 +34,32 @@ func Cwd() string { } // ShortPath returns an absolute or relative name for path, whatever is shorter. -// There are rare cases where the path produced by ShortPath could be incorrect -// so it should only be used when formatting paths for error messages, not to read -// a file. +// ShortPath should only be used when formatting paths for error messages. func ShortPath(path string) string { - if rel, err := filepath.Rel(Cwd(), path); err == nil && len(rel) < len(path) { + if rel, err := filepath.Rel(Cwd(), path); err == nil && len(rel) < len(path) && sameFile(rel, path) { return rel } return path } -// ShortPathConservative is similar to ShortPath, but returns the input if the result of ShortPath -// would meet conditions that could make it invalid. If the short path would reach into a -// parent directory and the base path contains a symlink, a ".." component can -// cross a symlink boundary. That could be a problem because the symlinks could be evaluated, -// changing the relative location of the boundary, before the ".." terms are applied to -// go to parents. The check here is a little more conservative: it checks -// whether the path starts with a ../ or ..\ component, and if any of the parent directories -// of the working directory are symlinks. -// See #68383 for a case where this could happen. -func ShortPathConservative(path string) string { - if rel, err := relConservative(Cwd(), path); err == nil && len(rel) < len(path) { - return rel +func sameFile(path1, path2 string) bool { + fi1, err1 := os.Stat(path1) + fi2, err2 := os.Stat(path2) + if err1 != nil || err2 != nil { + // If there were errors statting the files return false, + // unless both of the files don't exist. + return os.IsNotExist(err1) && os.IsNotExist(err2) } - return path + return os.SameFile(fi1, fi2) } -func relConservative(basepath, targpath string) (string, error) { - relpath, err := filepath.Rel(basepath, targpath) - if err != nil { - return "", err +// ShortPathError rewrites the path in err using base.ShortPath, if err is a wrapped PathError. +func ShortPathError(err error) error { + var pe *fs.PathError + if errors.As(err, &pe) { + pe.Path = ShortPath(pe.Path) } - if strings.HasPrefix(relpath, str.WithFilePathSeparator("..")) { - expanded, err := filepath.EvalSymlinks(basepath) - if err != nil || expanded != basepath { // The basepath contains a symlink. Be conservative and reject it. - return "", errors.New("conservatively rejecting relative path that may be invalid") - } - } - return relpath, nil + return err } // RelPaths returns a copy of paths with absolute paths @@ -80,11 +67,7 @@ func relConservative(basepath, targpath string) (string, error) { func RelPaths(paths []string) []string { out := make([]string, 0, len(paths)) for _, p := range paths { - rel, err := relConservative(Cwd(), p) - if err == nil && len(rel) < len(p) { - p = rel - } - out = append(out, p) + out = append(out, ShortPath(p)) } return out } diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go index c1bca7e7329..8fdcd0da632 100644 --- a/src/cmd/go/internal/modload/init.go +++ b/src/cmd/go/internal/modload/init.go @@ -944,7 +944,7 @@ func loadModFile(ctx context.Context, opts *PackageOpts) (*Requirements, error) err = errWorkTooOld(gomod, workFile, tooNew.GoVersion) } else { err = fmt.Errorf("cannot load module %s listed in go.work file: %w", - base.ShortPath(filepath.Dir(gomod)), err) + base.ShortPath(filepath.Dir(gomod)), base.ShortPathError(err)) } } errs = append(errs, err) diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index 746cefd256d..e8f8e7fa5cd 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -670,7 +670,7 @@ func resolveLocalPackage(ctx context.Context, dir string, rs *Requirements) (str } if inWorkspaceMode() { if mr := findModuleRoot(absDir); mr != "" { - return "", fmt.Errorf("%s is contained in a module that is not one of the workspace modules listed in go.work. You can add the module to the workspace using:\n\tgo work use %s", dirstr, base.ShortPathConservative(mr)) + return "", fmt.Errorf("%s is contained in a module that is not one of the workspace modules listed in go.work. You can add the module to the workspace using:\n\tgo work use %s", dirstr, base.ShortPath(mr)) } return "", fmt.Errorf("%s outside modules listed in go.work or their selected dependencies", dirstr) } diff --git a/src/cmd/go/internal/modload/modfile.go b/src/cmd/go/internal/modload/modfile.go index 636ad03c784..769d35e073d 100644 --- a/src/cmd/go/internal/modload/modfile.go +++ b/src/cmd/go/internal/modload/modfile.go @@ -30,10 +30,6 @@ import ( // ReadModFile reads and parses the mod file at gomod. ReadModFile properly applies the // overlay, locks the file while reading, and applies fix, if applicable. func ReadModFile(gomod string, fix modfile.VersionFixer) (data []byte, f *modfile.File, err error) { - // The path used to open the file shows up in errors. Use ShortPathConservative - // so a more convenient path is displayed in the errors. ShortPath isn't used - // because it's meant only to be used in errors, not to open files. - gomod = base.ShortPathConservative(gomod) if fsys.Replaced(gomod) { // Don't lock go.mod if it's part of the overlay. // On Plan 9, locking requires chmod, and we don't want to modify any file @@ -49,18 +45,18 @@ func ReadModFile(gomod string, fix modfile.VersionFixer) (data []byte, f *modfil f, err = modfile.Parse(gomod, data, fix) if err != nil { // Errors returned by modfile.Parse begin with file:line. - return nil, nil, fmt.Errorf("errors parsing %s:\n%w", gomod, err) + return nil, nil, fmt.Errorf("errors parsing %s:\n%w", base.ShortPath(gomod), err) } if f.Go != nil && gover.Compare(f.Go.Version, gover.Local()) > 0 { toolchain := "" if f.Toolchain != nil { toolchain = f.Toolchain.Name } - return nil, nil, &gover.TooNewError{What: gomod, GoVersion: f.Go.Version, Toolchain: toolchain} + return nil, nil, &gover.TooNewError{What: base.ShortPath(gomod), GoVersion: f.Go.Version, Toolchain: toolchain} } if f.Module == nil { // No module declaration. Must add module path. - return nil, nil, fmt.Errorf("error reading %s: missing module declaration. To specify the module path:\n\tgo mod edit -module=example.com/mod", gomod) + return nil, nil, fmt.Errorf("error reading %s: missing module declaration. To specify the module path:\n\tgo mod edit -module=example.com/mod", base.ShortPath(gomod)) } return data, f, err diff --git a/src/cmd/go/internal/workcmd/use.go b/src/cmd/go/internal/workcmd/use.go index 3e503bfac5b..afbe99d3a48 100644 --- a/src/cmd/go/internal/workcmd/use.go +++ b/src/cmd/go/internal/workcmd/use.go @@ -102,7 +102,7 @@ func workUse(ctx context.Context, gowork string, wf *modfile.WorkFile, args []st lookDir := func(dir string) { absDir, dir := pathRel(workDir, dir) - file := base.ShortPathConservative(filepath.Join(absDir, "go.mod")) + file := filepath.Join(absDir, "go.mod") fi, err := fsys.Stat(file) if err != nil { if os.IsNotExist(err) { @@ -114,7 +114,7 @@ func workUse(ctx context.Context, gowork string, wf *modfile.WorkFile, args []st } if !fi.Mode().IsRegular() { - sw.Error(fmt.Errorf("%v is not a regular file", file)) + sw.Error(fmt.Errorf("%v is not a regular file", base.ShortPath(file))) return } @@ -126,18 +126,17 @@ func workUse(ctx context.Context, gowork string, wf *modfile.WorkFile, args []st for _, useDir := range args { absArg, _ := pathRel(workDir, useDir) - useDirShort := base.ShortPathConservative(absArg) // relative to the working directory rather than the workspace - info, err := fsys.Stat(useDirShort) + info, err := fsys.Stat(absArg) if err != nil { // Errors raised from os.Stat are formatted to be more user-friendly. if os.IsNotExist(err) { - err = fmt.Errorf("directory %v does not exist", useDirShort) + err = fmt.Errorf("directory %v does not exist", base.ShortPath(absArg)) } sw.Error(err) continue } else if !info.IsDir() { - sw.Error(fmt.Errorf("%s is not a directory", useDirShort)) + sw.Error(fmt.Errorf("%s is not a directory", base.ShortPath(absArg))) continue } @@ -158,7 +157,7 @@ func workUse(ctx context.Context, gowork string, wf *modfile.WorkFile, args []st if !d.IsDir() { if d.Type()&fs.ModeSymlink != 0 { if target, err := fsys.Stat(path); err == nil && target.IsDir() { - fmt.Fprintf(os.Stderr, "warning: ignoring symlink %s\n", base.ShortPathConservative(path)) + fmt.Fprintf(os.Stderr, "warning: ignoring symlink %s\n", base.ShortPath(path)) } } return nil @@ -210,7 +209,7 @@ func workUse(ctx context.Context, gowork string, wf *modfile.WorkFile, args []st } else { abs = filepath.Join(workDir, use.Path) } - _, mf, err := modload.ReadModFile(base.ShortPathConservative(filepath.Join(abs, "go.mod")), nil) + _, mf, err := modload.ReadModFile(filepath.Join(abs, "go.mod"), nil) if err != nil { sw.Error(err) continue