From: Daniel Stenberg Date: Mon, 18 Aug 2025 12:52:13 +0000 (+0200) Subject: ftp: store dir components as start+len instead of memdup'ing X-Git-Tag: curl-8_16_0~156 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b9e3ea4edb6ed0bb9f4f101fd2c7c36b2b96e5a3;p=thirdparty%2Fcurl.git ftp: store dir components as start+len instead of memdup'ing - Avoids allocating every path segment separately - Improved directory handling in connection reuse Closes #18312 --- diff --git a/lib/ftp.c b/lib/ftp.c index b7ab845d4e..1da756b9dc 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -331,15 +331,10 @@ static void close_secondarysocket(struct Curl_easy *data, static void freedirs(struct ftp_conn *ftpc) { - if(ftpc->dirs) { - int i; - for(i = 0; i < ftpc->dirdepth; i++) - free(ftpc->dirs[i]); - Curl_safefree(ftpc->dirs); - ftpc->dirdepth = 0; - } + Curl_safefree(ftpc->dirs); + ftpc->dirdepth = 0; Curl_safefree(ftpc->rawpath); - Curl_safefree(ftpc->file); + ftpc->file = NULL; Curl_safefree(ftpc->newhost); } @@ -813,6 +808,20 @@ static CURLcode ftp_domore_pollset(struct Curl_easy *data, return Curl_pp_pollset(data, &ftpc->pp, ps); } +static int pathlen(struct ftp_conn *ftpc, int num) +{ + DEBUGASSERT(ftpc->dirs); + DEBUGASSERT(ftpc->dirdepth > num); + return ftpc->dirs[num].len; +} + +static const char *pathpiece(struct ftp_conn *ftpc, int num) +{ + DEBUGASSERT(ftpc->dirs); + DEBUGASSERT(ftpc->dirdepth > num); + return &ftpc->rawpath[ ftpc->dirs[num].start ]; +} + /* This is called after the FTP_QUOTE state is passed. ftp_state_cwd() sends the range of CWD commands to the server to change to @@ -831,13 +840,13 @@ static CURLcode ftp_state_cwd(struct Curl_easy *data, else { /* FTPFILE_NOCWD with full path: expect ftpc->cwddone! */ DEBUGASSERT((data->set.ftp_filemethod != FTPFILE_NOCWD) || - !(ftpc->dirdepth && ftpc->dirs[0][0] == '/')); + !(ftpc->dirdepth && ftpc->rawpath[0] == '/')); ftpc->count2 = 0; /* count2 counts failed CWDs */ if(data->conn->bits.reuse && ftpc->entrypath && /* no need to go to entrypath when we have an absolute path */ - !(ftpc->dirdepth && ftpc->dirs[0][0] == '/')) { + !(ftpc->dirdepth && ftpc->rawpath[0] == '/')) { /* This is a reused connection. Since we change directory to where the transfer is taking place, we must first get back to the original dir where we ended up after login: */ @@ -852,8 +861,8 @@ static CURLcode ftp_state_cwd(struct Curl_easy *data, ftpc->cwdcount = 1; /* issue the first CWD, the rest is sent when the CWD responses are received... */ - result = Curl_pp_sendf(data, &ftpc->pp, "CWD %s", - ftpc->dirs[ftpc->cwdcount -1]); + result = Curl_pp_sendf(data, &ftpc->pp, "CWD %.*s", + pathlen(ftpc, 0), pathpiece(ftpc, 0)); if(!result) ftp_state(data, ftpc, FTP_CWD); } @@ -3055,8 +3064,9 @@ static CURLcode ftp_pp_statemachine(struct Curl_easy *data, create the dir) this then allows for a second try to CWD to it. */ ftpc->count3 = (data->set.ftp_create_missing_dirs == 2) ? 1 : 0; - result = Curl_pp_sendf(data, &ftpc->pp, "MKD %s", - ftpc->dirs[ftpc->cwdcount - 1]); + result = Curl_pp_sendf(data, &ftpc->pp, "MKD %.*s", + pathlen(ftpc, ftpc->cwdcount - 1), + pathpiece(ftpc, ftpc->cwdcount - 1)); if(!result) ftp_state(data, ftpc, FTP_MKD); } @@ -3071,12 +3081,15 @@ static CURLcode ftp_pp_statemachine(struct Curl_easy *data, else { /* success */ ftpc->count2 = 0; - if(++ftpc->cwdcount <= ftpc->dirdepth) - /* send next CWD */ - result = Curl_pp_sendf(data, &ftpc->pp, "CWD %s", - ftpc->dirs[ftpc->cwdcount - 1]); - else + if(ftpc->cwdcount >= ftpc->dirdepth) result = ftp_state_mdtm(data, ftpc, ftp); + else { + ftpc->cwdcount++; + /* send next CWD */ + result = Curl_pp_sendf(data, &ftpc->pp, "CWD %.*s", + pathlen(ftpc, ftpc->cwdcount - 1), + pathpiece(ftpc, ftpc->cwdcount - 1)); + } } break; @@ -3089,8 +3102,9 @@ static CURLcode ftp_pp_statemachine(struct Curl_easy *data, else { ftp_state(data, ftpc, FTP_CWD); /* send CWD */ - result = Curl_pp_sendf(data, &ftpc->pp, "CWD %s", - ftpc->dirs[ftpc->cwdcount - 1]); + result = Curl_pp_sendf(data, &ftpc->pp, "CWD %.*s", + pathlen(ftpc, ftpc->cwdcount - 1), + pathpiece(ftpc, ftpc->cwdcount - 1)); } break; @@ -3258,7 +3272,6 @@ static CURLcode ftp_done(struct Curl_easy *data, CURLcode status, ssize_t nread; int ftpcode; CURLcode result = CURLE_OK; - const char *rawPath = NULL; if(!ftp || !ftpc) return CURLE_OK; @@ -3301,6 +3314,7 @@ static CURLcode ftp_done(struct Curl_easy *data, CURLcode status, Curl_set_in_callback(data, TRUE); data->set.chunk_end(data->set.wildcardptr); Curl_set_in_callback(data, FALSE); + freedirs(ftpc); } ftpc->known_filesize = -1; } @@ -3314,33 +3328,31 @@ static CURLcode ftp_done(struct Curl_easy *data, CURLcode status, ftpc->prevpath = NULL; /* no path remembering */ } else { /* remember working directory for connection reuse */ - rawPath = ftpc->rawpath; - if((data->set.ftp_filemethod == FTPFILE_NOCWD) && (rawPath[0] == '/')) - ; /* full path => no CWDs happened => keep ftpc->prevpath */ - else { - size_t pathLen = strlen(ftpc->rawpath); + const char *rawPath = ftpc->rawpath; + if(rawPath) { + if((data->set.ftp_filemethod == FTPFILE_NOCWD) && (rawPath[0] == '/')) + ; /* full path => no CWDs happened => keep ftpc->prevpath */ + else { + size_t pathLen = strlen(ftpc->rawpath); - free(ftpc->prevpath); + free(ftpc->prevpath); - if(!ftpc->cwdfail) { - if(data->set.ftp_filemethod == FTPFILE_NOCWD) - pathLen = 0; /* relative path => working directory is FTP home */ + if(!ftpc->cwdfail) { + if(data->set.ftp_filemethod == FTPFILE_NOCWD) + pathLen = 0; /* relative path => working directory is FTP home */ + else + /* file is url-decoded */ + pathLen -= ftpc->file ? strlen(ftpc->file) : 0; + ftpc->prevpath = Curl_memdup0(rawPath, pathLen); + } else - /* file is url-decoded */ - pathLen -= ftpc->file ? strlen(ftpc->file) : 0; - ftpc->prevpath = Curl_memdup0(rawPath, pathLen); + ftpc->prevpath = NULL; /* no path */ } - else - ftpc->prevpath = NULL; /* no path */ } - if(ftpc->prevpath) infof(data, "Remembering we are in dir \"%s\"", ftpc->prevpath); } - /* free the dir tree and file parts */ - freedirs(ftpc); - /* shut down the socket to inform the server we are done */ #ifdef UNDER_CE @@ -4158,6 +4170,20 @@ static CURLcode ftp_disconnect(struct Curl_easy *data, return CURLE_OK; } +static size_t numof_slashes(const char *str) +{ + const char *slashPos; + size_t num = 0; + do { + slashPos = strchr(str, '/'); + if(slashPos) { + ++num; + str = slashPos + 1; + } + } while(slashPos); + return num; +} + /*********************************************************************** * * ftp_parse_url_path() @@ -4179,6 +4205,8 @@ CURLcode ftp_parse_url_path(struct Curl_easy *data, ftpc->ctl_valid = FALSE; ftpc->cwdfail = FALSE; + if(ftpc->rawpath) + freedirs(ftpc); /* url-decode ftp path before further evaluation */ result = Curl_urldecode(ftp->path, 0, &ftpc->rawpath, &pathLen, REJECT_CTRL); if(result) { @@ -4209,15 +4237,11 @@ CURLcode ftp_parse_url_path(struct Curl_easy *data, dirlen = 1; ftpc->dirs = calloc(1, sizeof(ftpc->dirs[0])); - if(!ftpc->dirs) { + if(!ftpc->dirs) return CURLE_OUT_OF_MEMORY; - } - - ftpc->dirs[0] = Curl_memdup0(rawPath, dirlen); - if(!ftpc->dirs[0]) { - return CURLE_OUT_OF_MEMORY; - } + ftpc->dirs[0].start = 0; + ftpc->dirs[0].len = (int)dirlen; ftpc->dirdepth = 1; /* we consider it to be a single dir */ fileName = slashPos + 1; /* rest is filename */ } @@ -4230,49 +4254,45 @@ CURLcode ftp_parse_url_path(struct Curl_easy *data, /* current position: begin of next path component */ const char *curPos = rawPath; - /* number of entries allocated for the 'dirs' array */ - size_t dirAlloc = 0; - const char *str = rawPath; - for(; *str != 0; ++str) - if(*str == '/') - ++dirAlloc; + /* number of entries to allocate for the 'dirs' array */ + size_t dirAlloc = numof_slashes(rawPath); + + if(dirAlloc >= 1000) + /* suspiciously deep dir hierarchy */ + return CURLE_URL_MALFORMAT; if(dirAlloc) { ftpc->dirs = calloc(dirAlloc, sizeof(ftpc->dirs[0])); - if(!ftpc->dirs) { + if(!ftpc->dirs) return CURLE_OUT_OF_MEMORY; - } /* parse the URL path into separate path components */ - /* !checksrc! disable EQUALSNULL 1 */ - while((slashPos = strchr(curPos, '/')) != NULL) { - size_t compLen = slashPos - curPos; + while(dirAlloc--) { + const char *spos = strchr(curPos, '/'); + size_t clen = spos - curPos; /* path starts with a slash: add that as a directory */ - if((compLen == 0) && (ftpc->dirdepth == 0)) - ++compLen; + if(!clen && (ftpc->dirdepth == 0)) + ++clen; /* we skip empty path components, like "x//y" since the FTP command CWD requires a parameter and a non-existent parameter a) does not work on many servers and b) has no effect on the others. */ - if(compLen > 0) { - char *comp = Curl_memdup0(curPos, compLen); - if(!comp) { - return CURLE_OUT_OF_MEMORY; - } - ftpc->dirs[ftpc->dirdepth++] = comp; + if(clen) { + ftpc->dirs[ftpc->dirdepth].start = (int)(curPos - rawPath); + ftpc->dirs[ftpc->dirdepth].len = (int)clen; + ftpc->dirdepth++; } - curPos = slashPos + 1; + curPos = spos + 1; } } - DEBUGASSERT((size_t)ftpc->dirdepth <= dirAlloc); fileName = curPos; /* the rest is the filename (or empty) */ } break; } /* switch */ if(fileName && *fileName) - ftpc->file = strdup(fileName); + ftpc->file = fileName; else ftpc->file = NULL; /* instead of point to a zero byte, we make it a NULL pointer */ diff --git a/lib/ftp.h b/lib/ftp.h index 4560630daa..aba1db7f2d 100644 --- a/lib/ftp.h +++ b/lib/ftp.h @@ -120,6 +120,11 @@ struct FTP { curl_off_t downloadsize; }; +/* one struct entry for each path component (of 'rawpath') */ +struct pathcomp { + int start; /* start column */ + int len; /* length in bytes */ +}; /* ftp_conn is used for struct connection-oriented data in the connectdata struct */ @@ -128,9 +133,9 @@ struct ftp_conn { char *account; char *alternative_to_user; char *entrypath; /* the PWD reply when we logged on */ - char *file; /* url-decoded filename (or path) */ + const char *file; /* url-decoded filename (or path), points into rawpath */ char *rawpath; /* URL decoded, allocated, version of the path */ - char **dirs; /* realloc()ed array for path components */ + struct pathcomp *dirs; /* allocated array for path components */ char *newhost; /* the (allocated) IP addr or hostname to connect the data connection to */ char *prevpath; /* url-decoded conn->path from the previous transfer */ diff --git a/tests/data/test1113 b/tests/data/test1113 index c0e1230c35..c7eaf2e938 100644 --- a/tests/data/test1113 +++ b/tests/data/test1113 @@ -65,9 +65,6 @@ EPSV RETR file.txt EPSV RETR someothertext.txt -CWD / -CWD fully_simulated -CWD DOS EPSV TYPE A LIST diff --git a/tests/data/test142 b/tests/data/test142 index db17e3074e..e09b744f93 100644 --- a/tests/data/test142 +++ b/tests/data/test142 @@ -189,5 +189,8 @@ SIZE %TESTNUMBER RETR %TESTNUMBER QUIT + +Allocations: 170 + diff --git a/tests/data/test574 b/tests/data/test574 index 50d2120f04..46c4883b61 100644 --- a/tests/data/test574 +++ b/tests/data/test574 @@ -62,9 +62,6 @@ EPSV RETR file.txt EPSV RETR someothertext.txt -CWD / -CWD fully_simulated -CWD UNIX EPSV TYPE A LIST diff --git a/tests/data/test575 b/tests/data/test575 index fbb7343d96..4ed302fbb1 100644 --- a/tests/data/test575 +++ b/tests/data/test575 @@ -60,9 +60,6 @@ EPSV RETR file.txt EPSV RETR someothertext.txt -CWD / -CWD fully_simulated -CWD UNIX EPSV TYPE A LIST