cmd/go: remove base.ShortPathConservative

This CL first removes the base.ShortPathConservative function. It had
two classes of uses. The first was in opening files where the paths end
up in error messages. In all those cases, the non-shortened paths are
used to open the files, and ShortPath is only used for the error
messages. The second is in base.RelPaths. RelPaths will now call
ShortPath for each of the paths passed in instead of calling
RelConservative and then doing the same check as ShortPath to see if the
path is shorter.

To avoid the possibility of incorrect relative paths ending up in error
messages (that might have command lines suggested for users to run), and
to avoid the possibility of incorrect relative paths appearing in the
output of base.RelPaths, base.ShortPaths always does an os.SameFile
check to make sure that the relative path its providing is actually
correct. Since this makes ShortPath slower than just manipulating paths
(because we need to stat the files), we need to be continue to enforce
that ShortPath is only called for error messages (with the exception of
base.RelPaths and its callers).

This is a simpler way of solving the problem that base.ShortPaths
intended to solve.

For #68383

Change-Id: I474f464f51a9acb2250069dea3054b55d95a4ab4
Reviewed-on: https://go-review.googlesource.com/c/go/+/630276
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>

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
+// ShortPath should only be used when formatting paths for error messages.
-// so it should only be used when formatting paths for error messages, not to read
-// a file.
 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
+func sameFile(path1, path2 string) bool {
-// would meet conditions that could make it invalid. If the short path would reach into a
+	fi1, err1 := os.Stat(path1)
-// parent directory and the base path contains a symlink, a ".." component can
+	fi2, err2 := os.Stat(path2)
-// cross a symlink boundary. That could be a problem because the symlinks could be evaluated,
+	if err1 != nil || err2 != nil {
-// changing the relative location of the boundary, before the ".." terms are applied to
+		// If there were errors statting the files return false,
-// go to parents. The check here is a little more conservative: it checks
+		// unless both of the files don't exist.
-// whether the path starts with a ../ or ..\ component, and if any of the parent directories
+		return os.IsNotExist(err1) && os.IsNotExist(err2)
-// 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
 	}
-	return path
+	return os.SameFile(fi1, fi2)
 }
 
-func relConservative(basepath, targpath string) (string, error) {
+// ShortPathError rewrites the path in err using base.ShortPath, if err is a wrapped PathError.
-	relpath, err := filepath.Rel(basepath, targpath)
+func ShortPathError(err error) error {
-	if err != nil {
+	var pe *fs.PathError
-		return "", err
+	if errors.As(err, &pe) {
+		pe.Path = ShortPath(pe.Path)
 	}
-	if strings.HasPrefix(relpath, str.WithFilePathSeparator("..")) {
+	return err
-		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
 }
 
 // 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)
+		out = append(out, ShortPath(p))
-		if err == nil && len(rel) < len(p) {
-			p = rel
-		}
-		out = append(out, p)
 	}
 	return out
 }

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)

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)
 		}

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

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