Skip to content

Commit 6edfd65

Browse files
rafd123Rafael Dowling Goodman
authored and
Rafael Dowling Goodman
committed
syscall/exec: pass nil for CreateProcess() lpApplicationName on windows
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
1 parent e8c8b79 commit 6edfd65

File tree

3 files changed

+85
-11
lines changed

3 files changed

+85
-11
lines changed

src/path/filepath/path_windows_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func testWinSplitListTestIsValid(t *testing.T, ti int, tt SplitListTest,
7373
exp := []byte(d + "\r\n")
7474
cmd := &exec.Cmd{
7575
Path: comspec,
76-
Args: []string{`/c`, cmdfile},
76+
Args: []string{comspec, `/c`, cmdfile},
7777
Env: []string{`Path=` + systemRoot + "/System32;" + tt.list, `SystemRoot=` + systemRoot},
7878
Dir: tmp,
7979
}

src/syscall/exec_windows.go

+14-10
Original file line numberDiff line numberDiff line change
@@ -287,24 +287,28 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle
287287
return 0, 0, err
288288
}
289289
}
290-
argv0p, err := UTF16PtrFromString(argv0)
291-
if err != nil {
292-
return 0, 0, err
293-
}
294290

291+
var appnamep *uint16
295292
var cmdline string
296293
// Windows CreateProcess takes the command line as a single string:
297294
// use attr.CmdLine if set, else build the command line by escaping
298-
// and joining each argument with spaces
295+
// and joining each argument with spaces, and pass appnamep as nil to
296+
// allow CreateProcess to parse the module from the command line.
299297
if sys.CmdLine != "" {
300298
cmdline = sys.CmdLine
299+
appnamep, err = UTF16PtrFromString(argv0)
300+
if err != nil {
301+
return 0, 0, err
302+
}
301303
} else {
302-
cmdline = makeCmdLine(argv)
304+
// Use argv0 as the first element since it may have been updated
305+
// above to be an absolute path.
306+
cmdline = makeCmdLine(append([]string{argv0}, argv[1:]...))
303307
}
304308

305-
var argvp *uint16
309+
var cmdlinep *uint16
306310
if len(cmdline) != 0 {
307-
argvp, err = UTF16PtrFromString(cmdline)
311+
cmdlinep, err = UTF16PtrFromString(cmdline)
308312
if err != nil {
309313
return 0, 0, err
310314
}
@@ -413,9 +417,9 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle
413417
pi := new(ProcessInformation)
414418
flags := sys.CreationFlags | CREATE_UNICODE_ENVIRONMENT | _EXTENDED_STARTUPINFO_PRESENT
415419
if sys.Token != 0 {
416-
err = CreateProcessAsUser(sys.Token, argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, envBlock, dirp, &si.StartupInfo, pi)
420+
err = CreateProcessAsUser(sys.Token, appnamep, cmdlinep, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, envBlock, dirp, &si.StartupInfo, pi)
417421
} else {
418-
err = CreateProcess(argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, envBlock, dirp, &si.StartupInfo, pi)
422+
err = CreateProcess(appnamep, cmdlinep, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, envBlock, dirp, &si.StartupInfo, pi)
419423
}
420424
if err != nil {
421425
return 0, 0, err

src/syscall/exec_windows_test.go

+70
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@
55
package syscall_test
66

77
import (
8+
"bytes"
89
"fmt"
10+
"io"
911
"os"
1012
"os/exec"
13+
"path"
1114
"path/filepath"
1215
"syscall"
1316
"testing"
@@ -47,6 +50,73 @@ func TestEscapeArg(t *testing.T) {
4750
}
4851
}
4952

53+
func TestStartProcessBatchFile(t *testing.T) {
54+
const batchFileContent = "@echo %*"
55+
56+
dir, err := os.MkdirTemp("", "TestStartProcessBatchFile")
57+
if err != nil {
58+
t.Fatal(err)
59+
}
60+
defer os.RemoveAll(dir)
61+
62+
noSpacesInPath := path.Join(dir, "no-spaces-in-path.cmd")
63+
err = os.WriteFile(noSpacesInPath, []byte(batchFileContent), 0644)
64+
if err != nil {
65+
t.Fatal(err)
66+
}
67+
68+
spacesInPath := path.Join(dir, "spaces in path.cmd")
69+
err = os.WriteFile(spacesInPath, []byte(batchFileContent), 0644)
70+
if err != nil {
71+
t.Fatal(err)
72+
}
73+
74+
tests := []struct {
75+
batchFile string
76+
args []string
77+
want string
78+
}{
79+
{noSpacesInPath, []string{noSpacesInPath}, "ECHO is on."},
80+
{spacesInPath, []string{spacesInPath}, "ECHO is on."},
81+
{noSpacesInPath, []string{noSpacesInPath, "test-arg-no-spaces"}, "test-arg-no-spaces"},
82+
{spacesInPath, []string{spacesInPath, "test-arg-no-spaces"}, "test-arg-no-spaces"},
83+
{noSpacesInPath, []string{noSpacesInPath, "test arg spaces"}, `"test arg spaces"`},
84+
{spacesInPath, []string{spacesInPath, "test arg spaces"}, `"test arg spaces"`},
85+
{noSpacesInPath, []string{noSpacesInPath, "test arg spaces", "test-arg-no-spaces"}, `"test arg spaces" test-arg-no-spaces`},
86+
{spacesInPath, []string{spacesInPath, "test arg spaces", "test-arg-no-spaces"}, `"test arg spaces" test-arg-no-spaces`},
87+
}
88+
for _, test := range tests {
89+
pr, pw, err := os.Pipe()
90+
if err != nil {
91+
t.Fatal(err)
92+
}
93+
defer pr.Close()
94+
defer pw.Close()
95+
96+
attr := &os.ProcAttr{Files: []*os.File{nil, pw, pw}}
97+
p, err := os.StartProcess(test.batchFile, test.args, attr)
98+
if err != nil {
99+
t.Fatal(err)
100+
}
101+
102+
_, err = p.Wait()
103+
if err != nil {
104+
t.Fatal(err)
105+
}
106+
pw.Close()
107+
108+
var buf bytes.Buffer
109+
_, err = io.Copy(&buf, pr)
110+
if err != nil {
111+
t.Fatal(err)
112+
}
113+
114+
if got, want := string(buf.Bytes()), test.want+"\r\n"; got != want {
115+
t.Errorf("StartProcess(%#q, %#q) = %#q, want %#q", test.batchFile, test.args, got, want)
116+
}
117+
}
118+
}
119+
50120
func TestChangingProcessParent(t *testing.T) {
51121
if os.Getenv("GO_WANT_HELPER_PROCESS") == "parent" {
52122
// in parent process

0 commit comments

Comments
 (0)