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)

diff --git a/CHANGES b/CHANGES
index 2b49adb..02501a3 100644 (file)
--- 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 7639afe..49beade 100644 (file)
--- 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 c3a94fe..0df7a09 100644 (file)
--- 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"));