From 913c8d73978795e4f2cdd1f87de3af5239ebdc84 Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Thu, 24 Mar 2011 11:20:28 +1100 Subject: [PATCH] syscall: StartProcess fixes for windows - StartProcess will work with relative (to attr.Dir, not current directory) executable filenames - StartProcess will only work if executable filename points to the real file, it will not search for executable in the $PATH list and others (see CreateProcess manual for details) - StartProcess argv strings can contain any characters R=golang-dev, r CC=golang-dev https://golang.org/cl/4306041 --- src/pkg/exec/exec_test.go | 52 ++++ src/pkg/os/os_test.go | 44 ++-- src/pkg/syscall/exec_windows.go | 334 +++++++++++++++--------- src/pkg/syscall/syscall_windows.go | 3 +- src/pkg/syscall/zsyscall_windows_386.go | 20 +- 5 files changed, 311 insertions(+), 142 deletions(-) diff --git a/src/pkg/exec/exec_test.go b/src/pkg/exec/exec_test.go index 3a3d3b1a53e..5e37b99eeca 100644 --- a/src/pkg/exec/exec_test.go +++ b/src/pkg/exec/exec_test.go @@ -118,3 +118,55 @@ func TestAddEnvVar(t *testing.T) { t.Fatal("close:", err) } } + +var tryargs = []string{ + `2`, + `2 `, + "2 \t", + `2" "`, + `2 ab `, + `2 "ab" `, + `2 \ `, + `2 \\ `, + `2 \" `, + `2 \`, + `2\`, + `2"`, + `2\"`, + `2 "`, + `2 \"`, + ``, + `2 ^ `, + `2 \^`, +} + +func TestArgs(t *testing.T) { + for _, a := range tryargs { + argv := []string{ + "awk", + `BEGIN{printf("%s|%s|%s",ARGV[1],ARGV[2],ARGV[3])}`, + "/dev/null", + a, + "EOF", + } + exe, err := LookPath(argv[0]) + if err != nil { + t.Fatal("run:", err) + } + cmd, err := Run(exe, argv, nil, "", DevNull, Pipe, DevNull) + if err != nil { + t.Fatal("run:", err) + } + buf, err := ioutil.ReadAll(cmd.Stdout) + if err != nil { + t.Fatal("read:", err) + } + expect := "/dev/null|" + a + "|EOF" + if string(buf) != expect { + t.Errorf("read: got %q expect %q", buf, expect) + } + if err = cmd.Close(); err != nil { + t.Fatal("close:", err) + } + } +} diff --git a/src/pkg/os/os_test.go b/src/pkg/os/os_test.go index e06b2894024..1f34e54f5f0 100644 --- a/src/pkg/os/os_test.go +++ b/src/pkg/os/os_test.go @@ -10,6 +10,7 @@ import ( "io" "io/ioutil" . "os" + "path/filepath" "strings" "syscall" "testing" @@ -409,25 +410,12 @@ func TestRename(t *testing.T) { } } -func TestStartProcess(t *testing.T) { - var cmd, expect string - var args []string +func exec(t *testing.T, dir, cmd string, args []string, expect string) { r, w, err := Pipe() if err != nil { t.Fatalf("Pipe: %v", err) } - attr := &ProcAttr{Files: []*File{nil, w, Stderr}} - if syscall.OS == "windows" { - cmd = Getenv("COMSPEC") - args = []string{Getenv("COMSPEC"), "/c cd"} - attr.Dir = Getenv("SystemRoot") - expect = Getenv("SystemRoot") + "\r\n" - } else { - cmd = "/bin/pwd" - args = []string{"pwd"} - attr.Dir = "/" - expect = "/\n" - } + attr := &ProcAttr{Dir: dir, Files: []*File{nil, w, Stderr}} p, err := StartProcess(cmd, args, attr) if err != nil { t.Fatalf("StartProcess: %v", err) @@ -439,12 +427,34 @@ func TestStartProcess(t *testing.T) { io.Copy(&b, r) output := b.String() if output != expect { - args[0] = cmd - t.Errorf("exec %q returned %q wanted %q", strings.Join(args, " "), output, expect) + t.Errorf("exec %q returned %q wanted %q", + strings.Join(append([]string{cmd}, args...), " "), output, expect) } p.Wait(0) } +func TestStartProcess(t *testing.T) { + var dir, cmd, le string + var args []string + if syscall.OS == "windows" { + le = "\r\n" + cmd = Getenv("COMSPEC") + dir = Getenv("SystemRoot") + args = []string{"/c", "cd"} + } else { + le = "\n" + cmd = "/bin/pwd" + dir = "/" + args = []string{} + } + cmddir, cmdbase := filepath.Split(cmd) + args = append([]string{cmdbase}, args...) + // Test absolute executable path. + exec(t, dir, cmd, args, dir+le) + // Test relative executable path. + exec(t, cmddir, cmdbase, args, filepath.Clean(cmddir)+le) +} + func checkMode(t *testing.T, path string, mode uint32) { dir, err := Stat(path) if err != nil { diff --git a/src/pkg/syscall/exec_windows.go b/src/pkg/syscall/exec_windows.go index 06c33331fbb..1fa224efeaa 100644 --- a/src/pkg/syscall/exec_windows.go +++ b/src/pkg/syscall/exec_windows.go @@ -11,48 +11,92 @@ import ( "utf16" ) -// Windows doesn't have a good concept of just Exec in the documented API. -// However, the kernel32 CreateProcess does a good job with -// ForkExec. - var ForkLock sync.RWMutex -// Joins an array of string with sep -// From the "strings" package. Modified. -func stringJoin(a []string, sep string, escape escapeFunc) string { - if len(a) == 0 { - return "" +// escape rewrites command line argument s as prescribed +// in http://msdn.microsoft.com/en-us/library/ms880421. +// This function returns "" (2 double quotes) if s is empty. +// Alternatively, these transformations are done: +// - every back slash (\) is doubled, but only if immediately +// followed by double quote ("); +// - every double quote (") is escaped by back slash (\); +// - finally, s is wrapped with double quotes (arg -> "arg"), +// but only if there is space or tab inside s. +func escape(s string) string { + if len(s) == 0 { + return "\"\"" } - if len(a) == 1 { - return a[0] + n := len(s) + hasSpace := false + for i := 0; i < len(s); i++ { + switch s[i] { + case '"', '\\': + n++ + case ' ', '\t': + hasSpace = true + } } - n := len(sep) * (len(a) - 1) - for i := 0; i < len(a); i++ { - a[i] = escape(a[i]) - n += len(a[i]) + if hasSpace { + n += 2 + } + if n == len(s) { + return s } - b := make([]byte, n) - bp := 0 - for i := 0; i < len(a); i++ { - s := a[i] - for j := 0; j < len(s); j++ { - b[bp] = s[j] - bp++ - } - if i+1 < len(a) { - s = sep - for j := 0; j < len(s); j++ { - b[bp] = s[j] - bp++ - } - } + qs := make([]byte, n) + j := 0 + if hasSpace { + qs[j] = '"' + j++ } - return string(b) + slashes := 0 + for i := 0; i < len(s); i++ { + switch s[i] { + default: + slashes = 0 + qs[j] = s[i] + case '\\': + slashes++ + qs[j] = s[i] + case '"': + for ; slashes > 0; slashes-- { + qs[j] = '\\' + j++ + } + qs[j] = '\\' + j++ + qs[j] = s[i] + } + j++ + } + if hasSpace { + for ; slashes > 0; slashes-- { + qs[j] = '\\' + j++ + } + qs[j] = '"' + j++ + } + return string(qs[:j]) } -//Env block is a sequence of null terminated strings followed by a null. -//Last bytes are two unicode nulls, or four null bytes. +// makeCmdLine builds a command line out of args by escaping "special" +// characters and joining the arguments with spaces. +func makeCmdLine(args []string) string { + var s string + for _, v := range args { + if s != "" { + s += " " + } + s += escape(v) + } + return s +} + +// createEnvBlock converts an array of environment strings into +// the representation required by CreateProcess: a sequence of NUL +// terminated strings followed by a nil. +// Last bytes are two UCS-2 NULs, or four NUL bytes. func createEnvBlock(envv []string) *uint16 { if len(envv) == 0 { return &utf16.Encode([]int("\x00\x00"))[0] @@ -76,36 +120,6 @@ func createEnvBlock(envv []string) *uint16 { return &utf16.Encode([]int(string(b)))[0] } -type escapeFunc func(s string) string - -//escapes quotes by " -> "" -//Also string -> "string" -func escapeAddQuotes(s string) string { - //normal ascii char, one byte wide - rune := byte('"') - l := len(s) - n := 0 - for i := 0; i < l; i++ { - if s[i] == rune { - n++ - } - } - qs := make([]byte, l+n+2) - - qs[0] = rune - j := 1 - for i := 0; i < l; i++ { - qs[i+j] = s[i] - if s[i] == rune { - j++ - qs[i+j] = rune - } - } - qs[len(qs)-1] = rune - return string(qs) -} - - func CloseOnExec(fd int) { SetHandleInformation(int32(fd), HANDLE_FLAG_INHERIT, 0) } @@ -114,6 +128,94 @@ func SetNonblock(fd int, nonblocking bool) (errno int) { return 0 } +// getFullPath retrieves the full path of the specified file. +// Just a wrapper for Windows GetFullPathName api. +func getFullPath(name string) (path string, err int) { + p := StringToUTF16Ptr(name) + buf := make([]uint16, 100) + n, err := GetFullPathName(p, uint32(len(buf)), &buf[0], nil) + if err != 0 { + return "", err + } + if n > uint32(len(buf)) { + // Windows is asking for bigger buffer. + buf = make([]uint16, n) + n, err = GetFullPathName(p, uint32(len(buf)), &buf[0], nil) + if err != 0 { + return "", err + } + if n > uint32(len(buf)) { + return "", EINVAL + } + } + return UTF16ToString(buf[:n]), 0 +} + +func isSlash(c uint8) bool { + return c == '\\' || c == '/' +} + +func normalizeDir(dir string) (name string, err int) { + ndir, err := getFullPath(dir) + if err != 0 { + return "", err + } + if len(ndir) > 2 && isSlash(ndir[0]) && isSlash(ndir[1]) { + // dir cannot have \\server\share\path form + return "", EINVAL + } + return ndir, 0 +} + +func volToUpper(ch int) int { + if 'a' <= ch && ch <= 'z' { + ch += 'A' - 'a' + } + return ch +} + +func joinExeDirAndFName(dir, p string) (name string, err int) { + if len(p) == 0 { + return "", EINVAL + } + if len(p) > 2 && isSlash(p[0]) && isSlash(p[1]) { + // \\server\share\path form + return p, 0 + } + if len(p) > 1 && p[1] == ':' { + // has drive letter + if len(p) == 2 { + return "", EINVAL + } + if isSlash(p[2]) { + return p, 0 + } else { + d, err := normalizeDir(dir) + if err != 0 { + return "", err + } + if volToUpper(int(p[0])) == volToUpper(int(d[0])) { + return getFullPath(d + "\\" + p[2:]) + } else { + return getFullPath(p) + } + } + } else { + // no drive letter + d, err := normalizeDir(dir) + if err != 0 { + return "", err + } + if isSlash(p[0]) { + return getFullPath(d[:2] + p) + } else { + return getFullPath(d + "\\" + p) + } + } + // we shouldn't be here + return "", EINVAL +} + type ProcAttr struct { Dir string Env []string @@ -122,9 +224,10 @@ type ProcAttr struct { var zeroAttributes ProcAttr -// TODO(kardia): Add trace -//The command and arguments are passed via the Command line parameter. func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid, handle int, err int) { + if len(argv0) == 0 { + return 0, 0, EWINDOWS + } if attr == nil { attr = &zeroAttributes } @@ -132,25 +235,31 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid, handle int, return 0, 0, EWINDOWS } - // CreateProcess will throw an error if the dir is not set to a valid dir - // thus get the working dir if dir is empty. - dir := attr.Dir - if dir == "" { - if wd, ok := Getwd(); ok == 0 { - dir = wd + if len(attr.Dir) != 0 { + // StartProcess assumes that argv0 is relative to attr.Dir, + // because it implies Chdir(attr.Dir) before executing argv0. + // Windows CreateProcess assumes the opposite: it looks for + // argv0 relative to the current directory, and, only once the new + // process is started, it does Chdir(attr.Dir). We are adjusting + // for that difference here by making argv0 absolute. + var err int + argv0, err = joinExeDirAndFName(attr.Dir, argv0) + if err != 0 { + return 0, 0, err } } - fd := attr.Files + argv0p := StringToUTF16Ptr(argv0) - startupInfo := new(StartupInfo) - processInfo := new(ProcessInformation) + var argvp *uint16 + s := makeCmdLine(argv) + if len(s) != 0 { + argvp = StringToUTF16Ptr(s) + } - GetStartupInfo(startupInfo) - - startupInfo.Flags = STARTF_USESTDHANDLES - startupInfo.StdInput = 0 - startupInfo.StdOutput = 0 - startupInfo.StdErr = 0 + var dirp *uint16 + if len(attr.Dir) != 0 { + dirp = StringToUTF16Ptr(attr.Dir) + } // Acquire the fork lock so that no other threads // create new fds that are not yet close-on-exec @@ -158,54 +267,35 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid, handle int, ForkLock.Lock() defer ForkLock.Unlock() - var currentProc, _ = GetCurrentProcess() - if len(fd) > 0 && fd[0] > 0 { - err := DuplicateHandle(currentProc, int32(fd[0]), currentProc, &startupInfo.StdInput, 0, true, DUPLICATE_SAME_ACCESS) - if err != 0 { - return 0, 0, err + p, _ := GetCurrentProcess() + fd := make([]int32, len(attr.Files)) + for i, _ := range attr.Files { + if attr.Files[i] > 0 { + err := DuplicateHandle(p, int32(attr.Files[i]), p, &fd[i], 0, true, DUPLICATE_SAME_ACCESS) + if err != 0 { + return 0, 0, err + } + defer CloseHandle(int32(fd[i])) } - defer CloseHandle(int32(startupInfo.StdInput)) } - if len(fd) > 1 && fd[1] > 0 { - err := DuplicateHandle(currentProc, int32(fd[1]), currentProc, &startupInfo.StdOutput, 0, true, DUPLICATE_SAME_ACCESS) - if err != 0 { - return 0, 0, err - } - defer CloseHandle(int32(startupInfo.StdOutput)) - } - if len(fd) > 2 && fd[2] > 0 { - err := DuplicateHandle(currentProc, int32(fd[2]), currentProc, &startupInfo.StdErr, 0, true, DUPLICATE_SAME_ACCESS) - if err != 0 { - return 0, 0, err - } - defer CloseHandle(int32(startupInfo.StdErr)) - } - if len(argv) == 0 { - argv = []string{""} - } - // argv0 must not be longer then 256 chars - // but the entire cmd line can have up to 32k chars (msdn) - err = CreateProcess( - nil, - StringToUTF16Ptr(escapeAddQuotes(argv0)+" "+stringJoin(argv[1:], " ", escapeAddQuotes)), - nil, //ptr to struct lpProcessAttributes - nil, //ptr to struct lpThreadAttributes - true, //bInheritHandles - CREATE_UNICODE_ENVIRONMENT, //Flags - createEnvBlock(attr.Env), //env block, NULL uses parent env - StringToUTF16Ptr(dir), - startupInfo, - processInfo) + si := new(StartupInfo) + GetStartupInfo(si) + si.Flags = STARTF_USESTDHANDLES + si.StdInput = fd[0] + si.StdOutput = fd[1] + si.StdErr = fd[2] - if err == 0 { - pid = int(processInfo.ProcessId) - handle = int(processInfo.Process) - CloseHandle(processInfo.Thread) + pi := new(ProcessInformation) + + err = CreateProcess(argv0p, argvp, nil, nil, true, CREATE_UNICODE_ENVIRONMENT, createEnvBlock(attr.Env), dirp, si, pi) + if err != 0 { + return 0, 0, err } - return + defer CloseHandle(pi.Thread) + + return int(pi.ProcessId), int(pi.Process), 0 } -// Ordinary exec. func Exec(argv0 string, argv []string, envv []string) (err int) { return EWINDOWS } diff --git a/src/pkg/syscall/syscall_windows.go b/src/pkg/syscall/syscall_windows.go index 394e0644218..4f8230003ca 100644 --- a/src/pkg/syscall/syscall_windows.go +++ b/src/pkg/syscall/syscall_windows.go @@ -135,7 +135,7 @@ func NewCallback(fn interface{}) uintptr //sys CreateIoCompletionPort(filehandle int32, cphandle int32, key uint32, threadcnt uint32) (handle int32, errno int) //sys GetQueuedCompletionStatus(cphandle int32, qty *uint32, key *uint32, overlapped **Overlapped, timeout uint32) (errno int) //sys CancelIo(s uint32) (errno int) -//sys CreateProcess(appName *int16, commandLine *uint16, procSecurity *int16, threadSecurity *int16, inheritHandles bool, creationFlags uint32, env *uint16, currentDir *uint16, startupInfo *StartupInfo, outProcInfo *ProcessInformation) (errno int) = CreateProcessW +//sys CreateProcess(appName *uint16, commandLine *uint16, procSecurity *SecurityAttributes, threadSecurity *SecurityAttributes, inheritHandles bool, creationFlags uint32, env *uint16, currentDir *uint16, startupInfo *StartupInfo, outProcInfo *ProcessInformation) (errno int) = CreateProcessW //sys OpenProcess(da uint32, inheritHandle bool, pid uint32) (handle uint32, errno int) //sys GetExitCodeProcess(handle uint32, exitcode *uint32) (errno int) //sys GetStartupInfo(startupInfo *StartupInfo) (errno int) = GetStartupInfoW @@ -160,6 +160,7 @@ func NewCallback(fn interface{}) uintptr //sys LocalFree(hmem uint32) (handle uint32, errno int) [failretval!=0] //sys SetHandleInformation(handle int32, mask uint32, flags uint32) (errno int) //sys FlushFileBuffers(handle int32) (errno int) +//sys GetFullPathName(path *uint16, buflen uint32, buf *uint16, fname **uint16) (n uint32, errno int) = kernel32.GetFullPathNameW // syscall interface implementation for other packages diff --git a/src/pkg/syscall/zsyscall_windows_386.go b/src/pkg/syscall/zsyscall_windows_386.go index 543992ea657..f4cfdeed8b1 100644 --- a/src/pkg/syscall/zsyscall_windows_386.go +++ b/src/pkg/syscall/zsyscall_windows_386.go @@ -1,4 +1,4 @@ -// mksyscall_windows.sh -l32 syscall_windows.go syscall_windows_386.go +// mksyscall_windows.pl -l32 syscall_windows.go syscall_windows_386.go // MACHINE GENERATED BY THE COMMAND ABOVE; DO NOT EDIT package syscall @@ -69,6 +69,7 @@ var ( procLocalFree = getSysProcAddr(modkernel32, "LocalFree") procSetHandleInformation = getSysProcAddr(modkernel32, "SetHandleInformation") procFlushFileBuffers = getSysProcAddr(modkernel32, "FlushFileBuffers") + procGetFullPathNameW = getSysProcAddr(modkernel32, "GetFullPathNameW") procWSAStartup = getSysProcAddr(modwsock32, "WSAStartup") procWSACleanup = getSysProcAddr(modwsock32, "WSACleanup") procsocket = getSysProcAddr(modwsock32, "socket") @@ -515,7 +516,7 @@ func CancelIo(s uint32) (errno int) { return } -func CreateProcess(appName *int16, commandLine *uint16, procSecurity *int16, threadSecurity *int16, inheritHandles bool, creationFlags uint32, env *uint16, currentDir *uint16, startupInfo *StartupInfo, outProcInfo *ProcessInformation) (errno int) { +func CreateProcess(appName *uint16, commandLine *uint16, procSecurity *SecurityAttributes, threadSecurity *SecurityAttributes, inheritHandles bool, creationFlags uint32, env *uint16, currentDir *uint16, startupInfo *StartupInfo, outProcInfo *ProcessInformation) (errno int) { var _p0 uint32 if inheritHandles { _p0 = 1 @@ -885,6 +886,21 @@ func FlushFileBuffers(handle int32) (errno int) { return } +func GetFullPathName(path *uint16, buflen uint32, buf *uint16, fname **uint16) (n uint32, errno int) { + r0, _, e1 := Syscall6(procGetFullPathNameW, 4, uintptr(unsafe.Pointer(path)), uintptr(buflen), uintptr(unsafe.Pointer(buf)), uintptr(unsafe.Pointer(fname)), 0, 0) + n = uint32(r0) + if n == 0 { + if e1 != 0 { + errno = int(e1) + } else { + errno = EINVAL + } + } else { + errno = 0 + } + return +} + func WSAStartup(verreq uint32, data *WSAData) (sockerrno int) { r0, _, _ := Syscall(procWSAStartup, 2, uintptr(verreq), uintptr(unsafe.Pointer(data)), 0) sockerrno = int(r0)