SSH: corrected the inability to respect the timeout

Jason McDonald posted bug report #3006786 when he found that the
SFTP code didn't timeout properly in several places in the code
even if a timeout was set properly.

Based on his suggested patch, I wrote a different implementation
that I think addressed the issue better and also uses the connect
timeout for the initial part of the SSH/SFTP done during the
"protocol connect" phase.

(http://curl.haxx.se/bug/view.cgi?id=3006786)
This commit is contained in:
Daniel Stenberg 2010-06-02 23:33:51 +02:00
parent 51248a9bdd
commit 684830cb2a
3 changed files with 35 additions and 9 deletions

11
CHANGES
View File

@ -6,6 +6,17 @@
Changelog Changelog
Daniel Stenberg (2 June 2010)
- Jason McDonald posted bug report #3006786 when he found that the SFTP code
didn't timeout properly in several places in the code even if a timeout was
set properly.
Based on his suggested patch, I wrote a different implementation that I
think addressed the issue better and also uses the connect timeout for the
initial part of the SSH/SFTP done during the "protocol connect" phase.
(http://curl.haxx.se/bug/view.cgi?id=3006786)
Yang Tse (2 June 2010) Yang Tse (2 June 2010)
- Added missing new libcurl files to non-configure targets. Adjusted - Added missing new libcurl files to non-configure targets. Adjusted
libcurl standard internal header inclusions in new files. Fixed an libcurl standard internal header inclusions in new files. Fixed an

View File

@ -34,6 +34,7 @@ This release includes the following bugfixes:
o TFTP timeout option sent correctly o TFTP timeout option sent correctly
o TFTP block id wrap o TFTP block id wrap
o curl_multi_socket_action() timeout handles inaccuracy in timers better o curl_multi_socket_action() timeout handles inaccuracy in timers better
o SCP/SFTP failure to respect the timeout
This release includes the following known bugs: This release includes the following known bugs:
@ -45,6 +46,6 @@ advice from friends like these:
Rainer Canavan, Paul Howarth, Jerome Vouillon, Ruslan Gazizov, Yang Tse, Rainer Canavan, Paul Howarth, Jerome Vouillon, Ruslan Gazizov, Yang Tse,
Kamil Dudka, Alex Bligh, Ben Greear, Hoi-Ho Chan, Howard Chu, Dirk Manske, Kamil Dudka, Alex Bligh, Ben Greear, Hoi-Ho Chan, Howard Chu, Dirk Manske,
Pavel Raiskup, John-Mark Bell, Eric Mertens, Tor Arntsen, Douglas Kilpatrick, Pavel Raiskup, John-Mark Bell, Eric Mertens, Tor Arntsen, Douglas Kilpatrick,
Igor Novoseltsev Igor Novoseltsev, Jason McDonald
Thanks! (and sorry if I forgot to mention someone) Thanks! (and sorry if I forgot to mention someone)

View File

@ -2408,15 +2408,28 @@ static CURLcode ssh_multi_statemach(struct connectdata *conn, bool *done)
return result; return result;
} }
static CURLcode ssh_easy_statemach(struct connectdata *conn) static CURLcode ssh_easy_statemach(struct connectdata *conn,
bool duringconnect)
{ {
struct ssh_conn *sshc = &conn->proto.sshc; struct ssh_conn *sshc = &conn->proto.sshc;
CURLcode result = CURLE_OK; CURLcode result = CURLE_OK;
struct SessionHandle *data = conn->data;
while((sshc->state != SSH_STOP) && !result) { while((sshc->state != SSH_STOP) && !result) {
bool block; bool block;
long left;
result = ssh_statemach_act(conn, &block); result = ssh_statemach_act(conn, &block);
if(Curl_pgrsUpdate(conn))
return CURLE_ABORTED_BY_CALLBACK;
left = Curl_timeleft(conn, NULL, duringconnect);
if(left < 0) {
failf(data, "Operation timed out\n");
return CURLE_OPERATION_TIMEDOUT;
}
#ifdef HAVE_LIBSSH2_SESSION_BLOCK_DIRECTION #ifdef HAVE_LIBSSH2_SESSION_BLOCK_DIRECTION
if((CURLE_OK == result) && block) { if((CURLE_OK == result) && block) {
int dir = libssh2_session_block_directions(sshc->ssh_session); int dir = libssh2_session_block_directions(sshc->ssh_session);
@ -2430,7 +2443,8 @@ static CURLcode ssh_easy_statemach(struct connectdata *conn)
fd_write = sock; fd_write = sock;
} }
/* wait for the socket to become ready */ /* wait for the socket to become ready */
Curl_socket_ready(fd_read, fd_write, 1000); /* ignore result */ Curl_socket_ready(fd_read, fd_write,
left>1000?1000:left); /* ignore result */
} }
#endif #endif
@ -2548,7 +2562,7 @@ static CURLcode ssh_connect(struct connectdata *conn, bool *done)
if(data->state.used_interface == Curl_if_multi) if(data->state.used_interface == Curl_if_multi)
result = ssh_multi_statemach(conn, done); result = ssh_multi_statemach(conn, done);
else { else {
result = ssh_easy_statemach(conn); result = ssh_easy_statemach(conn, TRUE);
if(!result) if(!result)
*done = TRUE; *done = TRUE;
} }
@ -2584,7 +2598,7 @@ CURLcode scp_perform(struct connectdata *conn,
result = ssh_multi_statemach(conn, dophase_done); result = ssh_multi_statemach(conn, dophase_done);
} }
else { else {
result = ssh_easy_statemach(conn); result = ssh_easy_statemach(conn, FALSE);
*dophase_done = TRUE; /* with the easy interface we are done here */ *dophase_done = TRUE; /* with the easy interface we are done here */
} }
*connected = conn->bits.tcpconnect; *connected = conn->bits.tcpconnect;
@ -2665,7 +2679,7 @@ static CURLcode scp_disconnect(struct connectdata *conn)
state(conn, SSH_SESSION_DISCONNECT); state(conn, SSH_SESSION_DISCONNECT);
result = ssh_easy_statemach(conn); result = ssh_easy_statemach(conn, FALSE);
} }
return result; return result;
@ -2686,7 +2700,7 @@ static CURLcode ssh_done(struct connectdata *conn, CURLcode status)
non-blocking DONE operations, not in the multi state machine and with non-blocking DONE operations, not in the multi state machine and with
Curl_done() invokes on several places in the code! Curl_done() invokes on several places in the code!
*/ */
result = ssh_easy_statemach(conn); result = ssh_easy_statemach(conn, FALSE);
} }
else else
result = status; result = status;
@ -2788,7 +2802,7 @@ CURLcode sftp_perform(struct connectdata *conn,
result = ssh_multi_statemach(conn, dophase_done); result = ssh_multi_statemach(conn, dophase_done);
} }
else { else {
result = ssh_easy_statemach(conn); result = ssh_easy_statemach(conn, FALSE);
*dophase_done = TRUE; /* with the easy interface we are done here */ *dophase_done = TRUE; /* with the easy interface we are done here */
} }
*connected = conn->bits.tcpconnect; *connected = conn->bits.tcpconnect;
@ -2828,7 +2842,7 @@ static CURLcode sftp_disconnect(struct connectdata *conn)
if(conn->proto.sshc.ssh_session) { if(conn->proto.sshc.ssh_session) {
/* only if there's a session still around to use! */ /* only if there's a session still around to use! */
state(conn, SSH_SFTP_SHUTDOWN); state(conn, SSH_SFTP_SHUTDOWN);
result = ssh_easy_statemach(conn); result = ssh_easy_statemach(conn, FALSE);
} }
DEBUGF(infof(conn->data, "SSH DISCONNECT is done\n")); DEBUGF(infof(conn->data, "SSH DISCONNECT is done\n"));