From d7934b8bd49114cbb54f7401742f7fb088e2f796 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Fri, 21 Oct 2011 23:36:54 +0200
Subject: [PATCH] curl_multi_fdset: correct fdset with FTP PORT use
After a PORT has been issued, and the multi handle would switch to the
CURLM_STATE_DO_MORE state (which is unique for FTP), libcurl would
return the wrong fdset to wait for when curl_multi_fdset() is
called. The code would blindly assume that it was waiting for a connect
of the second connection, while that isn't true immediately after the
PORT command.
Also, the function multi.c:domore_getsock() was highly FTP-centric and
therefore ugly to keep in protocol-agnostic code. I solved this problem
by introducing a new function pointer in the Curl_handler struct called
domore_getsock() which is only called during the DOMORE state for
protocols that set that pointer.
The new ftp.c:ftp_domore_getsock() function now returns fdset info about
the control connection's command/response handling while such a state is
in use, and goes over to waiting for a writable second connection first
once the commands are done.
The original problem could be seen by running test 525 and checking the
time stamps in the FTP server log. I can verify that this fix at least
fixes this problem.
Bug: http://curl.haxx.se/mail/lib-2011-10/0250.html
Reported by: Gokhan Sengun
---
lib/curl_rtmp.c | 6 ++++++
lib/dict.c | 1 +
lib/file.c | 1 +
lib/ftp.c | 40 ++++++++++++++++++++++++++++++++++++++--
lib/gopher.c | 1 +
lib/http.c | 2 ++
lib/imap.c | 4 ++++
lib/ldap.c | 2 ++
lib/multi.c | 16 ++++------------
lib/openldap.c | 2 ++
lib/pop3.c | 4 ++++
lib/rtsp.c | 1 +
lib/smtp.c | 4 ++++
lib/ssh.c | 2 ++
lib/telnet.c | 1 +
lib/tftp.c | 1 +
lib/url.c | 1 +
lib/urldata.h | 6 ++++++
18 files changed, 81 insertions(+), 14 deletions(-)
diff --git a/lib/curl_rtmp.c b/lib/curl_rtmp.c
index 44b7e1524..725b8f995 100644
--- a/lib/curl_rtmp.c
+++ b/lib/curl_rtmp.c
@@ -71,6 +71,7 @@ const struct Curl_handler Curl_handler_rtmp = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
rtmp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -90,6 +91,7 @@ const struct Curl_handler Curl_handler_rtmpt = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
rtmp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -109,6 +111,7 @@ const struct Curl_handler Curl_handler_rtmpe = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
rtmp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -128,6 +131,7 @@ const struct Curl_handler Curl_handler_rtmpte = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
rtmp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -147,6 +151,7 @@ const struct Curl_handler Curl_handler_rtmps = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
rtmp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -166,6 +171,7 @@ const struct Curl_handler Curl_handler_rtmpts = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
rtmp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/dict.c b/lib/dict.c
index 36bb28923..b326054d1 100644
--- a/lib/dict.c
+++ b/lib/dict.c
@@ -92,6 +92,7 @@ const struct Curl_handler Curl_handler_dict = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/file.c b/lib/file.c
index d4ca3e5ef..00d5fc09b 100644
--- a/lib/file.c
+++ b/lib/file.c
@@ -113,6 +113,7 @@ const struct Curl_handler Curl_handler_file = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
file_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/ftp.c b/lib/ftp.c
index a078c5e71..12b41ecc5 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -134,9 +134,10 @@ static CURLcode ftp_connect(struct connectdata *conn, bool *done);
static CURLcode ftp_disconnect(struct connectdata *conn, bool dead_connection);
static CURLcode ftp_nextconnect(struct connectdata *conn);
static CURLcode ftp_multi_statemach(struct connectdata *conn, bool *done);
-static int ftp_getsock(struct connectdata *conn,
- curl_socket_t *socks,
+static int ftp_getsock(struct connectdata *conn, curl_socket_t *socks,
int numsocks);
+static int ftp_domore_getsock(struct connectdata *conn, curl_socket_t *socks,
+ int numsocks);
static CURLcode ftp_doing(struct connectdata *conn,
bool *dophase_done);
static CURLcode ftp_setup_connection(struct connectdata * conn);
@@ -171,6 +172,7 @@ const struct Curl_handler Curl_handler_ftp = {
ftp_doing, /* doing */
ftp_getsock, /* proto_getsock */
ftp_getsock, /* doing_getsock */
+ ftp_domore_getsock, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ftp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -196,6 +198,7 @@ const struct Curl_handler Curl_handler_ftps = {
ftp_doing, /* doing */
ftp_getsock, /* proto_getsock */
ftp_getsock, /* doing_getsock */
+ ftp_domore_getsock, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ftp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -222,6 +225,7 @@ static const struct Curl_handler Curl_handler_ftp_proxy = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -247,6 +251,7 @@ static const struct Curl_handler Curl_handler_ftps_proxy = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -634,6 +639,37 @@ static int ftp_getsock(struct connectdata *conn,
return Curl_pp_getsock(&conn->proto.ftpc.pp, socks, numsocks);
}
+/* For the FTP "DO_MORE" phase only */
+static int ftp_domore_getsock(struct connectdata *conn, curl_socket_t *socks,
+ int numsocks)
+{
+ struct ftp_conn *ftpc = &conn->proto.ftpc;
+
+ if(!numsocks)
+ return GETSOCK_BLANK;
+
+ /* When in DO_MORE state, we could be either waiting for us to connect to a
+ remote site, or we could wait for that site to connect to us. Or just
+ handle ordinary commands.
+
+ When waiting for a connect, we will be in FTP_STOP state and then we wait
+ for the secondary socket to become writeable. If we're in another state,
+ we're still handling commands on the control (primary) connection.
+
+ */
+
+ switch(ftpc->state) {
+ case FTP_STOP:
+ break;
+ default:
+ return Curl_pp_getsock(&conn->proto.ftpc.pp, socks, numsocks);
+ }
+
+ socks[0] = conn->sock[SECONDARYSOCKET];
+
+ return GETSOCK_READSOCK(0);
+}
+
/* This is called after the FTP_QUOTE state is passed.
ftp_state_cwd() sends the range of CWD commands to the server to change to
diff --git a/lib/gopher.c b/lib/gopher.c
index 38f1f64a9..b4efae8cc 100644
--- a/lib/gopher.c
+++ b/lib/gopher.c
@@ -97,6 +97,7 @@ const struct Curl_handler Curl_handler_gopher = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/http.c b/lib/http.c
index e41cb7af3..f85cd6c55 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -118,6 +118,7 @@ const struct Curl_handler Curl_handler_http = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
http_getsock_do, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -141,6 +142,7 @@ const struct Curl_handler Curl_handler_https = {
ZERO_NULL, /* doing */
https_getsock, /* proto_getsock */
http_getsock_do, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/imap.c b/lib/imap.c
index 37c0d0e73..986b79a56 100644
--- a/lib/imap.c
+++ b/lib/imap.c
@@ -119,6 +119,7 @@ const struct Curl_handler Curl_handler_imap = {
imap_doing, /* doing */
imap_getsock, /* proto_getsock */
imap_getsock, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
imap_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -144,6 +145,7 @@ const struct Curl_handler Curl_handler_imaps = {
imap_doing, /* doing */
imap_getsock, /* proto_getsock */
imap_getsock, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
imap_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -169,6 +171,7 @@ static const struct Curl_handler Curl_handler_imap_proxy = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -194,6 +197,7 @@ static const struct Curl_handler Curl_handler_imaps_proxy = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/ldap.c b/lib/ldap.c
index 11b9beeb7..737847649 100644
--- a/lib/ldap.c
+++ b/lib/ldap.c
@@ -130,6 +130,7 @@ const struct Curl_handler Curl_handler_ldap = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -154,6 +155,7 @@ const struct Curl_handler Curl_handler_ldaps = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/multi.c b/lib/multi.c
index e53d387f0..6e4ec37a8 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -816,20 +816,12 @@ static int waitconnect_getsock(struct connectdata *conn,
}
static int domore_getsock(struct connectdata *conn,
- curl_socket_t *sock,
+ curl_socket_t *socks,
int numsocks)
{
- if(!numsocks)
- return GETSOCK_BLANK;
-
- /* When in DO_MORE state, we could be either waiting for us
- to connect to a remote site, or we could wait for that site
- to connect to us. It makes a difference in the way: if we
- connect to the site we wait for the socket to become writable, if
- the site connects to us we wait for it to become readable */
- sock[0] = conn->sock[SECONDARYSOCKET];
-
- return GETSOCK_WRITESOCK(0);
+ if(conn && conn->handler->domore_getsock)
+ return conn->handler->domore_getsock(conn, socks, numsocks);
+ return GETSOCK_BLANK;
}
/* returns bitmapped flags for this handle and its sockets */
diff --git a/lib/openldap.c b/lib/openldap.c
index 070892524..e5a3369a1 100644
--- a/lib/openldap.c
+++ b/lib/openldap.c
@@ -83,6 +83,7 @@ const struct Curl_handler Curl_handler_ldap = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ldap_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -107,6 +108,7 @@ const struct Curl_handler Curl_handler_ldaps = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ldap_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/pop3.c b/lib/pop3.c
index c5a0f5b9b..cb70679c5 100644
--- a/lib/pop3.c
+++ b/lib/pop3.c
@@ -119,6 +119,7 @@ const struct Curl_handler Curl_handler_pop3 = {
pop3_doing, /* doing */
pop3_getsock, /* proto_getsock */
pop3_getsock, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
pop3_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -144,6 +145,7 @@ const struct Curl_handler Curl_handler_pop3s = {
pop3_doing, /* doing */
pop3_getsock, /* proto_getsock */
pop3_getsock, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
pop3_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -169,6 +171,7 @@ static const struct Curl_handler Curl_handler_pop3_proxy = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -194,6 +197,7 @@ static const struct Curl_handler Curl_handler_pop3s_proxy = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/rtsp.c b/lib/rtsp.c
index 198c25dc1..5d62ac7c7 100644
--- a/lib/rtsp.c
+++ b/lib/rtsp.c
@@ -113,6 +113,7 @@ const struct Curl_handler Curl_handler_rtsp = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
rtsp_getsock_do, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
rtsp_disconnect, /* disconnect */
rtsp_rtp_readwrite, /* readwrite */
diff --git a/lib/smtp.c b/lib/smtp.c
index f85f9c433..8835b07a5 100644
--- a/lib/smtp.c
+++ b/lib/smtp.c
@@ -127,6 +127,7 @@ const struct Curl_handler Curl_handler_smtp = {
smtp_doing, /* doing */
smtp_getsock, /* proto_getsock */
smtp_getsock, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
smtp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -151,6 +152,7 @@ const struct Curl_handler Curl_handler_smtps = {
smtp_doing, /* doing */
smtp_getsock, /* proto_getsock */
smtp_getsock, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
smtp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -176,6 +178,7 @@ static const struct Curl_handler Curl_handler_smtp_proxy = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -200,6 +203,7 @@ static const struct Curl_handler Curl_handler_smtps_proxy = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/ssh.c b/lib/ssh.c
index 089b76110..a9e4c56e4 100644
--- a/lib/ssh.c
+++ b/lib/ssh.c
@@ -165,6 +165,7 @@ const struct Curl_handler Curl_handler_scp = {
scp_doing, /* doing */
ssh_getsock, /* proto_getsock */
ssh_getsock, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ssh_perform_getsock, /* perform_getsock */
scp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
@@ -189,6 +190,7 @@ const struct Curl_handler Curl_handler_sftp = {
sftp_doing, /* doing */
ssh_getsock, /* proto_getsock */
ssh_getsock, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ssh_perform_getsock, /* perform_getsock */
sftp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/telnet.c b/lib/telnet.c
index 890632214..59094b674 100644
--- a/lib/telnet.c
+++ b/lib/telnet.c
@@ -182,6 +182,7 @@ const struct Curl_handler Curl_handler_telnet = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/tftp.c b/lib/tftp.c
index 8a85a1cdd..9b44a9b3d 100644
--- a/lib/tftp.c
+++ b/lib/tftp.c
@@ -184,6 +184,7 @@ const struct Curl_handler Curl_handler_tftp = {
tftp_doing, /* doing */
tftp_getsock, /* proto_getsock */
tftp_getsock, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
tftp_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/url.c b/lib/url.c
index c3beaaa6f..40dcd1568 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -246,6 +246,7 @@ static const struct Curl_handler Curl_handler_dummy = {
ZERO_NULL, /* doing */
ZERO_NULL, /* proto_getsock */
ZERO_NULL, /* doing_getsock */
+ ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
diff --git a/lib/urldata.h b/lib/urldata.h
index 45ef2434e..5b3dc059e 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -671,6 +671,12 @@ struct Curl_handler {
curl_socket_t *socks,
int numsocks);
+ /* Called from the multi interface during the DO_MORE phase, and it should
+ then return a proper fd set */
+ int (*domore_getsock)(struct connectdata *conn,
+ curl_socket_t *socks,
+ int numsocks);
+
/* Called from the multi interface during the DO_DONE, PERFORM and
WAITPERFORM phases, and it should then return a proper fd set. Not setting
this will make libcurl use the generic default one. */
--
2.50.1