Merge pull request #86 from cyphar/newpty-from-file

pty: add GetPtyFromFile as safer GetPty

Author: AkihiroSuda

.github/workflows/ci.yml

@@ -37,7 +37,7 @@ jobs:
         go install github.com/golangci/golangci-lint/cmd/[email protected]
 
     - name: Project Checks
-      uses: containerd/project-checks@v1.1.0
+      uses: containerd/project-checks@v1.2.2
       with:
         working-directory: src/github.com/containerd/console
 

console_test.go

@@ -54,8 +54,8 @@ func TestWinSize(t *testing.T) {
 	}
 }
 
-func TestConsolePty(t *testing.T) {
-	console, slavePath, err := NewPty()
+func testConsolePty(t *testing.T, newPty func() (Console, string, error)) {
+	console, slavePath, err := newPty()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -100,3 +100,18 @@ func TestConsolePty(t *testing.T) {
 		t.Errorf("unexpected output %q", out)
 	}
 }
+
+func TestConsolePty_NewPty(t *testing.T) {
+	testConsolePty(t, NewPty)
+}
+
+func TestConsolePty_NewPtyFromFile(t *testing.T) {
+	testConsolePty(t, func() (Console, string, error) {
+		// Equivalent to NewPty().
+		f, err := openpt()
+		if err != nil {
+			return nil, "", err
+		}
+		return NewPtyFromFile(f)
+	})
+}

console_unix.go

@@ -31,6 +31,15 @@ func NewPty() (Console, string, error) {
 	if err != nil {
 		return nil, "", err
 	}
+	return NewPtyFromFile(f)
+}
+
+// NewPtyFromFile creates a new pty pair, just like [NewPty] except that the
+// provided [os.File] is used as the master rather than automatically creating
+// a new master from /dev/ptmx. The ownership of [os.File] is passed to the
+// returned [Console], so the caller must be careful to not call Close on the
+// underlying file.
+func NewPtyFromFile(f File) (Console, string, error) {
 	slave, err := ptsname(f)
 	if err != nil {
 		return nil, "", err

tc_darwin.go

@@ -18,7 +18,6 @@ package console
 
 import (
 	"fmt"
-	"os"
 
 	"golang.org/x/sys/unix"
 )
@@ -30,12 +29,12 @@ const (
 
 // unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f.
 // unlockpt should be called before opening the slave side of a pty.
-func unlockpt(f *os.File) error {
+func unlockpt(f File) error {
 	return unix.IoctlSetPointerInt(int(f.Fd()), unix.TIOCPTYUNLK, 0)
 }
 
 // ptsname retrieves the name of the first available pts for the given master.
-func ptsname(f *os.File) (string, error) {
+func ptsname(f File) (string, error) {
 	n, err := unix.IoctlGetInt(int(f.Fd()), unix.TIOCPTYGNAME)
 	if err != nil {
 		return "", err

tc_freebsd_cgo.go

@@ -21,7 +21,6 @@ package console
 
 import (
 	"fmt"
-	"os"
 
 	"golang.org/x/sys/unix"
 )
@@ -39,7 +38,7 @@ const (
 
 // unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f.
 // unlockpt should be called before opening the slave side of a pty.
-func unlockpt(f *os.File) error {
+func unlockpt(f File) error {
 	fd := C.int(f.Fd())
 	if _, err := C.unlockpt(fd); err != nil {
 		C.close(fd)
@@ -49,7 +48,7 @@ func unlockpt(f *os.File) error {
 }
 
 // ptsname retrieves the name of the first available pts for the given master.
-func ptsname(f *os.File) (string, error) {
+func ptsname(f File) (string, error) {
 	n, err := unix.IoctlGetInt(int(f.Fd()), unix.TIOCGPTN)
 	if err != nil {
 		return "", err

tc_freebsd_nocgo.go

@@ -21,7 +21,6 @@ package console
 
 import (
 	"fmt"
-	"os"
 
 	"golang.org/x/sys/unix"
 )
@@ -42,12 +41,12 @@ const (
 
 // unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f.
 // unlockpt should be called before opening the slave side of a pty.
-func unlockpt(f *os.File) error {
+func unlockpt(f File) error {
 	panic("unlockpt() support requires cgo.")
 }
 
 // ptsname retrieves the name of the first available pts for the given master.
-func ptsname(f *os.File) (string, error) {
+func ptsname(f File) (string, error) {
 	n, err := unix.IoctlGetInt(int(f.Fd()), unix.TIOCGPTN)
 	if err != nil {
 		return "", err

tc_linux.go

@@ -18,7 +18,6 @@ package console
 
 import (
 	"fmt"
-	"os"
 	"unsafe"
 
 	"golang.org/x/sys/unix"
@@ -31,7 +30,7 @@ const (
 
 // unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f.
 // unlockpt should be called before opening the slave side of a pty.
-func unlockpt(f *os.File) error {
+func unlockpt(f File) error {
 	var u int32
 	// XXX do not use unix.IoctlSetPointerInt here, see commit dbd69c59b81.
 	if _, _, err := unix.Syscall(unix.SYS_IOCTL, f.Fd(), unix.TIOCSPTLCK, uintptr(unsafe.Pointer(&u))); err != 0 {
@@ -41,7 +40,7 @@ func unlockpt(f *os.File) error {
 }
 
 // ptsname retrieves the name of the first available pts for the given master.
-func ptsname(f *os.File) (string, error) {
+func ptsname(f File) (string, error) {
 	var u uint32
 	// XXX do not use unix.IoctlGetInt here, see commit dbd69c59b81.
 	if _, _, err := unix.Syscall(unix.SYS_IOCTL, f.Fd(), unix.TIOCGPTN, uintptr(unsafe.Pointer(&u))); err != 0 {

tc_netbsd.go

@@ -18,7 +18,6 @@ package console
 
 import (
 	"bytes"
-	"os"
 
 	"golang.org/x/sys/unix"
 )
@@ -31,12 +30,12 @@ const (
 // unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f.
 // unlockpt should be called before opening the slave side of a pty.
 // This does not exist on NetBSD, it does not allocate controlling terminals on open
-func unlockpt(f *os.File) error {
+func unlockpt(f File) error {
 	return nil
 }
 
 // ptsname retrieves the name of the first available pts for the given master.
-func ptsname(f *os.File) (string, error) {
+func ptsname(f File) (string, error) {
 	ptm, err := unix.IoctlGetPtmget(int(f.Fd()), unix.TIOCPTSNAME)
 	if err != nil {
 		return "", err

tc_openbsd_cgo.go

@@ -20,8 +20,6 @@
 package console
 
 import (
-	"os"
-
 	"golang.org/x/sys/unix"
 )
 
@@ -34,7 +32,7 @@ const (
 )
 
 // ptsname retrieves the name of the first available pts for the given master.
-func ptsname(f *os.File) (string, error) {
+func ptsname(f File) (string, error) {
 	ptspath, err := C.ptsname(C.int(f.Fd()))
 	if err != nil {
 		return "", err
@@ -44,7 +42,7 @@ func ptsname(f *os.File) (string, error) {
 
 // unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f.
 // unlockpt should be called before opening the slave side of a pty.
-func unlockpt(f *os.File) error {
+func unlockpt(f File) error {
 	if _, err := C.grantpt(C.int(f.Fd())); err != nil {
 		return err
 	}

tc_openbsd_nocgo.go

@@ -29,8 +29,6 @@
 package console
 
 import (
-	"os"
-
 	"golang.org/x/sys/unix"
 )
 
@@ -39,10 +37,10 @@ const (
 	cmdTcSet = unix.TIOCSETA
 )
 
-func ptsname(f *os.File) (string, error) {
+func ptsname(f File) (string, error) {
 	panic("ptsname() support requires cgo.")
 }
 
-func unlockpt(f *os.File) error {
+func unlockpt(f File) error {
 	panic("unlockpt() support requires cgo.")
 }

tc_zos.go

@@ -17,7 +17,6 @@
 package console
 
 import (
-	"os"
 	"strings"
 
 	"golang.org/x/sys/unix"
@@ -29,11 +28,11 @@ const (
 )
 
 // unlockpt is a no-op on zos.
-func unlockpt(_ *os.File) error {
+func unlockpt(File) error {
 	return nil
 }
 
 // ptsname retrieves the name of the first available pts for the given master.
-func ptsname(f *os.File) (string, error) {
+func ptsname(f File) (string, error) {
 	return "/dev/ttyp" + strings.TrimPrefix(f.Name(), "/dev/ptyp"), nil
 }