From 5dfb5143117631242560c2121c45642d50cc6f6d Mon Sep 17 00:00:00 2001 From: "Vojtech Vitek (V-Teq)" Date: Thu, 21 Aug 2014 10:06:32 +0200 Subject: [PATCH 1/3] Fix #6509: Interactive container hangs when redirecting stdout Cli IsTerminal() SYS_IOCTL operation should be determined from STDIN, not from STDOUT. Signed-off-by: Vojtech Vitek (V-Teq) Upstream-commit: 40c7b53791b598364ffe2290c88875e4fc65be11 Component: engine --- components/engine/api/client/cli.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/api/client/cli.go b/components/engine/api/client/cli.go index 6346d30cac..a158fc7d3c 100644 --- a/components/engine/api/client/cli.go +++ b/components/engine/api/client/cli.go @@ -104,7 +104,7 @@ func NewDockerCli(in io.ReadCloser, out, err io.Writer, proto, addr string, tlsC } if in != nil { - if file, ok := out.(*os.File); ok { + if file, ok := in.(*os.File); ok { terminalFd = file.Fd() isTerminal = term.IsTerminal(terminalFd) } From e3880837c423ab672fdbe4cddd8d360a8ea80508 Mon Sep 17 00:00:00 2001 From: "Vojtech Vitek (V-Teq)" Date: Wed, 10 Sep 2014 15:35:48 +0200 Subject: [PATCH 2/3] DockerCli: Check IsTerminal() for both STDIN and STDOUT `docker events > /tmp/out` should not print control characters to non-terminal STDOUT. This addresses commit 26b4a4920adca614f6be17a96f254f331271faf0 without creating regression described in issue #6509. Signed-off-by: Vojtech Vitek (V-Teq) Upstream-commit: d742c57f534352b6cad596d0c9fe8cf84044e92a Component: engine --- components/engine/api/client/cli.go | 50 ++++++++++++++++-------- components/engine/api/client/commands.go | 14 +++---- components/engine/api/client/hijack.go | 12 +++--- components/engine/api/client/utils.go | 6 +-- 4 files changed, 50 insertions(+), 32 deletions(-) diff --git a/components/engine/api/client/cli.go b/components/engine/api/client/cli.go index a158fc7d3c..5e7f45b119 100644 --- a/components/engine/api/client/cli.go +++ b/components/engine/api/client/cli.go @@ -22,10 +22,16 @@ type DockerCli struct { in io.ReadCloser out io.Writer err io.Writer - isTerminal bool - terminalFd uintptr tlsConfig *tls.Config scheme string + // inFd holds file descriptor of the client's STDIN, if it's a valid file + inFd uintptr + // outFd holds file descriptor of the client's STDOUT, if it's a valid file + outFd uintptr + // isTerminalIn describes if client's STDIN is a TTY + isTerminalIn bool + // isTerminalOut describes if client's STDOUT is a TTY + isTerminalOut bool } var funcMap = template.FuncMap{ @@ -94,9 +100,11 @@ func (cli *DockerCli) LoadConfigFile() (err error) { func NewDockerCli(in io.ReadCloser, out, err io.Writer, proto, addr string, tlsConfig *tls.Config) *DockerCli { var ( - isTerminal = false - terminalFd uintptr - scheme = "http" + inFd uintptr + outFd uintptr + isTerminalIn = false + isTerminalOut = false + scheme = "http" ) if tlsConfig != nil { @@ -105,23 +113,33 @@ func NewDockerCli(in io.ReadCloser, out, err io.Writer, proto, addr string, tlsC if in != nil { if file, ok := in.(*os.File); ok { - terminalFd = file.Fd() - isTerminal = term.IsTerminal(terminalFd) + inFd = file.Fd() + isTerminalIn = term.IsTerminal(inFd) + } + } + + if out != nil { + if file, ok := out.(*os.File); ok { + outFd = file.Fd() + isTerminalOut = term.IsTerminal(outFd) } } if err == nil { err = out } + return &DockerCli{ - proto: proto, - addr: addr, - in: in, - out: out, - err: err, - isTerminal: isTerminal, - terminalFd: terminalFd, - tlsConfig: tlsConfig, - scheme: scheme, + proto: proto, + addr: addr, + in: in, + out: out, + err: err, + inFd: inFd, + outFd: outFd, + isTerminalIn: isTerminalIn, + isTerminalOut: isTerminalOut, + tlsConfig: tlsConfig, + scheme: scheme, } } diff --git a/components/engine/api/client/commands.go b/components/engine/api/client/commands.go index fa0f69f3d6..6c26d152d6 100644 --- a/components/engine/api/client/commands.go +++ b/components/engine/api/client/commands.go @@ -277,14 +277,14 @@ func (cli *DockerCli) CmdLogin(args ...string) error { // the password or email from the config file, so prompt them if username != authconfig.Username { if password == "" { - oldState, _ := term.SaveState(cli.terminalFd) + oldState, _ := term.SaveState(cli.inFd) fmt.Fprintf(cli.out, "Password: ") - term.DisableEcho(cli.terminalFd, oldState) + term.DisableEcho(cli.inFd, oldState) password = readInput(cli.in, cli.out) fmt.Fprint(cli.out, "\n") - term.RestoreTerminal(cli.terminalFd, oldState) + term.RestoreTerminal(cli.inFd, oldState) if password == "" { return fmt.Errorf("Error : Password Required") } @@ -670,7 +670,7 @@ func (cli *DockerCli) CmdStart(args ...string) error { } if *openStdin || *attach { - if tty && cli.isTerminal { + if tty && cli.isTerminalOut { if err := cli.monitorTtySize(cmd.Arg(0), false); err != nil { log.Errorf("Error monitoring TTY size: %s", err) } @@ -1822,7 +1822,7 @@ func (cli *DockerCli) CmdAttach(args ...string) error { tty = config.GetBool("Tty") ) - if tty && cli.isTerminal { + if tty && cli.isTerminalOut { if err := cli.monitorTtySize(cmd.Arg(0), false); err != nil { log.Debugf("Error monitoring TTY size: %s", err) } @@ -2247,7 +2247,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { return err } - if (config.AttachStdin || config.AttachStdout || config.AttachStderr) && config.Tty && cli.isTerminal { + if (config.AttachStdin || config.AttachStdout || config.AttachStderr) && config.Tty && cli.isTerminalOut { if err := cli.monitorTtySize(runResult.Get("Id"), false); err != nil { log.Errorf("Error monitoring TTY size: %s", err) } @@ -2496,7 +2496,7 @@ func (cli *DockerCli) CmdExec(args ...string) error { } } - if execConfig.Tty && cli.isTerminal { + if execConfig.Tty && cli.isTerminalIn { if err := cli.monitorTtySize(execID, true); err != nil { log.Errorf("Error monitoring TTY size: %s", err) } diff --git a/components/engine/api/client/hijack.go b/components/engine/api/client/hijack.go index 4aba842372..7a934fde15 100644 --- a/components/engine/api/client/hijack.go +++ b/components/engine/api/client/hijack.go @@ -69,20 +69,20 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea var oldState *term.State - if in != nil && setRawTerminal && cli.isTerminal && os.Getenv("NORAW") == "" { - oldState, err = term.SetRawTerminal(cli.terminalFd) + if in != nil && setRawTerminal && cli.isTerminalIn && os.Getenv("NORAW") == "" { + oldState, err = term.SetRawTerminal(cli.inFd) if err != nil { return err } - defer term.RestoreTerminal(cli.terminalFd, oldState) + defer term.RestoreTerminal(cli.inFd, oldState) } if stdout != nil || stderr != nil { receiveStdout = utils.Go(func() (err error) { defer func() { if in != nil { - if setRawTerminal && cli.isTerminal { - term.RestoreTerminal(cli.terminalFd, oldState) + if setRawTerminal && cli.isTerminalIn { + term.RestoreTerminal(cli.inFd, oldState) } // For some reason this Close call blocks on darwin.. // As the client exists right after, simply discard the close @@ -129,7 +129,7 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea } } - if !cli.isTerminal { + if !cli.isTerminalIn { if err := <-sendStdin; err != nil { log.Debugf("Error sendStdin: %s", err) return err diff --git a/components/engine/api/client/utils.go b/components/engine/api/client/utils.go index 19e280614e..0d8310cbba 100644 --- a/components/engine/api/client/utils.go +++ b/components/engine/api/client/utils.go @@ -168,7 +168,7 @@ func (cli *DockerCli) streamHelper(method, path string, setRawTerminal bool, in } if api.MatchesContentType(resp.Header.Get("Content-Type"), "application/json") || api.MatchesContentType(resp.Header.Get("Content-Type"), "application/x-json-stream") { - return utils.DisplayJSONMessagesStream(resp.Body, stdout, cli.terminalFd, cli.isTerminal) + return utils.DisplayJSONMessagesStream(resp.Body, stdout, cli.outFd, cli.isTerminalOut) } if stdout != nil || stderr != nil { // When TTY is ON, use regular copy @@ -252,10 +252,10 @@ func (cli *DockerCli) monitorTtySize(id string, isExec bool) error { } func (cli *DockerCli) getTtySize() (int, int) { - if !cli.isTerminal { + if !cli.isTerminalOut { return 0, 0 } - ws, err := term.GetWinsize(cli.terminalFd) + ws, err := term.GetWinsize(cli.outFd) if err != nil { log.Debugf("Error getting size: %s", err) if ws == nil { From 1375b67b0515bf45eaff4427fc4f40b59c5bcf57 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Thu, 25 Sep 2014 10:50:10 -0400 Subject: [PATCH 3/3] Add DockerCli tests Signed-off-by: Tibor Vass Upstream-commit: 29a62ceefc62836cc918093b60aef6b23a649d76 Component: engine --- .../integration-cli/docker_cli_events_test.go | 49 +++++++++++++++++++ .../integration-cli/docker_cli_run_test.go | 39 +++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/components/engine/integration-cli/docker_cli_events_test.go b/components/engine/integration-cli/docker_cli_events_test.go index 98e50d97a2..dcd9e94f5f 100644 --- a/components/engine/integration-cli/docker_cli_events_test.go +++ b/components/engine/integration-cli/docker_cli_events_test.go @@ -1,12 +1,18 @@ package main import ( + "bufio" "fmt" + "io/ioutil" + "os" "os/exec" "strconv" "strings" "testing" "time" + "unicode" + + "github.com/kr/pty" ) func TestEventsUntag(t *testing.T) { @@ -166,3 +172,46 @@ func TestEventsImageUntagDelete(t *testing.T) { } logDone("events - image untag, delete is logged") } + +// #5979 +func TestEventsRedirectStdout(t *testing.T) { + + since := time.Now().Unix() + + cmd(t, "run", "busybox", "true") + + defer deleteAllContainers() + + file, err := ioutil.TempFile("", "") + if err != nil { + t.Fatalf("could not create temp file: %v", err) + } + defer os.Remove(file.Name()) + + command := fmt.Sprintf("%s events --since=%d --until=%d > %s", dockerBinary, since, time.Now().Unix(), file.Name()) + _, tty, err := pty.Open() + if err != nil { + t.Fatalf("Could not open pty: %v", err) + } + cmd := exec.Command("sh", "-c", command) + cmd.Stdin = tty + cmd.Stdout = tty + cmd.Stderr = tty + if err := cmd.Run(); err != nil { + t.Fatalf("run err for command %q: %v", command, err) + } + + scanner := bufio.NewScanner(file) + for scanner.Scan() { + for _, c := range scanner.Text() { + if unicode.IsControl(c) { + t.Fatalf("found control character %v", []byte(string(c))) + } + } + } + if err := scanner.Err(); err != nil { + t.Fatalf("Scan err for command %q: %v", command, err) + } + + logDone("events - redirect stdout") +} diff --git a/components/engine/integration-cli/docker_cli_run_test.go b/components/engine/integration-cli/docker_cli_run_test.go index f8ec8bd21a..4ee1fa092a 100644 --- a/components/engine/integration-cli/docker_cli_run_test.go +++ b/components/engine/integration-cli/docker_cli_run_test.go @@ -19,6 +19,7 @@ import ( "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/networkfs/resolvconf" + "github.com/kr/pty" ) // "test123" should be printed by docker run @@ -2162,3 +2163,41 @@ func TestRunExecDir(t *testing.T) { logDone("run - check execdriver dir behavior") } + +// #6509 +func TestRunRedirectStdout(t *testing.T) { + + defer deleteAllContainers() + + checkRedirect := func(command string) { + _, tty, err := pty.Open() + if err != nil { + t.Fatalf("Could not open pty: %v", err) + } + cmd := exec.Command("sh", "-c", command) + cmd.Stdin = tty + cmd.Stdout = tty + cmd.Stderr = tty + ch := make(chan struct{}) + if err := cmd.Start(); err != nil { + t.Fatalf("start err: %v", err) + } + go func() { + if err := cmd.Wait(); err != nil { + t.Fatalf("wait err=%v", err) + } + close(ch) + }() + + select { + case <-time.After(time.Second): + t.Fatal("command timeout") + case <-ch: + } + } + + checkRedirect(dockerBinary + " run -i busybox cat /etc/passwd | grep -q root") + checkRedirect(dockerBinary + " run busybox cat /etc/passwd | grep -q root") + + logDone("run - redirect stdout") +}