FTP: prevent the multi interface from blocking

As pointed out in Bug report #3579064, curl_multi_perform() would
wrongly use a blocking mechanism internally for some commands which
could lead to for example a very long block if the LIST response never
showed.

The solution was to make sure to properly continue to use the multi
interface non-blocking state machine.

The new test 1501 verifies the fix.

Bug: http://curl.haxx.se/bug/view.cgi?id=3579064
Reported by: Guido Berhoerster

1 files changed, 41 insertions, 15 deletions

diff --git a/lib/ftp.c b/lib/ftp.c
index 02c671a96..bb7d7a496 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -666,11 +666,18 @@ static CURLcode ftp_readresp(curl_socket_t sockfd,
   if(ftpcode)
     *ftpcode = code;
 
-  if(421 == code)
+  if(421 == code) {
     /* 421 means "Service not available, closing control connection." and FTP
      * servers use it to signal that idle session timeout has been exceeded.
-     * If we ignored the response, it could end up hanging in some cases. */
+     * If we ignored the response, it could end up hanging in some cases.
+     *
+     * This response code can come at any point so having it treated
+     * generically is a good idea.
+     */
+    infof(data, "We got a 421 - timeout!\n");
+    state(conn, FTP_STOP);
     return CURLE_OPERATION_TIMEDOUT;
+  }
 
   return result;
 }
@@ -2394,6 +2401,7 @@ static CURLcode ftp_state_stor_resp(struct connectdata *conn,
 
   if(ftpcode>=400) {
     failf(data, "Failed FTP upload: %0d", ftpcode);
+    state(conn, FTP_STOP);
     /* oops, we never close the sockets! */
     return CURLE_UPLOAD_FAILED;
   }
@@ -2411,9 +2419,6 @@ static CURLcode ftp_state_stor_resp(struct connectdata *conn,
     if(!connected) {
       struct ftp_conn *ftpc = &conn->proto.ftpc;
       infof(data, "Data conn was not available immediately\n");
-      /* as there's not necessarily an immediate action on the control
-         connection now, we halt the state machine */
-      state(conn, FTP_STOP);
       ftpc->wait_data_conn = TRUE;
     }
 
@@ -3663,6 +3668,8 @@ static CURLcode ftp_do_more(struct connectdata *conn, bool *complete)
   /* the ftp struct is inited in ftp_connect() */
   struct FTP *ftp = data->state.proto.ftp;
 
+  *complete = FALSE;
+
   /* if the second connection isn't done yet, wait for it */
   if(!conn->bits.tcpconnect[SECONDARYSOCKET]) {
     result = Curl_is_connected(conn, SECONDARYSOCKET, &connected);
@@ -3675,6 +3682,18 @@ static CURLcode ftp_do_more(struct connectdata *conn, bool *complete)
       return result;
   }
 
+  if((data->state.used_interface == Curl_if_multi) &&
+     ftpc->state) {
+    /* multi interface and already in a state so skip the intial commands.
+       They are only done to kickstart the do_more state */
+    result = ftp_multi_statemach(conn, complete);
+
+    /* if we got an error or if we don't wait for a data connection return
+       immediately */
+    if(result || (ftpc->wait_data_conn != TRUE))
+      return result;
+  }
+
   if(ftp->transfer <= FTPTRANSFER_INFO) {
     /* a transfer is about to take place, or if not a file name was given
        so we'll do a SIZE on it later and then we need the right TYPE first */
@@ -3728,7 +3747,13 @@ static CURLcode ftp_do_more(struct connectdata *conn, bool *complete)
           return result;
       }
     }
-    result = ftp_easy_statemach(conn);
+    if(data->state.used_interface == Curl_if_multi) {
+      result = ftp_multi_statemach(conn, complete);
+
+      return result;
+    }
+    else
+      result = ftp_easy_statemach(conn);
   }
 
   if((result == CURLE_OK) && (ftp->transfer != FTPTRANSFER_BODY))
@@ -4402,20 +4427,21 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
 static CURLcode ftp_dophase_done(struct connectdata *conn,
                                  bool connected)
 {
-  CURLcode result = CURLE_OK;
   struct FTP *ftp = conn->data->state.proto.ftp;
   struct ftp_conn *ftpc = &conn->proto.ftpc;
 
   if(connected) {
     bool completed;
-    result = ftp_do_more(conn, &completed);
-  }
+    CURLcode result = ftp_do_more(conn, &completed);
 
-  if(result && (conn->sock[SECONDARYSOCKET] != CURL_SOCKET_BAD)) {
-    /* Failure detected, close the second socket if it was created already */
-    Curl_closesocket(conn, conn->sock[SECONDARYSOCKET]);
-    conn->sock[SECONDARYSOCKET] = CURL_SOCKET_BAD;
-    return result;
+    if(result) {
+      if(conn->sock[SECONDARYSOCKET] != CURL_SOCKET_BAD) {
+        /* close the second socket if it was created already */
+        Curl_closesocket(conn, conn->sock[SECONDARYSOCKET]);
+        conn->sock[SECONDARYSOCKET] = CURL_SOCKET_BAD;
+      }
+      return result;
+    }
   }
 
   if(ftp->transfer != FTPTRANSFER_BODY)
@@ -4427,7 +4453,7 @@ static CURLcode ftp_dophase_done(struct connectdata *conn,
 
   ftpc->ctl_valid = TRUE; /* seems good */
 
-  return result;
+  return CURLE_OK;
 }
 
 /* called from multi.c while DOing */