Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/exec: on Windows look for extensions in Run if not already done #67035

Closed
wants to merge 13 commits into from
Closed
16 changes: 10 additions & 6 deletions src/os/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,10 @@ type Cmd struct {
// See https://go.dev/blog/path-security
// and https://go.dev/issue/43724 for more context.
lookPathErr error

// cachedLookExtensions caches the result of calling lookExtensions.
// This is only used on Windows.
cachedLookExtensions string
}

// A ctxResult reports the result of watching the Context associated with a
Expand Down Expand Up @@ -430,16 +434,13 @@ func Command(name string, arg ...string) *Cmd {
// We may need to add a filename extension from PATHEXT
// or verify an extension that is already present.
// 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.
// and independent of cmd.Dir, and we can go ahead and cache the lookup now.
//
// 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 := lookExtensions(name, "")
if lp != "" {
cmd.Path = lp
}
cmd.cachedLookExtensions = lp
if err != nil {
cmd.Err = err
}
Expand Down Expand Up @@ -641,7 +642,10 @@ func (c *Cmd) Start() error {
return c.Err
}
lp := c.Path
if runtime.GOOS == "windows" && !filepath.IsAbs(c.Path) {
if c.cachedLookExtensions != "" {
lp = c.cachedLookExtensions
}
if runtime.GOOS == "windows" && c.cachedLookExtensions == "" {
// If c.Path is relative, we had to wait until now
// to resolve it in case c.Dir was changed.
// (If it is absolute, we already resolved its extension in Command
Expand Down
29 changes: 29 additions & 0 deletions src/os/exec/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1835,3 +1835,32 @@ func TestPathRace(t *testing.T) {
t.Logf("running in background: %v", cmd)
<-done
}

func TestAbsPathExec(t *testing.T) {
testenv.MustHaveExec(t)
testenv.MustHaveGoBuild(t) // must have GOROOT/bin/gofmt, but close enough

// A simple exec of a full path should work.
// Go 1.22 broke this on Windows, requiring ".exe"; see #66586.
exe := filepath.Join(testenv.GOROOT(t), "bin/gofmt")
cmd := exec.Command(exe)
if cmd.Path != exe {
t.Errorf("exec.Command(%#q) set Path=%#q", exe, cmd.Path)
}
err := cmd.Run()
if err != nil {
t.Errorf("using exec.Command(%#q): %v", exe, err)
}

cmd = &exec.Cmd{Path: exe}
err = cmd.Run()
if err != nil {
t.Errorf("using exec.Cmd{Path: %#q}: %v", cmd.Path, err)
}

cmd = &exec.Cmd{Path: "gofmt", Dir: "/"}
err = cmd.Run()
if err == nil {
t.Errorf("using exec.Cmd{Path: %#q}: unexpected success", cmd.Path)
}
}