From a6315359d742bdf967ba3ee1db3e0b7e5a3956fe Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Wed, 26 Sep 2007 12:00:01 +0000 Subject: 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. --- CHANGES | 6 ++++ RELEASE-NOTES | 4 ++- lib/ftp.c | 63 +++++++++++++++++++++------------------- lib/urldata.h | 15 ++++++++-- tests/data/test542 | 57 ++++++++++++++++++++++++++++++++++++ tests/libtest/Makefile.am | 4 ++- tests/libtest/lib542.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 189 insertions(+), 34 deletions(-) create mode 100644 tests/data/test542 create mode 100644 tests/libtest/lib542.c diff --git a/CHANGES b/CHANGES index 47060eb7d..730f9cdd6 100644 --- 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 f425abd11..75676d519 100644 --- 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 c92b08e49..b2122fa6b 100644 --- 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 2d09be07b..435693ee0 100644 --- 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 index 000000000..5afe12470 --- /dev/null +++ b/tests/data/test542 @@ -0,0 +1,57 @@ + + + +FTP +PASV +RETR + + +# Server-side + + +data + to + see +that FTP +works + so does it? + + +Content-Length: 51 +Accept-ranges: bytes + + + +# Client-side + + +ftp + + +lib542 + + +FTP a file with NOBODY yes and HEADER no + + +ftp://%HOSTIP:%FTPPORT/542 + + + + +# 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 + + +USER anonymous +PASS ftp@example.com +PWD +TYPE I +SIZE 542 +REST 0 +QUIT + + + diff --git a/tests/libtest/Makefile.am b/tests/libtest/Makefile.am index dc55afce7..395564a6e 100644 --- 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 index 000000000..6aee9013f --- /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 +#endif +#ifdef HAVE_SYS_TYPES_H +#include +#endif +#ifdef HAVE_SYS_STAT_H +#include +#endif +#ifdef HAVE_FCNTL_H +#include +#endif + +#ifdef HAVE_UNISTD_H +#include +#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; +} -- cgit v1.2.3