[curl/f19] FTP: when EPSV gets a 229 but fails to connect, retry with PASV (#1002815)
Kamil Dudka
kdudka at fedoraproject.org
Fri Aug 30 20:00:16 UTC 2013
commit b53f8f22099f6aa66df541a1f8594e2fbe1b5fa2
Author: Kamil Dudka <kdudka at redhat.com>
Date: Fri Aug 30 21:53:36 2013 +0200
FTP: when EPSV gets a 229 but fails to connect, retry with PASV (#1002815)
0010-curl-7.29.0-7cc00d9a.patch | 395 +++++++++++++++++++++++++++++++++++++++
curl.spec | 9 +-
2 files changed, 403 insertions(+), 1 deletions(-)
---
diff --git a/0010-curl-7.29.0-7cc00d9a.patch b/0010-curl-7.29.0-7cc00d9a.patch
new file mode 100644
index 0000000..fb44274
--- /dev/null
+++ b/0010-curl-7.29.0-7cc00d9a.patch
@@ -0,0 +1,395 @@
+From 3f411052825386a95d039435eb139a63859c3c73 Mon Sep 17 00:00:00 2001
+From: Daniel Stenberg <daniel at haxx.se>
+Date: Mon, 5 Aug 2013 23:49:53 +0200
+Subject: [PATCH] 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
+
+[upstream commit 7cc00d9a832c42a330888aa5c11a2abad1bd5ac0]
+
+Signed-off-by: Kamil Dudka <kdudka at redhat.com>
+---
+ lib/ftp.c | 64 ++++++++++++++++++++++++++++-------------------
+ lib/multi.c | 11 ++++++--
+ lib/url.c | 10 ++++---
+ lib/url.h | 4 +-
+ lib/urldata.h | 2 +-
+ tests/data/Makefile.am | 2 +-
+ tests/data/test1233 | 46 ++++++++++++++++++++++++++++++++++
+ 7 files changed, 102 insertions(+), 37 deletions(-)
+ create mode 100644 tests/data/test1233
+
+diff --git a/lib/ftp.c b/lib/ftp.c
+index 469b887..4501116 100644
+--- a/lib/ftp.c
++++ b/lib/ftp.c
+@@ -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);
+@@ -1794,15 +1794,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, "PASV", NULL);
+ conn->proto.ftpc.count1++;
+- /* remain in the FTP_PASV state */
++ /* remain in/go to the FTP_PASV state */
++ state(conn, FTP_PASV);
+ return result;
+ }
+
+@@ -1931,15 +1931,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, "PASV", NULL);
+- ftpc->count1++;
+- /* remain in the FTP_PASV state */
+- return result;
++ return ftp_epsv_disable(conn);
+ }
+ else {
+ failf(data, "Bad PASV/EPSV response: %03d", ftpcode);
+@@ -2018,14 +2010,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:
+@@ -2077,8 +2072,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 */
+
+@@ -3664,20 +3658,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) {
+@@ -3694,14 +3691,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 */
+@@ -3712,7 +3717,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) {
+@@ -3735,6 +3740,9 @@ static CURLcode ftp_do_more(struct connectdata *conn, bool *complete)
+
+ if(result)
+ return result;
++
++ *completep = 1; /* this state is now complete when the server has
++ connected back to us */
+ }
+ }
+ else if(data->set.upload) {
+@@ -3742,7 +3750,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 */
+@@ -3770,7 +3779,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;
+ }
+@@ -3782,7 +3792,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));
+ }
+
+@@ -3825,7 +3835,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"));
+@@ -4445,7 +4457,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) {
+diff --git a/lib/multi.c b/lib/multi.c
+index 706df23..9a8e68e 100644
+--- a/lib/multi.c
++++ b/lib/multi.c
+@@ -906,6 +906,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->easy_handle))
+ return CURLM_BAD_EASY_HANDLE;
+@@ -1323,13 +1324,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
+diff --git a/lib/url.c b/lib/url.c
+index b269027..52f7e27 100644
+--- a/lib/url.c
++++ b/lib/url.c
+@@ -5394,18 +5394,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);
+
+diff --git a/lib/url.h b/lib/url.h
+index a026e90..c0d9c38 100644
+--- a/lib/url.h
++++ b/lib/url.h
+@@ -7,7 +7,7 @@
+ * | (__| |_| | _ <| |___
+ * \___|\___/|_| \_\_____|
+ *
+- * Copyright (C) 1998 - 2011, Daniel Stenberg, <daniel at haxx.se>, et al.
++ * Copyright (C) 1998 - 2013, Daniel Stenberg, <daniel at 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);
+diff --git a/lib/urldata.h b/lib/urldata.h
+index 7a275da..2be467b 100644
+--- a/lib/urldata.h
++++ b/lib/urldata.h
+@@ -550,7 +550,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);
+
+
+diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
+index 3e8dae0..3f6a047 100644
+--- a/tests/data/Makefile.am
++++ b/tests/data/Makefile.am
+@@ -78,7 +78,7 @@ test1118 test1119 test1120 test1121 test1122 test1123 test1124 test1125 \
+ test1126 test1127 test1128 test1129 test1130 test1131 test1132 test1133 \
+ test1200 test1201 test1202 test1203 test1204 test1205 test1206 test1207 \
+ test1208 test1209 test1210 test1211 test1216 test1218 \
+-test1220 test1221 test1222 test1223 \
++test1220 test1221 test1222 test1223 test1233 \
+ test1300 test1301 test1302 test1303 test1304 test1305 \
+ test1306 test1307 test1308 test1309 test1310 test1311 test1312 test1313 \
+ test1314 test1315 test1316 test1317 test1318 test1319 test1320 test1321 \
+diff --git a/tests/data/test1233 b/tests/data/test1233
+new file mode 100644
+index 0000000..caf0527
+--- /dev/null
++++ b/tests/data/test1233
+@@ -0,0 +1,46 @@
++<testcase>
++<info>
++<keywords>
++FTP
++</keywords>
++</info>
++
++# Server-side
++<reply>
++<servercmd>
++# Assuming there's nothing listening on port 1
++REPLY EPSV 229 Entering Passiv Mode (|||1|)
++</servercmd>
++<data>
++here are some bytes
++</data>
++</reply>
++
++# Client-side
++<client>
++<server>
++ftp
++</server>
++ <name>
++FTP failing to connect to EPSV port, switching to PASV
++ </name>
++ <command>
++ftp://%HOSTIP:%FTPPORT/1233
++</command>
++</client>
++
++# Verify data after the test has been "shot"
++<verify>
++<protocol>
++USER anonymous
++PASS ftp at example.com
++PWD
++EPSV
++PASV
++TYPE I
++SIZE 1233
++RETR 1233
++QUIT
++</protocol>
++</verify>
++</testcase>
+--
+1.7.1
+
diff --git a/curl.spec b/curl.spec
index eb41b77..34e6562 100644
--- a/curl.spec
+++ b/curl.spec
@@ -1,7 +1,7 @@
Summary: A utility for getting files from remote servers (FTP, HTTP, and others)
Name: curl
Version: 7.29.0
-Release: 8%{?dist}
+Release: 9%{?dist}
License: MIT
Group: Applications/Internet
Source: http://curl.haxx.se/download/%{name}-%{version}.tar.lzma
@@ -34,6 +34,9 @@ Patch8: 0008-curl-7.29.0-192c4f78.patch
# mention all option listed in 'curl --help' in curl.1 man page
Patch9: 0009-curl-7.29.0-3a0e931f.patch
+# FTP: when EPSV gets a 229 but fails to connect, retry with PASV (#1002815)
+Patch10: 0010-curl-7.29.0-7cc00d9a.patch
+
# patch making libcurl multilib ready
Patch101: 0101-curl-7.29.0-multilib.patch
@@ -136,6 +139,7 @@ documentation of the library, too.
%patch7 -p1
%patch8 -p1
%patch9 -p1
+%patch10 -p1
# Fedora patches
%patch101 -p1
@@ -253,6 +257,9 @@ rm -rf $RPM_BUILD_ROOT
%{_datadir}/aclocal/libcurl.m4
%changelog
+* Fri Aug 30 2013 Kamil Dudka <kdudka at redaht.com> 7.29.0-9
+- FTP: when EPSV gets a 229 but fails to connect, retry with PASV (#1002815)
+
* Tue Jul 09 2013 Kamil Dudka <kdudka at redaht.com> 7.29.0-8
- mention all option listed in 'curl --help' in curl.1 man page
More information about the scm-commits
mailing list