mirror of
https://github.com/moparisthebest/curl
synced 2024-12-21 23:58:49 -05:00
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
This commit is contained in:
parent
d67b75c9f9
commit
d7934b8bd4
@ -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 */
|
||||
|
@ -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 */
|
||||
|
@ -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 */
|
||||
|
40
lib/ftp.c
40
lib/ftp.c
@ -134,8 +134,9 @@ 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);
|
||||
@ -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
|
||||
|
@ -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 */
|
||||
|
@ -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 */
|
||||
|
@ -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 */
|
||||
|
@ -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 */
|
||||
|
14
lib/multi.c
14
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)
|
||||
if(conn && conn->handler->domore_getsock)
|
||||
return conn->handler->domore_getsock(conn, socks, 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);
|
||||
}
|
||||
|
||||
/* returns bitmapped flags for this handle and its sockets */
|
||||
|
@ -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 */
|
||||
|
@ -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 */
|
||||
|
@ -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 */
|
||||
|
@ -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 */
|
||||
|
@ -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 */
|
||||
|
@ -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 */
|
||||
|
@ -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 */
|
||||
|
@ -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 */
|
||||
|
@ -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. */
|
||||
|
Loading…
Reference in New Issue
Block a user