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 4d3216d29bef63cddd6bfc25a967b534605af232..d807af2cae760eae6ac79e76fc122e935f7a68ac 100644 (file)
--- 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 fc1784623894e8fd3f83e721639ab95b51fcdd4c..f40c2ff3b111589fe784261ec4f61e86ca7d70bc 100644 (file)
--- 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 4e4dd5346b81c52c7d779166f29100f55978ed7d..7175c99a7d05a0c669edb9927f7d36fdfe578aec 100644 (file)
--- 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 a028857e8bd332ae609471572568b0726d85d8f0..9a3c68d0b68ad159be338d1f065aa606738e5d64 100644 (file)
--- 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 d0abffa230c3da63a820483ae258722e26cc234a..e079012bc782b40e41feb9e2c1fa5176bc3a52d9 100644 (file)
--- 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 (file)
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\r
+PASS curl_by_daniel@haxx.se\r
+</protocol>
+</verify>