Alexey Simak found out that when doing FTP with the multi interface and
something went wrong like it got a bad response code back from the server,
libcurl would leak memory. Added test case 538 to verify the fix.
I also noted that the connection would get cached in that case, which
doesn't make sense since it cannot be re-use when the authentication has
failed. I fixed that issue too at the same time, and also that the path
would be "remembered" in vain for cases where the connection was about to
get closed.
diff --git a/CHANGES b/CHANGES
index 4d3216d..d807af2 100644
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,17 @@
Changelog
+Daniel (11 December 2006)
+- Alexey Simak found out that when doing FTP with the multi interface and
+ something went wrong like it got a bad response code back from the server,
+ libcurl would leak memory. Added test case 538 to verify the fix.
+
+ I also noted that the connection would get cached in that case, which
+ doesn't make sense since it cannot be re-use when the authentication has
+ failed. I fixed that issue too at the same time, and also that the path
+ would be "remembered" in vain for cases where the connection was about to
+ get closed.
+
Daniel (6 December 2006)
- Sebastien Willemijns reported bug #1603712
(http://curl.haxx.se/bug/view.cgi?id=1603712) which is about connections
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index fc17846..f40c2ff 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -33,6 +33,8 @@
o CURLOPT_FORBID_REUSE works again
o CURLOPT_MAXCONNECTS set to zero caused libcurl to SIGSEGV
o rate limiting works better
+ o getting FTP response code errors when using the multi-interface caused
+ libcurl to leak memory
Other curl-related news:
@@ -51,6 +53,6 @@
James Housley, Olaf Stueben, Yang Tse, Gisle Vanem, Bradford Bruce,
Ciprian Badescu, Dmitriy Sergeyev, Nir Soffer, Venkat Akella, Toon Verwaest,
Matt Witherspoon, Alexey Simak, Martin Skinner, Sh Diao, Jared Lundell,
- Stefan Krause, Sebastien Willemijns
+ Stefan Krause, Sebastien Willemijns, Alexey Simak
Thanks! (and sorry if I forgot to mention someone)
diff --git a/lib/ftp.c b/lib/ftp.c
index 4e4dd53..7175c99 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -2965,6 +2965,28 @@
*/
return CURLE_OK;
+ switch(status) {
+ case CURLE_BAD_DOWNLOAD_RESUME:
+ case CURLE_FTP_WEIRD_PASV_REPLY:
+ case CURLE_FTP_PORT_FAILED:
+ case CURLE_FTP_COULDNT_SET_BINARY:
+ case CURLE_FTP_COULDNT_RETR_FILE:
+ case CURLE_FTP_COULDNT_STOR_FILE:
+ case CURLE_FTP_ACCESS_DENIED:
+ /* the connection stays alive fine even though this happened */
+ /* fall-through */
+ case CURLE_OK: /* doesn't affect the control connection's status */
+ ftpc->ctl_valid = was_ctl_valid;
+ break;
+ default: /* by default, an error means the control connection is
+ wedged and should not be used anymore */
+ ftpc->ctl_valid = FALSE;
+ ftpc->cwdfail = TRUE; /* set this TRUE to prevent us to remember the
+ current path, as this connection is going */
+ conn->bits.close = TRUE; /* marked for closure */
+ break;
+ }
+
/* now store a copy of the directory we are in */
if(ftpc->prevpath)
free(ftpc->prevpath);
@@ -2990,25 +3012,6 @@
/* free the dir tree and file parts */
freedirs(conn);
- switch(status) {
- case CURLE_BAD_DOWNLOAD_RESUME:
- case CURLE_FTP_WEIRD_PASV_REPLY:
- case CURLE_FTP_PORT_FAILED:
- case CURLE_FTP_COULDNT_SET_BINARY:
- case CURLE_FTP_COULDNT_RETR_FILE:
- case CURLE_FTP_COULDNT_STOR_FILE:
- case CURLE_FTP_ACCESS_DENIED:
- /* the connection stays alive fine even though this happened */
- /* fall-through */
- case CURLE_OK: /* doesn't affect the control connection's status */
- ftpc->ctl_valid = was_ctl_valid;
- break;
- default: /* by default, an error means the control connection is
- wedged and should not be used anymore */
- ftpc->ctl_valid = FALSE;
- break;
- }
-
#ifdef HAVE_KRB4
Curl_sec_fflush_fd(conn, conn->sock[SECONDARYSOCKET]);
#endif
diff --git a/lib/url.c b/lib/url.c
index a028857..9a3c68d 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -2171,10 +2171,7 @@
ConnectionDone(struct connectdata *conn)
{
conn->inuse = FALSE;
- conn->data = NULL;
-
- if (conn->send_pipe == 0 &&
- conn->recv_pipe == 0)
+ if (!conn->send_pipe && !conn->recv_pipe)
conn->is_in_pipeline = FALSE;
}
@@ -3071,7 +3068,7 @@
conn->port = port;
conn->remote_port = (unsigned short)port;
- conn->protocol |= PROT_FTP|PROT_CLOSEACTION;
+ conn->protocol |= PROT_FTP;
if(data->change.proxy &&
*data->change.proxy &&
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index d0abffa..e079012 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -36,4 +36,4 @@
test265 test266 test267 test268 test269 test270 test271 test272 test273 \
test274 test275 test524 test525 test276 test277 test526 test527 test528 \
test530 DISABLED test278 test279 test531 test280 test529 test532 test533 \
- test534 test535 test281 test537 test282
+ test534 test535 test281 test537 test282 test538
diff --git a/tests/data/test538 b/tests/data/test538
new file mode 100644
index 0000000..6ad2aac
--- /dev/null
+++ b/tests/data/test538
@@ -0,0 +1,42 @@
+<info>
+<keywords>
+FTP
+FAILURE
+</keywords>
+</info>
+# Server-side
+<reply>
+</reply>
+
+# Client-side
+<client>
+<server>
+ftp
+</server>
+# NOTE that we use the 504 tool for this case
+<tool>
+lib504
+</tool>
+ <name>
+FTP multi-interface download, failed login: PASS not valid
+ </name>
+ <command>
+ftp://%HOSTIP:%FTPPORT/538
+</command>
+<file name="log/ftpserver.cmd">
+REPLY PASS 314 bluah you f00l!
+</file>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+# ok, the error code here is supposed to be 100 for the fine case since
+# that's just how lib504.c is written
+<errorcode>
+100
+</errorcode>
+<protocol>
+USER anonymous
+PASS curl_by_daniel@haxx.se
+</protocol>
+</verify>
