FTP: fix proxy connect race condition

When using the multi interface, a SOCKS proxy, and a connection that
wouldn't immediately consider itself connected (which my Linux tests do
by default), libcurl would be tricked into doing _two_ connects to the
SOCKS proxy when it setup the data connection and then of course the
second attempt would fail miserably and cause error.

This problem is a regression that was introduced by commit
4a42e5cdaa344755 that was introduced in the 7.21.7 release.

Bug: http://curl.haxx.se/mail/lib-2011-08/0199.html
Reported by: Fabian Keil

diff --git a/lib/connect.c b/lib/connect.c
index 9301f01..230d105 100644 (file)
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -659,7 +659,7 @@ CURLcode Curl_is_connected(struct connectdata *conn,
 
   *connected = FALSE; /* a very negative world view is best */
 
-  if(conn->bits.tcpconnect) {
+  if(conn->bits.tcpconnect[sockindex]) {
     /* we are connected already! */
     *connected = TRUE;
     return CURLE_OK;
@@ -698,7 +698,7 @@ CURLcode Curl_is_connected(struct connectdata *conn,
       if(code)
         return code;
 
-      conn->bits.tcpconnect = TRUE;
+      conn->bits.tcpconnect[sockindex] = TRUE;
       *connected = TRUE;
       Curl_pgrsTime(data, TIMER_CONNECT); /* connect done */
       Curl_verboseconnect(conn);

diff --git a/lib/ftp.c b/lib/ftp.c
index 689eda2..3d7f22b 100644 (file)
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -1043,7 +1043,7 @@ static CURLcode ftp_state_use_port(struct connectdata *conn,
      The *proper* fix is to make sure that the active connection from the
      server is done in a non-blocking way. Currently, it is still BLOCKING.
   */
-  conn->bits.tcpconnect = TRUE;
+  conn->bits.tcpconnect[SECONDARYSOCKET] = TRUE;
 
   state(conn, FTP_PORT);
   return result;
@@ -1703,7 +1703,7 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn,
   if(result)
     return result;
 
-  conn->bits.tcpconnect = connected; /* simply TRUE or FALSE */
+  conn->bits.tcpconnect[SECONDARYSOCKET] = connected;
 
   /*
    * When this is used from the multi interface, this might've returned with
@@ -1741,6 +1741,9 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn,
     break;
   }
 
+  if(result)
+    return result;
+
   if(conn->bits.tunnel_proxy && conn->bits.httpproxy) {
     /* FIX: this MUST wait for a proper connect first if 'connected' is
      * FALSE */
@@ -1763,10 +1766,11 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn,
 
     data->state.proto.ftp = ftp_save;
 
-    if(CURLE_OK != result)
+    if(result)
       return result;
   }
 
+  conn->bits.tcpconnect[SECONDARYSOCKET] = TRUE;
 
   state(conn, FTP_STOP); /* this phase is completed */
 
@@ -3473,7 +3477,7 @@ CURLcode ftp_perform(struct connectdata *conn,
     result = ftp_easy_statemach(conn);
     *dophase_done = TRUE; /* with the easy interface we are done here */
   }
-  *connected = conn->bits.tcpconnect;
+  *connected = conn->bits.tcpconnect[FIRSTSOCKET];
 
   if(*dophase_done)
     DEBUGF(infof(conn->data, "DO phase is complete\n"));

diff --git a/lib/url.c b/lib/url.c
index f888c1d..6d0b2af 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -3200,8 +3200,11 @@ static CURLcode ConnectPlease(struct SessionHandle *data,
     /* All is cool, we store the current information */
     conn->ip_addr = addr;
 
-    if(*connected)
+    if(*connected) {
       result = Curl_connected_proxy(conn);
+      if(!result)
+        conn->bits.tcpconnect[FIRSTSOCKET] = TRUE;
+    }
   }
 
   if(result)
@@ -3294,7 +3297,7 @@ CURLcode Curl_protocol_connect(struct connectdata *conn,
 
   *protocol_done = FALSE;
 
-  if(conn->bits.tcpconnect && conn->bits.protoconnstart) {
+  if(conn->bits.tcpconnect[FIRSTSOCKET] && conn->bits.protoconnstart) {
     /* We already are connected, get back. This may happen when the connect
        worked fine in the first call, like when we connect to a local server
        or proxy. Note that we don't know if the protocol is actually done.
@@ -3307,7 +3310,7 @@ CURLcode Curl_protocol_connect(struct connectdata *conn,
     return CURLE_OK;
   }
 
-  if(!conn->bits.tcpconnect) {
+  if(!conn->bits.tcpconnect[FIRSTSOCKET]) {
 
     Curl_pgrsTime(data, TIMER_CONNECT); /* connect done */
     Curl_verboseconnect(conn);
@@ -4870,7 +4873,7 @@ static CURLcode create_conn(struct SessionHandle *data,
     /* Setup a "faked" transfer that'll do nothing */
     if(CURLE_OK == result) {
       conn->data = data;
-      conn->bits.tcpconnect = TRUE; /* we are "connected */
+      conn->bits.tcpconnect[FIRSTSOCKET] = TRUE; /* we are "connected */
 
       ConnectionStore(data, conn);
 
@@ -5064,10 +5067,10 @@ CURLcode Curl_setup_conn(struct connectdata *conn,
       if(connected) {
         result = Curl_protocol_connect(conn, protocol_done);
         if(CURLE_OK == result)
-          conn->bits.tcpconnect = TRUE;
+          conn->bits.tcpconnect[FIRSTSOCKET] = TRUE;
       }
       else
-        conn->bits.tcpconnect = FALSE;
+        conn->bits.tcpconnect[FIRSTSOCKET] = FALSE;
 
       /* if the connection was closed by the server while exchanging
          authentication informations, retry with the new set
@@ -5086,7 +5089,7 @@ CURLcode Curl_setup_conn(struct connectdata *conn,
     else {
       Curl_pgrsTime(data, TIMER_CONNECT); /* we're connected already */
       Curl_pgrsTime(data, TIMER_APPCONNECT); /* we're connected already */
-      conn->bits.tcpconnect = TRUE;
+      conn->bits.tcpconnect[FIRSTSOCKET] = TRUE;
       *protocol_done = TRUE;
       Curl_verboseconnect(conn);
       Curl_updateconninfo(conn, conn->sock[FIRSTSOCKET]);

diff --git a/lib/urldata.h b/lib/urldata.h
index bd8ba6c..b9439e4 100644 (file)
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -411,7 +411,7 @@ struct ConnectBits {
   bool do_more; /* this is set TRUE if the ->curl_do_more() function is
                    supposed to be called, after ->curl_do() */
 
-  bool tcpconnect;    /* the TCP layer (or similar) is connected, this is set
+  bool tcpconnect[2]; /* the TCP layer (or similar) is connected, this is set
                          the first time on the first connect function call */
   bool protoconnstart;/* the protocol layer has STARTED its operation after
                          the TCP layer connect */