From d81fdb778bf2c40a91b24519d60cdc5767318829 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Tue, 5 Jul 2016 18:05:20 +0200 Subject: [PATCH] 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 Run-TryBot: Han-Wen Nienhuys TryBot-Result: Gobot Gobot --- ssh/session.go | 29 ++++++++++++++++++++++------- ssh/session_test.go | 8 ++------ 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/ssh/session.go b/ssh/session.go index a2b8ef4..17e2aa8 100644 --- a/ssh/session.go +++ b/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 diff --git a/ssh/session_test.go b/ssh/session_test.go index f7f0f76..f35a378 100644 --- a/ssh/session_test.go +++ b/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) } }