From 88c8d72a214864952b6d1c2347b6c3f5b7d69e84 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 11 Dec 2006 09:32:58 +0000 Subject: 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. --- CHANGES | 11 +++++++++++ RELEASE-NOTES | 4 +++- lib/ftp.c | 41 ++++++++++++++++++++++------------------- lib/url.c | 7 ++----- tests/data/Makefile.am | 2 +- tests/data/test538 | 42 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 81 insertions(+), 26 deletions(-) create mode 100644 tests/data/test538 diff --git a/CHANGES b/CHANGES index 4d3216d29..d807af2ca 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 fc1784623..f40c2ff3b 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -33,6 +33,8 @@ This release includes the following bugfixes: 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 @@ advice from friends like these: 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 4e4dd5346..7175c99a7 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -2965,6 +2965,28 @@ CURLcode Curl_ftp_done(struct connectdata *conn, CURLcode status) */ 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 @@ CURLcode Curl_ftp_done(struct connectdata *conn, CURLcode status) /* 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 a028857e8..9a3c68d0b 100644 --- a/lib/url.c +++ b/lib/url.c @@ -2171,10 +2171,7 @@ static void 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 @@ static CURLcode CreateConnection(struct SessionHandle *data, 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 d0abffa23..e079012bc 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -36,4 +36,4 @@ EXTRA_DIST = test1 test108 test117 test127 test20 test27 test34 test46 \ 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 000000000..6ad2aac13 --- /dev/null +++ b/tests/data/test538 @@ -0,0 +1,42 @@ + + +FTP +FAILURE + + +# Server-side + + + +# Client-side + + +ftp + +# NOTE that we use the 504 tool for this case + +lib504 + + +FTP multi-interface download, failed login: PASS not valid + + +ftp://%HOSTIP:%FTPPORT/538 + + +REPLY PASS 314 bluah you f00l! + + + +# Verify data after the test has been "shot" + +# ok, the error code here is supposed to be 100 for the fine case since +# that's just how lib504.c is written + +100 + + +USER anonymous +PASS curl_by_daniel@haxx.se + + -- cgit v1.2.3