]> Cypherpunks repositories - gostls13.git/commitdiff
os/exec: avoid calling LookPath in cmd.Start for resolved paths
authorBryan C. Mills <bcmills@google.com>
Wed, 13 Sep 2023 16:02:52 +0000 (12:02 -0400)
committerGopher Robot <gobot@golang.org>
Wed, 13 Sep 2023 19:38:12 +0000 (19:38 +0000)
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>

src/os/exec/exec.go
src/os/exec/lp_plan9.go
src/os/exec/lp_unix.go
src/os/exec/lp_wasm.go
src/os/exec/lp_windows.go
src/os/exec/lp_windows_test.go

index ea520f872abfab56c064f4b97fa66321944176a7..c88ee7f52c7b02873ba7c8d864d348a2607908b8 100644 (file)
@@ -429,9 +429,6 @@ 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.
@@ -439,7 +436,7 @@ func Command(name string, arg ...string) *Cmd {
                // 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.
index 9344b14e8cdfe6ffc24955018d7e9235c9c6f9b9..dffdbac35f8e01e0ef2a3c0f4f3a5f1dca2ae019 100644 (file)
@@ -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
+}
index fd2c6efbef08cd8e18774c651159e9be61883bfe..37871320786aef2f36203919273009a39dba8651 100644 (file)
@@ -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
+}
index f2c8e9c5de47903835a4db859f18b70c539017d8..3c819049bab3420e0a04e6b294e406bb557e855f 100644 (file)
@@ -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
+}
index ea83c19acd6036629ed6704ead5e3f6af5ac448e..698a97c40f6f948e11aab69f1f9eae2839f5cf0e 100644 (file)
@@ -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 {
index 0d5095e5348790a39763b1bb597a57d97511fbea..a92a29799f35f187d5734c7a4cb98b5438a59627 100644 (file)
@@ -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)
        }
 }