From cbbb1a80f5ccbbec5496f253cabce0f65df0f656 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 12 Jul 2021 17:17:47 -0400 Subject: [PATCH] Perform a one-sided close of HTTP attach conn on EOF On EOF of STDIN, we need to perform a one-sided close of the attach connection on the client side, to ensure that STDIN finishing will also cause the exec session to terminate, instead of hang. Fixes #7360 Signed-off-by: Matthew Heon --- libpod/oci_conmon_exec_linux.go | 3 +++ pkg/bindings/containers/attach.go | 19 +++++++++++++++++++ test/system/075-exec.bats | 2 -- test/system/500-networking.bats | 1 - 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go index 09d3d18334..05a4e19b0e 100644 --- a/libpod/oci_conmon_exec_linux.go +++ b/libpod/oci_conmon_exec_linux.go @@ -610,6 +610,9 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp _, err := utils.CopyDetachable(conn, httpBuf, detachKeys) logrus.Debugf("STDIN copy completed") stdinChan <- err + if connErr := conn.CloseWrite(); connErr != nil { + logrus.Errorf("Unable to close conn: %v", connErr) + } }() } diff --git a/pkg/bindings/containers/attach.go b/pkg/bindings/containers/attach.go index cc12c8ab7f..01c14d350a 100644 --- a/pkg/bindings/containers/attach.go +++ b/pkg/bindings/containers/attach.go @@ -25,6 +25,12 @@ import ( "golang.org/x/crypto/ssh/terminal" ) +// The CloseWriter interface is used to determine whether we can do a one-sided +// close of a hijacked connection. +type CloseWriter interface { + CloseWrite() error +} + // Attach attaches to a running container func Attach(ctx context.Context, nameOrID string, stdin io.Reader, stdout io.Writer, stderr io.Writer, attachReady chan bool, options *AttachOptions) error { if options == nil { @@ -161,6 +167,12 @@ func Attach(ctx context.Context, nameOrID string, stdin io.Reader, stdout io.Wri logrus.Error("failed to write input to service: " + err.Error()) } stdinChan <- err + + if closeWrite, ok := socket.(CloseWriter); ok { + if err := closeWrite.CloseWrite(); err != nil { + logrus.Warnf("Failed to close STDIN for writing: %v", err) + } + } }() } @@ -485,6 +497,13 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, options *ExecStar if err != nil { logrus.Error("failed to write input to service: " + err.Error()) } + + if closeWrite, ok := socket.(CloseWriter); ok { + logrus.Debugf("Closing STDIN") + if err := closeWrite.CloseWrite(); err != nil { + logrus.Warnf("Failed to close STDIN for writing: %v", err) + } + } }() } diff --git a/test/system/075-exec.bats b/test/system/075-exec.bats index badf44c490..3e8c3c1ea3 100644 --- a/test/system/075-exec.bats +++ b/test/system/075-exec.bats @@ -59,8 +59,6 @@ load helpers # Issue #4785 - piping to exec statement - fixed in #4818 # Issue #5046 - piping to exec truncates results (actually a conmon issue) @test "podman exec - cat from stdin" { - skip_if_remote "FIXME: pending #7360" - run_podman run -d $IMAGE sh -c 'while [ ! -e /stop ]; do sleep 0.1;done' cid="$output" diff --git a/test/system/500-networking.bats b/test/system/500-networking.bats index d55a786f79..4feb578077 100644 --- a/test/system/500-networking.bats +++ b/test/system/500-networking.bats @@ -20,7 +20,6 @@ load helpers # Copied from tsweeney's https://github.com/containers/podman/issues/4827 @test "podman networking: port on localhost" { - skip_if_remote "FIXME: reevaluate this one after #7360 is fixed" random_1=$(random_string 30) random_2=$(random_string 30)