]> granicus.if.org Git - curl/commitdiff
FTP improvements:
authorDaniel Stenberg <daniel@haxx.se>
Thu, 25 Nov 2004 22:21:49 +0000 (22:21 +0000)
committerDaniel Stenberg <daniel@haxx.se>
Thu, 25 Nov 2004 22:21:49 +0000 (22:21 +0000)
If EPSV, EPRT or LPRT is tried and doesn't work, it will not be retried on
the same server again even if a following request is made using a persistent
connection.

If a second request is made to a server, requesting a file from the same
directory as the previous request operated on, libcurl will no longer make
that long series of CWD commands just to end up on the same spot. Note that
this is only for *exactly* the same dir. There is still room for improvements
to optimize the CWD-sending when the dirs are only slightly different.

Added test 210, 211 and 212 to verify these changes. Had to improve the
test script too and added a new primitive to the test file format.

CHANGES
RELEASE-NOTES
lib/ftp.c
lib/url.c
lib/urldata.h
tests/FILEFORMAT
tests/data/Makefile.am
tests/data/test210 [new file with mode: 0644]
tests/data/test211 [new file with mode: 0644]
tests/data/test212 [new file with mode: 0644]
tests/runtests.pl

diff --git a/CHANGES b/CHANGES
index 7900474c359214fb906f0e4334fc07b0353d8d4f..93e77c7a354c5e1259a3fef113a064b26b907888 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,22 @@
 
                                   Changelog
 
+Daniel (25 November 2004)
+- FTP improvements:
+
+  If EPSV, EPRT or LPRT is tried and doesn't work, it will not be retried on
+  the same server again even if a following request is made using a persistent
+  connection.
+
+  If a second request is made to a server, requesting a file from the same
+  directory as the previous request operated on, libcurl will no longer make
+  that long series of CWD commands just to end up on the same spot. Note that
+  this is only for *exactly* the same dir. There is still room for improvements
+  to optimize the CWD-sending when the dirs are only slightly different.
+
+  Added test 210, 211 and 212 to verify these changes. Had to improve the
+  test script too and added a new primitive to the test file format.
+
 Daniel (24 November 2004)
 - Andrés García fixed the configure script to detect select properly when run
   with Msys/Mingw on Windows.
index 4a06b3681308b08613572607f7165081bd72504c..2c55f54276779c96389e2bc0d3325397a645cc6f 100644 (file)
@@ -10,6 +10,7 @@ Curl and libcurl 7.12.3
 
 This release includes the following changes:
 
+ o persistent ftp request improvements
  o CURLOPT_IOCTLFUNCTION and CURLOPT_IOCTLDATA added. If your app uses HTTP
    Digest, NTLM or Negotiate authentication, you will most likely want to use
    these
index af42f3fb711b039a0aaeb42ef1f94ce66bd2fc0b..f083388c6a954e0e50631606f6c7c1e625f9f520 100644 (file)
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -764,6 +764,24 @@ CURLcode Curl_ftp_done(struct connectdata *conn, CURLcode status)
 
   bool was_ctl_valid = ftp->ctl_valid;
 
+  /* now store a copy of the directory we are in */
+  if(ftp->prevpath)
+    free(ftp->prevpath);
+  {
+    size_t flen = ftp->file?strlen(ftp->file):0;
+    size_t dlen = conn->path?strlen(conn->path)-flen:0;
+    if(dlen) {
+      ftp->prevpath = malloc(dlen + 1);
+      if(!ftp->prevpath)
+        return CURLE_OUT_OF_MEMORY;
+      memcpy(ftp->prevpath, conn->path, dlen);
+      ftp->prevpath[dlen]=0; /* terminate */
+      infof(data, "Remembering we are in dir %s\n", ftp->prevpath);
+    }
+    else
+      ftp->prevpath = NULL; /* no path */
+  }
+
   /* free the dir tree and file parts */
   freedirs(ftp);
 
@@ -1085,6 +1103,7 @@ CURLcode ftp_use_port(struct connectdata *conn)
   unsigned char *pp;
   char portmsgbuf[1024], tmp[1024];
 
+  enum ftpcommand { EPRT, LPRT, PORT, DONE } fcmd;
   const char *mode[] = { "EPRT", "LPRT", "PORT", NULL };
   char **modep;
   int rc;
@@ -1165,11 +1184,18 @@ CURLcode ftp_use_port(struct connectdata *conn)
     return CURLE_FTP_PORT_FAILED;
   }
 
-  for (modep = (char **)(data->set.ftp_use_eprt?&mode[0]:&mode[2]);
-       modep && *modep; modep++) {
+  for (fcmd = EPRT; fcmd != DONE; fcmd++) {
     int lprtaf, eprtaf;
     int alen=0, plen=0;
 
+    if(!conn->bits.ftp_use_eprt && (EPRT == fcmd))
+      /* if disabled, goto next */
+      continue;
+
+    if(!conn->bits.ftp_use_lprt && (LPRT == fcmd))
+      /* if disabled, goto next */
+      continue;
+
     switch (sa->sa_family) {
     case AF_INET:
       ap = (unsigned char *)&((struct sockaddr_in *)&ss)->sin_addr;
@@ -1193,7 +1219,7 @@ CURLcode ftp_use_port(struct connectdata *conn)
       break;
     }
 
-    if (strcmp(*modep, "EPRT") == 0) {
+    if (EPRT == fcmd) {
       if (eprtaf < 0)
         continue;
       if (getnameinfo((struct sockaddr *)&ss, sslen,
@@ -1208,22 +1234,21 @@ CURLcode ftp_use_port(struct connectdata *conn)
           *q = '\0';
       }
 
-      result = Curl_ftpsendf(conn, "%s |%d|%s|%s|", *modep, eprtaf,
+      result = Curl_ftpsendf(conn, "%s |%d|%s|%s|", mode[fcmd], eprtaf,
                              portmsgbuf, tmp);
       if(result)
         return result;
     }
-    else if (strcmp(*modep, "LPRT") == 0 ||
-             strcmp(*modep, "PORT") == 0) {
+    else if ((LPRT == fcmd) || (PORT == fcmd)) {
       int i;
 
-      if (strcmp(*modep, "LPRT") == 0 && lprtaf < 0)
+      if ((LPRT == fcmd) && lprtaf < 0)
         continue;
-      if (strcmp(*modep, "PORT") == 0 && sa->sa_family != AF_INET)
+      if ((PORT == fcmd) && sa->sa_family != AF_INET)
         continue;
 
       portmsgbuf[0] = '\0';
-      if (strcmp(*modep, "LPRT") == 0) {
+      if (LPRT == fcmd) {
         snprintf(tmp, sizeof(tmp), "%d,%d", lprtaf, alen);
         if (strlcat(portmsgbuf, tmp, sizeof(portmsgbuf)) >=
             sizeof(portmsgbuf)) {
@@ -1243,7 +1268,7 @@ CURLcode ftp_use_port(struct connectdata *conn)
         }
       }
 
-      if (strcmp(*modep, "LPRT") == 0) {
+      if (LPRT == fcmd) {
         snprintf(tmp, sizeof(tmp), ",%d", plen);
 
         if (strlcat(portmsgbuf, tmp, sizeof(portmsgbuf)) >= sizeof(portmsgbuf))
@@ -1259,7 +1284,7 @@ CURLcode ftp_use_port(struct connectdata *conn)
         }
       }
 
-      result = Curl_ftpsendf(conn, "%s %s", *modep, portmsgbuf);
+      result = Curl_ftpsendf(conn, "%s %s", mode[fcmd], portmsgbuf);
       if(result)
         return result;
     }
@@ -1269,13 +1294,21 @@ CURLcode ftp_use_port(struct connectdata *conn)
       return result;
 
     if (ftpcode != 200) {
+      if (EPRT == fcmd) {
+        infof(data, "disabling EPRT usage\n");
+        conn->bits.ftp_use_eprt = FALSE;
+      }
+      else if (LPRT == fcmd) {
+        infof(data, "disabling LPRT usage\n");
+        conn->bits.ftp_use_lprt = FALSE;
+      }
       continue;
     }
     else
       break;
   }
 
-  if (!*modep) {
+  if (fcmd == DONE) {
     sclose(portsock);
     failf(data, "PORT command attempts failed");
     return CURLE_FTP_PORT_FAILED;
@@ -1479,7 +1512,7 @@ CURLcode ftp_use_pasv(struct connectdata *conn,
   char newhost[48];
   char *newhostp=NULL;
 
-  for (modeoff = (data->set.ftp_use_epsv?0:1);
+  for (modeoff = (conn->bits.ftp_use_epsv?0:1);
        mode[modeoff]; modeoff++) {
     result = Curl_ftpsendf(conn, "%s", mode[modeoff]);
     if(result)
@@ -1489,6 +1522,12 @@ CURLcode ftp_use_pasv(struct connectdata *conn,
       return result;
     if (ftpcode == results[modeoff])
       break;
+
+    if(modeoff == 0) {
+      /* EPSV is not supported, disable it for next transfer */
+      conn->bits.ftp_use_epsv = FALSE;
+      infof(data, "disabling EPSV usage\n");
+    }
   }
 
   if (!mode[modeoff]) {
@@ -2362,6 +2401,8 @@ CURLcode Curl_ftp_disconnect(struct connectdata *conn)
       ftp->cache = NULL;
     }
     freedirs(ftp);
+    if(ftp->prevpath)
+      free(ftp->prevpath);
   }
   return CURLE_OK;
 }
@@ -2707,6 +2748,19 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
     ftp->file=NULL; /* instead of point to a zero byte, we make it a NULL
                        pointer */
 
+  ftp->cwddone = FALSE; /* default to not done */
+  {
+    size_t dlen = conn->path?strlen(conn->path):0;
+    if(dlen && ftp->prevpath) {
+      dlen -= ftp->file?strlen(ftp->file):0;
+      if((dlen == strlen(ftp->prevpath)) &&
+         curl_strnequal(conn->path, ftp->prevpath, dlen)) {
+        infof(data, "Request has same path as previous transfer\n");
+        ftp->cwddone = TRUE;
+      }
+    }
+  }
+
   return retcode;
 }
 
@@ -2727,6 +2781,10 @@ CURLcode ftp_cwd_and_create_path(struct connectdata *conn)
   struct FTP *ftp = conn->proto.ftp;
   int i;
 
+  if(ftp->cwddone)
+    /* already done and fine */
+    return CURLE_OK;
+
   /* This is a re-used connection. Since we change directory to where the
      transfer is taking place, we must now get back to the original dir
      where we ended up after login: */
index 1b15d69578ab825c5656c19144f55272c2deaa13..cfc791104219ae1e05efc450dc9a0ef5b2c63df8 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -321,6 +321,7 @@ CURLcode Curl_open(struct SessionHandle **curl)
     data->set.httpreq = HTTPREQ_GET; /* Default HTTP request */
     data->set.ftp_use_epsv = TRUE;   /* FTP defaults to EPSV operations */
     data->set.ftp_use_eprt = TRUE;   /* FTP defaults to EPRT operations */
+    data->set.ftp_use_lprt = TRUE;   /* FTP defaults to EPRT operations */
 
     data->set.dns_cache_timeout = 60; /* Timeout every 60 seconds by default */
 
@@ -911,6 +912,7 @@ CURLcode Curl_setopt(struct SessionHandle *data, CURLoption option, ...)
 
   case CURLOPT_FTP_USE_EPRT:
     data->set.ftp_use_eprt = va_arg(param, long)?TRUE:FALSE;
+    data->set.ftp_use_lprt = data->set.ftp_use_eprt;
     break;
 
   case CURLOPT_FTP_USE_EPSV:
@@ -1439,7 +1441,7 @@ CURLcode Curl_setopt(struct SessionHandle *data, CURLoption option, ...)
     result = CURLE_FAILED_INIT; /* correct this */
     break;
   }
-  
+
   return result;
 }
 
@@ -2262,6 +2264,9 @@ static CURLcode CreateConnection(struct SessionHandle *data,
   conn->bits.proxy_user_passwd = data->set.proxyuserpwd?1:0;
   conn->bits.no_body = data->set.opt_no_body;
   conn->bits.tunnel_proxy = data->set.tunnel_thru_httpproxy;
+  conn->bits.ftp_use_epsv = data->set.ftp_use_epsv;
+  conn->bits.ftp_use_eprt = data->set.ftp_use_eprt;
+  conn->bits.ftp_use_lprt = data->set.ftp_use_lprt;
 
   /* This initing continues below, see the comment "Continue connectdata
    * initialization here" */
index e161e8083e42c5a7c1cfd61acb28d71a23fe9004..903f47a46459e237d106976b2b355651368bcd69 100644 (file)
@@ -268,10 +268,12 @@ struct FTP {
   long response_time; /* When no timeout is given, this is the amount of
                          seconds we await for an FTP response. Initialized
                          in Curl_ftp_connect() */
-  bool ctl_valid;     /* Tells Curl_ftp_quit() whether or not to do
-                         anything. If the connection has timed out or
-                         been closed, this should be FALSE when it gets
-                         to Curl_ftp_quit() */
+  bool ctl_valid;   /* Tells Curl_ftp_quit() whether or not to do anything. If
+                       the connection has timed out or been closed, this
+                       should be FALSE when it gets to Curl_ftp_quit() */
+  bool cwddone;     /* if it has been determined that the proper CWD combo
+                       already has been done */
+  char *prevpath;   /* conn->path from the previous transfer */
 };
 
 /****************************************************************************
@@ -327,6 +329,16 @@ struct ConnectBits {
                           though it will be discarded. When the whole send
                           operation is done, we must call the data rewind
                           callback. */
+  bool ftp_use_epsv;  /* As set with CURLOPT_FTP_USE_EPSV, but if we find out
+                         EPSV doesn't work we disable it for the forthcoming
+                         requests */
+
+  bool ftp_use_eprt;  /* As set with CURLOPT_FTP_USE_EPRT, but if we find out
+                         EPRT doesn't work we disable it for the forthcoming
+                         requests */
+  bool ftp_use_lprt;  /* As set with CURLOPT_FTP_USE_EPRT, but if we find out
+                         LPRT doesn't work we disable it for the forthcoming
+                         requests */
 };
 
 struct hostname {
@@ -931,6 +943,7 @@ struct UserDefined {
   bool expect100header;  /* TRUE if we added Expect: 100-continue */
   bool ftp_use_epsv;     /* if EPSV is to be attempted or not */
   bool ftp_use_eprt;     /* if EPRT is to be attempted or not */
+  bool ftp_use_lprt;     /* if LPRT is to be attempted or not */
   curl_ftpssl ftp_ssl;   /* if AUTH TLS is to be attempted etc */
   curl_ftpauth ftpsslauth; /* what AUTH XXX to be attempted */
   bool no_signal;        /* do not use any signal/alarm handler */
index a3b8a5511df435b3c0d13b11d785891c5ad1dd70..b65b6b2ca3aa43e9228e8e6d1d08aa57885768e8 100644 (file)
@@ -86,6 +86,7 @@ netrc_debug
 large_file
 idn
 getrlimit
+ipv6
 </features>
 
 <killserver>
@@ -165,6 +166,10 @@ One regex per line that is removed from the protocol dumps before the
 comparison is made. This is very useful to remove dependencies on dynamicly
 changing protocol data such as port numbers or user-agent strings.
 </strip>
+<strippart>
+One perl op per line that operates on the protocol dump. This is pretty
+advanced. Example: "s/^EPRT .*/EPRT stripped/"
+</strippart>
 <protocol [nonewline=yes]>
 the protocol dump curl should transmit, if 'nonewline' is set, we will cut
 off the trailing newline of this given data before comparing with the one
index 16841cf71b90db7e2292c197f2983bc4bb0f8f6d..e7a5fce850b6cf3a2ddad095fc9c728ed4e3c329 100644 (file)
@@ -28,7 +28,7 @@ EXTRA_DIST = test1 test108 test117 test127 test20 test27 test34 test46        \
  test513 test514 test178 test179 test180 test181 test182 test183       \
  test184 test185 test186 test187 test188 test189 test191 test192       \
  test193 test194 test195 test196 test197 test198 test515 test516       \
- test517 test518
+ test517 test518 test210 test211 test212
 
 # The following tests have been removed from the dist since they no longer
 # work. We need to fix the test suite's FTPS server first, then bring them
diff --git a/tests/data/test210 b/tests/data/test210
new file mode 100644 (file)
index 0000000..7235080
--- /dev/null
@@ -0,0 +1,43 @@
+# Server-side
+<reply>
+<data>
+data blobb
+</data>
+<datacheck>
+data blobb
+data blobb
+</datacheck>
+</reply>
+
+# Client-side
+<client>
+<server>
+ftp
+</server>
+ <name>
+Get two FTP files from the same remote dir: no second CWD
+ </name>
+ <command>
+ftp://%HOSTIP:%FTPPORT/a/path/210 ftp://%HOSTIP:%FTPPORT/a/path/210
+</command>
+</test>
+
+# Verify data after the test has been "shot"
+<verify>
+<protocol>
+USER anonymous\r
+PASS curl_by_daniel@haxx.se\r
+PWD\r
+CWD a\r
+CWD path\r
+EPSV\r
+TYPE I\r
+SIZE 210\r
+RETR 210\r
+EPSV\r
+TYPE I\r
+SIZE 210\r
+RETR 210\r
+QUIT\r
+</protocol>
+</verify>
diff --git a/tests/data/test211 b/tests/data/test211
new file mode 100644 (file)
index 0000000..c59da0b
--- /dev/null
@@ -0,0 +1,47 @@
+# Server-side
+<reply>
+<data>
+data blobb
+</data>
+<datacheck>
+data blobb
+data blobb
+</datacheck>
+</reply>
+
+# Client-side
+<client>
+<server>
+ftp
+</server>
+ <name>
+Get two FTP files with no remote EPSV support
+ </name>
+ <command>
+ftp://%HOSTIP:%FTPPORT/a/path/211 ftp://%HOSTIP:%FTPPORT/a/path/211
+</command>
+<file name="log/ftpserver.cmd">
+REPLY EPSV 500 no such command
+</file>
+</test>
+
+# Verify data after the test has been "shot"
+<verify>
+<protocol>
+USER anonymous\r
+PASS curl_by_daniel@haxx.se\r
+PWD\r
+CWD a\r
+CWD path\r
+EPSV\r
+PASV\r
+TYPE I\r
+SIZE 211\r
+RETR 211\r
+PASV\r
+TYPE I\r
+SIZE 211\r
+RETR 211\r
+QUIT\r
+</protocol>
+</verify>
diff --git a/tests/data/test212 b/tests/data/test212
new file mode 100644 (file)
index 0000000..cae7baf
--- /dev/null
@@ -0,0 +1,57 @@
+# Server-side
+<reply>
+<data>
+data blobb
+</data>
+<datacheck>
+data blobb
+data blobb
+</datacheck>
+</reply>
+
+# Client-side
+<client>
+<features>
+ipv6
+</features>
+<server>
+ftp
+</server>
+ <name>
+Get two FTP files with no remote EPRT or LPRT support
+ </name>
+ <command>
+ftp://%HOSTIP:%FTPPORT/a/path/212 ftp://%HOSTIP:%FTPPORT/a/path/212 -P -
+</command>
+<file name="log/ftpserver.cmd">
+REPLY EPRT 500 no such command
+REPLY LPRT 500 no such command
+</file>
+</test>
+
+# Verify data after the test has been "shot"
+<verify>
+<strippart>
+s/^EPRT .*/EPRT stripped/
+s/^LPRT .*/LPRT stripped/
+s/^PORT .*/PORT stripped/
+</strippart>
+<protocol>
+USER anonymous\r
+PASS curl_by_daniel@haxx.se\r
+PWD\r
+CWD a\r
+CWD path\r
+EPRT stripped
+LPRT stripped
+PORT stripped
+TYPE I\r
+SIZE 212\r
+RETR 212\r
+PORT stripped\r
+TYPE I\r
+SIZE 212\r
+RETR 212\r
+QUIT\r
+</protocol>
+</verify>
index 0acef67b4076e03ae90b89e11b0c8dc9308ac4cc..aba8c76498f381b3c765a42d938ceecdac0d188f 100755 (executable)
@@ -95,6 +95,7 @@ my $gdb = checkcmd("gdb");
 my $ssl_version; # set if libcurl is built with SSL support
 my $large_file;  # set if libcurl is built with large file support
 my $has_idn;     # set if libcurl is built with IDN support
+my $has_ipv6;    # set if libcurl is built with IPv6 support
 my $has_getrlimit;  # set if system has getrlimit()
 
 my $skipped=0;  # number of tests skipped; reported in main loop
@@ -758,6 +759,9 @@ sub checkcurl {
                 # IDN support
                 $has_idn=1;
             }
+            if($feat =~ /IPv6/i) {
+                $has_ipv6 = 1;
+            }
         }
     }
     if(!$curl) {
@@ -784,6 +788,7 @@ sub checkcurl {
     print "********* System characteristics ******** \n",
     "* $curl\n",
     "* $libcurl\n",
+    "* Features: $feat\n"
     "* Host: $hostname",
     "* System: $hosttype";
 
@@ -873,6 +878,11 @@ sub singletest {
                 next;
             }
         }
+        elsif($f eq "ipv6") {
+            if($has_ipv6) {
+                next;
+            }
+        }
         elsif($f eq "getrlimit") {
             if($has_getrlimit) {
                 next;
@@ -1259,6 +1269,16 @@ sub singletest {
             @protstrip= striparray( $_, \@protstrip);
         }
 
+        # what parts to cut off from the protocol
+        my @strippart = getpart("verify", "strippart");
+        my $strip;
+        for $strip (@strippart) {
+            chomp $strip;
+            for(@out) {
+                eval $strip;
+            }
+        }
+
         $res = compare("protocol", \@out, \@protstrip);
         if($res) {
             return 1;