Merge pull request #8233 from tiborvass/pr-7658

Fix Interactive container hangs when redirecting stdout

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 {
@@ -104,24 +112,34 @@ func NewDockerCli(in io.ReadCloser, out, err io.Writer, proto, addr string, tlsC
 	}
 
 	if in != nil {
+		if file, ok := in.(*os.File); ok {
+			inFd = file.Fd()
+			isTerminalIn = term.IsTerminal(inFd)
+		}
+	}
+
+	if out != nil {
 		if file, ok := out.(*os.File); ok {
-			terminalFd = file.Fd()
-			isTerminal = term.IsTerminal(terminalFd)
+			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,
 	}
 }

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")
 			}
@@ -669,7 +669,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)
 			}
@@ -1821,7 +1821,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)
 		}
@@ -2241,7 +2241,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)
 		}
@@ -2490,7 +2490,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)
 		}

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

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 {

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")
+}

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
@@ -2182,3 +2183,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")
+}