Max Katsev reported that when doing a libcurl FTP request with

CURLOPT_NOBODY enabled but not CURLOPT_HEADER, libcurl wouldn't do TYPE
before it does SIZE which makes it less useful. I walked over the code and
made it do this properly, and added test case 542 to verify it.

diff --git a/CHANGES b/CHANGES
index 47060eb7df59cf2422a079e4a2638ad238420f60..730f9cdd6429fc93dd159555176b5fc45181dff2 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,12 @@
 
                                   Changelog
 
+Daniel S (26 September 2007)
+- Max Katsev reported that when doing a libcurl FTP request with
+  CURLOPT_NOBODY enabled but not CURLOPT_HEADER, libcurl wouldn't do TYPE
+  before it does SIZE which makes it less useful. I walked over the code and
+  made it do this properly, and added test case 542 to verify it.
+
 Daniel S (24 September 2007)
 - Immanuel Gregoire fixed KNOWN_BUGS #44: --ftp-method nocwd did not handle
   URLs ending with a slash properly (it should list the contents of that

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index f425abd119175f3ac91b28a96fc05f348f4a86b2..75676d519d856ae0e2cbe84e7ea91ce40129354e 100644 (file)
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -23,6 +23,8 @@ This release includes the following bugfixes:
  o no HOME and no key given caused SSH auth failure
  o Negotiate authentication over proxy
  o --ftp-method nocwd on directory listings
+ o FTP, CURLOPT_NOBODY enabled and CURLOPT_HEADER disabled now does TYPE
+   before SIZE
 
 This release includes the following known bugs:
 
@@ -40,6 +42,6 @@ This release would not have looked like this without help, code, reports and
 advice from friends like these:
 
  Dan Fandrich, Michal Marek, Günter Knauf, Rob Crittenden, Immanuel Gregoire,
- Mark Davies
+ Mark Davies, Max Katsev
  
         Thanks! (and sorry if I forgot to mention someone)

diff --git a/lib/ftp.c b/lib/ftp.c
index c92b08e497bcdab3ca980ca97597e16df658da08..b2122fa6b3c13615be9d6b5a035ec7e10a091b3c 100644 (file)
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -1212,7 +1212,7 @@ static CURLcode ftp_state_post_rest(struct connectdata *conn)
   struct FTP *ftp = conn->data->reqdata.proto.ftp;
   struct SessionHandle *data = conn->data;
 
-  if(ftp->no_transfer) {
+  if(ftp->transfer != FTPTRANSFER_BODY) {
     /* doesn't transfer any data */
 
     /* still possibly do PRE QUOTE jobs */
@@ -1235,7 +1235,7 @@ static CURLcode ftp_state_post_size(struct connectdata *conn)
   CURLcode result = CURLE_OK;
   struct FTP *ftp = conn->data->reqdata.proto.ftp;
 
-  if(ftp->no_transfer && ftp->file) {
+  if((ftp->transfer != FTPTRANSFER_BODY) && ftp->file) {
     /* if a "head"-like request is being made (on a file) */
 
     /* Determine if server can respond to REST command and therefore
@@ -1255,7 +1255,7 @@ static CURLcode ftp_state_post_type(struct connectdata *conn)
   CURLcode result = CURLE_OK;
   struct FTP *ftp = conn->data->reqdata.proto.ftp;
 
-  if(ftp->no_transfer && ftp->file) {
+  if((ftp->transfer == FTPTRANSFER_INFO) && ftp->file) {
     /* if a "head"-like request is being made (on a file) */
 
     /* we know ftp->file is a valid pointer to a file name */
@@ -1362,13 +1362,14 @@ static CURLcode ftp_state_post_mdtm(struct connectdata *conn)
   /* If we have selected NOBODY and HEADER, it means that we only want file
      information. Which in FTP can't be much more than the file size and
      date. */
-  if(conn->bits.no_body && data->set.include_header && ftp->file &&
+  if(conn->bits.no_body && ftp->file &&
      ftp_need_type(conn, data->set.prefer_ascii)) {
     /* The SIZE command is _not_ RFC 959 specified, and therefor many servers
        may not support it! It is however the only way we have to get a file's
        size! */
 
-    ftp->no_transfer = TRUE; /* this means no actual transfer will be made */
+    ftp->transfer = FTPTRANSFER_INFO;
+    /* this means no actual transfer will be made */
 
     /* Some servers return different sizes for different modes, and thus we
        must set the proper type before we check the size */
@@ -1475,9 +1476,9 @@ static CURLcode ftp_state_ul_setup(struct connectdata *conn,
         /* no data to transfer */
         result=Curl_setup_transfer(conn, -1, -1, FALSE, NULL, -1, NULL);
 
-        /* Set no_transfer so that we won't get any error in
+        /* Set ->transfer so that we won't get any error in
          * Curl_ftp_done() because we didn't transfer anything! */
-        ftp->no_transfer = TRUE;
+        ftp->transfer = FTPTRANSFER_NONE;
 
         state(conn, FTP_STOP);
         return CURLE_OK;
@@ -1547,7 +1548,7 @@ static CURLcode ftp_state_quote(struct connectdata *conn,
       result = ftp_state_cwd(conn);
       break;
     case FTP_RETR_PREQUOTE:
-      if (ftp->no_transfer)
+      if (ftp->transfer != FTPTRANSFER_BODY)
         state(conn, FTP_STOP);
       else {
         NBFTPSENDF(conn, "SIZE %s", ftp->file);
@@ -1877,7 +1878,6 @@ static CURLcode ftp_state_mdtm_resp(struct connectdata *conn,
          we "emulate" a HTTP-style header in our output. */
 
       if(conn->bits.no_body &&
-         data->set.include_header &&
          ftp->file &&
          data->set.get_filetime &&
          (data->info.filetime>=0) ) {
@@ -1922,7 +1922,7 @@ static CURLcode ftp_state_mdtm_resp(struct connectdata *conn,
       default:
         if(data->info.filetime <= data->set.timevalue) {
           infof(data, "The requested document is not new enough\n");
-          ftp->no_transfer = TRUE; /* mark this to not transfer data */
+          ftp->transfer = FTPTRANSFER_NONE; /* mark this to not transfer data */
           state(conn, FTP_STOP);
           return CURLE_OK;
         }
@@ -1930,7 +1930,7 @@ static CURLcode ftp_state_mdtm_resp(struct connectdata *conn,
       case CURL_TIMECOND_IFUNMODSINCE:
         if(data->info.filetime > data->set.timevalue) {
           infof(data, "The requested document is not old enough\n");
-          ftp->no_transfer = TRUE; /* mark this to not transfer data */
+          ftp->transfer = FTPTRANSFER_NONE; /* mark this to not transfer data */
           state(conn, FTP_STOP);
           return CURLE_OK;
         }
@@ -2034,9 +2034,9 @@ static CURLcode ftp_state_post_retr_size(struct connectdata *conn,
       result = Curl_setup_transfer(conn, -1, -1, FALSE, NULL, -1, NULL);
       infof(data, "File already completely downloaded\n");
 
-      /* Set no_transfer so that we won't get any error in Curl_ftp_done()
+      /* Set ->transfer so that we won't get any error in Curl_ftp_done()
        * because we didn't transfer the any file */
-      ftp->no_transfer = TRUE;
+      ftp->transfer = FTPTRANSFER_NONE;
       state(conn, FTP_STOP);
       return CURLE_OK;
     }
@@ -2291,7 +2291,7 @@ static CURLcode ftp_state_get_resp(struct connectdata *conn,
   else {
     if((instate == FTP_LIST) && (ftpcode == 450)) {
       /* simply no matching files in the dir listing */
-      ftp->no_transfer = TRUE; /* don't download anything */
+      ftp->transfer = FTPTRANSFER_NONE; /* don't download anything */
       state(conn, FTP_STOP); /* this phase is over */
     }
     else {
@@ -3121,7 +3121,7 @@ CURLcode Curl_ftp_done(struct connectdata *conn, CURLcode status,
 
   conn->sock[SECONDARYSOCKET] = CURL_SOCKET_BAD;
 
-  if(!ftp->no_transfer && !status && !premature) {
+  if((ftp->transfer == FTPTRANSFER_BODY) && !status && !premature) {
     /*
      * Let's see what the server says about the transfer we just performed,
      * but lower the timeout as sometimes this connection has died while the
@@ -3162,7 +3162,7 @@ CURLcode Curl_ftp_done(struct connectdata *conn, CURLcode status,
     if((-1 != data->set.infilesize) &&
        (data->set.infilesize != *ftp->bytecountp) &&
        !data->set.crlf &&
-       !ftp->no_transfer) {
+       (ftp->transfer == FTPTRANSFER_BODY)) {
       failf(data, "Uploaded unaligned file size (%" FORMAT_OFF_T
             " out of %" FORMAT_OFF_T " bytes)",
             *ftp->bytecountp, data->set.infilesize);
@@ -3192,7 +3192,7 @@ CURLcode Curl_ftp_done(struct connectdata *conn, CURLcode status,
   }
 
   /* clear these for next connection */
-  ftp->no_transfer = FALSE;
+  ftp->transfer = FTPTRANSFER_BODY;
   ftpc->dont_check = FALSE;
 
   /* Send any post-transfer QUOTE strings? */
@@ -3375,12 +3375,12 @@ CURLcode Curl_ftp_nextconnect(struct connectdata *conn)
 
   DEBUGF(infof(data, "DO-MORE phase starts\n"));
 
-  if(!ftp->no_transfer) {
-    /* a transfer is about to take place */
+  if(ftp->transfer <= FTPTRANSFER_INFO) {
+    /* a transfer is about to take place, or if not a file name was given
+       so we'll do a SIZE on it later and then we need the right TYPE first */
 
     if(data->set.upload) {
-      result = ftp_nb_type(conn, data->set.prefer_ascii,
-                                      FTP_STOR_TYPE);
+      result = ftp_nb_type(conn, data->set.prefer_ascii, FTP_STOR_TYPE);
       if (result)
         return result;
     }
@@ -3391,13 +3391,18 @@ CURLcode Curl_ftp_nextconnect(struct connectdata *conn)
       result = ftp_range(conn);
       if(result)
         ;
-      else if((data->set.ftp_list_only) || !ftp->file) {
+      else if(data->set.ftp_list_only || !ftp->file) {
         /* The specified path ends with a slash, and therefore we think this
            is a directory that is requested, use LIST. But before that we
            need to set ASCII transfer mode. */
-        result = ftp_nb_type(conn, 1, FTP_LIST_TYPE);
-        if (result)
-          return result;
+
+        /* But only if a body transfer was requested. */
+        if(ftp->transfer == FTPTRANSFER_BODY) {
+          result = ftp_nb_type(conn, 1, FTP_LIST_TYPE);
+          if (result)
+            return result;
+        }
+        /* otherwise just fall through */
       }
       else {
         result = ftp_nb_type(conn, data->set.prefer_ascii, FTP_RETR_TYPE);
@@ -3408,7 +3413,7 @@ CURLcode Curl_ftp_nextconnect(struct connectdata *conn)
     result = ftp_easy_statemach(conn);
   }
 
-  if(ftp->no_transfer)
+  if(ftp->transfer != FTPTRANSFER_BODY)
     /* no data to transfer. FIX: it feels like a kludge to have this here
        too! */
     result=Curl_setup_transfer(conn, -1, -1, FALSE, NULL, -1, NULL);
@@ -3442,7 +3447,7 @@ CURLcode ftp_perform(struct connectdata *conn,
   if(conn->bits.no_body) {
     /* requested no body means no transfer... */
     struct FTP *ftp = conn->data->reqdata.proto.ftp;
-    ftp->no_transfer = TRUE;
+    ftp->transfer = FTPTRANSFER_INFO;
   }
 
 
@@ -3865,7 +3870,7 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
     ftp->file=NULL; /* instead of point to a zero byte, we make it a NULL
                        pointer */
 
-  if(data->set.upload && !ftp->file && !ftp->no_transfer) {
+  if(data->set.upload && !ftp->file && (ftp->transfer == FTPTRANSFER_BODY)) {
     /* We need a file name when uploading. Return error! */
     failf(data, "Uploading to a URL without a file name!");
     return CURLE_URL_MALFORMAT;
@@ -3912,7 +3917,7 @@ static CURLcode ftp_dophase_done(struct connectdata *conn,
     return result;
   }
 
-  if(ftp->no_transfer)
+  if(ftp->transfer != FTPTRANSFER_BODY)
     /* no data to transfer */
     result=Curl_setup_transfer(conn, -1, -1, FALSE, NULL, -1, NULL);
   else if(!connected)

diff --git a/lib/urldata.h b/lib/urldata.h
index 2d09be07b8dce3efb13677282f500db94b60edff..435693ee08b1fb6e6aed10cb4ddf38e69601ea91 100644 (file)
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -362,6 +362,13 @@ typedef enum {
   FTPFILE_SINGLECWD = 3  /* make one CWD, then SIZE / RETR / STOR on the file */
 } curl_ftpfile;
 
+typedef enum {
+  FTPTRANSFER_BODY, /* yes do transfer a body */
+  FTPTRANSFER_INFO, /* do still go through to get info/headers */
+  FTPTRANSFER_NONE, /* don't get anything and don't get info */
+  FTPTRANSFER_LAST  /* end of list marker, never used */
+} curl_ftptransfer;
+
 /* This FTP struct is used in the SessionHandle. All FTP data that is
    connection-oriented must be in FTP_conn to properly deal with the fact that
    perhaps the SessionHandle is changed between the times the connection is
@@ -372,8 +379,10 @@ struct FTP {
   char *passwd;  /* password string */
   char *urlpath; /* the originally given path part of the URL */
   char *file;    /* decoded file */
-  bool no_transfer; /* nothing was transfered, (possibly because a resumed
-                       transfer already was complete) */
+
+  /* transfer a file/body or not, done as a typedefed enum just to make
+     debuggers display the full symbol and not just the numerical value */
+  curl_ftptransfer transfer;
   curl_off_t downloadsize;
 };
 
@@ -570,7 +579,7 @@ struct ConnectBits {
                          proxies, but can also be enabled explicitly by
                          apps */
   bool tunnel_connecting; /* TRUE while we're still waiting for a proxy CONNECT
-                          */
+                           */
   bool authneg;       /* TRUE when the auth phase has started, which means
                          that we are creating a request with an auth header,
                          but it is not the final request in the auth

diff --git a/tests/data/test542 b/tests/data/test542
new file mode 100644 (file)
index 0000000..5afe124
--- /dev/null
+++ b/tests/data/test542
@@ -0,0 +1,57 @@
+<testcase>
+<info>
+<keywords>
+FTP
+PASV
+RETR
+</keywords>
+</info>
+# Server-side
+<reply>
+<data>
+data
+    to
+      see
+that FTP
+works
+  so does it?
+</data>
+<datacheck>
+Content-Length: 51\r
+Accept-ranges: bytes\r
+</datacheck>
+</reply>
+
+# Client-side
+<client>
+<server>
+ftp
+</server>
+<tool>
+lib542
+</tool>
+ <name>
+FTP a file with NOBODY yes and HEADER no
+ </name>
+ <command>
+ftp://%HOSTIP:%FTPPORT/542
+</command>
+
+</client>
+
+# Verify data after the test has been "shot"
+#
+# There's no MTDM in the protocol here since this code doesn't ask for the
+# time/date of the file
+<verify>
+<protocol>
+USER anonymous\r
+PASS ftp@example.com\r
+PWD\r
+TYPE I\r
+SIZE 542\r
+REST 0\r
+QUIT\r
+</protocol>
+</verify>
+</testcase>

diff --git a/tests/libtest/Makefile.am b/tests/libtest/Makefile.am
index dc55afce70e6d1836a37ab95e7e735526fe6ca3a..395564a6edd84c3e2b6acd4bf27f00af490d0497 100644 (file)
--- a/tests/libtest/Makefile.am
+++ b/tests/libtest/Makefile.am
@@ -47,7 +47,7 @@ SUPPORTFILES = first.c test.h
 noinst_PROGRAMS = lib500 lib501 lib502 lib503 lib504 lib505 lib506     \
   lib507 lib508 lib509 lib510 lib511 lib512 lib513 lib514 lib515 lib516        \
   lib517 lib518 lib519 lib520 lib521 lib523 lib524 lib525 lib526 lib527        \
-  lib529 lib530 lib532 lib533 lib536 lib537 lib540 lib541
+  lib529 lib530 lib532 lib533 lib536 lib537 lib540 lib541 lib542
 
 # Dependencies (may need to be overriden)
 LDADD = $(LIBDIR)/libcurl.la
@@ -128,3 +128,5 @@ lib537_SOURCES = lib537.c $(SUPPORTFILES)
 lib540_SOURCES = lib540.c $(SUPPORTFILES)
 
 lib541_SOURCES = lib541.c $(SUPPORTFILES)
+
+lib542_SOURCES = lib542.c $(SUPPORTFILES)

diff --git a/tests/libtest/lib542.c b/tests/libtest/lib542.c
new file mode 100644 (file)
index 0000000..6aee901
--- /dev/null
+++ b/tests/libtest/lib542.c
@@ -0,0 +1,74 @@
+/*****************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * $Id$
+ */
+
+#include "setup.h" /* struct_stat etc. */
+#include "test.h"
+
+#ifdef HAVE_SYS_SOCKET_H
+#include <sys/socket.h>
+#endif
+#ifdef HAVE_SYS_TYPES_H
+#include <sys/types.h>
+#endif
+#ifdef HAVE_SYS_STAT_H
+#include <sys/stat.h>
+#endif
+#ifdef HAVE_FCNTL_H
+#include <fcntl.h>
+#endif
+
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
+/*
+ * FTP get with NOBODY but no HEADER
+ */
+
+int test(char *URL)
+{
+  CURL *curl;
+  CURLcode res = CURLE_OK;
+  FILE *hd_src ;
+
+  if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK) {
+    fprintf(stderr, "curl_global_init() failed\n");
+    fclose(hd_src);
+    return TEST_ERR_MAJOR_BAD;
+  }
+
+  /* get a curl handle */
+  if ((curl = curl_easy_init()) == NULL) {
+    fprintf(stderr, "curl_easy_init() failed\n");
+    curl_global_cleanup();
+    fclose(hd_src);
+    return TEST_ERR_MAJOR_BAD;
+  }
+
+  /* enable verbose */
+  curl_easy_setopt(curl, CURLOPT_VERBOSE, TRUE) ;
+
+  /* enable NOBODY */
+  curl_easy_setopt(curl, CURLOPT_NOBODY, TRUE) ;
+
+  /* disable HEADER */
+  curl_easy_setopt(curl, CURLOPT_HEADER, FALSE) ;
+
+  /* specify target */
+  curl_easy_setopt(curl,CURLOPT_URL, URL);
+
+  /* Now run off and do what you've been told! */
+  res = curl_easy_perform(curl);
+
+  curl_easy_cleanup(curl);
+  curl_global_cleanup();
+
+  return res;
+}