From eea28f480db435dbef4a275de9776b9934818b8c Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Mon, 24 Oct 2016 22:09:01 +1100 Subject: [PATCH 1/2] libcontainer: io: stop screwing with \n in console output The default terminal setting for a new pty on Linux (unix98) has +ONLCR, resulting in '\n' writes by a container process to be converted to '\r\n' reads by the managing process. This is quite unexpected, and causes multiple issues with things like bats testing. To fix it, make the terminal sane after opening it by setting -ONLCR. This patch might need to be rewritten after the console rewrite patchset is merged. Signed-off-by: Aleksa Sarai --- libcontainer/console_linux.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/libcontainer/console_linux.go b/libcontainer/console_linux.go index 5bc2fd75..0ca59393 100644 --- a/libcontainer/console_linux.go +++ b/libcontainer/console_linux.go @@ -17,6 +17,9 @@ func NewConsole(uid, gid int) (Console, error) { if err != nil { return nil, err } + if err := saneTerminal(master); err != nil { + return nil, err + } console, err := ptsname(master) if err != nil { return nil, err @@ -143,3 +146,26 @@ func ptsname(f *os.File) (string, error) { } return fmt.Sprintf("/dev/pts/%d", n), nil } + +// saneTerminal sets the necessary tty_ioctl(4)s to ensure that a pty pair +// created by us acts normally. In particular, a not-very-well-known default of +// Linux unix98 ptys is that they have +onlcr by default. While this isn't a +// problem for terminal emulators, because we relay data from the terminal we +// also relay that funky line discipline. +func saneTerminal(terminal *os.File) error { + // Go doesn't have a wrapper for any of the termios ioctls. + var termios syscall.Termios + + if err := ioctl(terminal.Fd(), syscall.TCGETS, uintptr(unsafe.Pointer(&termios))); err != nil { + return fmt.Errorf("ioctl(tty, tcgets): %s", err.Error()) + } + + // Set -onlcr so we don't have to deal with \r. + termios.Oflag &^= syscall.ONLCR + + if err := ioctl(terminal.Fd(), syscall.TCSETS, uintptr(unsafe.Pointer(&termios))); err != nil { + return fmt.Errorf("ioctl(tty, tcsets): %s", err.Error()) + } + + return nil +} From fd7ab60a70d20025deef869959de2305910b288e Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Mon, 24 Oct 2016 22:22:59 +1100 Subject: [PATCH 2/2] libcontainer: make tests to make sure we don't mess with \r Signed-off-by: Aleksa Sarai --- libcontainer/integration/execin_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index 174b5f06..f25ef357 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -61,6 +61,9 @@ func TestExecIn(t *testing.T) { if !strings.Contains(out, "cat") || !strings.Contains(out, "ps") { t.Fatalf("unexpected running process, output %q", out) } + if strings.Contains(out, "\r") { + t.Fatalf("unexpected carriage-return in output") + } } func TestExecInUsernsRlimit(t *testing.T) { @@ -296,9 +299,12 @@ func TestExecInTTY(t *testing.T) { waitProcess(process, t) out := stdout.String() - if !strings.Contains(out, "cat") || !strings.Contains(string(out), "ps") { + if !strings.Contains(out, "cat") || !strings.Contains(out, "ps") { t.Fatalf("unexpected running process, output %q", out) } + if strings.Contains(out, "\r") { + t.Fatalf("unexpected carriage-return in output") + } } func TestExecInEnvironment(t *testing.T) {