Skip to content

Commit

Permalink
syscall/exec: pass nil for CreateProcess() lpApplicationName on windows
Browse files Browse the repository at this point in the history
This addresses golang#17149 by calling CreateProcess() and
CreateProcessAsUser()in the same way that that dotnet does for its
equivalent syscall abstraction since dotnet doesn't suffer from the same
problem due to this difference.

See the dotnet equivalent here: https://github.com/dotnet/runtime/blob/2d411c4dfc1d71b2387ac64089014ec811ad7af0/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs#L578
...and here: https://github.com/dotnet/runtime/blob/2d411c4dfc1d71b2387ac64089014ec811ad7af0/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs#L554

This change assumes that argv0 always represents the name/path of the
executable and is represented as the argv[0] element per:
- How exec.Command() builds exec.Cmd.Args
- The following syscall/exec_unix code path: https://github.com/golang/go/blob/e8c8b79f000515e086012df632f01fc0ec21076b/src/syscall/exec_unix.go#L169-L171
  • Loading branch information
rafd123 committed Apr 28, 2023
1 parent e8c8b79 commit d24b468
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/path/filepath/path_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func testWinSplitListTestIsValid(t *testing.T, ti int, tt SplitListTest,
exp := []byte(d + "\r\n")
cmd := &exec.Cmd{
Path: comspec,
Args: []string{`/c`, cmdfile},
Args: []string{comspec, `/c`, cmdfile},
Env: []string{`Path=` + systemRoot + "/System32;" + tt.list, `SystemRoot=` + systemRoot},
Dir: tmp,
}
Expand Down
24 changes: 14 additions & 10 deletions src/syscall/exec_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,24 +287,28 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle
return 0, 0, err
}
}
argv0p, err := UTF16PtrFromString(argv0)
if err != nil {
return 0, 0, err
}

var appnamep *uint16
var cmdline string
// Windows CreateProcess takes the command line as a single string:
// use attr.CmdLine if set, else build the command line by escaping
// and joining each argument with spaces
// and joining each argument with spaces, and pass appnamep as nil to
// allow CreateProcess to parse the module from the command line.
if sys.CmdLine != "" {
cmdline = sys.CmdLine
appnamep, err = UTF16PtrFromString(argv0)
if err != nil {
return 0, 0, err
}
} else {
cmdline = makeCmdLine(argv)
// Use argv0 as the first element since it may have been updated
// above to be an absolute path.
cmdline = makeCmdLine(append([]string{argv0}, argv[1:]...))
}

var argvp *uint16
var cmdlinep *uint16
if len(cmdline) != 0 {
argvp, err = UTF16PtrFromString(cmdline)
cmdlinep, err = UTF16PtrFromString(cmdline)
if err != nil {
return 0, 0, err
}
Expand Down Expand Up @@ -413,9 +417,9 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle
pi := new(ProcessInformation)
flags := sys.CreationFlags | CREATE_UNICODE_ENVIRONMENT | _EXTENDED_STARTUPINFO_PRESENT
if sys.Token != 0 {
err = CreateProcessAsUser(sys.Token, argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, envBlock, dirp, &si.StartupInfo, pi)
err = CreateProcessAsUser(sys.Token, appnamep, cmdlinep, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, envBlock, dirp, &si.StartupInfo, pi)
} else {
err = CreateProcess(argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, envBlock, dirp, &si.StartupInfo, pi)
err = CreateProcess(appnamep, cmdlinep, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, envBlock, dirp, &si.StartupInfo, pi)
}
if err != nil {
return 0, 0, err
Expand Down
70 changes: 70 additions & 0 deletions src/syscall/exec_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
package syscall_test

import (
"bytes"
"fmt"
"io"
"os"
"os/exec"
"path"
"path/filepath"
"syscall"
"testing"
Expand Down Expand Up @@ -47,6 +50,73 @@ func TestEscapeArg(t *testing.T) {
}
}

func TestStartProcessBatchFile(t *testing.T) {
const batchFileContent = "@echo %*"

dir, err := os.MkdirTemp("", "TestStartProcessBatchFile")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)

noSpacesInPath := path.Join(dir, "no-spaces-in-path.cmd")
err = os.WriteFile(noSpacesInPath, []byte(batchFileContent), 0644)
if err != nil {
t.Fatal(err)
}

spacesInPath := path.Join(dir, "spaces in path.cmd")
err = os.WriteFile(spacesInPath, []byte(batchFileContent), 0644)
if err != nil {
t.Fatal(err)
}

tests := []struct {
batchFile string
args []string
want string
}{
{noSpacesInPath, []string{noSpacesInPath}, "ECHO is on."},
{spacesInPath, []string{spacesInPath}, "ECHO is on."},
{noSpacesInPath, []string{noSpacesInPath, "test-arg-no-spaces"}, "test-arg-no-spaces"},
{spacesInPath, []string{spacesInPath, "test-arg-no-spaces"}, "test-arg-no-spaces"},
{noSpacesInPath, []string{noSpacesInPath, "test arg spaces"}, `"test arg spaces"`},
{spacesInPath, []string{spacesInPath, "test arg spaces"}, `"test arg spaces"`},
{noSpacesInPath, []string{noSpacesInPath, "test arg spaces", "test-arg-no-spaces"}, `"test arg spaces" test-arg-no-spaces`},
{spacesInPath, []string{spacesInPath, "test arg spaces", "test-arg-no-spaces"}, `"test arg spaces" test-arg-no-spaces`},
}
for _, test := range tests {
pr, pw, err := os.Pipe()
if err != nil {
t.Fatal(err)
}
defer pr.Close()
defer pw.Close()

attr := &os.ProcAttr{Files: []*os.File{nil, pw, pw}}
p, err := os.StartProcess(test.batchFile, test.args, attr)
if err != nil {
t.Fatal(err)
}

_, err = p.Wait()
if err != nil {
t.Fatal(err)
}
pw.Close()

var buf bytes.Buffer
_, err = io.Copy(&buf, pr)
if err != nil {
t.Fatal(err)
}

if got, want := string(buf.Bytes()), test.want+"\r\n"; got != want {
t.Errorf("StartProcess(%#q, %#q) = %#q, want %#q", test.batchFile, test.args, got, want)
}
}
}

func TestChangingProcessParent(t *testing.T) {
if os.Getenv("GO_WANT_HELPER_PROCESS") == "parent" {
// in parent process
Expand Down

0 comments on commit d24b468

Please sign in to comment.