HTTP Attach: Wait until both STDIN and STDOUT finish

In the old code, there was a chance that we could return when
only one of STDIN or STDOUT had finished - this could lead to us
dropping either input to the container, or output from it, in the
case that one stream terminated early.

To resolve this, use separate channels to return STDOUT and STDIN
errors, and track which ones have returned cleanly to ensure that
we need bith in order to return from the HTTP attach function and
pass control back to the HTTP handler (which would assume we
exited cleanly and close the client's attach connection).

Signed-off-by: Matthew Heon <mheon@redhat.com>
This commit is contained in:
Matthew Heon
2020-09-23 16:30:51 -04:00
parent 08cc91926d
commit 00cca405d2
2 changed files with 32 additions and 22 deletions

View File

@ -537,9 +537,6 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp
}
}()
// Make a channel to pass errors back
errChan := make(chan error)
attachStdout := true
attachStderr := true
attachStdin := true
@ -580,13 +577,16 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp
hijackWriteErrorAndClose(deferredErr, c.ID(), isTerminal, httpCon, httpBuf)
}()
stdoutChan := make(chan error)
stdinChan := make(chan error)
// Next, STDIN. Avoid entirely if attachStdin unset.
if attachStdin {
go func() {
logrus.Debugf("Beginning STDIN copy")
_, err := utils.CopyDetachable(conn, httpBuf, detachKeys)
logrus.Debugf("STDIN copy completed")
errChan <- err
stdinChan <- err
}()
}
@ -613,19 +613,24 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp
logrus.Debugf("Performing non-terminal HTTP attach for container %s", c.ID())
err = httpAttachNonTerminalCopy(conn, httpBuf, c.ID(), attachStdin, attachStdout, attachStderr)
}
errChan <- err
stdoutChan <- err
logrus.Debugf("STDOUT/ERR copy completed")
}()
if cancel != nil {
for {
select {
case err := <-errChan:
return err
case err := <-stdoutChan:
if err != nil {
return err
}
return nil
case err := <-stdinChan:
if err != nil {
return err
}
case <-cancel:
return nil
}
} else {
var connErr error = <-errChan
return connErr
}
}

View File

@ -555,9 +555,6 @@ func (r *ConmonOCIRuntime) HTTPAttach(ctr *Container, req *http.Request, w http.
return err
}
// Make a channel to pass errors back
errChan := make(chan error)
attachStdout := true
attachStderr := true
attachStdin := true
@ -672,6 +669,9 @@ func (r *ConmonOCIRuntime) HTTPAttach(ctr *Container, req *http.Request, w http.
logrus.Debugf("Forwarding attach output for container %s", ctr.ID())
stdoutChan := make(chan error)
stdinChan := make(chan error)
// Handle STDOUT/STDERR
go func() {
var err error
@ -690,7 +690,7 @@ func (r *ConmonOCIRuntime) HTTPAttach(ctr *Container, req *http.Request, w http.
logrus.Debugf("Performing non-terminal HTTP attach for container %s", ctr.ID())
err = httpAttachNonTerminalCopy(conn, httpBuf, ctr.ID(), attachStdin, attachStdout, attachStderr)
}
errChan <- err
stdoutChan <- err
logrus.Debugf("STDOUT/ERR copy completed")
}()
// Next, STDIN. Avoid entirely if attachStdin unset.
@ -698,20 +698,25 @@ func (r *ConmonOCIRuntime) HTTPAttach(ctr *Container, req *http.Request, w http.
go func() {
_, err := utils.CopyDetachable(conn, httpBuf, detach)
logrus.Debugf("STDIN copy completed")
errChan <- err
stdinChan <- err
}()
}
if cancel != nil {
for {
select {
case err := <-errChan:
return err
case err := <-stdoutChan:
if err != nil {
return err
}
return nil
case err := <-stdinChan:
if err != nil {
return err
}
case <-cancel:
return nil
}
} else {
var connErr error = <-errChan
return connErr
}
}