From 6cbca87a62683a0342b3bf9265dc833cf7f3e918 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 3 Jun 2018 06:46:28 +0100 Subject: [PATCH] Try harder not to call connection_fatal twice. If the server sends an SSH_MSG_DISCONNECT, then we call connection_fatal(). But if the server closes the network connection, then we call connection_fatal(). In situations where the former happens, the latter happens too. Currently, calling connection_fatal twice is especially bad on GTK because all dialogs are now non-modal and an assertion fails in the GTK front end when two fatal message boxes try to exist at the same time (the register_dialog system finds that slot is already occupied). But regardless of that, we'd rather not even _try_ to print two fatal boxes, because even if the front end doesn't fail an assertion, there's no guarantee that the _more useful_ one of the messages will end up being displayed. So a better fix is to have ssh.c make a sensible decision about which message is the helpful one - in this case, the actual error message out of the SSH_MSG_DISCONNECT, rather than the predictable fact of the connection having been slammed shut immediately afterwards - and only pass that one to the front end in the first place. --- ssh.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/ssh.c b/ssh.c index 012d7062..547ffb5b 100644 --- a/ssh.c +++ b/ssh.c @@ -930,6 +930,7 @@ struct ssh_tag { int exitcode; int close_expected; int clean_exit; + int disconnect_message_seen; tree234 *rportfwds, *portfwds; @@ -1593,6 +1594,20 @@ static void ssh1_rdpkt(Ssh ssh) BinarySource_INIT(st->pktin, st->pktin->body, st->pktin->length); + /* + * Mild layer violation: if the message is a DISCONNECT, we + * should unset the close_expected flag, because now we _do_ + * expect the server to close the network connection + * afterwards. That way, the more informative connection_fatal + * message for the disconnect itself won't fight with 'Server + * unexpectedly closed network connection'. + */ + if (st->pktin->type == SSH1_MSG_DISCONNECT) { + ssh->clean_exit = FALSE; + ssh->close_expected = TRUE; + ssh->disconnect_message_seen = TRUE; + } + pq_push(&ssh->pq_full, st->pktin); queue_idempotent_callback(&ssh->pq_full_consumer); } @@ -2015,6 +2030,20 @@ static void ssh2_rdpkt(Ssh ssh) BinarySource_INIT(st->pktin, st->pktin->body, st->pktin->length); + /* + * Mild layer violation: if the message is a DISCONNECT, we + * should unset the close_expected flag, because now we _do_ + * expect the server to close the network connection + * afterwards. That way, the more informative connection_fatal + * message for the disconnect itself won't fight with 'Server + * unexpectedly closed network connection'. + */ + if (st->pktin->type == SSH2_MSG_DISCONNECT) { + ssh->clean_exit = FALSE; + ssh->close_expected = TRUE; + ssh->disconnect_message_seen = TRUE; + } + pq_push(&ssh->pq_full, st->pktin); queue_idempotent_callback(&ssh->pq_full_consumer); if (st->pktin->type == SSH2_MSG_NEWKEYS) { @@ -2079,6 +2108,20 @@ static void ssh2_bare_connection_rdpkt(Ssh ssh) BinarySource_INIT(st->pktin, st->pktin->body, st->pktin->length); + /* + * Mild layer violation: if the message is a DISCONNECT, we + * should unset the close_expected flag, because now we _do_ + * expect the server to close the network connection + * afterwards. That way, the more informative connection_fatal + * message for the disconnect itself won't fight with 'Server + * unexpectedly closed network connection'. + */ + if (st->pktin->type == SSH2_MSG_DISCONNECT) { + ssh->clean_exit = FALSE; + ssh->close_expected = TRUE; + ssh->disconnect_message_seen = TRUE; + } + pq_push(&ssh->pq_full, st->pktin); queue_idempotent_callback(&ssh->pq_full_consumer); } @@ -3379,7 +3422,8 @@ static void ssh_process_incoming_data(void *ctx) if (error_msg) logevent(error_msg); - if (!ssh->close_expected || !ssh->clean_exit) + if ((!ssh->close_expected || !ssh->clean_exit) && + !ssh->disconnect_message_seen) connection_fatal(ssh->frontend, "%s", error_msg); } } @@ -11959,6 +12003,7 @@ static const char *ssh_init(void *frontend_handle, void **backend_handle, ssh->exitcode = -1; ssh->close_expected = FALSE; ssh->clean_exit = FALSE; + ssh->disconnect_message_seen = FALSE; ssh->state = SSH_STATE_PREPACKET; ssh->size_needed = FALSE; ssh->eof_needed = FALSE;