x/crypto/ssh: handle missing exit status more gracefully.

According to RFC 4254 section 6.10, SSH server implementations may
omit the exit-status and exit-signal messages.  If this happens, we
now return &ExitMissingError{}, so clients can handle this case
specifically.

This came up in the discussion of issue #16194.

Change-Id: Iae5e916b18aa5bd8e95618e9fcfcab8b19e147d9
Reviewed-on: https://go-review.googlesource.com/24727
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh/session.go

@@ -282,9 +282,10 @@ func (s *Session) Start(cmd string) error {
 // copying stdin, stdout, and stderr, and exits with a zero exit
 // status.
 //
-// If the command fails to run or doesn't complete successfully, the
-// error is of type *ExitError. Other error types may be
-// returned for I/O problems.
+// If the remote server does not send an exit status, an error of type
+// *ExitMissingError is returned. If the command completes
+// unsuccessfully or is interrupted by a signal, the error is of type
+// *ExitError. Other error types may be returned for I/O problems.
 func (s *Session) Run(cmd string) error {
 	err := s.Start(cmd)
 	if err != nil {
@@ -371,9 +372,10 @@ func (s *Session) start() error {
 // copying stdin, stdout, and stderr, and exits with a zero exit
 // status.
 //
-// If the command fails to run or doesn't complete successfully, the
-// error is of type *ExitError. Other error types may be
-// returned for I/O problems.
+// If the remote server does not send an exit status, an error of type
+// *ExitMissingError is returned. If the command completes
+// unsuccessfully or is interrupted by a signal, the error is of type
+// *ExitError. Other error types may be returned for I/O problems.
 func (s *Session) Wait() error {
 	if !s.started {
 		return errors.New("ssh: session not started")
@@ -431,16 +433,29 @@ func (s *Session) wait(reqs <-chan *Request) error {
 	if wm.status == -1 {
 		// exit-status was never sent from server
 		if wm.signal == "" {
-			return errors.New("wait: remote command exited without exit status or exit signal")
+			// signal was not sent either.  RFC 4254
+			// section 6.10 recommends against this
+			// behavior, but it is allowed, so we let
+			// clients handle it.
+			return &ExitMissingError{}
 		}
 		wm.status = 128
 		if _, ok := signals[Signal(wm.signal)]; ok {
 			wm.status += signals[Signal(wm.signal)]
 		}
 	}
+
 	return &ExitError{wm}
 }
 
+// ExitMissingError is returned if a session is torn down cleanly, but
+// the server sends no confirmation of the exit status.
+type ExitMissingError struct{}
+
+func (e *ExitMissingError) Error() string {
+	return "wait: remote command exited without exit status or exit signal"
+}
+
 func (s *Session) stdin() {
 	if s.stdinpipe {
 		return

ssh/session_test.go

@@ -297,7 +297,6 @@ func TestUnknownExitSignal(t *testing.T) {
 	}
 }
 
-// Test WaitMsg is not returned if the channel closes abruptly.
 func TestExitWithoutStatusOrSignal(t *testing.T) {
 	conn := dial(exitWithoutSignalOrStatus, t)
 	defer conn.Close()
@@ -313,11 +312,8 @@ func TestExitWithoutStatusOrSignal(t *testing.T) {
 	if err == nil {
 		t.Fatalf("expected command to fail but it didn't")
 	}
-	_, ok := err.(*ExitError)
-	if ok {
-		// you can't actually test for errors.errorString
-		// because it's not exported.
-		t.Fatalf("expected *errorString but got %T", err)
+	if _, ok := err.(*ExitMissingError); !ok {
+		t.Fatalf("got %T want *ExitMissingError", err)
 	}
 }