Commit 7cc00d9a authored by Daniel Stenberg's avatar Daniel Stenberg
Browse files

FTP: when EPSV gets a 229 but fails to connect, retry with PASV 

This is a regression as this logic used to work. It isn't clear when it
broke, but I'm assuming in 7.28.0 when we went all-multi internally.

This likely never worked with the multi interface. As the failed
connection is detected once the multi state has reached DO_MORE, the
Curl_do_more() function was now expanded somewhat so that the
ftp_do_more() function can request to go "back" to the previous state
when it makes another attempt - using PASV.

Added test case 1233 to verify this fix. It has the little issue that it
assumes no service is listening/accepting connections on port 1...

Reported-by: byte_bucket in the #curl IRC channel
parent 230e16dc

lib/ftp.c

+37 −28
Original line number Diff line number Diff line
@@ -136,7 +136,7 @@ static CURLcode ftp_done(struct connectdata *conn, 
                         CURLcode, bool premature);

static CURLcode ftp_connect(struct connectdata *conn, bool *done);

static CURLcode ftp_disconnect(struct connectdata *conn, bool dead_connection);

static CURLcode ftp_do_more(struct connectdata *conn, bool *completed);

static CURLcode ftp_do_more(struct connectdata *conn, int *completed);

static CURLcode ftp_multi_statemach(struct connectdata *conn, bool *done);

static int ftp_getsock(struct connectdata *conn, curl_socket_t *socks,

                       int numsocks);
@@ -1799,15 +1799,15 @@ static CURLcode ftp_state_quote(struct connectdata *conn, 
static CURLcode ftp_epsv_disable(struct connectdata *conn)

{

  CURLcode result = CURLE_OK;

  infof(conn->data, "got positive EPSV response, but can't connect. "

        "Disabling EPSV\n");

  infof(conn->data, "Failed EPSV attempt. Disabling EPSV\n");

  /* disable it for next transfer */

  conn->bits.ftp_use_epsv = FALSE;

  conn->data->state.errorbuf = FALSE; /* allow error message to get

                                         rewritten */

  PPSENDF(&conn->proto.ftpc.pp, "%s", "PASV");

  conn->proto.ftpc.count1++;

  /* remain in the FTP_PASV state */

  /* remain in/go to the FTP_PASV state */

  state(conn, FTP_PASV);

  return result;

}
@@ -1936,15 +1936,7 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn, 
  }

  else if(ftpc->count1 == 0) {

    /* EPSV failed, move on to PASV */



    /* disable it for next transfer */

    conn->bits.ftp_use_epsv = FALSE;

    infof(data, "disabling EPSV usage\n");



    PPSENDF(&ftpc->pp, "%s", "PASV");

    ftpc->count1++;

    /* remain in the FTP_PASV state */

    return result;

    return ftp_epsv_disable(conn);

  }

  else {

    failf(data, "Bad PASV/EPSV response: %03d", ftpcode);
@@ -2021,14 +2013,17 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn, 
  case CURLPROXY_SOCKS5_HOSTNAME:

    result = Curl_SOCKS5(conn->proxyuser, conn->proxypasswd, newhost, newport,

                         SECONDARYSOCKET, conn);

    connected = TRUE;

    break;

  case CURLPROXY_SOCKS4:

    result = Curl_SOCKS4(conn->proxyuser, newhost, newport,

                         SECONDARYSOCKET, conn, FALSE);

    connected = TRUE;

    break;

  case CURLPROXY_SOCKS4A:

    result = Curl_SOCKS4(conn->proxyuser, newhost, newport,

                         SECONDARYSOCKET, conn, TRUE);

    connected = TRUE;

    break;

  case CURLPROXY_HTTP:

  case CURLPROXY_HTTP_1_0:
@@ -2080,8 +2075,7 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn, 
    }

  }



  conn->bits.tcpconnect[SECONDARYSOCKET] = TRUE;



  conn->bits.tcpconnect[SECONDARYSOCKET] = connected;

  conn->bits.do_more = TRUE;

  state(conn, FTP_STOP); /* this phase is completed */
@@ -3677,20 +3671,23 @@ static CURLcode ftp_range(struct connectdata *conn) 
 *

 * This function shall be called when the second FTP (data) connection is

 * connected.

 *

 * 'complete' can return 0 for incomplete, 1 for done and -1 for go back

 * (which basically is only for when PASV is being sent to retry a failed

 * EPSV).

 */



static CURLcode ftp_do_more(struct connectdata *conn, bool *complete)

static CURLcode ftp_do_more(struct connectdata *conn, int *completep)

{

  struct SessionHandle *data=conn->data;

  struct ftp_conn *ftpc = &conn->proto.ftpc;

  CURLcode result = CURLE_OK;

  bool connected = FALSE;

  bool complete = FALSE;



  /* 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]) {

    if(conn->tunnel_state[SECONDARYSOCKET] == TUNNEL_CONNECT) {
@@ -3707,14 +3704,22 @@ static CURLcode ftp_do_more(struct connectdata *conn, bool *complete) 
    if(connected) {

      DEBUGF(infof(data, "DO-MORE connected phase starts\n"));

    }

    else

    else {

      if(result && (ftpc->count1 == 0)) {

        *completep = -1; /* go back to DOING please */

        /* this is a EPSV connect failing, try PASV instead */

        return ftp_epsv_disable(conn);

      }

      return result;

    }

  }



  if(ftpc->state) {

    /* 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);

    result = ftp_multi_statemach(conn, &complete);



    *completep = (int)complete;



    /* if we got an error or if we don't wait for a data connection return

       immediately */
@@ -3725,7 +3730,7 @@ static CURLcode ftp_do_more(struct connectdata *conn, bool *complete) 
      /* if we reach the end of the FTP state machine here, *complete will be

         TRUE but so is ftpc->wait_data_conn, which says we need to wait for

         the data connection and therefore we're not actually complete */

      *complete = FALSE;

      *completep = 0;

  }



  if(ftp->transfer <= FTPTRANSFER_INFO) {
@@ -3749,7 +3754,7 @@ static CURLcode ftp_do_more(struct connectdata *conn, bool *complete) 
        if(result)

          return result;



        *complete = TRUE; /* this state is now complete when the server has

        *completep = 1; /* this state is now complete when the server has

                           connected back to us */

      }

    }
@@ -3758,7 +3763,8 @@ static CURLcode ftp_do_more(struct connectdata *conn, bool *complete) 
      if(result)

        return result;



      result = ftp_multi_statemach(conn, complete);

      result = ftp_multi_statemach(conn, &complete);

      *completep = (int)complete;

    }

    else {

      /* download */
@@ -3786,7 +3792,8 @@ static CURLcode ftp_do_more(struct connectdata *conn, bool *complete) 
          return result;

      }



      result = ftp_multi_statemach(conn, complete);

      result = ftp_multi_statemach(conn, &complete);

      *completep = (int)complete;

    }

    return result;

  }
@@ -3798,7 +3805,7 @@ static CURLcode ftp_do_more(struct connectdata *conn, bool *complete) 


  if(!ftpc->wait_data_conn) {

    /* no waiting for the data connection so this is now complete */

    *complete = TRUE;

    *completep = 1;

    DEBUGF(infof(data, "DO-MORE phase ends with %d\n", (int)result));

  }
@@ -3841,7 +3848,9 @@ CURLcode ftp_perform(struct connectdata *conn, 
  /* run the state-machine */

  result = ftp_multi_statemach(conn, dophase_done);



  *connected = conn->bits.tcpconnect[FIRSTSOCKET];

  *connected = conn->bits.tcpconnect[SECONDARYSOCKET];



  infof(conn->data, "ftp_perform ends with SECONDARY: %d\n", *connected);



  if(*dophase_done)

    DEBUGF(infof(conn->data, "DO phase is complete1\n"));
@@ -4469,7 +4478,7 @@ static CURLcode ftp_dophase_done(struct connectdata *conn, 
  struct ftp_conn *ftpc = &conn->proto.ftpc;



  if(connected) {

    bool completed;

    int completed;

    CURLcode result = ftp_do_more(conn, &completed);



    if(result) {

lib/multi.c

+8 −3
Original line number Diff line number Diff line
@@ -929,6 +929,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, 
  struct SingleRequest *k;

  struct SessionHandle *data;

  long timeout_ms;

  int control;



  if(!GOOD_EASY_HANDLE(easy))

    return CURLM_BAD_EASY_HANDLE;
@@ -1359,13 +1360,17 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, 
      /*

       * When we are connected, DO MORE and then go DO_DONE

       */

      easy->result = Curl_do_more(easy->easy_conn, &dophase_done);

      easy->result = Curl_do_more(easy->easy_conn, &control);



      /* No need to remove this handle from the send pipeline here since that

         is done in Curl_done() */

      if(CURLE_OK == easy->result) {

        if(dophase_done) {

          multistate(easy, CURLM_STATE_DO_DONE);

        if(control) {

          /* if positive, advance to DO_DONE

             if negative, go back to DOING */

          multistate(easy, control==1?

                     CURLM_STATE_DO_DONE:

                     CURLM_STATE_DOING);

          result = CURLM_CALL_MULTI_PERFORM;

        }

        else

lib/url.c

+6 −4
Original line number Diff line number Diff line
@@ -5841,18 +5841,20 @@ CURLcode Curl_do(struct connectdata **connp, bool *done) 
 *

 * TODO: A future libcurl should be able to work away this state.

 *

 * 'complete' can return 0 for incomplete, 1 for done and -1 for go back to

 * DOING state there's more work to do!

 */



CURLcode Curl_do_more(struct connectdata *conn, bool *completed)

CURLcode Curl_do_more(struct connectdata *conn, int *complete)

{

  CURLcode result=CURLE_OK;



  *completed = FALSE;

  *complete = 0;



  if(conn->handler->do_more)

    result = conn->handler->do_more(conn, completed);

    result = conn->handler->do_more(conn, complete);



  if(!result && *completed)

  if(!result && (*complete == 1))

    /* do_complete must be called after the protocol-specific DO function */

    do_complete(conn);

lib/url.h

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

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

 *

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

 * Copyright (C) 1998 - 2013, 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
@@ -37,7 +37,7 @@ CURLcode Curl_close(struct SessionHandle *data); /* opposite of curl_open() */ 
CURLcode Curl_connect(struct SessionHandle *, struct connectdata **,

                      bool *async, bool *protocol_connect);

CURLcode Curl_do(struct connectdata **, bool *done);

CURLcode Curl_do_more(struct connectdata *, bool *completed);

CURLcode Curl_do_more(struct connectdata *, int *completed);

CURLcode Curl_done(struct connectdata **, CURLcode, bool premature);

CURLcode Curl_disconnect(struct connectdata *, bool dead_connection);

CURLcode Curl_protocol_connect(struct connectdata *conn, bool *done);

lib/urldata.h

+1 −1
Original line number Diff line number Diff line
@@ -577,7 +577,7 @@ struct Curl_async { 
/* These function pointer types are here only to allow easier typecasting

   within the source when we need to cast between data pointers (such as NULL)

   and function pointers. */

typedef CURLcode (*Curl_do_more_func)(struct connectdata *, bool *);

typedef CURLcode (*Curl_do_more_func)(struct connectdata *, int *);

typedef CURLcode (*Curl_done_func)(struct connectdata *, CURLcode, bool);