Skip to content

Commit

Permalink
SSH: corrected the inability to respect the timeout
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
bagder committed Jun 2, 2010
1 parent 51248a9 commit 684830c
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 9 deletions.
11 changes: 11 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@

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)
- Added missing new libcurl files to non-configure targets. Adjusted
libcurl standard internal header inclusions in new files. Fixed an
Expand Down
3 changes: 2 additions & 1 deletion RELEASE-NOTES
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ This release includes the following bugfixes:
o TFTP timeout option sent correctly
o TFTP block id wrap
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:

Expand All @@ -45,6 +46,6 @@ advice from friends like these:
Rainer Canavan, Paul Howarth, Jerome Vouillon, Ruslan Gazizov, Yang Tse,
Kamil Dudka, Alex Bligh, Ben Greear, Hoi-Ho Chan, Howard Chu, Dirk Manske,
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)
30 changes: 22 additions & 8 deletions lib/ssh.c
Original file line number Diff line number Diff line change
Expand Up @@ -2408,15 +2408,28 @@ static CURLcode ssh_multi_statemach(struct connectdata *conn, bool *done)
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;
CURLcode result = CURLE_OK;
struct SessionHandle *data = conn->data;

while((sshc->state != SSH_STOP) && !result) {
bool block;
long left;

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
if((CURLE_OK == result) && block) {
int dir = libssh2_session_block_directions(sshc->ssh_session);
Expand All @@ -2430,7 +2443,8 @@ static CURLcode ssh_easy_statemach(struct connectdata *conn)
fd_write = sock;
}
/* 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

Expand Down Expand Up @@ -2548,7 +2562,7 @@ static CURLcode ssh_connect(struct connectdata *conn, bool *done)
if(data->state.used_interface == Curl_if_multi)
result = ssh_multi_statemach(conn, done);
else {
result = ssh_easy_statemach(conn);
result = ssh_easy_statemach(conn, TRUE);
if(!result)
*done = TRUE;
}
Expand Down Expand Up @@ -2584,7 +2598,7 @@ CURLcode scp_perform(struct connectdata *conn,
result = ssh_multi_statemach(conn, dophase_done);
}
else {
result = ssh_easy_statemach(conn);
result = ssh_easy_statemach(conn, FALSE);
*dophase_done = TRUE; /* with the easy interface we are done here */
}
*connected = conn->bits.tcpconnect;
Expand Down Expand Up @@ -2665,7 +2679,7 @@ static CURLcode scp_disconnect(struct connectdata *conn)

state(conn, SSH_SESSION_DISCONNECT);

result = ssh_easy_statemach(conn);
result = ssh_easy_statemach(conn, FALSE);
}

return result;
Expand All @@ -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
Curl_done() invokes on several places in the code!
*/
result = ssh_easy_statemach(conn);
result = ssh_easy_statemach(conn, FALSE);
}
else
result = status;
Expand Down Expand Up @@ -2788,7 +2802,7 @@ CURLcode sftp_perform(struct connectdata *conn,
result = ssh_multi_statemach(conn, dophase_done);
}
else {
result = ssh_easy_statemach(conn);
result = ssh_easy_statemach(conn, FALSE);
*dophase_done = TRUE; /* with the easy interface we are done here */
}
*connected = conn->bits.tcpconnect;
Expand Down Expand Up @@ -2828,7 +2842,7 @@ static CURLcode sftp_disconnect(struct connectdata *conn)
if(conn->proto.sshc.ssh_session) {
/* only if there's a session still around to use! */
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"));
Expand Down

0 comments on commit 684830c

Please sign in to comment.