diff options
| -rw-r--r-- | CHANGES | 17 | ||||
| -rw-r--r-- | RELEASE-NOTES | 1 | ||||
| -rw-r--r-- | lib/file.c | 28 | ||||
| -rw-r--r-- | lib/ftp.c | 92 | ||||
| -rw-r--r-- | lib/http.c | 7 | ||||
| -rw-r--r-- | lib/ssh.c | 7 | ||||
| -rw-r--r-- | lib/url.c | 16 | ||||
| -rw-r--r-- | lib/url.h | 5 | ||||
| -rw-r--r-- | lib/urldata.h | 5 |
9 files changed, 116 insertions, 62 deletions
@@ -7,6 +7,23 @@ Changelog Daniel S (22 October 2007) +- Michal Marek forwarded the bug report + https://bugzilla.novell.com/show_bug.cgi?id=332917 about a HTTP redirect to + FTP that caused memory havoc. His work together with my efforts created two + fixes: + + #1 - FTP::file was moved to struct ftp_conn, because is has to be dealt with + at connection cleanup, at which time the struct HandleData could be + used by another connection. + Also, the unused char *urlpath member is removed from struct FTP. + + #2 - provide a Curl_reset_reqproto() function that frees + data->reqdata.proto.* on connection setup if needed (that is if the + SessionHandle was used by a different connection). + + A long-term goal is of course to somehow get rid of how the reqdata struct + is used, as it is too error-prone. + - Bug report #1815530 (http://curl.haxx.se/bug/view.cgi?id=1815530) points out that specifying a proxy with a trailing slash didn't work (unless it also contained a port number). diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 91a938262..cbc24b8ba 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -40,6 +40,7 @@ This release includes the following bugfixes: o CURLOPT_POSTFIELDS could fail to send binary data o specifying a proxy with a trailing slash didn't work (unless it also contained a port number) + o redirect from HTTP to FTP memory problem This release includes the following known bugs: diff --git a/lib/file.c b/lib/file.c index 6d4ae9892..bce92bd2a 100644 --- a/lib/file.c +++ b/lib/file.c @@ -125,8 +125,8 @@ const struct Curl_handler Curl_handler_file = { */ CURLcode Curl_file_connect(struct connectdata *conn) { - char *real_path = curl_easy_unescape(conn->data, conn->data->reqdata.path, 0, - NULL); + struct SessionHandle *data = conn->data; + char *real_path = curl_easy_unescape(data, data->reqdata.path, 0, NULL); struct FILEPROTO *file; int fd; #if defined(WIN32) || defined(MSDOS) || defined(__EMX__) @@ -137,16 +137,18 @@ CURLcode Curl_file_connect(struct connectdata *conn) if(!real_path) return CURLE_OUT_OF_MEMORY; - file = (struct FILEPROTO *)calloc(sizeof(struct FILEPROTO), 1); - if(!file) { - free(real_path); - return CURLE_OUT_OF_MEMORY; - } - - if (conn->data->reqdata.proto.file) - free(conn->data->reqdata.proto.file); + /* If there already is a protocol-specific struct allocated for this + sessionhandle, deal with it */ + Curl_reset_reqproto(conn); - conn->data->reqdata.proto.file = file; + if (!data->reqdata.proto.file) { + file = (struct FILEPROTO *)calloc(sizeof(struct FILEPROTO), 1); + if(!file) { + free(real_path); + return CURLE_OUT_OF_MEMORY; + } + data->reqdata.proto.file = file; + } #if defined(WIN32) || defined(MSDOS) || defined(__EMX__) /* If the first character is a slash, and there's @@ -186,8 +188,8 @@ CURLcode Curl_file_connect(struct connectdata *conn) file->freepath = real_path; /* free this when done */ file->fd = fd; - if(!conn->data->set.upload && (fd == -1)) { - failf(conn->data, "Couldn't open file %s", conn->data->reqdata.path); + if(!data->set.upload && (fd == -1)) { + failf(data, "Couldn't open file %s", data->reqdata.path); Curl_file_done(conn, CURLE_FILE_COULDNT_READ_FILE, FALSE); return CURLE_FILE_COULDNT_READ_FILE; } @@ -90,6 +90,7 @@ #include "parsedate.h" /* for the week day and month names */ #include "sockaddr.h" /* required for Curl_sockaddr_storage */ #include "multiif.h" +#include "url.h" #if defined(HAVE_INET_NTOA_R) && !defined(HAVE_INET_NTOA_R_DECL) #include "inet_ntoa_r.h" @@ -261,7 +262,6 @@ const struct Curl_handler Curl_handler_ftps_proxy = { static void freedirs(struct connectdata *conn) { struct ftp_conn *ftpc = &conn->proto.ftpc; - struct FTP *ftp = conn->data->reqdata.proto.ftp; int i; if(ftpc->dirs) { @@ -274,9 +274,9 @@ static void freedirs(struct connectdata *conn) free(ftpc->dirs); ftpc->dirs = NULL; } - if(ftp->file) { - free(ftp->file); - ftp->file = NULL; + if(ftpc->file) { + free(ftpc->file); + ftpc->file = NULL; } } @@ -1339,8 +1339,9 @@ static CURLcode ftp_state_post_size(struct connectdata *conn) { CURLcode result = CURLE_OK; struct FTP *ftp = conn->data->reqdata.proto.ftp; + struct ftp_conn *ftpc = &conn->proto.ftpc; - if((ftp->transfer != FTPTRANSFER_BODY) && ftp->file) { + if((ftp->transfer != FTPTRANSFER_BODY) && ftpc->file) { /* if a "head"-like request is being made (on a file) */ /* Determine if server can respond to REST command and therefore @@ -1359,12 +1360,13 @@ static CURLcode ftp_state_post_type(struct connectdata *conn) { CURLcode result = CURLE_OK; struct FTP *ftp = conn->data->reqdata.proto.ftp; + struct ftp_conn *ftpc = &conn->proto.ftpc; - if((ftp->transfer == FTPTRANSFER_INFO) && ftp->file) { + if((ftp->transfer == FTPTRANSFER_INFO) && ftpc->file) { /* if a "head"-like request is being made (on a file) */ - /* we know ftp->file is a valid pointer to a file name */ - NBFTPSENDF(conn, "SIZE %s", ftp->file); + /* we know ftpc->file is a valid pointer to a file name */ + NBFTPSENDF(conn, "SIZE %s", ftpc->file); state(conn, FTP_SIZE); } @@ -1466,11 +1468,12 @@ static CURLcode ftp_state_post_mdtm(struct connectdata *conn) CURLcode result = CURLE_OK; struct FTP *ftp = conn->data->reqdata.proto.ftp; struct SessionHandle *data = conn->data; + struct ftp_conn *ftpc = &conn->proto.ftpc; /* 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 && ftp->file && + if(conn->bits.no_body && ftpc->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 @@ -1496,15 +1499,15 @@ static CURLcode ftp_state_post_mdtm(struct connectdata *conn) static CURLcode ftp_state_post_cwd(struct connectdata *conn) { CURLcode result = CURLE_OK; - struct FTP *ftp = conn->data->reqdata.proto.ftp; struct SessionHandle *data = conn->data; + struct ftp_conn *ftpc = &conn->proto.ftpc; /* Requested time of file or time-depended transfer? */ - if((data->set.get_filetime || data->set.timecondition) && ftp->file) { + if((data->set.get_filetime || data->set.timecondition) && ftpc->file) { /* we have requested to get the modified-time of the file, this is a white spot as the MDTM is not mentioned in RFC959 */ - NBFTPSENDF(conn, "MDTM %s", ftp->file); + NBFTPSENDF(conn, "MDTM %s", ftpc->file); state(conn, FTP_MDTM); } @@ -1522,6 +1525,7 @@ static CURLcode ftp_state_ul_setup(struct connectdata *conn, CURLcode result = CURLE_OK; struct FTP *ftp = conn->data->reqdata.proto.ftp; struct SessionHandle *data = conn->data; + struct ftp_conn *ftpc = &conn->proto.ftpc; curl_off_t passed=0; if((data->reqdata.resume_from && !sizechecked) || @@ -1541,7 +1545,7 @@ static CURLcode ftp_state_ul_setup(struct connectdata *conn, if(data->reqdata.resume_from < 0 ) { /* Got no given size to start from, figure it out */ - NBFTPSENDF(conn, "SIZE %s", ftp->file); + NBFTPSENDF(conn, "SIZE %s", ftpc->file); state(conn, FTP_STOR_SIZE); return result; } @@ -1596,7 +1600,7 @@ static CURLcode ftp_state_ul_setup(struct connectdata *conn, } /* resume_from */ NBFTPSENDF(conn, data->set.ftp_append?"APPE %s":"STOR %s", - ftp->file); + ftpc->file); state(conn, FTP_STOR); @@ -1659,7 +1663,7 @@ static CURLcode ftp_state_quote(struct connectdata *conn, if (ftp->transfer != FTPTRANSFER_BODY) state(conn, FTP_STOP); else { - NBFTPSENDF(conn, "SIZE %s", ftp->file); + NBFTPSENDF(conn, "SIZE %s", ftpc->file); state(conn, FTP_RETR_SIZE); } break; @@ -1961,6 +1965,7 @@ static CURLcode ftp_state_mdtm_resp(struct connectdata *conn, CURLcode result = CURLE_OK; struct SessionHandle *data=conn->data; struct FTP *ftp = data->reqdata.proto.ftp; + struct ftp_conn *ftpc = &conn->proto.ftpc; switch(ftpcode) { case 213: @@ -1986,7 +1991,7 @@ static CURLcode ftp_state_mdtm_resp(struct connectdata *conn, we "emulate" a HTTP-style header in our output. */ if(conn->bits.no_body && - ftp->file && + ftpc->file && data->set.get_filetime && (data->info.filetime>=0) ) { struct tm *tm; @@ -2092,6 +2097,7 @@ static CURLcode ftp_state_post_retr_size(struct connectdata *conn, CURLcode result = CURLE_OK; struct SessionHandle *data=conn->data; struct FTP *ftp = data->reqdata.proto.ftp; + struct ftp_conn *ftpc = &conn->proto.ftpc; if (data->set.max_filesize && (filesize > data->set.max_filesize)) { failf(data, "Maximum file size exceeded"); @@ -2160,7 +2166,7 @@ static CURLcode ftp_state_post_retr_size(struct connectdata *conn, } else { /* no resume */ - NBFTPSENDF(conn, "RETR %s", ftp->file); + NBFTPSENDF(conn, "RETR %s", ftpc->file); state(conn, FTP_RETR); } @@ -2209,7 +2215,7 @@ static CURLcode ftp_state_rest_resp(struct connectdata *conn, ftpstate instate) { CURLcode result = CURLE_OK; - struct FTP *ftp = conn->data->reqdata.proto.ftp; + struct ftp_conn *ftpc = &conn->proto.ftpc; switch(instate) { case FTP_REST: @@ -2231,7 +2237,7 @@ static CURLcode ftp_state_rest_resp(struct connectdata *conn, result = CURLE_FTP_COULDNT_USE_REST; } else { - NBFTPSENDF(conn, "RETR %s", ftp->file); + NBFTPSENDF(conn, "RETR %s", ftpc->file); state(conn, FTP_RETR); } break; @@ -3047,11 +3053,9 @@ static CURLcode Curl_ftp_connect(struct connectdata *conn, *done = FALSE; /* default to not done yet */ - if (data->reqdata.proto.ftp) { - Curl_ftp_disconnect(conn); - free(data->reqdata.proto.ftp); - data->reqdata.proto.ftp = NULL; - } + /* If there already is a protocol-specific struct allocated for this + sessionhandle, deal with it */ + Curl_reset_reqproto(conn); result = ftp_init(conn); if(result) @@ -3183,7 +3187,7 @@ static CURLcode Curl_ftp_done(struct connectdata *conn, CURLcode status, ftpc->prevpath = NULL; /* no path */ } else { - size_t flen = ftp->file?strlen(ftp->file):0; /* file is "raw" already */ + size_t flen = ftpc->file?strlen(ftpc->file):0; /* file is "raw" already */ size_t dlen = strlen(path)-flen; if(!ftpc->cwdfail) { if(dlen && (data->set.ftp_filemethod != FTPFILE_NOCWD)) { @@ -3476,6 +3480,7 @@ static CURLcode ftp_range(struct connectdata *conn) static CURLcode Curl_ftp_nextconnect(struct connectdata *conn) { struct SessionHandle *data=conn->data; + struct ftp_conn *ftpc = &conn->proto.ftpc; CURLcode result = CURLE_OK; /* the ftp struct is inited in Curl_ftp_connect() */ @@ -3499,7 +3504,7 @@ static 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 || !ftpc->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. */ @@ -3602,6 +3607,7 @@ static CURLcode Curl_ftp(struct connectdata *conn, bool *done) make sure we have a good 'struct FTP' to play with. For new connections, the struct FTP is allocated and setup in the Curl_ftp_connect() function. */ + Curl_reset_reqproto(conn); retcode = ftp_init(conn); if(retcode) return retcode; @@ -3800,12 +3806,16 @@ static CURLcode Curl_ftp_disconnect(struct connectdata *conn) */ /* The FTP session may or may not have been allocated/setup at this point! */ + /* FIXME: checking for conn->data->reqdata.proto.ftp is not correct here, + * the reqdata structure could be used by another connection already */ if(conn->data->reqdata.proto.ftp) { (void)ftp_quit(conn); /* ignore errors on the QUIT */ if(ftpc->entrypath) { struct SessionHandle *data = conn->data; - data->state.most_recent_ftp_entrypath = NULL; + if (data->state.most_recent_ftp_entrypath == ftpc->entrypath) { + data->state.most_recent_ftp_entrypath = NULL; + } free(ftpc->entrypath); ftpc->entrypath = NULL; } @@ -3861,11 +3871,11 @@ CURLcode ftp_parse_url_path(struct connectdata *conn) if(data->reqdata.path && data->reqdata.path[0] && (data->reqdata.path[strlen(data->reqdata.path) - 1] != '/') ) - ftp->file = data->reqdata.path; /* this is a full file path */ + ftpc->file = data->reqdata.path; /* this is a full file path */ else - ftp->file = NULL; + ftpc->file = NULL; /* - ftp->file is not used anywhere other than for operations on a file. + ftpc->file is not used anywhere other than for operations on a file. In other words, never for directory operations. So we can safely set it to NULL here and use it as a argument in dir/file decisions. @@ -3877,7 +3887,7 @@ CURLcode ftp_parse_url_path(struct connectdata *conn) if(!path_to_use[0]) { /* no dir, no file */ ftpc->dirdepth = 0; - ftp->file = NULL; + ftpc->file = NULL; break; } slash_pos=strrchr(cur_pos, '/'); @@ -3894,10 +3904,10 @@ CURLcode ftp_parse_url_path(struct connectdata *conn) return CURLE_OUT_OF_MEMORY; } ftpc->dirdepth = 1; /* we consider it to be a single dir */ - ftp->file = slash_pos ? slash_pos+1 : cur_pos; /* rest is file name */ + ftpc->file = slash_pos ? slash_pos+1 : cur_pos; /* rest is file name */ } else - ftp->file = cur_pos; /* this is a file name only */ + ftpc->file = cur_pos; /* this is a file name only */ break; default: /* allow pretty much anything */ @@ -3959,26 +3969,26 @@ CURLcode ftp_parse_url_path(struct connectdata *conn) } } } - ftp->file = cur_pos; /* the rest is the file name */ + ftpc->file = cur_pos; /* the rest is the file name */ } - if(ftp->file && *ftp->file) { - ftp->file = curl_easy_unescape(conn->data, ftp->file, 0, NULL); - if(NULL == ftp->file) { + if(ftpc->file && *ftpc->file) { + ftpc->file = curl_easy_unescape(conn->data, ftpc->file, 0, NULL); + if(NULL == ftpc->file) { freedirs(conn); failf(data, "no memory"); return CURLE_OUT_OF_MEMORY; } - if (isBadFtpString(ftp->file)) { + if (isBadFtpString(ftpc->file)) { freedirs(conn); return CURLE_URL_MALFORMAT; } } else - ftp->file=NULL; /* instead of point to a zero byte, we make it a NULL + ftpc->file=NULL; /* instead of point to a zero byte, we make it a NULL pointer */ - if(data->set.upload && !ftp->file && (ftp->transfer == FTPTRANSFER_BODY)) { + if(data->set.upload && !ftpc->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; @@ -3995,7 +4005,7 @@ CURLcode ftp_parse_url_path(struct connectdata *conn) return CURLE_OUT_OF_MEMORY; } - dlen = strlen(path) - (ftp->file?strlen(ftp->file):0); + dlen = strlen(path) - (ftpc->file?strlen(ftpc->file):0); if((dlen == strlen(ftpc->prevpath)) && curl_strnequal(path, ftpc->prevpath, dlen)) { infof(data, "Request has same path as previous transfer\n"); diff --git a/lib/http.c b/lib/http.c index b32ca1d5b..7c63097c9 100644 --- a/lib/http.c +++ b/lib/http.c @@ -1894,13 +1894,16 @@ CURLcode Curl_http(struct connectdata *conn, bool *done) the rest of the request in the PERFORM phase. */ *done = TRUE; + /* If there already is a protocol-specific struct allocated for this + sessionhandle, deal with it */ + Curl_reset_reqproto(conn); + if(!data->reqdata.proto.http) { /* Only allocate this struct if we don't already have it! */ - http = (struct HTTP *)malloc(sizeof(struct HTTP)); + http = (struct HTTP *)calloc(sizeof(struct HTTP), 1); if(!http) return CURLE_OUT_OF_MEMORY; - memset(http, 0, sizeof(struct HTTP)); data->reqdata.proto.http = http; } else @@ -1846,10 +1846,9 @@ static CURLcode Curl_ssh_connect(struct connectdata *conn, bool *done) CURLcode result; struct SessionHandle *data = conn->data; - if (data->reqdata.proto.ssh) { - Curl_safefree(data->reqdata.proto.ssh); - data->reqdata.proto.ssh = NULL; - } + /* If there already is a protocol-specific struct allocated for this + sessionhandle, deal with it */ + Curl_reset_reqproto(conn); result = ssh_init(conn); if (result) @@ -2058,6 +2058,9 @@ static void conn_free(struct connectdata *conn) if(CURL_SOCKET_BAD != conn->sock[FIRSTSOCKET]) sclose(conn->sock[FIRSTSOCKET]); + if (conn->data->reqdata.current_conn == conn) { + conn->data->reqdata.current_conn = NULL; + } Curl_safefree(conn->user); Curl_safefree(conn->passwd); Curl_safefree(conn->proxyuser); @@ -4514,3 +4517,16 @@ CURLcode Curl_do_more(struct connectdata *conn) return result; } + +/* Called on connect, and if there's already a protocol-specific struct + allocated for a different connection, this frees it that it can be setup + properly later on. */ +void Curl_reset_reqproto(struct connectdata *conn) +{ + struct SessionHandle *data = conn->data; + if (data->reqdata.proto.generic && data->reqdata.current_conn != conn) { + free(data->reqdata.proto.generic); + data->reqdata.proto.generic = NULL; + } + data->reqdata.current_conn = conn; +} @@ -84,4 +84,9 @@ CURLcode Curl_doing_fdset(struct connectdata *conn, int *max_fdp); #endif +/* Called on connect, and if there's already a protocol-specific struct + allocated for a different connection, this frees it that it can be setup + properly later on. */ +void Curl_reset_reqproto(struct connectdata *conn); + #endif diff --git a/lib/urldata.h b/lib/urldata.h index 8ee1d1989..b13ba42f1 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -376,8 +376,6 @@ struct FTP { curl_off_t *bytecountp; char *user; /* user name string */ char *passwd; /* password string */ - char *urlpath; /* the originally given path part of the URL */ - char *file; /* decoded file */ /* 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 */ @@ -392,6 +390,7 @@ struct ftp_conn { char **dirs; /* realloc()ed array for path components */ int dirdepth; /* number of entries used in the 'dirs' array */ int diralloc; /* number of entries allocated for the 'dirs' array */ + char *file; /* decoded file */ char *cache; /* data cache between getresponse()-calls */ curl_off_t cache_size; /* size of cache in bytes */ bool dont_check; /* Set to TRUE to prevent the final (post-transfer) @@ -807,6 +806,8 @@ struct HandleData { void *generic; struct SSHPROTO *ssh; } proto; + /* current user of this HandleData instance, or NULL */ + struct connectdata *current_conn; }; /* |