Skip to content

Commit

Permalink
curl_path: make Curl_get_pathname use dynbuf
Browse files Browse the repository at this point in the history
... instead of malloc and memcpy

- changed the prototype: the first argument no longer gets updated. No
  caller used this functionality.

- unit test 2604 verifies Curl_get_pathname()
  • Loading branch information
bagder committed May 7, 2024
1 parent 62ae1f1 commit df6a3af
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 43 deletions.
62 changes: 30 additions & 32 deletions lib/curl_path.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ CURLcode Curl_getworkingpath(struct Curl_easy *data,
return CURLE_OK;
}

/* The get_pathname() function is being borrowed from OpenSSH sftp.c
version 4.6p1. */
/* The original get_pathname() function came from OpenSSH sftp.c version
4.6p1. */
/*
* Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
*
Expand All @@ -115,38 +115,37 @@ CURLcode Curl_getworkingpath(struct Curl_easy *data,
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
CURLcode Curl_get_pathname(const char **cpp, char **path, char *homedir)

#define MAX_PATHLENGTH 65535 /* arbitrary long */

CURLcode Curl_get_pathname(const char *cp, char **path, const char *homedir)
{
const char *cp = *cpp, *end;
const char *end;
char quot;
unsigned int i, j;
size_t fullPathLength, pathLength;
bool relativePath = false;
unsigned int i;
static const char WHITESPACE[] = " \t\r\n";
struct dynbuf out;
CURLcode result;

DEBUGASSERT(homedir);
if(!*cp || !homedir) {
*cpp = NULL;
*path = NULL;
return CURLE_QUOTE_ERROR;
}

Curl_dyn_init(&out, MAX_PATHLENGTH);

/* Ignore leading whitespace */
cp += strspn(cp, WHITESPACE);
/* Allocate enough space for home directory and filename + separator */
fullPathLength = strlen(cp) + strlen(homedir) + 2;
*path = malloc(fullPathLength);
if(!*path)
return CURLE_OUT_OF_MEMORY;

/* Check for quoted filenames */
if(*cp == '\"' || *cp == '\'') {
quot = *cp++;

/* Search for terminating quote, unescape some chars */
for(i = j = 0; i <= strlen(cp); i++) {
for(i = 0; i <= strlen(cp); i++) {
if(cp[i] == quot) { /* Found quote */
i++;
(*path)[j] = '\0';
break;
}
if(cp[i] == '\0') { /* End of string */
Expand All @@ -159,40 +158,39 @@ CURLcode Curl_get_pathname(const char **cpp, char **path, char *homedir)
goto fail;
}
}
(*path)[j++] = cp[i];
result = Curl_dyn_addn(&out, &cp[i], 1);
if(result)
return result;
}

if(j == 0) {
if(!Curl_dyn_len(&out))
goto fail;
}
*cpp = cp + i + strspn(cp + i, WHITESPACE);
}
else {
/* Read to end of filename - either to whitespace or terminator */
end = strpbrk(cp, WHITESPACE);
if(!end)
end = strchr(cp, '\0');
/* return pointer to second parameter if it exists */
*cpp = end + strspn(end, WHITESPACE);
pathLength = 0;
relativePath = (cp[0] == '/' && cp[1] == '~' && cp[2] == '/');

/* Handling for relative path - prepend home directory */
if(relativePath) {
strcpy(*path, homedir);
pathLength = strlen(homedir);
(*path)[pathLength++] = '/';
(*path)[pathLength] = '\0';
if(cp[0] == '/' && cp[1] == '~' && cp[2] == '/') {
result = Curl_dyn_add(&out, homedir);
if(!result)
result = Curl_dyn_addn(&out, "/", 1);
if(result)
return result;
cp += 3;
}
/* Copy path name up until first "whitespace" */
memcpy(&(*path)[pathLength], cp, (int)(end - cp));
pathLength += (int)(end - cp);
(*path)[pathLength] = '\0';
result = Curl_dyn_addn(&out, cp, (end - cp));
if(result)
return result;
}
*path = Curl_dyn_ptr(&out);
return CURLE_OK;

fail:
Curl_safefree(*path);
Curl_dyn_free(&out);
return CURLE_QUOTE_ERROR;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/curl_path.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,5 @@ CURLcode Curl_getworkingpath(struct Curl_easy *data,
char *homedir,
char **path);

CURLcode Curl_get_pathname(const char **cpp, char **path, char *homedir);
CURLcode Curl_get_pathname(const char *cp, char **path, const char *homedir);
#endif /* HEADER_CURL_PATH_H */
8 changes: 4 additions & 4 deletions lib/vssh/libssh.c
Original file line number Diff line number Diff line change
Expand Up @@ -2704,7 +2704,7 @@ static void sftp_quote(struct Curl_easy *data)
* also, every command takes at least one argument so we get that
* first argument right now
*/
result = Curl_get_pathname(&cp, &sshc->quote_path1, sshc->homedir);
result = Curl_get_pathname(cp, &sshc->quote_path1, sshc->homedir);
if(result) {
if(result == CURLE_OUT_OF_MEMORY)
failf(data, "Out of memory");
Expand All @@ -2731,7 +2731,7 @@ static void sftp_quote(struct Curl_easy *data)

/* sshc->quote_path1 contains the mode to set */
/* get the destination */
result = Curl_get_pathname(&cp, &sshc->quote_path2, sshc->homedir);
result = Curl_get_pathname(cp, &sshc->quote_path2, sshc->homedir);
if(result) {
if(result == CURLE_OUT_OF_MEMORY)
failf(data, "Out of memory");
Expand All @@ -2753,7 +2753,7 @@ static void sftp_quote(struct Curl_easy *data)
/* symbolic linking */
/* sshc->quote_path1 is the source */
/* get the destination */
result = Curl_get_pathname(&cp, &sshc->quote_path2, sshc->homedir);
result = Curl_get_pathname(cp, &sshc->quote_path2, sshc->homedir);
if(result) {
if(result == CURLE_OUT_OF_MEMORY)
failf(data, "Out of memory");
Expand All @@ -2777,7 +2777,7 @@ static void sftp_quote(struct Curl_easy *data)
/* rename file */
/* first param is the source path */
/* second param is the dest. path */
result = Curl_get_pathname(&cp, &sshc->quote_path2, sshc->homedir);
result = Curl_get_pathname(cp, &sshc->quote_path2, sshc->homedir);
if(result) {
if(result == CURLE_OUT_OF_MEMORY)
failf(data, "Out of memory");
Expand Down
8 changes: 4 additions & 4 deletions lib/vssh/libssh2.c
Original file line number Diff line number Diff line change
Expand Up @@ -1558,7 +1558,7 @@ static CURLcode ssh_statemach_act(struct Curl_easy *data, bool *block)
* also, every command takes at least one argument so we get that
* first argument right now
*/
result = Curl_get_pathname(&cp, &sshc->quote_path1, sshc->homedir);
result = Curl_get_pathname(cp, &sshc->quote_path1, sshc->homedir);
if(result) {
if(result == CURLE_OUT_OF_MEMORY)
failf(data, "Out of memory");
Expand All @@ -1585,7 +1585,7 @@ static CURLcode ssh_statemach_act(struct Curl_easy *data, bool *block)

/* sshc->quote_path1 contains the mode to set */
/* get the destination */
result = Curl_get_pathname(&cp, &sshc->quote_path2, sshc->homedir);
result = Curl_get_pathname(cp, &sshc->quote_path2, sshc->homedir);
if(result) {
if(result == CURLE_OUT_OF_MEMORY)
failf(data, "Out of memory");
Expand All @@ -1606,7 +1606,7 @@ static CURLcode ssh_statemach_act(struct Curl_easy *data, bool *block)
/* symbolic linking */
/* sshc->quote_path1 is the source */
/* get the destination */
result = Curl_get_pathname(&cp, &sshc->quote_path2, sshc->homedir);
result = Curl_get_pathname(cp, &sshc->quote_path2, sshc->homedir);
if(result) {
if(result == CURLE_OUT_OF_MEMORY)
failf(data, "Out of memory");
Expand All @@ -1631,7 +1631,7 @@ static CURLcode ssh_statemach_act(struct Curl_easy *data, bool *block)
/* rename file */
/* first param is the source path */
/* second param is the dest. path */
result = Curl_get_pathname(&cp, &sshc->quote_path2, sshc->homedir);
result = Curl_get_pathname(cp, &sshc->quote_path2, sshc->homedir);
if(result) {
if(result == CURLE_OUT_OF_MEMORY)
failf(data, "Out of memory");
Expand Down
2 changes: 1 addition & 1 deletion tests/data/Makefile.inc
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ test2400 test2401 test2402 test2403 test2404 test2405 test2406 \
\
test2500 test2501 test2502 test2503 \
\
test2600 test2601 test2602 test2603 \
test2600 test2601 test2602 test2603 test2604 \
\
test3000 test3001 test3002 test3003 test3004 test3005 test3006 test3007 \
test3008 test3009 test3010 test3011 test3012 test3013 test3014 test3015 \
Expand Down
22 changes: 22 additions & 0 deletions tests/data/test2604
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<testcase>
<info>
<keywords>
unittest
</keywords>
</info>

#
# Client-side
<client>
<server>
none
</server>
<features>
unittest
sftp
</features>
<name>
Curl_get_pathname unit test
</name>
</client>
</testcase>
4 changes: 3 additions & 1 deletion tests/unit/Makefile.inc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ UNITPROGS = unit1300 unit1302 unit1303 unit1304 unit1305 unit1307 \
unit1620 unit1621 \
unit1650 unit1651 unit1652 unit1653 unit1654 unit1655 \
unit1660 unit1661 \
unit2600 unit2601 unit2602 unit2603 \
unit2600 unit2601 unit2602 unit2603 unit2604 \
unit3200 \
unit3205

Expand Down Expand Up @@ -134,6 +134,8 @@ unit2602_SOURCES = unit2602.c $(UNITFILES)

unit2603_SOURCES = unit2603.c $(UNITFILES)

unit2604_SOURCES = unit2604.c $(UNITFILES)

unit3200_SOURCES = unit3200.c $(UNITFILES)

unit3205_SOURCES = unit3205.c $(UNITFILES)
83 changes: 83 additions & 0 deletions tests/unit/unit2604.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/***************************************************************************
* _ _ ____ _
* Project ___| | | | _ \| |
* / __| | | | |_) | |
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
* Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
* are also available at https://curl.se/docs/copyright.html.
*
* You may opt to use, copy, modify, merge, publish, distribute and/or sell
* copies of the Software, and permit persons to whom the Software is
* furnished to do so, under the terms of the COPYING file.
*
* This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
* KIND, either express or implied.
*
* SPDX-License-Identifier: curl
*
***************************************************************************/
#include "curlcheck.h"

#include "curl_path.h"

static CURLcode unit_setup(void)
{
return CURLE_OK;
}

static void unit_stop(void)
{
}


struct set {
const char *cp;
const char *expect;
const char *home;
CURLcode result;
};

struct set list[] = {
{ "a a", "a", "/home/", CURLE_OK},
{ "b a", "b", "/", CURLE_OK},
{ "\"foo bar\"\tb", "foo bar", "/", CURLE_OK},
{ "/~/hej", "/home/user/hej", "/home/user", CURLE_OK},
{ "\"foo bar", "", "/", CURLE_QUOTE_ERROR},
{ "\"foo\\\"bar\" a", "foo\"bar", "/", CURLE_OK},
{ "\"foo\\\'bar\" b", "foo\'bar", "/", CURLE_OK},
{ "\"foo\\\\bar\" c", "foo\\bar", "/", CURLE_OK},
{ "foo\"", "foo\"", "/", CURLE_OK},
{ "foo \"", "foo", "/", CURLE_OK},
{ NULL, NULL, NULL, CURLE_OK }
};

UNITTEST_START
{
int i;
int error = 0;
for(i = 0; list[i].home; i++) {
char *path;
CURLcode result = Curl_get_pathname(list[i].cp, &path, list[i].home);
printf("%u - Curl_get_pathname(\"%s\", ... \"%s\") == %u\n", i,
list[i].cp, list[i].home, list[i].result);
if(result != list[i].result) {
printf("... returned %d\n", result);
error++;
}
if(!result && path) {
if(strcmp(path, list[i].expect)) {
printf("... gave '%s', not '%s' as expected \n",
path, list[i].expect);
error++;
}
curl_free(path);
}
}
return error;
}
UNITTEST_STOP

0 comments on commit df6a3af

Please sign in to comment.