Commit b2954e66 authored by Daniel Stenberg's avatar Daniel Stenberg
Browse files

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
parent 7c0f2010

lib/ftp.c

+41 −15
Original line number Diff line number Diff line
@@ -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,6 +3747,12 @@ static CURLcode ftp_do_more(struct connectdata *conn, bool *complete) 
          return result;

      }

    }

    if(data->state.used_interface == Curl_if_multi) {

      result = ftp_multi_statemach(conn, complete);



      return result;

    }

    else

      result = ftp_easy_statemach(conn);

  }
@@ -4402,21 +4427,22 @@ 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 */

    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)

    /* no data to transfer */
@@ -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 */

lib/pingpong.c

+4 −1
Original line number Diff line number Diff line
@@ -5,7 +5,7 @@ 
 *                            | (__| |_| |  _ <| |___

 *                             \___|\___/|_| \_\_____|

 *

 * Copyright (C) 1998 - 2011, Daniel Stenberg, <daniel@haxx.se>, et al.

 * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.

 *

 * This software is licensed as described in the file COPYING, which

 * you should have received as part of this distribution. The terms
@@ -424,6 +424,9 @@ CURLcode Curl_pp_readresp(curl_socket_t sockfd, 
           it may actually contain another end of response already! */

        clipamount = gotbytes - i;

        restart = TRUE;

        DEBUGF(infof(data, "Curl_pp_readresp_ %d bytes of trailing "

                     "server response left\n",

                     (int)clipamount));

      }

      else if(keepon) {

tests/data/Makefile.am

+1 −1
Original line number Diff line number Diff line
@@ -93,7 +93,7 @@ test1379 test1380 test1381 test1382 test1383 test1384 test1385 test1386 \ 
test1387 test1388 test1389 test1390 test1391 test1392 test1393 \

test1400 test1401 test1402 test1403 test1404 test1405 test1406 test1407 \

test1408 test1409 test1410 test1411 \

test1500 \

test1500 test1501 \

test2000 test2001 test2002 test2003 test2004 test2005 test2006 test2007 \

test2008 test2009 test2010 test2011 test2012 test2013 test2014 test2015 \

test2016 test2017 test2018 test2019 test2020 test2021 test2022 \

tests/data/test1501

0 → 100644
+53 −0
Original line number Diff line number Diff line 
<testcase>

<info>

<keywords>

FTP

RETR

multi

LIST

</keywords>

</info>



# Server-side

<reply>

<data>

</data>

<servercmd>

DELAY LIST 2

DELAY TYPE 2

</servercmd>

</reply>



# Client-side

<client>

<server>

ftp

</server>

<tool>

lib1501

</tool>

 <name>

FTP with multi interface and slow LIST response 

 </name>

 <command>

ftp://%HOSTIP:%FTPPORT/1501/

</command>

</client>

# Verify data after the test has been "shot"

<verify>

<errorcode>

0

</errorcode>

<protocol>

USER anonymous

PASS ftp@example.com

PWD

CWD 1501

EPSV

TYPE A

LIST

QUIT

</protocol>



</verify>

</testcase>

tests/data/test591

+2 −1
Original line number Diff line number Diff line
@@ -63,8 +63,9 @@ TYPE I 
STOR 591

QUIT

</protocol>

# CURLE_UPLOAD_FAILED = 25

<errorcode>

10

25

</errorcode>

<upload>

</upload>