From 684830cb2a2f10e987658ba36915a681271be941 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Wed, 2 Jun 2010 23:33:51 +0200 Subject: [PATCH] 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) --- CHANGES | 11 +++++++++++ RELEASE-NOTES | 3 ++- lib/ssh.c | 30 ++++++++++++++++++++++-------- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/CHANGES b/CHANGES index 2b49adb5a..02501a350 100644 --- a/CHANGES +++ b/CHANGES @@ -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 diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 7639afe28..49beadeec 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -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: @@ -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) diff --git a/lib/ssh.c b/lib/ssh.c index c3a94fe32..0df7a0991 100644 --- a/lib/ssh.c +++ b/lib/ssh.c @@ -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); @@ -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 @@ -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; } @@ -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; @@ -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; @@ -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; @@ -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; @@ -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"));