os/exec: avoid calling LookPath in cmd.Start for resolved paths

This reapplies CL 512155, which was previously reverted in CL 527337.
The race that prompted the revert should be fixed by CL 527820,
which will be submitted before this one.

For #36768.
Updates #62596.

Change-Id: I3c3cd92470254072901b6ef91c0ac52c8071e0a2
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-race,gotip-windows-amd64-race,gotip-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/528038
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>

Author: Bryan C. Mills

src/os/exec/exec.go

@@ -429,17 +429,14 @@ func Command(name string, arg ...string) *Cmd {
 	} else if runtime.GOOS == "windows" && filepath.IsAbs(name) {
 		// We may need to add a filename extension from PATHEXT
 		// or verify an extension that is already present.
-		// (We need to do this even for names that already have an extension
-		// in case of weird names like "foo.bat.exe".)
-		//
 		// Since the path is absolute, its extension should be unambiguous
 		// and independent of cmd.Dir, and we can go ahead and update cmd.Path to
 		// reflect it.
 		//
 		// Note that we cannot add an extension here for relative paths, because
 		// cmd.Dir may be set after we return from this function and that may cause
 		// the command to resolve to a different extension.
-		lp, err := LookPath(name)
+		lp, err := lookExtensions(name, "")
 		if lp != "" {
 			cmd.Path = lp
 		}
@@ -610,32 +607,6 @@ func (c *Cmd) Run() error {
 	return c.Wait()
 }
 
-// lookExtensions finds windows executable by its dir and path.
-// It uses LookPath to try appropriate extensions.
-// lookExtensions does not search PATH, instead it converts `prog` into `.\prog`.
-func lookExtensions(path, dir string) (string, error) {
-	if filepath.Base(path) == path {
-		path = "." + string(filepath.Separator) + path
-	}
-	if dir == "" {
-		return LookPath(path)
-	}
-	if filepath.VolumeName(path) != "" {
-		return LookPath(path)
-	}
-	if len(path) > 1 && os.IsPathSeparator(path[0]) {
-		return LookPath(path)
-	}
-	dirandpath := filepath.Join(dir, path)
-	// We assume that LookPath will only add file extension.
-	lp, err := LookPath(dirandpath)
-	if err != nil {
-		return "", err
-	}
-	ext := strings.TrimPrefix(lp, dirandpath)
-	return path + ext, nil
-}
-
 // Start starts the specified command but does not wait for it to complete.
 //
 // If Start returns successfully, the c.Process field will be set.

src/os/exec/lp_plan9.go

@@ -64,3 +64,9 @@ func LookPath(file string) (string, error) {
 	}
 	return "", &Error{file, ErrNotFound}
 }
+
+// lookExtensions is a no-op on non-Windows platforms, since
+// they do not restrict executables to specific extensions.
+func lookExtensions(path, dir string) (string, error) {
+	return path, nil
+}

src/os/exec/lp_unix.go

@@ -80,3 +80,9 @@ func LookPath(file string) (string, error) {
 	}
 	return "", &Error{file, ErrNotFound}
 }
+
+// lookExtensions is a no-op on non-Windows platforms, since
+// they do not restrict executables to specific extensions.
+func lookExtensions(path, dir string) (string, error) {
+	return path, nil
+}

src/os/exec/lp_wasm.go

@@ -21,3 +21,9 @@ func LookPath(file string) (string, error) {
 	// Wasm can not execute processes, so act as if there are no executables at all.
 	return "", &Error{file, ErrNotFound}
 }
+
+// lookExtensions is a no-op on non-Windows platforms, since
+// they do not restrict executables to specific extensions.
+func lookExtensions(path, dir string) (string, error) {
+	return path, nil
+}

src/os/exec/lp_windows.go

@@ -68,6 +68,51 @@ func findExecutable(file string, exts []string) (string, error) {
 // As of Go 1.19, LookPath will instead return that path along with an error satisfying
 // errors.Is(err, ErrDot). See the package documentation for more details.
 func LookPath(file string) (string, error) {
+	return lookPath(file, pathExt())
+}
+
+// lookExtensions finds windows executable by its dir and path.
+// It uses LookPath to try appropriate extensions.
+// lookExtensions does not search PATH, instead it converts `prog` into `.\prog`.
+//
+// If the path already has an extension found in PATHEXT,
+// lookExtensions returns it directly without searching
+// for additional extensions. For example,
+// "C:\foo\example.com" would be returned as-is even if the
+// program is actually "C:\foo\example.com.exe".
+func lookExtensions(path, dir string) (string, error) {
+	if filepath.Base(path) == path {
+		path = "." + string(filepath.Separator) + path
+	}
+	exts := pathExt()
+	if ext := filepath.Ext(path); ext != "" {
+		for _, e := range exts {
+			if strings.EqualFold(ext, e) {
+				// Assume that path has already been resolved.
+				return path, nil
+			}
+		}
+	}
+	if dir == "" {
+		return lookPath(path, exts)
+	}
+	if filepath.VolumeName(path) != "" {
+		return lookPath(path, exts)
+	}
+	if len(path) > 1 && os.IsPathSeparator(path[0]) {
+		return lookPath(path, exts)
+	}
+	dirandpath := filepath.Join(dir, path)
+	// We assume that LookPath will only add file extension.
+	lp, err := lookPath(dirandpath, exts)
+	if err != nil {
+		return "", err
+	}
+	ext := strings.TrimPrefix(lp, dirandpath)
+	return path + ext, nil
+}
+
+func pathExt() []string {
 	var exts []string
 	x := os.Getenv(`PATHEXT`)
 	if x != "" {
@@ -83,7 +128,11 @@ func LookPath(file string) (string, error) {
 	} else {
 		exts = []string{".com", ".exe", ".bat", ".cmd"}
 	}
+	return exts
+}
 
+// lookPath implements LookPath for the given PATHEXT list.
+func lookPath(file string, exts []string) (string, error) {
 	if strings.ContainsAny(file, `:\/`) {
 		f, err := findExecutable(file, exts)
 		if err == nil {

src/os/exec/lp_windows_test.go

@@ -611,22 +611,27 @@ func TestCommand(t *testing.T) {
 func TestAbsCommandWithDoubledExtension(t *testing.T) {
 	t.Parallel()
 
+	// We expect that ".com" is always included in PATHEXT, but it may also be
+	// found in the import path of a Go package. If it is at the root of the
+	// import path, the resulting executable may be named like "example.com.exe".
+	//
+	// Since "example.com" looks like a proper executable name, it is probably ok
+	// for exec.Command to try to run it directly without re-resolving it.
+	// However, exec.LookPath should try a little harder to figure it out.
+
 	comPath := filepath.Join(t.TempDir(), "example.com")
 	batPath := comPath + ".bat"
 	installBat(t, batPath)
 
 	cmd := exec.Command(comPath)
 	out, err := cmd.CombinedOutput()
-	t.Logf("%v:\n%s", cmd, out)
-	if err == nil {
-		got := strings.TrimSpace(string(out))
-		if got != batPath {
-			t.Errorf("wanted output %#q", batPath)
-		}
-	} else {
-		t.Errorf("%v: %v", cmd, err)
+	t.Logf("%v: %v\n%s", cmd, err, out)
+	if !errors.Is(err, fs.ErrNotExist) {
+		t.Errorf("Command(%#q).Run: %v\nwant fs.ErrNotExist", comPath, err)
 	}
-	if cmd.Path != batPath {
-		t.Errorf("exec.Command(%#q).Path =\n     %#q\nwant %#q", comPath, cmd.Path, batPath)
+
+	resolved, err := exec.LookPath(comPath)
+	if err != nil || resolved != batPath {
+		t.Fatalf("LookPath(%#q) = %v, %v; want %#q, <nil>", comPath, resolved, err, batPath)
 	}
 }