diff --git a/api/client/cli.go b/api/client/cli.go index a477d0b3a9..e54eb8056e 100644 --- a/api/client/cli.go +++ b/api/client/cli.go @@ -3,6 +3,7 @@ package client import ( "crypto/tls" "encoding/json" + "errors" "fmt" "io" "net" @@ -104,6 +105,16 @@ func (cli *DockerCli) LoadConfigFile() (err error) { return err } +func (cli *DockerCli) CheckTtyInput(attachStdin, ttyMode bool) error { + // In order to attach to a container tty, input stream for the client must + // be a tty itself: redirecting or piping the client standard input is + // incompatible with `docker run -t`, `docker exec -t` or `docker attach`. + if ttyMode && attachStdin && !cli.isTerminalIn { + return errors.New("cannot enable tty mode on non tty input") + } + return nil +} + func NewDockerCli(in io.ReadCloser, out, err io.Writer, key libtrust.PrivateKey, proto, addr string, tlsConfig *tls.Config) *DockerCli { var ( inFd uintptr diff --git a/api/client/commands.go b/api/client/commands.go index 5203513d90..89e5796bbb 100644 --- a/api/client/commands.go +++ b/api/client/commands.go @@ -1974,6 +1974,10 @@ func (cli *DockerCli) CmdAttach(args ...string) error { tty = config.GetBool("Tty") ) + if err := cli.CheckTtyInput(!*noStdin, tty); err != nil { + return err + } + if tty && cli.isTerminalOut { if err := cli.monitorTtySize(cmd.Arg(0), false); err != nil { log.Debugf("Error monitoring TTY size: %s", err) @@ -2288,7 +2292,11 @@ func (cli *DockerCli) CmdRun(args ...string) error { return nil } - if *flDetach { + if !*flDetach { + if err := cli.CheckTtyInput(config.AttachStdin, config.Tty); err != nil { + return err + } + } else { if fl := cmd.Lookup("attach"); fl != nil { flAttach = fl.Value.(*opts.ListOpts) if flAttach.Len() != 0 { @@ -2600,7 +2608,11 @@ func (cli *DockerCli) CmdExec(args ...string) error { return nil } - if execConfig.Detach { + if !execConfig.Detach { + if err := cli.CheckTtyInput(execConfig.AttachStdin, execConfig.Tty); err != nil { + return err + } + } else { if _, _, err := readBody(cli.call("POST", "/exec/"+execID+"/start", execConfig, false)); err != nil { return err } diff --git a/docs/man/docker-attach.1.md b/docs/man/docker-attach.1.md index 21bd566406..19fbaceb4a 100644 --- a/docs/man/docker-attach.1.md +++ b/docs/man/docker-attach.1.md @@ -20,6 +20,9 @@ container, or `CTRL-\` to get a stacktrace of the Docker client when it quits. When you detach from a container the exit code will be returned to the client. +It is forbidden to redirect the standard input of a docker attach command while +attaching to a tty-enabled container (i.e.: launched with -t`). + # OPTIONS **--no-stdin**=*true*|*false* Do not attach STDIN. The default is *false*. diff --git a/docs/man/docker-exec.1.md b/docs/man/docker-exec.1.md index c4e649016a..3db296ed76 100644 --- a/docs/man/docker-exec.1.md +++ b/docs/man/docker-exec.1.md @@ -31,5 +31,8 @@ container is unpaused, and then run **-t**, **--tty**=*true*|*false* Allocate a pseudo-TTY. The default is *false*. +The **-t** option is incompatible with a redirection of the docker client +standard input. + # HISTORY November 2014, updated by Sven Dowideit diff --git a/docs/man/docker-run.1.md b/docs/man/docker-run.1.md index f0129bedc9..44c5545084 100644 --- a/docs/man/docker-run.1.md +++ b/docs/man/docker-run.1.md @@ -267,6 +267,9 @@ outside of a container on the host. input of any container. This can be used, for example, to run a throwaway interactive shell. The default is value is false. +The **-t** option is incompatible with a redirection of the docker client +standard input. + **-u**, **--user**="" Username or UID diff --git a/docs/sources/reference/run.md b/docs/sources/reference/run.md index 8ac9f9d789..74a567c00b 100644 --- a/docs/sources/reference/run.md +++ b/docs/sources/reference/run.md @@ -94,9 +94,10 @@ specify to which of the three standard streams (`STDIN`, `STDOUT`, $ sudo docker run -a stdin -a stdout -i -t ubuntu /bin/bash -For interactive processes (like a shell) you will typically want a tty -as well as persistent standard input (`STDIN`), so you'll use `-i -t` -together in most interactive cases. +For interactive processes (like a shell), you must use `-i -t` together in +order to allocate a tty for the container process. Specifying `-t` is however +forbidden when the client standard output is redirected or pipe, such as in: +`echo test | docker run -i busybox cat`. ## Container identification diff --git a/integration-cli/docker_cli_attach_test.go b/integration-cli/docker_cli_attach_test.go index d03a986e48..0530d3896e 100644 --- a/integration-cli/docker_cli_attach_test.go +++ b/integration-cli/docker_cli_attach_test.go @@ -87,3 +87,50 @@ func TestAttachMultipleAndRestart(t *testing.T) { logDone("attach - multiple attach") } + +func TestAttachTtyWithoutStdin(t *testing.T) { + defer deleteAllContainers() + + cmd := exec.Command(dockerBinary, "run", "-d", "-ti", "busybox") + out, _, err := runCommandWithOutput(cmd) + if err != nil { + t.Fatalf("failed to start container: %v (%v)", out, err) + } + + id := strings.TrimSpace(out) + if err := waitRun(id); err != nil { + t.Fatal(err) + } + + defer func() { + cmd := exec.Command(dockerBinary, "kill", id) + if out, _, err := runCommandWithOutput(cmd); err != nil { + t.Fatalf("failed to kill container: %v (%v)", out, err) + } + }() + + done := make(chan struct{}) + go func() { + defer close(done) + + cmd := exec.Command(dockerBinary, "attach", id) + if _, err := cmd.StdinPipe(); err != nil { + t.Fatal(err) + } + + expected := "cannot enable tty mode" + if out, _, err := runCommandWithOutput(cmd); err == nil { + t.Fatal("attach should have failed") + } else if !strings.Contains(out, expected) { + t.Fatal("attach failed with error %q: expected %q", out, expected) + } + }() + + select { + case <-done: + case <-time.After(attachWait): + t.Fatal("attach is running but should have failed") + } + + logDone("attach - forbid piped stdin to tty enabled container") +} diff --git a/integration-cli/docker_cli_exec_test.go b/integration-cli/docker_cli_exec_test.go index 82ad9afe7b..b07f215a36 100644 --- a/integration-cli/docker_cli_exec_test.go +++ b/integration-cli/docker_cli_exec_test.go @@ -263,3 +263,91 @@ func TestExecPausedContainer(t *testing.T) { logDone("exec - exec should not exec a pause container") } + +// regression test for #9476 +func TestExecTtyCloseStdin(t *testing.T) { + defer deleteAllContainers() + + cmd := exec.Command(dockerBinary, "run", "-d", "-it", "--name", "exec_tty_stdin", "busybox") + if out, _, err := runCommandWithOutput(cmd); err != nil { + t.Fatal(out, err) + } + + cmd = exec.Command(dockerBinary, "exec", "-i", "exec_tty_stdin", "cat") + stdinRw, err := cmd.StdinPipe() + if err != nil { + t.Fatal(err) + } + + stdinRw.Write([]byte("test")) + stdinRw.Close() + + if out, _, err := runCommandWithOutput(cmd); err != nil { + t.Fatal(out, err) + } + + cmd = exec.Command(dockerBinary, "top", "exec_tty_stdin") + out, _, err := runCommandWithOutput(cmd) + if err != nil { + t.Fatal(out, err) + } + + outArr := strings.Split(out, "\n") + if len(outArr) > 3 || strings.Contains(out, "nsenter-exec") { + // This is the really bad part + if out, _, err := runCommandWithOutput(exec.Command(dockerBinary, "rm", "-f", "exec_tty_stdin")); err != nil { + t.Fatal(out, err) + } + + t.Fatalf("exec process left running\n\t %s", out) + } + + logDone("exec - stdin is closed properly with tty enabled") +} + +func TestExecTtyWithoutStdin(t *testing.T) { + defer deleteAllContainers() + + cmd := exec.Command(dockerBinary, "run", "-d", "-ti", "busybox") + out, _, err := runCommandWithOutput(cmd) + if err != nil { + t.Fatalf("failed to start container: %v (%v)", out, err) + } + + id := strings.TrimSpace(out) + if err := waitRun(id); err != nil { + t.Fatal(err) + } + + defer func() { + cmd := exec.Command(dockerBinary, "kill", id) + if out, _, err := runCommandWithOutput(cmd); err != nil { + t.Fatalf("failed to kill container: %v (%v)", out, err) + } + }() + + done := make(chan struct{}) + go func() { + defer close(done) + + cmd := exec.Command(dockerBinary, "exec", "-ti", id, "true") + if _, err := cmd.StdinPipe(); err != nil { + t.Fatal(err) + } + + expected := "cannot enable tty mode" + if out, _, err := runCommandWithOutput(cmd); err == nil { + t.Fatal("exec should have failed") + } else if !strings.Contains(out, expected) { + t.Fatal("exec failed with error %q: expected %q", out, expected) + } + }() + + select { + case <-done: + case <-time.After(3 * time.Second): + t.Fatal("exec is running but should have failed") + } + + logDone("exec - forbid piped stdin to tty enabled container") +} diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 20a096f196..0b56f235fe 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -2742,3 +2742,32 @@ func TestRunPortFromDockerRangeInUse(t *testing.T) { logDone("run - find another port if port from autorange already bound") } + +func TestRunTtyWithPipe(t *testing.T) { + defer deleteAllContainers() + + done := make(chan struct{}) + go func() { + defer close(done) + + cmd := exec.Command(dockerBinary, "run", "-ti", "busybox", "true") + if _, err := cmd.StdinPipe(); err != nil { + t.Fatal(err) + } + + expected := "cannot enable tty mode" + if out, _, err := runCommandWithOutput(cmd); err == nil { + t.Fatal("run should have failed") + } else if !strings.Contains(out, expected) { + t.Fatal("run failed with error %q: expected %q", out, expected) + } + }() + + select { + case <-done: + case <-time.After(3 * time.Second): + t.Fatal("container is running but should have failed") + } + + logDone("run - forbid piped stdin with tty") +} diff --git a/integration/commands_test.go b/integration/commands_test.go index b00c68641e..aa21791b50 100644 --- a/integration/commands_test.go +++ b/integration/commands_test.go @@ -15,6 +15,7 @@ import ( "github.com/docker/docker/pkg/term" "github.com/docker/docker/utils" "github.com/docker/libtrust" + "github.com/kr/pty" ) func closeWrap(args ...io.Closer) error { @@ -162,72 +163,20 @@ func TestRunDisconnect(t *testing.T) { }) } -// Expected behaviour: the process stay alive when the client disconnects -// but the client detaches. -func TestRunDisconnectTty(t *testing.T) { - - stdin, stdinPipe := io.Pipe() - stdout, stdoutPipe := io.Pipe() - key, err := libtrust.GenerateECP256PrivateKey() - if err != nil { - t.Fatal(err) - } - - cli := client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) - defer cleanup(globalEngine, t) - - c1 := make(chan struct{}) - go func() { - defer close(c1) - // We're simulating a disconnect so the return value doesn't matter. What matters is the - // fact that CmdRun returns. - if err := cli.CmdRun("-i", "-t", unitTestImageID, "/bin/cat"); err != nil { - log.Debugf("Error CmdRun: %s", err) - } - }() - - container := waitContainerStart(t, 10*time.Second) - - state := setRaw(t, container) - defer unsetRaw(t, container, state) - - // Client disconnect after run -i should keep stdin out in TTY mode - setTimeout(t, "Read/Write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { - t.Fatal(err) - } - }) - - // Close pipes (simulate disconnect) - if err := closeWrap(stdin, stdinPipe, stdout, stdoutPipe); err != nil { - t.Fatal(err) - } - - // wait for CmdRun to return - setTimeout(t, "Waiting for CmdRun timed out", 5*time.Second, func() { - <-c1 - }) - - // In tty mode, we expect the process to stay alive even after client's stdin closes. - - // Give some time to monitor to do his thing - container.WaitStop(500 * time.Millisecond) - if !container.IsRunning() { - t.Fatalf("/bin/cat should still be running after closing stdin (tty mode)") - } -} - // TestRunDetach checks attaching and detaching with the escape sequence. func TestRunDetach(t *testing.T) { - - stdin, stdinPipe := io.Pipe() stdout, stdoutPipe := io.Pipe() + cpty, tty, err := pty.Open() + if err != nil { + t.Fatal(err) + } + key, err := libtrust.GenerateECP256PrivateKey() if err != nil { t.Fatal(err) } - cli := client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) + cli := client.NewDockerCli(tty, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) defer cleanup(globalEngine, t) ch := make(chan struct{}) @@ -242,22 +191,22 @@ func TestRunDetach(t *testing.T) { defer unsetRaw(t, container, state) setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { + if err := assertPipe("hello\n", "hello", stdout, cpty, 150); err != nil { t.Fatal(err) } }) setTimeout(t, "Escape sequence timeout", 5*time.Second, func() { - stdinPipe.Write([]byte{16}) + cpty.Write([]byte{16}) time.Sleep(100 * time.Millisecond) - stdinPipe.Write([]byte{17}) + cpty.Write([]byte{17}) }) // wait for CmdRun to return setTimeout(t, "Waiting for CmdRun timed out", 15*time.Second, func() { <-ch }) - closeWrap(stdin, stdinPipe, stdout, stdoutPipe) + closeWrap(cpty, stdout, stdoutPipe) time.Sleep(500 * time.Millisecond) if !container.IsRunning() { @@ -271,14 +220,18 @@ func TestRunDetach(t *testing.T) { // TestAttachDetach checks that attach in tty mode can be detached using the long container ID func TestAttachDetach(t *testing.T) { - stdin, stdinPipe := io.Pipe() stdout, stdoutPipe := io.Pipe() + cpty, tty, err := pty.Open() + if err != nil { + t.Fatal(err) + } + key, err := libtrust.GenerateECP256PrivateKey() if err != nil { t.Fatal(err) } - cli := client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) + cli := client.NewDockerCli(tty, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) defer cleanup(globalEngine, t) ch := make(chan struct{}) @@ -309,9 +262,13 @@ func TestAttachDetach(t *testing.T) { state := setRaw(t, container) defer unsetRaw(t, container, state) - stdin, stdinPipe = io.Pipe() stdout, stdoutPipe = io.Pipe() - cli = client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) + cpty, tty, err = pty.Open() + if err != nil { + t.Fatal(err) + } + + cli = client.NewDockerCli(tty, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) ch = make(chan struct{}) go func() { @@ -324,7 +281,7 @@ func TestAttachDetach(t *testing.T) { }() setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { + if err := assertPipe("hello\n", "hello", stdout, cpty, 150); err != nil { if err != io.ErrClosedPipe { t.Fatal(err) } @@ -332,9 +289,9 @@ func TestAttachDetach(t *testing.T) { }) setTimeout(t, "Escape sequence timeout", 5*time.Second, func() { - stdinPipe.Write([]byte{16}) + cpty.Write([]byte{16}) time.Sleep(100 * time.Millisecond) - stdinPipe.Write([]byte{17}) + cpty.Write([]byte{17}) }) // wait for CmdRun to return @@ -342,7 +299,7 @@ func TestAttachDetach(t *testing.T) { <-ch }) - closeWrap(stdin, stdinPipe, stdout, stdoutPipe) + closeWrap(cpty, stdout, stdoutPipe) time.Sleep(500 * time.Millisecond) if !container.IsRunning() { @@ -356,14 +313,18 @@ func TestAttachDetach(t *testing.T) { // TestAttachDetachTruncatedID checks that attach in tty mode can be detached func TestAttachDetachTruncatedID(t *testing.T) { - stdin, stdinPipe := io.Pipe() stdout, stdoutPipe := io.Pipe() + cpty, tty, err := pty.Open() + if err != nil { + t.Fatal(err) + } + key, err := libtrust.GenerateECP256PrivateKey() if err != nil { t.Fatal(err) } - cli := client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) + cli := client.NewDockerCli(tty, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) defer cleanup(globalEngine, t) // Discard the CmdRun output @@ -379,9 +340,13 @@ func TestAttachDetachTruncatedID(t *testing.T) { state := setRaw(t, container) defer unsetRaw(t, container, state) - stdin, stdinPipe = io.Pipe() stdout, stdoutPipe = io.Pipe() - cli = client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) + cpty, tty, err = pty.Open() + if err != nil { + t.Fatal(err) + } + + cli = client.NewDockerCli(tty, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) ch := make(chan struct{}) go func() { @@ -394,7 +359,7 @@ func TestAttachDetachTruncatedID(t *testing.T) { }() setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { + if err := assertPipe("hello\n", "hello", stdout, cpty, 150); err != nil { if err != io.ErrClosedPipe { t.Fatal(err) } @@ -402,16 +367,16 @@ func TestAttachDetachTruncatedID(t *testing.T) { }) setTimeout(t, "Escape sequence timeout", 5*time.Second, func() { - stdinPipe.Write([]byte{16}) + cpty.Write([]byte{16}) time.Sleep(100 * time.Millisecond) - stdinPipe.Write([]byte{17}) + cpty.Write([]byte{17}) }) // wait for CmdRun to return setTimeout(t, "Waiting for CmdAttach timed out", 15*time.Second, func() { <-ch }) - closeWrap(stdin, stdinPipe, stdout, stdoutPipe) + closeWrap(cpty, stdout, stdoutPipe) time.Sleep(500 * time.Millisecond) if !container.IsRunning() { @@ -425,14 +390,18 @@ func TestAttachDetachTruncatedID(t *testing.T) { // Expected behaviour, the process stays alive when the client disconnects func TestAttachDisconnect(t *testing.T) { - stdin, stdinPipe := io.Pipe() stdout, stdoutPipe := io.Pipe() + cpty, tty, err := pty.Open() + if err != nil { + t.Fatal(err) + } + key, err := libtrust.GenerateECP256PrivateKey() if err != nil { t.Fatal(err) } - cli := client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) + cli := client.NewDockerCli(tty, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil) defer cleanup(globalEngine, t) go func() { @@ -470,12 +439,12 @@ func TestAttachDisconnect(t *testing.T) { }() setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { + if err := assertPipe("hello\n", "hello", stdout, cpty, 150); err != nil { t.Fatal(err) } }) // Close pipes (client disconnects) - if err := closeWrap(stdin, stdinPipe, stdout, stdoutPipe); err != nil { + if err := closeWrap(cpty, stdout, stdoutPipe); err != nil { t.Fatal(err) }