From 580600d366937a31ffe6cda9b0849c3fae486f9b Mon Sep 17 00:00:00 2001 From: photostorm Date: Mon, 25 Jul 2022 17:24:45 -0400 Subject: [PATCH 01/16] new windows support --- README.md | 4 +- cmd_windows.go | 391 +++++++++++++++++++++++++++++++++++++++++ doc.go | 34 +++- go.mod | 2 + go.sum | 2 + pty_unsupported.go | 4 +- pty_windows.go | 195 ++++++++++++++++++++ run.go | 45 +---- run_unix.go | 69 ++++++++ run_windows.go | 235 +++++++++++++++++++++++++ start.go | 25 --- start_windows.go | 19 -- test_crosscompile.sh | 4 +- winsize.go | 18 +- winsize_unix.go | 13 +- winsize_unsupported.go | 23 --- winsize_windows.go | 89 ++++++++++ 17 files changed, 1035 insertions(+), 137 deletions(-) create mode 100644 cmd_windows.go create mode 100644 go.sum create mode 100644 pty_windows.go create mode 100644 run_unix.go create mode 100644 run_windows.go delete mode 100644 start.go delete mode 100644 start_windows.go delete mode 100644 winsize_unsupported.go create mode 100644 winsize_windows.go diff --git a/README.md b/README.md index a4fe767..df4bcce 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # pty -Pty is a Go package for using unix pseudo-terminals. +Pty is a Go package for using unix pseudo-terminals and windows ConPty. ## Install @@ -12,6 +12,8 @@ go get github.com/creack/pty Note that those examples are for demonstration purpose only, to showcase how to use the library. They are not meant to be used in any kind of production environment. +__NOTE:__ This package requires `ConPty` support on windows platform, please make sure your windows system meet [these requirements](https://docs.microsoft.com/en-us/windows/console/createpseudoconsole#requirements) + ### Command ```go diff --git a/cmd_windows.go b/cmd_windows.go new file mode 100644 index 0000000..200e18a --- /dev/null +++ b/cmd_windows.go @@ -0,0 +1,391 @@ +//go:build windows +// +build windows + +package pty + +import ( + "bytes" + "errors" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + "syscall" + "unicode/utf16" + "unsafe" + + "golang.org/x/sys/windows" +) + +// copied from os/exec.Cmd for platform compatibility +// we need to use startupInfoEx for pty support, but os/exec.Cmd only have +// support for startupInfo on windows, so we have to rewrite some internal +// logic for windows while keep its behavior compatible with other platforms. + +// WindowExecCmd represents an external command being prepared or run. +// +// A cmd cannot be reused after calling its Run, Output or CombinedOutput +// methods. +type WindowExecCmd struct { + cmd *exec.Cmd + waitCalled bool + consoleHandle syscall.Handle + tty *WindowsTty + conPty *WindowsPty + attrList *windows.ProcThreadAttributeListContainer +} + +var errProcessNotStarted = errors.New("exec: process has not started yet") + +func (c *WindowExecCmd) close() error { + c.attrList.Delete() + _ = c.conPty.Close() + _ = c.tty.Close() + return nil +} + +func (c *WindowExecCmd) Run() error { + err := c.Start() + if err != nil { + return err + } + + return c.Wait() +} + +func (c *WindowExecCmd) Wait() error { + if c.cmd.Process == nil { + return errProcessNotStarted + } + + var err error + + if c.waitCalled { + return errors.New("exec: wait was already called") + } + + c.waitCalled = true + c.cmd.ProcessState, err = c.cmd.Process.Wait() + if err != nil { + return err + } + + err = c.close() + if err != nil { + return err + } + + if !c.cmd.ProcessState.Success() { + return &exec.ExitError{ProcessState: c.cmd.ProcessState} + } + + return nil +} + +func (c *WindowExecCmd) StdinPipe() (io.WriteCloser, error) { + if c.cmd.Stdin != nil { + return nil, errors.New("exec: Stdin already set") + } + if c.cmd.Process != nil { + return nil, errors.New("exec: StdinPipe after process started") + } + + if c.conPty != nil { + return c.conPty.InputPipe(), nil + } + + return nil, ErrUnsupported +} + +func (c *WindowExecCmd) StdoutPipe() (io.ReadCloser, error) { + if c.cmd.Stdout != nil { + return nil, errors.New("exec: Stdout already set") + } + if c.cmd.Process != nil { + return nil, errors.New("exec: StdoutPipe after process started") + } + + if c.conPty != nil { + return c.conPty.OutputPipe(), nil + } + + return nil, ErrUnsupported +} + +func (c *WindowExecCmd) StderrPipe() (io.ReadCloser, error) { + if c.cmd.Stderr != nil { + return nil, errors.New("exec: Stderr already set") + } + if c.cmd.Process != nil { + return nil, errors.New("exec: StderrPipe after process started") + } + + if c.conPty != nil { + return c.conPty.OutputPipe(), nil + } + + return nil, ErrUnsupported +} + +func (c *WindowExecCmd) Output() ([]byte, error) { + if c.cmd.Stdout != nil { + return nil, errors.New("exec: Stdout already set") + } + + var stdout bytes.Buffer + c.cmd.Stdout = &stdout + + err := c.Run() + return stdout.Bytes(), err +} + +func (c *WindowExecCmd) CombinedOutput() ([]byte, error) { + if c.cmd.Stdout != nil { + return nil, errors.New("exec: Stdout already set") + } + if c.cmd.Stderr != nil { + return nil, errors.New("exec: Stderr already set") + } + var b bytes.Buffer + c.cmd.Stdout = &b + c.cmd.Stderr = &b + err := c.Run() + return b.Bytes(), err +} + +func (c *WindowExecCmd) argv() []string { + if len(c.cmd.Args) > 0 { + return c.cmd.Args + } + + return []string{c.cmd.Path} +} + +// +// Helpers for working with Windows. These are exact copies of the same utilities found in the go stdlib. +// + +func lookExtensions(path, dir string) (string, error) { + if filepath.Base(path) == path { + path = filepath.Join(".", path) + } + + if dir == "" { + return exec.LookPath(path) + } + + if filepath.VolumeName(path) != "" { + return exec.LookPath(path) + } + + if len(path) > 1 && os.IsPathSeparator(path[0]) { + return exec.LookPath(path) + } + + dirandpath := filepath.Join(dir, path) + + // We assume that LookPath will only add file extension. + lp, err := exec.LookPath(dirandpath) + if err != nil { + return "", err + } + + ext := strings.TrimPrefix(lp, dirandpath) + + return path + ext, nil +} + +func dedupEnvCase(caseInsensitive bool, env []string) []string { + // Construct the output in reverse order, to preserve the + // last occurrence of each key. + out := make([]string, 0, len(env)) + saw := make(map[string]bool, len(env)) + for n := len(env); n > 0; n-- { + kv := env[n-1] + + i := strings.Index(kv, "=") + if i == 0 { + // We observe in practice keys with a single leading "=" on Windows. + // TODO(#49886): Should we consume only the first leading "=" as part + // of the key, or parse through arbitrarily many of them until a non-"="? + i = strings.Index(kv[1:], "=") + 1 + } + if i < 0 { + if kv != "" { + // The entry is not of the form "key=value" (as it is required to be). + // Leave it as-is for now. + // TODO(#52436): should we strip or reject these bogus entries? + out = append(out, kv) + } + continue + } + k := kv[:i] + if caseInsensitive { + k = strings.ToLower(k) + } + if saw[k] { + continue + } + + saw[k] = true + out = append(out, kv) + } + + // Now reverse the slice to restore the original order. + for i := 0; i < len(out)/2; i++ { + j := len(out) - i - 1 + out[i], out[j] = out[j], out[i] + } + + return out +} + +func addCriticalEnv(env []string) []string { + for _, kv := range env { + eq := strings.Index(kv, "=") + if eq < 0 { + continue + } + k := kv[:eq] + if strings.EqualFold(k, "SYSTEMROOT") { + // We already have it. + return env + } + } + + return append(env, "SYSTEMROOT="+os.Getenv("SYSTEMROOT")) +} + +func execEnvDefault(sys *syscall.SysProcAttr) (env []string, err error) { + if sys == nil || sys.Token == 0 { + return syscall.Environ(), nil + } + + var block *uint16 + err = windows.CreateEnvironmentBlock(&block, windows.Token(sys.Token), false) + if err != nil { + return nil, err + } + + defer windows.DestroyEnvironmentBlock(block) + blockp := uintptr(unsafe.Pointer(block)) + + for { + + // find NUL terminator + end := unsafe.Pointer(blockp) + for *(*uint16)(end) != 0 { + end = unsafe.Pointer(uintptr(end) + 2) + } + + n := (uintptr(end) - uintptr(unsafe.Pointer(blockp))) / 2 + if n == 0 { + // environment block ends with empty string + break + } + + entry := (*[(1 << 30) - 1]uint16)(unsafe.Pointer(blockp))[:n:n] + env = append(env, string(utf16.Decode(entry))) + blockp += 2 * (uintptr(len(entry)) + 1) + } + return +} + +func createEnvBlock(envv []string) *uint16 { + if len(envv) == 0 { + return &utf16.Encode([]rune("\x00\x00"))[0] + } + length := 0 + for _, s := range envv { + length += len(s) + 1 + } + length += 1 + + b := make([]byte, length) + i := 0 + for _, s := range envv { + l := len(s) + copy(b[i:i+l], []byte(s)) + copy(b[i+l:i+l+1], []byte{0}) + i = i + l + 1 + } + copy(b[i:i+1], []byte{0}) + + return &utf16.Encode([]rune(string(b)))[0] +} + +func makeCmdLine(args []string) string { + var s string + for _, v := range args { + if s != "" { + s += " " + } + s += windows.EscapeArg(v) + } + return s +} + +func isSlash(c uint8) bool { + return c == '\\' || c == '/' +} + +func normalizeDir(dir string) (name string, err error) { + ndir, err := syscall.FullPath(dir) + if err != nil { + return "", err + } + if len(ndir) > 2 && isSlash(ndir[0]) && isSlash(ndir[1]) { + // dir cannot have \\server\share\path form + return "", syscall.EINVAL + } + return ndir, nil +} + +func volToUpper(ch int) int { + if 'a' <= ch && ch <= 'z' { + ch += 'A' - 'a' + } + return ch +} + +func joinExeDirAndFName(dir, p string) (name string, err error) { + if len(p) == 0 { + return "", syscall.EINVAL + } + if len(p) > 2 && isSlash(p[0]) && isSlash(p[1]) { + // \\server\share\path form + return p, nil + } + if len(p) > 1 && p[1] == ':' { + // has drive letter + if len(p) == 2 { + return "", syscall.EINVAL + } + if isSlash(p[2]) { + return p, nil + } else { + d, err := normalizeDir(dir) + if err != nil { + return "", err + } + if volToUpper(int(p[0])) == volToUpper(int(d[0])) { + return syscall.FullPath(d + "\\" + p[2:]) + } else { + return syscall.FullPath(p) + } + } + } else { + // no drive letter + d, err := normalizeDir(dir) + if err != nil { + return "", err + } + + if isSlash(p[0]) { + return windows.FullPath(d[:2] + p) + } else { + return windows.FullPath(d + "\\" + p) + } + } +} diff --git a/doc.go b/doc.go index 3c8b324..033d43a 100644 --- a/doc.go +++ b/doc.go @@ -3,7 +3,7 @@ package pty import ( "errors" - "os" + "io" ) // ErrUnsupported is returned if a function is not @@ -11,6 +11,36 @@ import ( var ErrUnsupported = errors.New("unsupported") // Open a pty and its corresponding tty. -func Open() (pty, tty *os.File, err error) { +func Open() (Pty, Tty, error) { return open() } + +type FdHolder interface { + Fd() uintptr +} + +// Pty for terminal control in current process +// for unix systems, the real type is *os.File +// for windows, the real type is a *WindowsPty for ConPTY handle +type Pty interface { + // FdHolder Fd intended to resize Tty of child process in current process + FdHolder + + Name() string + + // WriteString is only used to identify Pty and Tty + WriteString(s string) (n int, err error) + io.ReadWriteCloser +} + +// Tty for data i/o in child process +// for unix systems, the real type is *os.File +// for windows, the real type is a *WindowsTty, which is a combination of two pipe file +type Tty interface { + // FdHolder Fd only intended for manual InheritSize from Pty + FdHolder + + Name() string + + io.ReadWriteCloser +} diff --git a/go.mod b/go.mod index 7731235..332c3a4 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,5 @@ module github.com/creack/pty go 1.13 + +require golang.org/x/sys v0.0.0-20220721230656-c6bc011c0c49 diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..ba6707d --- /dev/null +++ b/go.sum @@ -0,0 +1,2 @@ +golang.org/x/sys v0.0.0-20220721230656-c6bc011c0c49 h1:TMjZDarEwf621XDryfitp/8awEhiZNiwgphKlTMGRIg= +golang.org/x/sys v0.0.0-20220721230656-c6bc011c0c49/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/pty_unsupported.go b/pty_unsupported.go index c771020..c0ef327 100644 --- a/pty_unsupported.go +++ b/pty_unsupported.go @@ -1,5 +1,5 @@ -//go:build !linux && !darwin && !freebsd && !dragonfly && !netbsd && !openbsd && !solaris -// +build !linux,!darwin,!freebsd,!dragonfly,!netbsd,!openbsd,!solaris +//go:build !linux && !darwin && !freebsd && !dragonfly && !netbsd && !openbsd && !solaris && !windows +// +build !linux,!darwin,!freebsd,!dragonfly,!netbsd,!openbsd,!solaris,!windows package pty diff --git a/pty_windows.go b/pty_windows.go new file mode 100644 index 0000000..04fda46 --- /dev/null +++ b/pty_windows.go @@ -0,0 +1,195 @@ +//go:build windows +// +build windows + +package pty + +import ( + "fmt" + "os" + "syscall" + "unsafe" + + "golang.org/x/sys/windows" +) + +const ( + PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE = 0x20016 +) + +type WindowsPty struct { + handle windows.Handle + r, w *os.File +} + +type WindowsTty struct { + handle windows.Handle + r, w *os.File +} + +var ( + // NOTE(security): as noted by the comment of syscall.NewLazyDLL and syscall.LoadDLL + // user need to call internal/syscall/windows/sysdll.Add("kernel32.dll") to make sure + // the kernel32.dll is loaded from windows system path + // + // ref: https://pkg.go.dev/syscall@go1.13?GOOS=windows#LoadDLL + kernel32DLL = windows.NewLazyDLL("kernel32.dll") + + // https://docs.microsoft.com/en-us/windows/console/createpseudoconsole + createPseudoConsole = kernel32DLL.NewProc("CreatePseudoConsole") + closePseudoConsole = kernel32DLL.NewProc("ClosePseudoConsole") + + resizePseudoConsole = kernel32DLL.NewProc("ResizePseudoConsole") + getConsoleScreenBufferInfo = kernel32DLL.NewProc("GetConsoleScreenBufferInfo") +) + +func open() (_ Pty, _ Tty, err error) { + pr, consoleW, err := os.Pipe() + if err != nil { + return nil, nil, err + } + + consoleR, pw, err := os.Pipe() + if err != nil { + _ = consoleW.Close() + _ = pr.Close() + return nil, nil, err + } + + var consoleHandle windows.Handle + + err = procCreatePseudoConsole(windows.Handle(consoleR.Fd()), windows.Handle(consoleW.Fd()), + 0, &consoleHandle) + if err != nil { + _ = consoleW.Close() + _ = pr.Close() + _ = pw.Close() + _ = consoleR.Close() + return nil, nil, err + } + + // These pipes can be closed here without any worry + err = consoleW.Close() + if err != nil { + return nil, nil, fmt.Errorf("failed to close pseudo console handle: %w", err) + } + + err = consoleR.Close() + if err != nil { + return nil, nil, fmt.Errorf("failed to close pseudo console handle: %w", err) + } + + return &WindowsPty{ + handle: consoleHandle, + r: pr, + w: pw, + }, &WindowsTty{ + handle: consoleHandle, + r: consoleR, + w: consoleW, + }, nil +} + +func (p *WindowsPty) Name() string { + return p.r.Name() +} + +func (p *WindowsPty) Fd() uintptr { + return uintptr(p.handle) +} + +func (p *WindowsPty) Read(data []byte) (int, error) { + return p.r.Read(data) +} + +func (p *WindowsPty) Write(data []byte) (int, error) { + return p.w.Write(data) +} + +func (p *WindowsPty) WriteString(s string) (int, error) { + return p.w.WriteString(s) +} + +func (p *WindowsPty) InputPipe() *os.File { + return p.w +} + +func (p *WindowsPty) OutputPipe() *os.File { + return p.r +} + +func (p *WindowsPty) UpdateProcThreadAttribute(attrList *windows.ProcThreadAttributeListContainer) error { + var err error + + if err = attrList.Update( + PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, + unsafe.Pointer(p.handle), + unsafe.Sizeof(p.handle), + ); err != nil { + return fmt.Errorf("failed to update proc thread attributes for pseudo console: %w", err) + } + + return nil +} + +func (p *WindowsPty) Close() error { + _ = p.r.Close() + _ = p.w.Close() + + err := closePseudoConsole.Find() + if err != nil { + return err + } + + _, _, err = closePseudoConsole.Call(uintptr(p.handle)) + return err +} + +func (t *WindowsTty) Name() string { + return t.r.Name() +} + +func (t *WindowsTty) Fd() uintptr { + return uintptr(t.handle) +} + +func (t *WindowsTty) Read(p []byte) (int, error) { + return t.r.Read(p) +} + +func (t *WindowsTty) Write(p []byte) (int, error) { + return t.w.Write(p) +} + +func (t *WindowsTty) Close() error { + _ = t.r.Close() + return t.w.Close() +} + +func procCreatePseudoConsole(hInput windows.Handle, hOutput windows.Handle, dwFlags uint32, consoleHandle *windows.Handle) error { + var r0 uintptr + var err error + + err = createPseudoConsole.Find() + if err != nil { + return err + } + + r0, _, err = createPseudoConsole.Call( + (windowsCoord{X: 80, Y: 30}).Pack(), // size: default 80x30 window + uintptr(hInput), // console input + uintptr(hOutput), // console output + uintptr(dwFlags), // console flags, currently only PSEUDOCONSOLE_INHERIT_CURSOR supported + uintptr(unsafe.Pointer(consoleHandle)), // console handler value return + ) + + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + + // S_OK: 0 + return syscall.Errno(r0) + } + + return nil +} diff --git a/run.go b/run.go index 4755366..0b97b94 100644 --- a/run.go +++ b/run.go @@ -1,9 +1,7 @@ package pty import ( - "os" "os/exec" - "syscall" ) // Start assigns a pseudo-terminal tty os.File to c.Stdin, c.Stdout, @@ -11,47 +9,6 @@ import ( // corresponding pty. // // Starts the process in a new session and sets the controlling terminal. -func Start(cmd *exec.Cmd) (*os.File, error) { +func Start(cmd *exec.Cmd) (Pty, error) { return StartWithSize(cmd, nil) } - -// StartWithAttrs assigns a pseudo-terminal tty os.File to c.Stdin, c.Stdout, -// and c.Stderr, calls c.Start, and returns the File of the tty's -// corresponding pty. -// -// This will resize the pty to the specified size before starting the command if a size is provided. -// The `attrs` parameter overrides the one set in c.SysProcAttr. -// -// This should generally not be needed. Used in some edge cases where it is needed to create a pty -// without a controlling terminal. -func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (*os.File, error) { - pty, tty, err := Open() - if err != nil { - return nil, err - } - defer func() { _ = tty.Close() }() // Best effort. - - if sz != nil { - if err := Setsize(pty, sz); err != nil { - _ = pty.Close() // Best effort. - return nil, err - } - } - if c.Stdout == nil { - c.Stdout = tty - } - if c.Stderr == nil { - c.Stderr = tty - } - if c.Stdin == nil { - c.Stdin = tty - } - - c.SysProcAttr = attrs - - if err := c.Start(); err != nil { - _ = pty.Close() // Best effort. - return nil, err - } - return pty, err -} diff --git a/run_unix.go b/run_unix.go new file mode 100644 index 0000000..6d43291 --- /dev/null +++ b/run_unix.go @@ -0,0 +1,69 @@ +//go:build !windows +// +build !windows + +package pty + +import ( + "os/exec" + "syscall" +) + +// StartWithSize assigns a pseudo-terminal Tty to c.Stdin, c.Stdout, +// and c.Stderr, calls c.Start, and returns the File of the tty's +// corresponding Pty. +// +// This will resize the Pty to the specified size before starting the command. +// Starts the process in a new session and sets the controlling terminal. +func StartWithSize(c *exec.Cmd, sz *Winsize) (Pty, error) { + if c.SysProcAttr == nil { + c.SysProcAttr = &syscall.SysProcAttr{} + } + c.SysProcAttr.Setsid = true + c.SysProcAttr.Setctty = true + return StartWithAttrs(c, sz, c.SysProcAttr) +} + +// StartWithAttrs assigns a pseudo-terminal Tty to c.Stdin, c.Stdout, +// and c.Stderr, calls c.Start, and returns the File of the tty's +// corresponding Pty. +// +// This will resize the Pty to the specified size before starting the command if a size is provided. +// The `attrs` parameter overrides the one set in c.SysProcAttr. +// +// This should generally not be needed. Used in some edge cases where it is needed to create a pty +// without a controlling terminal. +func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (Pty, error) { + pty, tty, err := open() + if err != nil { + return nil, err + } + defer func() { + // always close tty fds since it's being used in another process + // but pty is kept to resize tty + _ = tty.Close() + }() + + if sz != nil { + if err := Setsize(pty, sz); err != nil { + _ = pty.Close() + return nil, err + } + } + if c.Stdout == nil { + c.Stdout = tty + } + if c.Stderr == nil { + c.Stderr = tty + } + if c.Stdin == nil { + c.Stdin = tty + } + + c.SysProcAttr = attrs + + if err := c.Start(); err != nil { + _ = pty.Close() + return nil, err + } + return pty, err +} diff --git a/run_windows.go b/run_windows.go new file mode 100644 index 0000000..e4da40d --- /dev/null +++ b/run_windows.go @@ -0,0 +1,235 @@ +//go:build windows +// +build windows + +package pty + +import ( + "errors" + "fmt" + "os" + "os/exec" + "syscall" + "unsafe" + + "golang.org/x/sys/windows" +) + +// StartWithSize assigns a pseudo-terminal Tty to c.Stdin, c.Stdout, +// and c.Stderr, calls c.Start, and returns the File of the tty's +// corresponding Pty. +// +// This will resize the Pty to the specified size before starting the command. +// Starts the process in a new session and sets the controlling terminal. +func StartWithSize(c *exec.Cmd, sz *Winsize) (Pty, error) { + return StartWithAttrs(c, sz, c.SysProcAttr) +} + +// StartWithAttrs assigns a pseudo-terminal Tty to c.Stdin, c.Stdout, +// and c.Stderr, calls c.Start, and returns the File of the tty's +// corresponding Pty. +// +// This will resize the Pty to the specified size before starting the command if a size is provided. +// The `attrs` parameter overrides the one set in c.SysProcAttr. +// +// This should generally not be needed. Used in some edge cases where it is needed to create a pty +// without a controlling terminal. +func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (_ Pty, err error) { + pty, tty, err := open() + if err != nil { + return nil, err + } + + defer func() { + // unlike unix command exec, do not close tty unless error happened + if err != nil { + _ = tty.Close() + _ = pty.Close() + } + }() + + if sz != nil { + if err = Setsize(pty, sz); err != nil { + return nil, err + } + } + + // unlike unix command exec, do not set stdin/stdout/stderr + + c.SysProcAttr = attrs + + // do not use os/exec.Start since we need to append console handler to startup info + + w := WindowExecCmd{ + cmd: c, + waitCalled: false, + consoleHandle: syscall.Handle(tty.Fd()), + tty: tty.(*WindowsTty), + conPty: pty.(*WindowsPty), + } + + err = w.Start() + if err != nil { + _ = tty.Close() + _ = pty.Close() + return nil, err + } + + return pty, err +} + +// Start the specified command but does not wait for it to complete. +// +// If Start returns successfully, the c.Process field will be set. +// +// The Wait method will return the exit code and release associated resources +// once the command exits. +func (c *WindowExecCmd) Start() error { + if c.cmd.Process != nil { + return errors.New("exec: already started") + } + + var argv0 = c.cmd.Path + var argv0p *uint16 + var argvp *uint16 + var dirp *uint16 + var err error + + sys := c.cmd.SysProcAttr + if sys == nil { + sys = &syscall.SysProcAttr{} + } + + if c.cmd.Env == nil { + c.cmd.Env, err = execEnvDefault(sys) + if err != nil { + return err + } + } + + var lp string + + lp, err = lookExtensions(c.cmd.Path, c.cmd.Dir) + if err != nil { + return err + } + + c.cmd.Path = lp + + if len(c.cmd.Dir) != 0 { + // Windows CreateProcess 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. + + argv0, err = joinExeDirAndFName(c.cmd.Dir, c.cmd.Path) + if err != nil { + return err + } + } + + argv0p, err = syscall.UTF16PtrFromString(argv0) + if err != nil { + return err + } + + 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 + if sys.CmdLine != "" { + cmdline = sys.CmdLine + } else { + cmdline = makeCmdLine(c.argv()) + } + + if len(cmdline) != 0 { + argvp, err = windows.UTF16PtrFromString(cmdline) + if err != nil { + return err + } + } + + if len(c.cmd.Dir) != 0 { + dirp, err = windows.UTF16PtrFromString(c.cmd.Dir) + if err != nil { + return err + } + } + + // Acquire the fork lock so that no other threads + // create new fds that are not yet close-on-exec + // before we fork. + syscall.ForkLock.Lock() + defer syscall.ForkLock.Unlock() + + siEx := new(windows.StartupInfoEx) + siEx.Flags = windows.STARTF_USESTDHANDLES + + if sys.HideWindow { + siEx.Flags |= syscall.STARTF_USESHOWWINDOW + siEx.ShowWindow = syscall.SW_HIDE + } + + pi := new(windows.ProcessInformation) + + // Need EXTENDED_STARTUPINFO_PRESENT as we're making use of the attribute list field. + flags := sys.CreationFlags | uint32(windows.CREATE_UNICODE_ENVIRONMENT) | windows.EXTENDED_STARTUPINFO_PRESENT + + c.attrList, err = windows.NewProcThreadAttributeList(3) + if err != nil { + return fmt.Errorf("failed to initialize process thread attribute list: %w", err) + } + + if c.conPty != nil { + if err = c.conPty.UpdateProcThreadAttribute(c.attrList); err != nil { + return err + } + } + + siEx.ProcThreadAttributeList = c.attrList.List() + siEx.Cb = uint32(unsafe.Sizeof(*siEx)) + + if sys.Token != 0 { + err = windows.CreateProcessAsUser( + windows.Token(sys.Token), + argv0p, + argvp, + nil, + nil, + false, + flags, + createEnvBlock(addCriticalEnv(dedupEnvCase(true, c.cmd.Env))), + dirp, + &siEx.StartupInfo, + pi, + ) + } else { + err = windows.CreateProcess( + argv0p, + argvp, + nil, + nil, + false, + flags, + createEnvBlock(addCriticalEnv(dedupEnvCase(true, c.cmd.Env))), + dirp, + &siEx.StartupInfo, + pi, + ) + } + if err != nil { + return err + } + + defer func() { + _ = windows.CloseHandle(pi.Thread) + }() + + c.cmd.Process, err = os.FindProcess(int(pi.ProcessId)) + if err != nil { + return err + } + + return nil +} diff --git a/start.go b/start.go deleted file mode 100644 index 9b51635..0000000 --- a/start.go +++ /dev/null @@ -1,25 +0,0 @@ -//go:build !windows -// +build !windows - -package pty - -import ( - "os" - "os/exec" - "syscall" -) - -// StartWithSize assigns a pseudo-terminal tty os.File to c.Stdin, c.Stdout, -// and c.Stderr, calls c.Start, and returns the File of the tty's -// corresponding pty. -// -// This will resize the pty to the specified size before starting the command. -// Starts the process in a new session and sets the controlling terminal. -func StartWithSize(cmd *exec.Cmd, ws *Winsize) (*os.File, error) { - if cmd.SysProcAttr == nil { - cmd.SysProcAttr = &syscall.SysProcAttr{} - } - cmd.SysProcAttr.Setsid = true - cmd.SysProcAttr.Setctty = true - return StartWithAttrs(cmd, ws, cmd.SysProcAttr) -} diff --git a/start_windows.go b/start_windows.go deleted file mode 100644 index 7e9530b..0000000 --- a/start_windows.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build windows -// +build windows - -package pty - -import ( - "os" - "os/exec" -) - -// StartWithSize assigns a pseudo-terminal tty os.File to c.Stdin, c.Stdout, -// and c.Stderr, calls c.Start, and returns the File of the tty's -// corresponding pty. -// -// This will resize the pty to the specified size before starting the command. -// Starts the process in a new session and sets the controlling terminal. -func StartWithSize(cmd *exec.Cmd, ws *Winsize) (*os.File, error) { - return nil, ErrUnsupported -} diff --git a/test_crosscompile.sh b/test_crosscompile.sh index 47e8b10..5bf80f9 100755 --- a/test_crosscompile.sh +++ b/test_crosscompile.sh @@ -32,9 +32,7 @@ cross netbsd amd64 386 arm arm64 cross openbsd amd64 386 arm arm64 cross dragonfly amd64 cross solaris amd64 - -# Not expected to work but should still compile. -cross windows amd64 386 arm +cross windows amd64 386 arm # TODO: Fix compilation error on openbsd/arm. # TODO: Merge the solaris PR. diff --git a/winsize.go b/winsize.go index 57323f4..7d3d1fc 100644 --- a/winsize.go +++ b/winsize.go @@ -1,15 +1,22 @@ package pty -import "os" +// Winsize describes the terminal size. +type Winsize struct { + Rows uint16 // ws_row: Number of rows (in cells) + Cols uint16 // ws_col: Number of columns (in cells) + X uint16 // ws_xpixel: Width in pixels + Y uint16 // ws_ypixel: Height in pixels +} // InheritSize applies the terminal size of pty to tty. This should be run // in a signal handler for syscall.SIGWINCH to automatically resize the tty when // the pty receives a window size change notification. -func InheritSize(pty, tty *os.File) error { +func InheritSize(pty Pty, tty Tty) error { size, err := GetsizeFull(pty) if err != nil { return err } + if err := Setsize(tty, size); err != nil { return err } @@ -18,10 +25,7 @@ func InheritSize(pty, tty *os.File) error { // Getsize returns the number of rows (lines) and cols (positions // in each line) in terminal t. -func Getsize(t *os.File) (rows, cols int, err error) { +func Getsize(t FdHolder) (rows, cols int, err error) { ws, err := GetsizeFull(t) - if err != nil { - return 0, 0, err - } - return int(ws.Rows), int(ws.Cols), nil + return int(ws.Rows), int(ws.Cols), err } diff --git a/winsize_unix.go b/winsize_unix.go index 5d99c3d..d3e7d15 100644 --- a/winsize_unix.go +++ b/winsize_unix.go @@ -4,27 +4,18 @@ package pty import ( - "os" "syscall" "unsafe" ) -// Winsize describes the terminal size. -type Winsize struct { - Rows uint16 // ws_row: Number of rows (in cells) - Cols uint16 // ws_col: Number of columns (in cells) - X uint16 // ws_xpixel: Width in pixels - Y uint16 // ws_ypixel: Height in pixels -} - // Setsize resizes t to s. -func Setsize(t *os.File, ws *Winsize) error { +func Setsize(t FdHolder, ws *Winsize) error { //nolint:gosec // Expected unsafe pointer for Syscall call. return ioctl(t.Fd(), syscall.TIOCSWINSZ, uintptr(unsafe.Pointer(ws))) } // GetsizeFull returns the full terminal size description. -func GetsizeFull(t *os.File) (size *Winsize, err error) { +func GetsizeFull(t FdHolder) (size *Winsize, err error) { var ws Winsize //nolint:gosec // Expected unsafe pointer for Syscall call. diff --git a/winsize_unsupported.go b/winsize_unsupported.go deleted file mode 100644 index 0d21099..0000000 --- a/winsize_unsupported.go +++ /dev/null @@ -1,23 +0,0 @@ -//go:build windows -// +build windows - -package pty - -import ( - "os" -) - -// Winsize is a dummy struct to enable compilation on unsupported platforms. -type Winsize struct { - Rows, Cols, X, Y uint16 -} - -// Setsize resizes t to s. -func Setsize(*os.File, *Winsize) error { - return ErrUnsupported -} - -// GetsizeFull returns the full terminal size description. -func GetsizeFull(*os.File) (*Winsize, error) { - return nil, ErrUnsupported -} diff --git a/winsize_windows.go b/winsize_windows.go new file mode 100644 index 0000000..5b96004 --- /dev/null +++ b/winsize_windows.go @@ -0,0 +1,89 @@ +//go:build windows +// +build windows + +package pty + +import ( + "syscall" + "unsafe" +) + +// types from golang.org/x/sys/windows +// copy of https://pkg.go.dev/golang.org/x/sys/windows#Coord +type windowsCoord struct { + X int16 + Y int16 +} + +// copy of https://pkg.go.dev/golang.org/x/sys/windows#SmallRect +type windowsSmallRect struct { + Left int16 + Top int16 + Right int16 + Bottom int16 +} + +// copy of https://pkg.go.dev/golang.org/x/sys/windows#ConsoleScreenBufferInfo +type windowsConsoleScreenBufferInfo struct { + Size windowsCoord + CursorPosition windowsCoord + Attributes uint16 + Window windowsSmallRect + MaximumWindowSize windowsCoord +} + +func (c windowsCoord) Pack() uintptr { + return uintptr((int32(c.Y) << 16) | int32(c.X)) +} + +// Setsize resizes t to ws. +func Setsize(t FdHolder, ws *Winsize) error { + var r0 uintptr + var err error + + err = resizePseudoConsole.Find() + if err != nil { + return err + } + + r0, _, err = resizePseudoConsole.Call( + t.Fd(), + (windowsCoord{X: int16(ws.Cols), Y: int16(ws.Rows)}).Pack(), + ) + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + + // S_OK: 0 + return syscall.Errno(r0) + } + + return nil +} + +// GetsizeFull returns the full terminal size description. +func GetsizeFull(t FdHolder) (size *Winsize, err error) { + err = getConsoleScreenBufferInfo.Find() + if err != nil { + return nil, err + } + + var info windowsConsoleScreenBufferInfo + var r0 uintptr + + r0, _, err = getConsoleScreenBufferInfo.Call(t.Fd(), uintptr(unsafe.Pointer(&info))) + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + + // S_OK: 0 + return nil, syscall.Errno(r0) + } + + return &Winsize{ + Rows: uint16(info.Window.Bottom - info.Window.Top + 1), + Cols: uint16(info.Window.Right - info.Window.Left + 1), + }, nil +} From d44d7f77e385f0bd1557aed33da25b93a491c918 Mon Sep 17 00:00:00 2001 From: photostorm Date: Mon, 25 Jul 2022 17:28:52 -0400 Subject: [PATCH 02/16] Update README.md --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index df4bcce..a6cb5aa 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,9 @@ + # pty +[![Crosscompile](https://github.com/photostorm/pty/actions/workflows/crosscompile.yml/badge.svg)](https://github.com/photostorm/pty/actions/workflows/crosscompile.yml) +[![Test](https://github.com/photostorm/pty/actions/workflows/test.yml/badge.svg)](https://github.com/photostorm/pty/actions/workflows/test.yml) + Pty is a Go package for using unix pseudo-terminals and windows ConPty. ## Install From 2aa609e6e2086afec572193405d26ab1033e960f Mon Sep 17 00:00:00 2001 From: photostorm Date: Mon, 25 Jul 2022 17:38:18 -0400 Subject: [PATCH 03/16] temporary module path --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 332c3a4..46b43e6 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/creack/pty +module github.com/photostorm/pty go 1.13 From d193ef77c7d748f734eb49cc14a174254ef687e3 Mon Sep 17 00:00:00 2001 From: photostorm Date: Mon, 25 Jul 2022 18:23:54 -0400 Subject: [PATCH 04/16] Revert "temporary module path" This reverts commit 2aa609e6e2086afec572193405d26ab1033e960f. --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 46b43e6..332c3a4 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/photostorm/pty +module github.com/creack/pty go 1.13 From f40884c37bb354d724e2479d62056251ecfa1b84 Mon Sep 17 00:00:00 2001 From: photostorm Date: Mon, 25 Jul 2022 23:44:28 -0400 Subject: [PATCH 05/16] made exec cmd struct private --- cmd_windows.go | 55 ++++++++++---------------------------------------- run_windows.go | 4 ++-- 2 files changed, 13 insertions(+), 46 deletions(-) diff --git a/cmd_windows.go b/cmd_windows.go index 200e18a..fc30d2f 100644 --- a/cmd_windows.go +++ b/cmd_windows.go @@ -23,11 +23,11 @@ import ( // support for startupInfo on windows, so we have to rewrite some internal // logic for windows while keep its behavior compatible with other platforms. -// WindowExecCmd represents an external command being prepared or run. +// windowExecCmd represents an external command being prepared or run. // // A cmd cannot be reused after calling its Run, Output or CombinedOutput // methods. -type WindowExecCmd struct { +type windowExecCmd struct { cmd *exec.Cmd waitCalled bool consoleHandle syscall.Handle @@ -38,14 +38,14 @@ type WindowExecCmd struct { var errProcessNotStarted = errors.New("exec: process has not started yet") -func (c *WindowExecCmd) close() error { +func (c *windowExecCmd) close() error { c.attrList.Delete() _ = c.conPty.Close() _ = c.tty.Close() return nil } -func (c *WindowExecCmd) Run() error { +func (c *windowExecCmd) Run() error { err := c.Start() if err != nil { return err @@ -54,7 +54,7 @@ func (c *WindowExecCmd) Run() error { return c.Wait() } -func (c *WindowExecCmd) Wait() error { +func (c *windowExecCmd) Wait() error { if c.cmd.Process == nil { return errProcessNotStarted } @@ -83,52 +83,19 @@ func (c *WindowExecCmd) Wait() error { return nil } -func (c *WindowExecCmd) StdinPipe() (io.WriteCloser, error) { - if c.cmd.Stdin != nil { - return nil, errors.New("exec: Stdin already set") - } - if c.cmd.Process != nil { - return nil, errors.New("exec: StdinPipe after process started") - } - - if c.conPty != nil { - return c.conPty.InputPipe(), nil - } - +func (c *windowExecCmd) StdinPipe() (io.WriteCloser, error) { return nil, ErrUnsupported } -func (c *WindowExecCmd) StdoutPipe() (io.ReadCloser, error) { - if c.cmd.Stdout != nil { - return nil, errors.New("exec: Stdout already set") - } - if c.cmd.Process != nil { - return nil, errors.New("exec: StdoutPipe after process started") - } - - if c.conPty != nil { - return c.conPty.OutputPipe(), nil - } - +func (c *windowExecCmd) StdoutPipe() (io.ReadCloser, error) { return nil, ErrUnsupported } -func (c *WindowExecCmd) StderrPipe() (io.ReadCloser, error) { - if c.cmd.Stderr != nil { - return nil, errors.New("exec: Stderr already set") - } - if c.cmd.Process != nil { - return nil, errors.New("exec: StderrPipe after process started") - } - - if c.conPty != nil { - return c.conPty.OutputPipe(), nil - } - +func (c *windowExecCmd) StderrPipe() (io.ReadCloser, error) { return nil, ErrUnsupported } -func (c *WindowExecCmd) Output() ([]byte, error) { +func (c *windowExecCmd) Output() ([]byte, error) { if c.cmd.Stdout != nil { return nil, errors.New("exec: Stdout already set") } @@ -140,7 +107,7 @@ func (c *WindowExecCmd) Output() ([]byte, error) { return stdout.Bytes(), err } -func (c *WindowExecCmd) CombinedOutput() ([]byte, error) { +func (c *windowExecCmd) CombinedOutput() ([]byte, error) { if c.cmd.Stdout != nil { return nil, errors.New("exec: Stdout already set") } @@ -154,7 +121,7 @@ func (c *WindowExecCmd) CombinedOutput() ([]byte, error) { return b.Bytes(), err } -func (c *WindowExecCmd) argv() []string { +func (c *windowExecCmd) argv() []string { if len(c.cmd.Args) > 0 { return c.cmd.Args } diff --git a/run_windows.go b/run_windows.go index e4da40d..a22f258 100644 --- a/run_windows.go +++ b/run_windows.go @@ -59,7 +59,7 @@ func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (_ Pty // do not use os/exec.Start since we need to append console handler to startup info - w := WindowExecCmd{ + w := windowExecCmd{ cmd: c, waitCalled: false, consoleHandle: syscall.Handle(tty.Fd()), @@ -83,7 +83,7 @@ func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (_ Pty // // The Wait method will return the exit code and release associated resources // once the command exits. -func (c *WindowExecCmd) Start() error { +func (c *windowExecCmd) Start() error { if c.cmd.Process != nil { return errors.New("exec: already started") } From 82ad6226c1ca68fa4b30ad91d331b68563686360 Mon Sep 17 00:00:00 2001 From: photostorm Date: Tue, 26 Jul 2022 13:03:55 -0400 Subject: [PATCH 06/16] Revert "Update README.md" This reverts commit d44d7f77e385f0bd1557aed33da25b93a491c918. --- README.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/README.md b/README.md index a6cb5aa..df4bcce 100644 --- a/README.md +++ b/README.md @@ -1,9 +1,5 @@ - # pty -[![Crosscompile](https://github.com/photostorm/pty/actions/workflows/crosscompile.yml/badge.svg)](https://github.com/photostorm/pty/actions/workflows/crosscompile.yml) -[![Test](https://github.com/photostorm/pty/actions/workflows/test.yml/badge.svg)](https://github.com/photostorm/pty/actions/workflows/test.yml) - Pty is a Go package for using unix pseudo-terminals and windows ConPty. ## Install From 0a71ca4f0f8cbd4bbf38919907c896e5209adbb8 Mon Sep 17 00:00:00 2001 From: photostorm Date: Tue, 25 Oct 2022 21:23:44 -0400 Subject: [PATCH 07/16] cleanup some unused fields --- cmd_windows.go | 12 +++++------- pty_windows.go | 8 -------- run_windows.go | 16 ++++++---------- 3 files changed, 11 insertions(+), 25 deletions(-) diff --git a/cmd_windows.go b/cmd_windows.go index fc30d2f..d36924f 100644 --- a/cmd_windows.go +++ b/cmd_windows.go @@ -28,12 +28,10 @@ import ( // A cmd cannot be reused after calling its Run, Output or CombinedOutput // methods. type windowExecCmd struct { - cmd *exec.Cmd - waitCalled bool - consoleHandle syscall.Handle - tty *WindowsTty - conPty *WindowsPty - attrList *windows.ProcThreadAttributeListContainer + cmd *exec.Cmd + waitCalled bool + conPty *WindowsPty + attrList *windows.ProcThreadAttributeListContainer } var errProcessNotStarted = errors.New("exec: process has not started yet") @@ -41,7 +39,7 @@ var errProcessNotStarted = errors.New("exec: process has not started yet") func (c *windowExecCmd) close() error { c.attrList.Delete() _ = c.conPty.Close() - _ = c.tty.Close() + return nil } diff --git a/pty_windows.go b/pty_windows.go index 04fda46..aa8eb0b 100644 --- a/pty_windows.go +++ b/pty_windows.go @@ -109,14 +109,6 @@ func (p *WindowsPty) WriteString(s string) (int, error) { return p.w.WriteString(s) } -func (p *WindowsPty) InputPipe() *os.File { - return p.w -} - -func (p *WindowsPty) OutputPipe() *os.File { - return p.r -} - func (p *WindowsPty) UpdateProcThreadAttribute(attrList *windows.ProcThreadAttributeListContainer) error { var err error diff --git a/run_windows.go b/run_windows.go index a22f258..37474fb 100644 --- a/run_windows.go +++ b/run_windows.go @@ -33,8 +33,8 @@ func StartWithSize(c *exec.Cmd, sz *Winsize) (Pty, error) { // // This should generally not be needed. Used in some edge cases where it is needed to create a pty // without a controlling terminal. -func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (_ Pty, err error) { - pty, tty, err := open() +func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (Pty, error) { + pty, _, err := open() if err != nil { return nil, err } @@ -42,7 +42,6 @@ func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (_ Pty defer func() { // unlike unix command exec, do not close tty unless error happened if err != nil { - _ = tty.Close() _ = pty.Close() } }() @@ -60,16 +59,13 @@ func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (_ Pty // do not use os/exec.Start since we need to append console handler to startup info w := windowExecCmd{ - cmd: c, - waitCalled: false, - consoleHandle: syscall.Handle(tty.Fd()), - tty: tty.(*WindowsTty), - conPty: pty.(*WindowsPty), + cmd: c, + waitCalled: false, + conPty: pty.(*WindowsPty), } err = w.Start() if err != nil { - _ = tty.Close() _ = pty.Close() return nil, err } @@ -176,7 +172,7 @@ func (c *windowExecCmd) Start() error { // Need EXTENDED_STARTUPINFO_PRESENT as we're making use of the attribute list field. flags := sys.CreationFlags | uint32(windows.CREATE_UNICODE_ENVIRONMENT) | windows.EXTENDED_STARTUPINFO_PRESENT - c.attrList, err = windows.NewProcThreadAttributeList(3) + c.attrList, err = windows.NewProcThreadAttributeList(1) if err != nil { return fmt.Errorf("failed to initialize process thread attribute list: %w", err) } From db6cfc0728c8449f6c27c707e3b9c382890af7d2 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Thu, 23 Mar 2023 13:25:45 -0700 Subject: [PATCH 08/16] Don't close a pty that was never started as it causes a panic --- run_windows.go | 1 - 1 file changed, 1 deletion(-) diff --git a/run_windows.go b/run_windows.go index 37474fb..8b2a511 100644 --- a/run_windows.go +++ b/run_windows.go @@ -66,7 +66,6 @@ func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (Pty, err = w.Start() if err != nil { - _ = pty.Close() return nil, err } From 31354506054b161701ca5acfd543be211b35eb6e Mon Sep 17 00:00:00 2001 From: photostorm Date: Sun, 3 Sep 2023 14:24:54 -0400 Subject: [PATCH 09/16] cleanup some used code, added code to monitor the spawned process --- cmd_windows.go | 81 -------------------------------------------------- pty_windows.go | 1 + run_windows.go | 7 +++++ 3 files changed, 8 insertions(+), 81 deletions(-) diff --git a/cmd_windows.go b/cmd_windows.go index d36924f..d74dbf3 100644 --- a/cmd_windows.go +++ b/cmd_windows.go @@ -4,9 +4,6 @@ package pty import ( - "bytes" - "errors" - "io" "os" "os/exec" "path/filepath" @@ -34,8 +31,6 @@ type windowExecCmd struct { attrList *windows.ProcThreadAttributeListContainer } -var errProcessNotStarted = errors.New("exec: process has not started yet") - func (c *windowExecCmd) close() error { c.attrList.Delete() _ = c.conPty.Close() @@ -43,82 +38,6 @@ func (c *windowExecCmd) close() error { return nil } -func (c *windowExecCmd) Run() error { - err := c.Start() - if err != nil { - return err - } - - return c.Wait() -} - -func (c *windowExecCmd) Wait() error { - if c.cmd.Process == nil { - return errProcessNotStarted - } - - var err error - - if c.waitCalled { - return errors.New("exec: wait was already called") - } - - c.waitCalled = true - c.cmd.ProcessState, err = c.cmd.Process.Wait() - if err != nil { - return err - } - - err = c.close() - if err != nil { - return err - } - - if !c.cmd.ProcessState.Success() { - return &exec.ExitError{ProcessState: c.cmd.ProcessState} - } - - return nil -} - -func (c *windowExecCmd) StdinPipe() (io.WriteCloser, error) { - return nil, ErrUnsupported -} - -func (c *windowExecCmd) StdoutPipe() (io.ReadCloser, error) { - return nil, ErrUnsupported -} - -func (c *windowExecCmd) StderrPipe() (io.ReadCloser, error) { - return nil, ErrUnsupported -} - -func (c *windowExecCmd) Output() ([]byte, error) { - if c.cmd.Stdout != nil { - return nil, errors.New("exec: Stdout already set") - } - - var stdout bytes.Buffer - c.cmd.Stdout = &stdout - - err := c.Run() - return stdout.Bytes(), err -} - -func (c *windowExecCmd) CombinedOutput() ([]byte, error) { - if c.cmd.Stdout != nil { - return nil, errors.New("exec: Stdout already set") - } - if c.cmd.Stderr != nil { - return nil, errors.New("exec: Stderr already set") - } - var b bytes.Buffer - c.cmd.Stdout = &b - c.cmd.Stderr = &b - err := c.Run() - return b.Bytes(), err -} - func (c *windowExecCmd) argv() []string { if len(c.cmd.Args) > 0 { return c.cmd.Args diff --git a/pty_windows.go b/pty_windows.go index aa8eb0b..35214d0 100644 --- a/pty_windows.go +++ b/pty_windows.go @@ -133,6 +133,7 @@ func (p *WindowsPty) Close() error { } _, _, err = closePseudoConsole.Call(uintptr(p.handle)) + return err } diff --git a/run_windows.go b/run_windows.go index 8b2a511..dd5201a 100644 --- a/run_windows.go +++ b/run_windows.go @@ -225,6 +225,13 @@ func (c *windowExecCmd) Start() error { if err != nil { return err } + + go c.waitProcess(c.cmd.Process) return nil } + +func (c *windowExecCmd) waitProcess(process *os.Process) { + _, _ = process.Wait() + _ = c.close() +} From 0ff5399118de7cb8b32b130897d895ad8d47b4df Mon Sep 17 00:00:00 2001 From: photostorm Date: Sat, 28 Oct 2023 11:44:36 -0400 Subject: [PATCH 10/16] fixed type issue from recent merge --- go.mod | 2 ++ go.sum | 4 ++-- winsize_unix.go | 5 +++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 14948d3..d7bfbdd 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,5 @@ module github.com/creack/pty/v2 go 1.18 + +require golang.org/x/sys v0.13.0 diff --git a/go.sum b/go.sum index ba6707d..d4673ec 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,2 @@ -golang.org/x/sys v0.0.0-20220721230656-c6bc011c0c49 h1:TMjZDarEwf621XDryfitp/8awEhiZNiwgphKlTMGRIg= -golang.org/x/sys v0.0.0-20220721230656-c6bc011c0c49/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= +golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/winsize_unix.go b/winsize_unix.go index 2868015..883a823 100644 --- a/winsize_unix.go +++ b/winsize_unix.go @@ -4,6 +4,7 @@ package pty import ( + "os" "syscall" "unsafe" ) @@ -11,7 +12,7 @@ import ( // Setsize resizes t to s. func Setsize(t FdHolder, ws *Winsize) error { //nolint:gosec // Expected unsafe pointer for Syscall call. - return ioctl(t, syscall.TIOCSWINSZ, uintptr(unsafe.Pointer(ws))) + return ioctl(t.(*os.File), syscall.TIOCSWINSZ, uintptr(unsafe.Pointer(ws))) } // GetsizeFull returns the full terminal size description. @@ -19,7 +20,7 @@ func GetsizeFull(t FdHolder) (size *Winsize, err error) { var ws Winsize //nolint:gosec // Expected unsafe pointer for Syscall call. - if err := ioctl(t, syscall.TIOCGWINSZ, uintptr(unsafe.Pointer(&ws))); err != nil { + if err := ioctl(t.(*os.File), syscall.TIOCGWINSZ, uintptr(unsafe.Pointer(&ws))); err != nil { return nil, err } return &ws, nil From c3e5fb6a7b36fb6f77b5c9b95832c7854db19ead Mon Sep 17 00:00:00 2001 From: photostorm Date: Sat, 28 Oct 2023 11:58:29 -0400 Subject: [PATCH 11/16] fixed unit tests --- doc.go | 4 ++++ io_test.go | 5 ++--- pty_windows.go | 9 +++++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/doc.go b/doc.go index 033d43a..c0235ce 100644 --- a/doc.go +++ b/doc.go @@ -4,6 +4,7 @@ package pty import ( "errors" "io" + "time" ) // ErrUnsupported is returned if a function is not @@ -30,6 +31,8 @@ type Pty interface { // WriteString is only used to identify Pty and Tty WriteString(s string) (n int, err error) + SetDeadline(t time.Time) error + io.ReadWriteCloser } @@ -41,6 +44,7 @@ type Tty interface { FdHolder Name() string + SetDeadline(t time.Time) error io.ReadWriteCloser } diff --git a/io_test.go b/io_test.go index 0837d9e..9fb682e 100644 --- a/io_test.go +++ b/io_test.go @@ -4,13 +4,12 @@ package pty import ( - "testing" - "context" "errors" "os" "runtime" "sync" + "testing" "time" ) @@ -71,7 +70,7 @@ func TestReadClose(t *testing.T) { } // Open pty and setup watchdogs for graceful and not so graceful failure modes -func prepare(t *testing.T) (ptmx *os.File, done func()) { +func prepare(t *testing.T) (ptmx Pty, done func()) { if runtime.GOOS == "darwin" { t.Log("creack/pty uses blocking i/o on darwin intentionally:") t.Log("> https://github.com/creack/pty/issues/52") diff --git a/pty_windows.go b/pty_windows.go index 35214d0..cf417b4 100644 --- a/pty_windows.go +++ b/pty_windows.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "syscall" + "time" "unsafe" "golang.org/x/sys/windows" @@ -137,6 +138,10 @@ func (p *WindowsPty) Close() error { return err } +func (t *WindowsPty) SetDeadline(value time.Time) error { + return nil +} + func (t *WindowsTty) Name() string { return t.r.Name() } @@ -158,6 +163,10 @@ func (t *WindowsTty) Close() error { return t.w.Close() } +func (t *WindowsTty) SetDeadline(value time.Time) error { + return nil +} + func procCreatePseudoConsole(hInput windows.Handle, hOutput windows.Handle, dwFlags uint32, consoleHandle *windows.Handle) error { var r0 uintptr var err error From 96e45d8b5a96753082482975bc5a7ed84dbe16b0 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sat, 28 Oct 2023 12:02:02 -0400 Subject: [PATCH 12/16] Minor cosmetic fixes, add few comments/questions. --- doc.go | 25 ++++++++++-------- pty_windows.go | 65 ++++++++++++++++++++++------------------------ run_windows.go | 19 +++++++------- winsize_windows.go | 27 +++++++++---------- 4 files changed, 66 insertions(+), 70 deletions(-) diff --git a/doc.go b/doc.go index c0235ce..833c9f5 100644 --- a/doc.go +++ b/doc.go @@ -16,35 +16,38 @@ func Open() (Pty, Tty, error) { return open() } +// FdHolder surfaces the Fd() method of the underlying handle. type FdHolder interface { Fd() uintptr } -// Pty for terminal control in current process -// for unix systems, the real type is *os.File -// for windows, the real type is a *WindowsPty for ConPTY handle +// Pty for terminal control in current process. +// +// - For Unix systems, the real type is *os.File. +// - For Windows, the real type is a *WindowsPty for ConPTY handle. type Pty interface { - // FdHolder Fd intended to resize Tty of child process in current process + // FdHolder is intended to resize / control ioctls of the TTY of the child process in current process. FdHolder Name() string - // WriteString is only used to identify Pty and Tty + // WriteString is only used to identify Pty and Tty. WriteString(s string) (n int, err error) - SetDeadline(t time.Time) error + SetDeadline(t time.Time) error // TODO: Maybe move to FdHolder? io.ReadWriteCloser } -// Tty for data i/o in child process -// for unix systems, the real type is *os.File -// for windows, the real type is a *WindowsTty, which is a combination of two pipe file +// Tty for data I/O in child process. +// +// - For Unix systems, the real type is *os.File. +// - For Windows, the real type is a *WindowsTty, which is a combination of two pipe file. type Tty interface { - // FdHolder Fd only intended for manual InheritSize from Pty + // FdHolder Fd only intended for manual InheritSize from Pty. FdHolder Name() string - SetDeadline(t time.Time) error + SetDeadline(t time.Time) error // TODO: Maybe move to FdHolder? io.ReadWriteCloser } diff --git a/pty_windows.go b/pty_windows.go index cf417b4..c10e7d0 100644 --- a/pty_windows.go +++ b/pty_windows.go @@ -14,6 +14,7 @@ import ( ) const ( + // Ref: https://pkg.go.dev/golang.org/x/sys/windows#pkg-constants PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE = 0x20016 ) @@ -28,11 +29,11 @@ type WindowsTty struct { } var ( - // NOTE(security): as noted by the comment of syscall.NewLazyDLL and syscall.LoadDLL - // user need to call internal/syscall/windows/sysdll.Add("kernel32.dll") to make sure - // the kernel32.dll is loaded from windows system path + // NOTE(security): As noted by the comment of syscall.NewLazyDLL and syscall.LoadDLL + // user need to call internal/syscall/windows/sysdll.Add("kernel32.dll") to make sure + // the kernel32.dll is loaded from windows system path. // - // ref: https://pkg.go.dev/syscall@go1.13?GOOS=windows#LoadDLL + // Ref: https://pkg.go.dev/syscall@go1.13?GOOS=windows#LoadDLL kernel32DLL = windows.NewLazyDLL("kernel32.dll") // https://docs.microsoft.com/en-us/windows/console/createpseudoconsole @@ -51,6 +52,7 @@ func open() (_ Pty, _ Tty, err error) { consoleR, pw, err := os.Pipe() if err != nil { + // Closing everything. Best effort. _ = consoleW.Close() _ = pr.Close() return nil, nil, err @@ -58,9 +60,13 @@ func open() (_ Pty, _ Tty, err error) { var consoleHandle windows.Handle - err = procCreatePseudoConsole(windows.Handle(consoleR.Fd()), windows.Handle(consoleW.Fd()), - 0, &consoleHandle) - if err != nil { + // TODO: As we removed the use of `.Fd()` on Unix (https://github.com/creack/pty/pull/168), we need to check if we should do the same here. + if err := procCreatePseudoConsole( + windows.Handle(consoleR.Fd()), + windows.Handle(consoleW.Fd()), + 0, + &consoleHandle); err != nil { + // Closing everything. Best effort. _ = consoleW.Close() _ = pr.Close() _ = pw.Close() @@ -68,15 +74,12 @@ func open() (_ Pty, _ Tty, err error) { return nil, nil, err } - // These pipes can be closed here without any worry - err = consoleW.Close() - if err != nil { - return nil, nil, fmt.Errorf("failed to close pseudo console handle: %w", err) + // These pipes can be closed here without any worry. + if err := consoleW.Close(); err != nil { + return nil, nil, fmt.Errorf("failed to close pseudo console write handle: %w", err) } - - err = consoleR.Close() - if err != nil { - return nil, nil, fmt.Errorf("failed to close pseudo console handle: %w", err) + if err := consoleR.Close(); err != nil { + return nil, nil, fmt.Errorf("failed to close pseudo console read handle: %w", err) } return &WindowsPty{ @@ -111,9 +114,7 @@ func (p *WindowsPty) WriteString(s string) (int, error) { } func (p *WindowsPty) UpdateProcThreadAttribute(attrList *windows.ProcThreadAttributeListContainer) error { - var err error - - if err = attrList.Update( + if err := attrList.Update( PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, unsafe.Pointer(p.handle), unsafe.Sizeof(p.handle), @@ -125,16 +126,15 @@ func (p *WindowsPty) UpdateProcThreadAttribute(attrList *windows.ProcThreadAttri } func (p *WindowsPty) Close() error { + // Best effort. _ = p.r.Close() _ = p.w.Close() - err := closePseudoConsole.Find() - if err != nil { + if err := closePseudoConsole.Find(); err != nil { return err } - _, _, err = closePseudoConsole.Call(uintptr(p.handle)) - + _, _, err := closePseudoConsole.Call(uintptr(p.handle)) return err } @@ -159,7 +159,7 @@ func (t *WindowsTty) Write(p []byte) (int, error) { } func (t *WindowsTty) Close() error { - _ = t.r.Close() + _ = t.r.Close() // Best effort. return t.w.Close() } @@ -168,20 +168,17 @@ func (t *WindowsTty) SetDeadline(value time.Time) error { } func procCreatePseudoConsole(hInput windows.Handle, hOutput windows.Handle, dwFlags uint32, consoleHandle *windows.Handle) error { - var r0 uintptr - var err error - - err = createPseudoConsole.Find() - if err != nil { + if err := createPseudoConsole.Find(); err != nil { return err } - r0, _, err = createPseudoConsole.Call( - (windowsCoord{X: 80, Y: 30}).Pack(), // size: default 80x30 window - uintptr(hInput), // console input - uintptr(hOutput), // console output - uintptr(dwFlags), // console flags, currently only PSEUDOCONSOLE_INHERIT_CURSOR supported - uintptr(unsafe.Pointer(consoleHandle)), // console handler value return + // TODO: Check if it is expected to ignore `err` here. + r0, _, _ := createPseudoConsole.Call( + (windowsCoord{X: 80, Y: 30}).Pack(), // Size: default 80x30 window. + uintptr(hInput), // Console input. + uintptr(hOutput), // Console output. + uintptr(dwFlags), // Console flags, currently only PSEUDOCONSOLE_INHERIT_CURSOR supported. + uintptr(unsafe.Pointer(consoleHandle)), // Console handler value return. ) if int32(r0) < 0 { diff --git a/run_windows.go b/run_windows.go index dd5201a..d63ea7a 100644 --- a/run_windows.go +++ b/run_windows.go @@ -40,23 +40,23 @@ func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (Pty, } defer func() { - // unlike unix command exec, do not close tty unless error happened + // Unlike unix command exec, do not close tty unless error happened. if err != nil { - _ = pty.Close() + _ = pty.Close() // Best effort. } }() if sz != nil { - if err = Setsize(pty, sz); err != nil { + if err := Setsize(pty, sz); err != nil { return nil, err } } - // unlike unix command exec, do not set stdin/stdout/stderr + // Unlike unix command exec, do not set stdin/stdout/stderr. c.SysProcAttr = attrs - // do not use os/exec.Start since we need to append console handler to startup info + // Do not use os/exec.Start since we need to append console handler to startup info. w := windowExecCmd{ cmd: c, @@ -64,8 +64,7 @@ func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (Pty, conPty: pty.(*WindowsPty), } - err = w.Start() - if err != nil { + if err := w.Start(); err != nil { return nil, err } @@ -83,7 +82,7 @@ func (c *windowExecCmd) Start() error { return errors.New("exec: already started") } - var argv0 = c.cmd.Path + argv0 := c.cmd.Path var argv0p *uint16 var argvp *uint16 var dirp *uint16 @@ -131,7 +130,7 @@ func (c *windowExecCmd) Start() error { // 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. if sys.CmdLine != "" { cmdline = sys.CmdLine } else { @@ -225,7 +224,7 @@ func (c *windowExecCmd) Start() error { if err != nil { return err } - + go c.waitProcess(c.cmd.Process) return nil diff --git a/winsize_windows.go b/winsize_windows.go index 5b96004..c9a3111 100644 --- a/winsize_windows.go +++ b/winsize_windows.go @@ -8,14 +8,15 @@ import ( "unsafe" ) -// types from golang.org/x/sys/windows -// copy of https://pkg.go.dev/golang.org/x/sys/windows#Coord +// Types from golang.org/x/sys/windows. + +// Ref: https://pkg.go.dev/golang.org/x/sys/windows#Coord type windowsCoord struct { X int16 Y int16 } -// copy of https://pkg.go.dev/golang.org/x/sys/windows#SmallRect +// Ref: https://pkg.go.dev/golang.org/x/sys/windows#SmallRect type windowsSmallRect struct { Left int16 Top int16 @@ -23,7 +24,7 @@ type windowsSmallRect struct { Bottom int16 } -// copy of https://pkg.go.dev/golang.org/x/sys/windows#ConsoleScreenBufferInfo +// Ref: https://pkg.go.dev/golang.org/x/sys/windows#ConsoleScreenBufferInfo type windowsConsoleScreenBufferInfo struct { Size windowsCoord CursorPosition windowsCoord @@ -38,15 +39,13 @@ func (c windowsCoord) Pack() uintptr { // Setsize resizes t to ws. func Setsize(t FdHolder, ws *Winsize) error { - var r0 uintptr - var err error - - err = resizePseudoConsole.Find() - if err != nil { + if err := resizePseudoConsole.Find(); err != nil { return err } - r0, _, err = resizePseudoConsole.Call( + // TODO: As we removed the use of `.Fd()` on Unix (https://github.com/creack/pty/pull/168), we need to check if we should do the same here. + // TODO: Check if it is expected to ignore `err` here. + r0, _, _ := resizePseudoConsole.Call( t.Fd(), (windowsCoord{X: int16(ws.Cols), Y: int16(ws.Rows)}).Pack(), ) @@ -64,15 +63,13 @@ func Setsize(t FdHolder, ws *Winsize) error { // GetsizeFull returns the full terminal size description. func GetsizeFull(t FdHolder) (size *Winsize, err error) { - err = getConsoleScreenBufferInfo.Find() - if err != nil { + if err := getConsoleScreenBufferInfo.Find(); err != nil { return nil, err } var info windowsConsoleScreenBufferInfo - var r0 uintptr - - r0, _, err = getConsoleScreenBufferInfo.Call(t.Fd(), uintptr(unsafe.Pointer(&info))) + // TODO: Check if it is expected to ignore `err` here. + r0, _, _ := getConsoleScreenBufferInfo.Call(t.Fd(), uintptr(unsafe.Pointer(&info))) if int32(r0) < 0 { if r0&0x1fff0000 == 0x00070000 { r0 &= 0xffff From 7f5c495d2413769c2a4db2b2dd2e86f6b9b3c505 Mon Sep 17 00:00:00 2001 From: photostorm Date: Sat, 28 Oct 2023 18:00:10 -0400 Subject: [PATCH 13/16] moved deadline method to FdHolder --- doc.go | 3 +-- pty_windows.go | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/doc.go b/doc.go index 833c9f5..0cfb503 100644 --- a/doc.go +++ b/doc.go @@ -19,6 +19,7 @@ func Open() (Pty, Tty, error) { // FdHolder surfaces the Fd() method of the underlying handle. type FdHolder interface { Fd() uintptr + SetDeadline(t time.Time) error } // Pty for terminal control in current process. @@ -33,7 +34,6 @@ type Pty interface { // WriteString is only used to identify Pty and Tty. WriteString(s string) (n int, err error) - SetDeadline(t time.Time) error // TODO: Maybe move to FdHolder? io.ReadWriteCloser } @@ -47,7 +47,6 @@ type Tty interface { FdHolder Name() string - SetDeadline(t time.Time) error // TODO: Maybe move to FdHolder? io.ReadWriteCloser } diff --git a/pty_windows.go b/pty_windows.go index c10e7d0..cde03e7 100644 --- a/pty_windows.go +++ b/pty_windows.go @@ -34,7 +34,7 @@ var ( // the kernel32.dll is loaded from windows system path. // // Ref: https://pkg.go.dev/syscall@go1.13?GOOS=windows#LoadDLL - kernel32DLL = windows.NewLazyDLL("kernel32.dll") + kernel32DLL = windows.NewLazySystemDLL("kernel32.dll") // https://docs.microsoft.com/en-us/windows/console/createpseudoconsole createPseudoConsole = kernel32DLL.NewProc("CreatePseudoConsole") @@ -138,8 +138,8 @@ func (p *WindowsPty) Close() error { return err } -func (t *WindowsPty) SetDeadline(value time.Time) error { - return nil +func (p *WindowsPty) SetDeadline(value time.Time) error { + return os.ErrNoDeadline } func (t *WindowsTty) Name() string { @@ -164,7 +164,7 @@ func (t *WindowsTty) Close() error { } func (t *WindowsTty) SetDeadline(value time.Time) error { - return nil + return os.ErrNoDeadline } func procCreatePseudoConsole(hInput windows.Handle, hOutput windows.Handle, dwFlags uint32, consoleHandle *windows.Handle) error { From e9c1f000f465e8dedfe486c6f3f6cb1addf0e68a Mon Sep 17 00:00:00 2001 From: photostorm Date: Sat, 28 Oct 2023 18:11:18 -0400 Subject: [PATCH 14/16] added DeadlineHolder --- doc.go | 4 ++++ io_test.go | 14 ++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/doc.go b/doc.go index 0cfb503..989e286 100644 --- a/doc.go +++ b/doc.go @@ -19,6 +19,10 @@ func Open() (Pty, Tty, error) { // FdHolder surfaces the Fd() method of the underlying handle. type FdHolder interface { Fd() uintptr +} + +// DeadlineHolder surfaces the SetDeadline() method to sets the read and write deadlines. +type DeadlineHolder interface { SetDeadline(t time.Time) error } diff --git a/io_test.go b/io_test.go index 9fb682e..cc18ccd 100644 --- a/io_test.go +++ b/io_test.go @@ -27,12 +27,14 @@ var mu sync.Mutex func TestReadDeadline(t *testing.T) { ptmx, success := prepare(t) - err := ptmx.SetDeadline(time.Now().Add(timeout / 10)) - if err != nil { - if errors.Is(err, os.ErrNoDeadline) { - t.Skipf("deadline is not supported on %s/%s", runtime.GOOS, runtime.GOARCH) - } else { - t.Fatalf("error: set deadline: %v\n", err) + if ptmxd, ok := ptmx.(DeadlineHolder); ok { + err := ptmxd.SetDeadline(time.Now().Add(timeout / 10)) + if err != nil { + if errors.Is(err, os.ErrNoDeadline) { + t.Skipf("deadline is not supported on %s/%s", runtime.GOOS, runtime.GOARCH) + } else { + t.Fatalf("error: set deadline: %v\n", err) + } } } From 164ea9c03925463671d9fbc89eaa7a01f69d2494 Mon Sep 17 00:00:00 2001 From: Larry Clapp Date: Thu, 28 Mar 2024 09:39:17 -0400 Subject: [PATCH 15/16] Don't close consoleR & consoleW in Windows pty_windows.go closed consoleW and consoleR in the initial open(), with the comment "These pipes can be closed here without any worry." This isn't actually true. If you interact with the terminal via Go code (i.e. don't immediately fork a new process, but use (for example) github.com/mvdan/sh), you need those pipes to stay open, and to close them yourself. --- cmd_windows.go | 2 ++ go.mod | 2 +- pty_windows.go | 8 -------- run_windows.go | 3 ++- 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/cmd_windows.go b/cmd_windows.go index d74dbf3..5e628f8 100644 --- a/cmd_windows.go +++ b/cmd_windows.go @@ -28,12 +28,14 @@ type windowExecCmd struct { cmd *exec.Cmd waitCalled bool conPty *WindowsPty + conTty *WindowsTty attrList *windows.ProcThreadAttributeListContainer } func (c *windowExecCmd) close() error { c.attrList.Delete() _ = c.conPty.Close() + _ = c.conTty.Close() return nil } diff --git a/go.mod b/go.mod index d7bfbdd..a5ab0c7 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,5 @@ module github.com/creack/pty/v2 -go 1.18 +go 1.21.5 require golang.org/x/sys v0.13.0 diff --git a/pty_windows.go b/pty_windows.go index cde03e7..7dbe189 100644 --- a/pty_windows.go +++ b/pty_windows.go @@ -74,14 +74,6 @@ func open() (_ Pty, _ Tty, err error) { return nil, nil, err } - // These pipes can be closed here without any worry. - if err := consoleW.Close(); err != nil { - return nil, nil, fmt.Errorf("failed to close pseudo console write handle: %w", err) - } - if err := consoleR.Close(); err != nil { - return nil, nil, fmt.Errorf("failed to close pseudo console read handle: %w", err) - } - return &WindowsPty{ handle: consoleHandle, r: pr, diff --git a/run_windows.go b/run_windows.go index d63ea7a..e933a87 100644 --- a/run_windows.go +++ b/run_windows.go @@ -34,7 +34,7 @@ func StartWithSize(c *exec.Cmd, sz *Winsize) (Pty, error) { // This should generally not be needed. Used in some edge cases where it is needed to create a pty // without a controlling terminal. func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (Pty, error) { - pty, _, err := open() + pty, tty, err := open() if err != nil { return nil, err } @@ -62,6 +62,7 @@ func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (Pty, cmd: c, waitCalled: false, conPty: pty.(*WindowsPty), + conTty: tty.(*WindowsTty), } if err := w.Start(); err != nil { From bf40468acd654f4db3914093606756d7af04b030 Mon Sep 17 00:00:00 2001 From: Larry Clapp Date: Fri, 5 Apr 2024 14:07:24 -0400 Subject: [PATCH 16/16] Make tests pass --- helpers_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/helpers_test.go b/helpers_test.go index fbc98b7..d712955 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -4,12 +4,11 @@ import ( "bytes" "fmt" "io" - "os" "testing" ) -// openCloses opens a pty/tty pair and stages the closing as part of the cleanup. -func openClose(t *testing.T) (pty, tty *os.File) { +// openClose opens a pty/tty pair and stages the closing as part of the cleanup. +func openClose(t *testing.T) (Pty, Tty) { t.Helper() pty, tty, err := Open()