1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-10 01:48:00 +00:00

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.
This commit is contained in:
Simon Tatham 2018-06-03 06:46:28 +01:00
parent 6dc6392596
commit 6cbca87a62

47
ssh.c
View File

@ -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;