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 <[email protected]>
containers · Jul 13, 2021 · cbbb1a8 · cbbb1a8
1 parent 31c3b95
commit cbbb1a8
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)