From b998d95b4d6de388ddc59a48714a2a1d9a43dc43 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 18 Aug 2011 23:35:15 +0200 Subject: 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 --- lib/connect.c | 4 ++-- lib/ftp.c | 12 ++++++++---- lib/url.c | 17 ++++++++++------- lib/urldata.h | 2 +- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/connect.c b/lib/connect.c index 9301f0108..230d1055a 100644 --- 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 689eda294..3d7f22b58 100644 --- 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 f888c1dcb..6d0b2aff8 100644 --- 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 bd8ba6cd8..b9439e458 100644 --- 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 */ -- cgit v1.2.3