Bugzilla – Attachment 1358 Details for
Bug 778
sftp client globs entire path, directories enclosed in square brackets are unusable
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Tweaked diff with regress tests
sftp-argparse3.diff (text/plain), 16.83 KB, created by
Damien Miller
on 2007-09-21 18:11:14 AEST
(
hide
)
Description:
Tweaked diff with regress tests
Filename:
MIME Type:
Creator:
Damien Miller
Created:
2007-09-21 18:11:14 AEST
Size:
16.83 KB
patch
obsolete
>Index: regress/usr.bin/ssh/sftp-glob.sh >=================================================================== >RCS file: /cvs/src/regress/usr.bin/ssh/sftp-glob.sh,v >retrieving revision 1.1 >diff -u -p -r1.1 sftp-glob.sh >--- regress/usr.bin/ssh/sftp-glob.sh 10 Dec 2004 01:31:30 -0000 1.1 >+++ regress/usr.bin/ssh/sftp-glob.sh 21 Sep 2007 08:07:47 -0000 >@@ -3,26 +3,63 @@ > > tid="sftp glob" > >+sftp_ls() { >+ target=$1 >+ errtag=$2 >+ expected=$3 >+ unexpected=$4 >+ verbose "$tid: $errtag" >+ echo -E "ls -l ${target}" | \ >+ ${SFTP} -b - -P ${SFTPSERVER} 2>/dev/null | \ >+ grep -v "^sftp>" > ${RESULTS} >+ if [ $? -ne 0 ]; then >+ fail "$errtag failed" >+ fi >+ if test "x$expected" != "x" && \ >+ ! fgrep "$expected" ${RESULTS} >/dev/null 2>&1 ; then >+ fail "$expected missing from $errtag results" >+ fi >+ if test "x$unexpected" != "x" && \ >+ fgrep "$unexpected" ${RESULTS} >/dev/null 2>&1 ; then >+ fail "$unexpected present in $errtag results" >+ fi >+ rm -f ${RESULTS} >+} >+ > BASE=${OBJ}/glob >+RESULTS=${OBJ}/results > DIR=${BASE}/dir > DATA=${DIR}/file > >+GLOB1="${DIR}/g-wild*" >+GLOB2="${DIR}/g-wildx" >+QUOTE="${DIR}/g-quote\"" >+SLASH="${DIR}/g-sl\\ash" >+ESLASH="${DIR}/g-slash\\" >+QSLASH="${DIR}/g-qs\\\"" >+SPACE="${DIR}/g-q space" >+ > rm -rf ${BASE} > mkdir -p ${DIR} >-touch ${DATA} >+touch "${DATA}" "${GLOB1}" "${GLOB2}" "${QUOTE}" >+touch "${QSLASH}" "${ESLASH}" "${SLASH}" "${SPACE}" > >-verbose "$tid: ls file" >-echo "ls -l ${DIR}/fil*" | ${SFTP} -P ${SFTPSERVER} 2>/dev/null | \ >- grep ${DATA} >/dev/null 2>&1 >-if [ $? -ne 0 ]; then >- fail "globbed ls file failed" >-fi >- >-verbose "$tid: ls dir" >-echo "ls -l ${BASE}/d*" | ${SFTP} -P ${SFTPSERVER} 2>/dev/null | \ >- grep file >/dev/null 2>&1 >-if [ $? -ne 0 ]; then >- fail "globbed ls dir failed" >-fi >+# target message expected unexpected >+sftp_ls "${DIR}/fil*" "file glob" "${DATA}" "" >+sftp_ls "${BASE}/d*" "dir glob" "`basename ${DATA}`" "" >+sftp_ls "${DIR}/g-wild\"*\"" "quoted glob" "g-wild*" "g-wildx" >+sftp_ls "${DIR}/g-wild\*" "escaped glob" "g-wild*" "g-wildx" >+sftp_ls "${DIR}/g-quote\\\"" "escaped quote" "g-quote\"" "" >+sftp_ls "\"${DIR}/g-quote\\\"\"" "quoted quote" "g-quote\"" "" >+sftp_ls "'${DIR}/g-quote\"'" "single-quoted quote" "g-quote\"" "" >+sftp_ls "${DIR}/g-sl\\\\ash" "escaped slash" "g-sl\\ash" "" >+sftp_ls "'${DIR}/g-sl\\\\ash'" "quoted slash" "g-sl\\ash" "" >+sftp_ls "${DIR}/g-slash\\\\" "escaped slash at EOL" "g-slash\\" "" >+sftp_ls "'${DIR}/g-slash\\\\'" "quoted slash at EOL" "g-slash\\" "" >+sftp_ls "${DIR}/g-qs\\\\\\\"" "escaped slash+quote" "g-qs\\\"" "" >+sftp_ls "'${DIR}/g-qs\\\\\"'" "quoted slash+quote" "g-qs\\\"" "" >+sftp_ls "${DIR}/g-q\\ space" "escaped space" "g-q space" "" >+sftp_ls "'${DIR}/g-q space'" "quoted space" "g-q space" "" > > rm -rf ${BASE} >+ >Index: regress/usr.bin/ssh/test-exec.sh >=================================================================== >RCS file: /cvs/src/regress/usr.bin/ssh/test-exec.sh,v >retrieving revision 1.28 >diff -u -p -r1.28 test-exec.sh >--- regress/usr.bin/ssh/test-exec.sh 20 May 2005 23:14:15 -0000 1.28 >+++ regress/usr.bin/ssh/test-exec.sh 21 Sep 2007 08:07:47 -0000 >@@ -109,7 +109,7 @@ cleanup () > > trace () > { >- echo "trace: $@" >>$TEST_SSH_LOGFILE >+ echo -E "trace: $@" >>$TEST_SSH_LOGFILE > if [ "X$TEST_SSH_TRACE" = "Xyes" ]; then > echo "$@" > fi >@@ -117,7 +117,7 @@ trace () > > verbose () > { >- echo "verbose: $@" >>$TEST_SSH_LOGFILE >+ echo -E "verbose: $@" >>$TEST_SSH_LOGFILE > if [ "X$TEST_SSH_QUIET" != "Xyes" ]; then > echo "$@" > fi >@@ -126,14 +126,14 @@ verbose () > > fail () > { >- echo "FAIL: $@" >>$TEST_SSH_LOGFILE >+ echo -E "FAIL: $@" >>$TEST_SSH_LOGFILE > RESULT=1 > echo "$@" > } > > fatal () > { >- echo "FATAL: $@" >>$TEST_SSH_LOGFILE >+ echo -E "FATAL: $@" >>$TEST_SSH_LOGFILE > echo -n "FATAL: " > fail "$@" > cleanup >Index: usr.bin/ssh/sftp.c >=================================================================== >RCS file: /cvs/src/usr.bin/ssh/sftp.c,v >retrieving revision 1.96 >diff -u -p -r1.96 sftp.c >--- usr.bin/ssh/sftp.c 3 Jan 2007 04:09:15 -0000 1.96 >+++ usr.bin/ssh/sftp.c 21 Sep 2007 08:07:47 -0000 >@@ -22,6 +22,7 @@ > #include <sys/socket.h> > #include <sys/param.h> > >+#include <ctype.h> > #include <errno.h> > #include <glob.h> > #include <histedit.h> >@@ -334,144 +335,78 @@ infer_path(const char *p, char **ifp) > } > > static int >-parse_getput_flags(const char **cpp, int *pflag) >+parse_getput_flags(const char *cmd, char **argv, int argc, int *pflag) > { >- const char *cp = *cpp; >+ extern int optind, optreset, opterr; >+ int ch; > >- /* Check for flags */ >- if (cp[0] == '-' && cp[1] && strchr(WHITESPACE, cp[2])) { >- switch (cp[1]) { >+ optind = optreset = 1; >+ opterr = 0; >+ >+ *pflag = 0; >+ while ((ch = getopt(argc, argv, "Pp")) != -1) { >+ switch (ch) { > case 'p': > case 'P': > *pflag = 1; > break; > default: >- error("Invalid flag -%c", cp[1]); >- return(-1); >- } >- cp += 2; >- *cpp = cp + strspn(cp, WHITESPACE); >- } >- >- return(0); >-} >- >-static int >-parse_ls_flags(const char **cpp, int *lflag) >-{ >- const char *cp = *cpp; >- >- /* Defaults */ >- *lflag = LS_NAME_SORT; >- >- /* Check for flags */ >- if (cp++[0] == '-') { >- for (; strchr(WHITESPACE, *cp) == NULL; cp++) { >- switch (*cp) { >- case 'l': >- *lflag &= ~VIEW_FLAGS; >- *lflag |= LS_LONG_VIEW; >- break; >- case '1': >- *lflag &= ~VIEW_FLAGS; >- *lflag |= LS_SHORT_VIEW; >- break; >- case 'n': >- *lflag &= ~VIEW_FLAGS; >- *lflag |= LS_NUMERIC_VIEW|LS_LONG_VIEW; >- break; >- case 'S': >- *lflag &= ~SORT_FLAGS; >- *lflag |= LS_SIZE_SORT; >- break; >- case 't': >- *lflag &= ~SORT_FLAGS; >- *lflag |= LS_TIME_SORT; >- break; >- case 'r': >- *lflag |= LS_REVERSE_SORT; >- break; >- case 'f': >- *lflag &= ~SORT_FLAGS; >- break; >- case 'a': >- *lflag |= LS_SHOW_ALL; >- break; >- default: >- error("Invalid flag -%c", *cp); >- return(-1); >- } >+ error("%s: Invalid flag -%c", cmd, ch); >+ return -1; > } >- *cpp = cp + strspn(cp, WHITESPACE); > } > >- return(0); >+ return optind; > } > > static int >-get_pathname(const char **cpp, char **path) >+parse_ls_flags(char **argv, int argc, int *lflag) > { >- const char *cp = *cpp, *end; >- char quot; >- u_int i, j; >- >- cp += strspn(cp, WHITESPACE); >- if (!*cp) { >- *cpp = cp; >- *path = NULL; >- return (0); >- } >+ extern int optind, optreset, opterr; >+ int ch; > >- *path = xmalloc(strlen(cp) + 1); >+ optind = optreset = 1; >+ opterr = 0; > >- /* Check for quoted filenames */ >- if (*cp == '\"' || *cp == '\'') { >- quot = *cp++; >- >- /* Search for terminating quote, unescape some chars */ >- for (i = j = 0; i <= strlen(cp); i++) { >- if (cp[i] == quot) { /* Found quote */ >- i++; >- (*path)[j] = '\0'; >- break; >- } >- if (cp[i] == '\0') { /* End of string */ >- error("Unterminated quote"); >- goto fail; >- } >- if (cp[i] == '\\') { /* Escaped characters */ >- i++; >- if (cp[i] != '\'' && cp[i] != '\"' && >- cp[i] != '\\') { >- error("Bad escaped character '\\%c'", >- cp[i]); >- goto fail; >- } >- } >- (*path)[j++] = cp[i]; >+ *lflag = LS_NAME_SORT; >+ while ((ch = getopt(argc, argv, "1Saflnrt")) != -1) { >+ switch (ch) { >+ case '1': >+ *lflag &= ~VIEW_FLAGS; >+ *lflag |= LS_SHORT_VIEW; >+ break; >+ case 'S': >+ *lflag &= ~SORT_FLAGS; >+ *lflag |= LS_SIZE_SORT; >+ break; >+ case 'a': >+ *lflag |= LS_SHOW_ALL; >+ break; >+ case 'f': >+ *lflag &= ~SORT_FLAGS; >+ break; >+ case 'l': >+ *lflag &= ~VIEW_FLAGS; >+ *lflag |= LS_LONG_VIEW; >+ break; >+ case 'n': >+ *lflag &= ~VIEW_FLAGS; >+ *lflag |= LS_NUMERIC_VIEW|LS_LONG_VIEW; >+ break; >+ case 'r': >+ *lflag |= LS_REVERSE_SORT; >+ break; >+ case 't': >+ *lflag &= ~SORT_FLAGS; >+ *lflag |= LS_TIME_SORT; >+ break; >+ default: >+ error("ls: Invalid flag -%c", ch); >+ return -1; > } >- >- if (j == 0) { >- error("Empty quotes"); >- goto fail; >- } >- *cpp = cp + i + strspn(cp + i, WHITESPACE); >- } else { >- /* Read to end of filename */ >- end = strpbrk(cp, WHITESPACE); >- if (end == NULL) >- end = strchr(cp, '\0'); >- *cpp = end + strspn(end, WHITESPACE); >- >- memcpy(*path, cp, end - cp); >- (*path)[end - cp] = '\0'; > } >- return (0); > >- fail: >- xfree(*path); >- *path = NULL; >- return (-1); >+ return optind; > } > > static int >@@ -854,15 +789,189 @@ do_globbed_ls(struct sftp_conn *conn, ch > return (0); > } > >+/* >+ * Undo escaping of glob sequences in place. Used to undo extra escaping >+ * applied in makeargv() when the string is destined for a function that >+ * does not glob it. >+ */ >+static void >+undo_glob_escape(char *s) >+{ >+ size_t i, j; >+ >+ for (i = j = 0;;) { >+ if (s[i] == '\0') { >+ s[j] = '\0'; >+ return; >+ } >+ if (s[i] != '\\') { >+ s[j++] = s[i++]; >+ continue; >+ } >+ /* s[i] == '\\' */ >+ ++i; >+ switch (s[i]) { >+ case '?': >+ case '[': >+ case '*': >+ case '\\': >+ s[j++] = s[i++]; >+ break; >+ case '\0': >+ s[j++] = '\\'; >+ s[j] = '\0'; >+ return; >+ default: >+ s[j++] = '\\'; >+ s[j++] = s[i++]; >+ break; >+ } >+ } >+} >+ >+/* >+ * Split a string into an argument vector using sh(1)-style quoting, >+ * comment and escaping rules, but with some tweaks to handle glob(3) >+ * wildcards. >+ * Returns NULL on error or up to MAXARGS in static buffer (NULL terminated) >+ */ >+#define MAXARGS 128 >+#define MAXARGLEN 8192 >+static char ** >+makeargv(const char *arg, int *argcp) >+{ >+ int argc, quot; >+ size_t i, j; >+ static char argvs[MAXARGLEN]; >+ static char *argv[MAXARGS + 1]; >+ enum { MA_START, MA_SQUOTE, MA_DQUOTE, MA_UNQUOTED } state, q; >+ >+ *argcp = argc = 0; >+ if (strlen(arg) > sizeof(argvs) - 1) { >+ args_too_longs: >+ error("string too long"); >+ return NULL; >+ } >+ state = MA_START; >+ i = j = 0; >+ for (;;) { >+ if (isspace(arg[i])) { >+ if (state == MA_UNQUOTED) { >+ /* Terminate current argument */ >+ argvs[j++] = '\0'; >+ argc++; >+ state = MA_START; >+ } else if (state != MA_START) >+ argvs[j++] = arg[i]; >+ } else if (arg[i] == '"' || arg[i] == '\'') { >+ q = arg[i] == '"' ? MA_DQUOTE : MA_SQUOTE; >+ if (state == MA_START) { >+ argv[argc] = argvs + j; >+ state = q; >+ } else if (state == MA_UNQUOTED) >+ state = q; >+ else if (state == q) >+ state = MA_UNQUOTED; >+ else >+ argvs[j++] = arg[i]; >+ } else if (arg[i] == '\\') { >+ if (state == MA_SQUOTE || state == MA_DQUOTE) { >+ quot = state == MA_SQUOTE ? '\'' : '"'; >+ /* Unescape quote we are in */ >+ /* XXX support \n and friends? */ >+ if (arg[i + 1] == quot) { >+ i++; >+ argvs[j++] = arg[i]; >+ } else if (arg[i + 1] == '?' || >+ arg[i + 1] == '[' || arg[i + 1] == '*') { >+ /* >+ * Special case for sftp: append >+ * double-escaped glob sequence - >+ * glob will undo one level of >+ * escaping. NB. string can grow here. >+ */ >+ if (j >= sizeof(argvs) - 5) >+ goto args_too_longs; >+ argvs[j++] = '\\'; >+ argvs[j++] = arg[i++]; >+ argvs[j++] = '\\'; >+ argvs[j++] = arg[i]; >+ } else { >+ argvs[j++] = arg[i++]; >+ argvs[j++] = arg[i]; >+ } >+ } else { >+ if (state == MA_START) { >+ argv[argc] = argvs + j; >+ state = MA_UNQUOTED; >+ } >+ if (arg[i + 1] == '?' || arg[i + 1] == '[' || >+ arg[i + 1] == '*' || arg[i + 1] == '\\') { >+ /* >+ * Special case for sftp: append >+ * escaped glob sequence - >+ * glob will undo one level of >+ * escaping. >+ */ >+ argvs[j++] = arg[i++]; >+ argvs[j++] = arg[i]; >+ } else { >+ /* Unescape everything */ >+ /* XXX support \n and friends? */ >+ i++; >+ argvs[j++] = arg[i]; >+ } >+ } >+ } else if (arg[i] == '#') { >+ if (state == MA_SQUOTE || state == MA_DQUOTE) >+ argvs[j++] = arg[i]; >+ else >+ goto string_done; >+ } else if (arg[i] == '\0') { >+ if (state == MA_SQUOTE || state == MA_DQUOTE) { >+ error("Unterminated quoted argument"); >+ return NULL; >+ } >+ string_done: >+ if (state == MA_UNQUOTED) { >+ argvs[j++] = '\0'; >+ argc++; >+ } >+ break; >+ } else { >+ if (state == MA_START) { >+ argv[argc] = argvs + j; >+ state = MA_UNQUOTED; >+ } >+ if ((state == MA_SQUOTE || state == MA_DQUOTE) && >+ (arg[i] == '?' || arg[i] == '[' || arg[i] == '*')) { >+ /* >+ * Special case for sftp: escape quoted >+ * glob(3) wildcards. NB. string can grow >+ * here. >+ */ >+ if (j >= sizeof(argvs) - 3) >+ goto args_too_longs; >+ argvs[j++] = '\\'; >+ argvs[j++] = arg[i]; >+ } else >+ argvs[j++] = arg[i]; >+ } >+ i++; >+ } >+ *argcp = argc; >+ return argv; >+} >+ > static int > parse_args(const char **cpp, int *pflag, int *lflag, int *iflag, > unsigned long *n_arg, char **path1, char **path2) > { > const char *cmd, *cp = *cpp; >- char *cp2; >+ char *cp2, **argv; > int base = 0; > long l; >- int i, cmdnum; >+ int i, cmdnum, optidx, argc; > > /* Skip leading whitespace */ > cp = cp + strspn(cp, WHITESPACE); >@@ -878,17 +987,13 @@ parse_args(const char **cpp, int *pflag, > cp++; > } > >- /* Figure out which command we have */ >- for (i = 0; cmds[i].c; i++) { >- int cmdlen = strlen(cmds[i].c); >+ if ((argv = makeargv(cp, &argc)) == NULL) >+ return -1; > >- /* Check for command followed by whitespace */ >- if (!strncasecmp(cp, cmds[i].c, cmdlen) && >- strchr(WHITESPACE, cp[cmdlen])) { >- cp += cmdlen; >- cp = cp + strspn(cp, WHITESPACE); >+ /* Figure out which command we have */ >+ for (i = 0; cmds[i].c != NULL; i++) { >+ if (strcasecmp(cmds[i].c, argv[0]) == 0) > break; >- } > } > cmdnum = cmds[i].n; > cmd = cmds[i].c; >@@ -899,40 +1004,44 @@ parse_args(const char **cpp, int *pflag, > cmdnum = I_SHELL; > } else if (cmdnum == -1) { > error("Invalid command."); >- return (-1); >+ return -1; > } > > /* Get arguments and parse flags */ > *lflag = *pflag = *n_arg = 0; > *path1 = *path2 = NULL; >+ optidx = 1; > switch (cmdnum) { > case I_GET: > case I_PUT: >- if (parse_getput_flags(&cp, pflag)) >- return(-1); >+ if ((optidx = parse_getput_flags(cmd, argv, argc, pflag)) == -1) >+ return -1; > /* Get first pathname (mandatory) */ >- if (get_pathname(&cp, path1)) >- return(-1); >- if (*path1 == NULL) { >+ if (argc - optidx < 1) { > error("You must specify at least one path after a " > "%s command.", cmd); >- return(-1); >+ return -1; >+ } >+ *path1 = xstrdup(argv[optidx]); >+ /* Get second pathname (optional) */ >+ if (argc - optidx > 1) { >+ *path2 = xstrdup(argv[optidx + 1]); >+ /* Destination is not globbed */ >+ undo_glob_escape(*path2); > } >- /* Try to get second pathname (optional) */ >- if (get_pathname(&cp, path2)) >- return(-1); > break; > case I_RENAME: > case I_SYMLINK: >- if (get_pathname(&cp, path1)) >- return(-1); >- if (get_pathname(&cp, path2)) >- return(-1); >- if (!*path1 || !*path2) { >+ if (argc - optidx < 2) { > error("You must specify two paths after a %s " > "command.", cmd); >- return(-1); >+ return -1; > } >+ *path1 = xstrdup(argv[optidx]); >+ *path2 = xstrdup(argv[optidx + 1]); >+ /* Paths are not globbed */ >+ undo_glob_escape(*path1); >+ undo_glob_escape(*path2); > break; > case I_RM: > case I_MKDIR: >@@ -941,59 +1050,55 @@ parse_args(const char **cpp, int *pflag, > case I_LCHDIR: > case I_LMKDIR: > /* Get pathname (mandatory) */ >- if (get_pathname(&cp, path1)) >- return(-1); >- if (*path1 == NULL) { >+ if (argc - optidx < 1) { > error("You must specify a path after a %s command.", > cmd); >- return(-1); >+ return -1; > } >+ *path1 = xstrdup(argv[optidx]); >+ /* Only "rm" globs */ >+ if (cmdnum != I_RM) >+ undo_glob_escape(*path1); > break; > case I_LS: >- if (parse_ls_flags(&cp, lflag)) >+ if ((optidx = parse_ls_flags(argv, argc, lflag)) == -1) > return(-1); > /* Path is optional */ >- if (get_pathname(&cp, path1)) >- return(-1); >+ if (argc - optidx > 0) >+ *path1 = xstrdup(argv[optidx]); > break; > case I_LLS: > case I_SHELL: > /* Uses the rest of the line */ > break; > case I_LUMASK: >- base = 8; > case I_CHMOD: > base = 8; > case I_CHOWN: > case I_CHGRP: > /* Get numeric arg (mandatory) */ >+ if (argc - optidx < 1) >+ goto need_num_arg; > errno = 0; >- l = strtol(cp, &cp2, base); >- if (cp2 == cp || ((l == LONG_MIN || l == LONG_MAX) && >- errno == ERANGE) || l < 0) { >+ l = strtol(argv[optidx], &cp2, base); >+ if (cp2 == argv[optidx] || *cp2 != '\0' || >+ ((l == LONG_MIN || l == LONG_MAX) && errno == ERANGE) || >+ l < 0) { >+ need_num_arg: > error("You must supply a numeric argument " > "to the %s command.", cmd); >- return(-1); >+ return -1; > } >- cp = cp2; > *n_arg = l; >- if (cmdnum == I_LUMASK && strchr(WHITESPACE, *cp)) >+ if (cmdnum == I_LUMASK) > break; >- if (cmdnum == I_LUMASK || !strchr(WHITESPACE, *cp)) { >- error("You must supply a numeric argument " >- "to the %s command.", cmd); >- return(-1); >- } >- cp += strspn(cp, WHITESPACE); >- > /* Get pathname (mandatory) */ >- if (get_pathname(&cp, path1)) >- return(-1); >- if (*path1 == NULL) { >+ if (argc - optidx < 2) { > error("You must specify a path after a %s command.", > cmd); >- return(-1); >+ return -1; > } >+ *path1 = xstrdup(argv[optidx + 1]); > break; > case I_QUIT: > case I_PWD:
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 778
:
1284
|
1291
|
1295
|
1357
| 1358