[Nmh-commits] nmh ChangeLog uip/whatnowsbr.c test/tests/whatn...

[Peter Maydell]

CVSROOT:        /sources/nmh
Module name:    nmh
Changes by:     Peter Maydell <pm215>   08/08/03 18:47:57

Modified files:
        .              : ChangeLog 
        uip            : whatnowsbr.c 
Added files:
        test/tests/whatnow: test-attach-detach test-cd test-ls 

Log message:
        bug #23319: rework the way we handle the working directory when invoking
        the user's shell, so that we don't have issues when the cwd contains a
        space or shell special character.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/nmh/ChangeLog?cvsroot=nmh&r1=1.282&r2=1.283
http://cvs.savannah.gnu.org/viewcvs/nmh/uip/whatnowsbr.c?cvsroot=nmh&r1=1.10&r2=1.11
http://cvs.savannah.gnu.org/viewcvs/nmh/test/tests/whatnow/test-attach-detach?cvsroot=nmh&rev=1.1
http://cvs.savannah.gnu.org/viewcvs/nmh/test/tests/whatnow/test-cd?cvsroot=nmh&rev=1.1
http://cvs.savannah.gnu.org/viewcvs/nmh/test/tests/whatnow/test-ls?cvsroot=nmh&rev=1.1

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/nmh/nmh/ChangeLog,v
retrieving revision 1.282
retrieving revision 1.283
diff -u -b -r1.282 -r1.283
--- ChangeLog   3 Aug 2008 15:19:53 -0000       1.282
+++ ChangeLog   3 Aug 2008 18:47:56 -0000       1.283
@@ -1,5 +1,14 @@
 2008-08-03  Peter Maydell  <address@hidden>
 
+       * test/setup-test: use 'set -e' so we stop on compile failure.
+       Configure --enable-debug for convenience in debugging.
+
+       * uip/whatnowsbr.c: bug #23319: rework the way we handle the working
+       directory when invoking the user's shell, so that we don't have
+       issues when the cwd contains a space or shell special character.
+       * test/tests/whatnow/test-attach-detach, test/tests/whatnow/test-cd,
+       test/tests/whatnow/test-ls: test cases for this.
+
        * sbr/fmt_compile.c: add 'do { ... } while (0)' wrappers
        to various multi-statement macros to avoid nasty surprises
        if the macros are used in if() clauses.

Index: uip/whatnowsbr.c
===================================================================
RCS file: /sources/nmh/nmh/uip/whatnowsbr.c,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -b -r1.10 -r1.11
--- uip/whatnowsbr.c    22 May 2008 10:21:48 -0000      1.10
+++ uip/whatnowsbr.c    3 Aug 2008 18:47:56 -0000       1.11
@@ -2,7 +2,7 @@
 /*
  * whatnowsbr.c -- the WhatNow shell
  *
- * $Id: whatnowsbr.c,v 1.10 2008/05/22 10:21:48 pm215 Exp $
+ * $Id: whatnowsbr.c,v 1.11 2008/08/03 18:47:56 pm215 Exp $
  *
  * This code is Copyright (c) 2002, by the authors of nmh.  See the
  * COPYRIGHT file in the root directory of the nmh distribution for
@@ -120,7 +120,11 @@
 static int check_draft (char *);
 static int whomfile (char **, char *);
 static int removefile (char *);
-static void writelscmd(char *, int, char *, char **);
+static void writelscmd(char *, int, char **);
+static void writesomecmd(char *buf, int bufsz, char *cmd, char *trailcmd, char 
**argp);
+static FILE* popen_in_dir(const char *dir, const char *cmd, const char *type);
+static int system_in_dir(const char *dir, const char *cmd);
+
 
 #ifdef HAVE_LSTAT
 static int copyf (char *, char *);
@@ -320,16 +324,13 @@
             *  is accustomed to.  Read back the absolute path.
             */
 
-           if (*++argp == (char *)0) {
+           if (*(argp+1) == (char *)0) {
                (void)sprintf(buf, "$SHELL -c \"cd;pwd\"");
            }
-           else if (strlen(*argp) >= BUFSIZ) {
-               adios((char *)0, "arguments too long");
-           }
            else {
-               (void)sprintf(buf, "$SHELL -c \"cd %s;cd %s;pwd\"", cwd, *argp);
+               writesomecmd(buf, BUFSIZ, "cd", "pwd", argp);
            }
-           if ((f = popen(buf, "r")) != (FILE *)0) {
+           if ((f = popen_in_dir(cwd, buf, "r")) != (FILE *)0) {
                fgets(cwd, sizeof (cwd), f);
 
                if (strchr(cwd, '\n') != (char *)0)
@@ -354,8 +355,8 @@
             *  Use the user's shell so that we can take advantage of any
             *  syntax that the user is accustomed to.
             */
-           writelscmd(buf, sizeof(buf), cwd, argp);
-           (void)system(buf);
+           writelscmd(buf, sizeof(buf), argp);
+           (void)system_in_dir(cwd, buf);
            break;
 
        case ALISTCMDSW:
@@ -414,7 +415,7 @@
             *  Build a command line that causes the user's shell to list the 
file name
             *  arguments.  This handles and wildcard expansion, tilde 
expansion, etc.
             */
-           writelscmd(buf, sizeof(buf), cwd, argp);
+           writelscmd(buf, sizeof(buf), argp);
 
            /*
             *  Read back the response from the shell, which contains a number 
of lines
@@ -424,7 +425,7 @@
             *  draft.
             */
 
-           if ((f = popen(buf, "r")) != (FILE *)0) {
+           if ((f = popen_in_dir(cwd, buf, "r")) != (FILE *)0) {
                while (fgets(shell, sizeof (shell), f) != (char *)0) {
                    *(strchr(shell, '\n')) = '\0';
 
@@ -499,36 +500,20 @@
             *  user's shell for wildcard expansion and other goodies.  Do this 
from
             *  the current working directory if the argument is not an 
absolute path
             *  name (does not begin with a /).
+            *
+            * We feed all the file names to the shell at once, otherwise you 
can't
+            * provide a file name with a space in it.
             */
-
-           else {
-               for (arguments = argp + 1; *arguments != (char *)0; 
arguments++) {
-                   if (**arguments == '/') {
-                       if (strlen(*arguments) + sizeof ("$SHELL -c \"ls \"") 
>= sizeof (buf))
-                           adios((char *)0, "arguments too long");
-
-                       (void)sprintf(buf, "$SHELL -c \"ls %s\"", *arguments);
-                   }
-                   else {
-                       if (strlen(cwd) + strlen(*arguments) + sizeof ("$SHELL 
-c \" cd ; ls \"") >= sizeof (buf))
-                           adios((char *)0, "arguments too long");
-
-                       (void)sprintf(buf, "$SHELL -c \" cd %s;ls %s\"", cwd, 
*arguments);
-                   }
-
-                   if ((f = popen(buf, "r")) != (FILE *)0) {
-                       while (fgets(shell, sizeof (cwd), f) != (char *)0) {
+           writelscmd(buf, sizeof(buf), argp);
+           if ((f = popen_in_dir(cwd, buf, "r")) != (FILE *)0) {
+               while (fgets(shell, sizeof (shell), f) != (char *)0) {
                            *(strchr(shell, '\n')) = '\0';
                            (void)annotate(drft, attach, shell, 1, 0, 0, 1);
                        }
-
                        pclose(f);
-                   }
-                   else {
+           } else {
                        advise("popen", "could not get file from shell");
                    }
-               }
-           }
 
            break;
 
@@ -542,40 +527,96 @@
 }
 
 
-/*
- * Build a command line that causes the user's shell to list the file name
- * arguments.  This handles and wildcard expansion, tilde expansion, etc.
- */
+
+/* Build a command line of the form $SHELL -c "cd 'cwd'; cmd argp ... ; 
trailcmd". */
 static void
-writelscmd(char *buf, int bufsz, char *cwd, char **argp)
+writesomecmd(char *buf, int bufsz, char *cmd, char *trailcmd, char **argp)
 {
     char *cp;
-    int ln = snprintf(buf, bufsz, "$SHELL -c \" cd %s;ls", cwd);
+    /* Note that we do not quote -- the argp from the user
+     * is assumed to be quoted as they desire. (We can't treat
+     * it as pure literal as that would prevent them using ~,
+     * wildcards, etc.) The buffer produced by this function
+     * should be given to popen_in_dir() or system_in_dir() so
+     * that the current working directory is set correctly.
+     */
+    int ln = snprintf(buf, bufsz, "$SHELL -c \"%s", cmd);
     /* NB that some snprintf() return -1 on overflow rather than the
      * new C99 mandated 'number of chars that would have been written'
      */
     /* length checks here and inside the loop allow for the
-     * trailing " and NUL
+     * trailing ';', trailcmd, '"' and NUL
      */
-    if (ln < 0 || ln + 2 > bufsz)
+    int trailln = strlen(trailcmd) + 3;
+    if (ln < 0 || ln + trailln > bufsz)
        adios((char *)0, "arguments too long");
     
     cp = buf + ln;
     
     while (*++argp != (char *)0) {
        ln = strlen(*argp);
-       /* +3 for leading space and trailing quote and NUL */
-       if (ln + 3 > bufsz - (cp-buf))
+       /* +1 for leading space */
+       if (ln + trailln + 1 > bufsz - (cp-buf))
            adios((char *)0, "arguments too long");
        *cp++ = ' ';
        memcpy(cp, *argp, ln+1);
        cp += ln;
     }
+    if (*trailcmd) {
+       *cp++ = ';';
+       strcpy(cp, trailcmd);
+       cp += trailln - 3;
+    }
     *cp++ = '"';
     *cp = 0;
 }
 
 /*
+ * Build a command line that causes the user's shell to list the file name
+ * arguments.  This handles and wildcard expansion, tilde expansion, etc.
+ */
+static void
+writelscmd(char *buf, int bufsz, char **argp)
+{
+    writesomecmd(buf, bufsz, "ls", "", argp);
+}
+
+/* Like system(), but run the command in directory dir.
+ * This assumes the program is single-threaded!
+ */
+static int
+system_in_dir(const char *dir, const char *cmd)
+{
+    char olddir[BUFSIZ];
+    int r;
+    if (getcwd(olddir, sizeof(olddir)) == 0)
+       adios("getcwd", "could not get working directory");
+    if (chdir(dir) != 0)
+       adios("chdir", "could not change working directory");
+    r = system(cmd);
+    if (chdir(olddir) != 0)
+       adios("chdir", "could not change working directory");
+    return r;
+}
+
+/* ditto for popen() */
+static FILE*
+popen_in_dir(const char *dir, const char *cmd, const char *type)
+{
+    char olddir[BUFSIZ];
+    FILE *f;
+    if (getcwd(olddir, sizeof(olddir)) == 0)
+       adios("getcwd", "could not get working directory");
+    if (chdir(dir) != 0)
+       adios("chdir", "could not change working directory");
+    f = popen(cmd, type);
+    if (chdir(olddir) != 0)
+       adios("chdir", "could not change working directory");
+    return f;
+}
+
+
+/*
  * EDIT
  */
 

Index: test/tests/whatnow/test-attach-detach
===================================================================
RCS file: test/tests/whatnow/test-attach-detach
diff -N test/tests/whatnow/test-attach-detach
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ test/tests/whatnow/test-attach-detach       3 Aug 2008 18:47:56 -0000       
1.1
@@ -0,0 +1,38 @@
+#!/bin/sh
+######################################################
+#
+# Test that whatnow's "ls" copes with directory names
+# which have spaces in them (see bug #23319)
+#
+######################################################
+
+set -e
+
+cd "$MH_TEST_DIR"
+rm -f "baz's boz"
+touch "baz's boz"
+# whatnow's attach stuff needs a draft to work on
+cp "$MH_TEST_DIR/Mail/inbox/1" "$MH_TEST_DIR/Mail/draft"
+
+expectederr=$MH_TEST_DIR/$$.expectederr
+actualerr=$MH_TEST_DIR/$$.actualerr
+expected=$MH_TEST_DIR/$$.expected
+actual=$MH_TEST_DIR/$$.actual
+
+rm -f $expected $expectederr $actual $actualerr
+touch $expected $expectederr $actual $actualerr
+
+cat > $expected <<EOF
+baz's boz
+EOF
+
+# whatnow's exit status is always 1 so that is not a failure
+set +e
+echo "attach baz\\'s\\ boz" | whatnow -attach foo -noedit -prompt '' 2>> 
$actualerr >> $actual
+echo "alist" | whatnow -attach foo -noedit -prompt '' 2>> $actualerr >> $actual
+echo "detach baz\\'s\\ boz" | whatnow -attach foo -noedit -prompt '' 2>> 
$actualerr >> $actual
+echo "alist" | whatnow -attach foo -noedit -prompt '' 2>> $actualerr >> $actual
+set -e
+
+diff -u $expectederr $actualerr
+diff -u $expected $actual

Index: test/tests/whatnow/test-cd
===================================================================
RCS file: test/tests/whatnow/test-cd
diff -N test/tests/whatnow/test-cd
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ test/tests/whatnow/test-cd  3 Aug 2008 18:47:57 -0000       1.1
@@ -0,0 +1,34 @@
+#!/bin/sh
+######################################################
+#
+# Test that whatnow's "ls" copes with directory names
+# which have spaces and quotes in them (see bug #23319)
+#
+######################################################
+
+set -e
+SPDIR="$MH_TEST_DIR/foo's bar"
+rm -rf "$SPDIR"
+mkdir "$SPDIR"
+cd "$SPDIR"
+touch baz boz
+cd
+
+expectederr=$MH_TEST_DIR/$$.expectederr
+actualerr=$MH_TEST_DIR/$$.actualerr
+expected=$MH_TEST_DIR/$$.expected
+actual=$MH_TEST_DIR/$$.actual
+
+cat > $expected <<EOF
+$SPDIR
+EOF
+
+cat > $expectederr <<EOF
+EOF
+
+# ||true to ignore whatnow's exit status
+# watch the quoting -- shell and printf and then the shell run inside whatnow
+printf "cd $MH_TEST_DIR/foo\\\\'s\\\\ bar\npwd\n" | whatnow -noedit -prompt '' 
2> "$actualerr" > "$actual" || true
+
+diff -u $expectederr $actualerr
+diff -u $expected $actual

Index: test/tests/whatnow/test-ls
===================================================================
RCS file: test/tests/whatnow/test-ls
diff -N test/tests/whatnow/test-ls
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ test/tests/whatnow/test-ls  3 Aug 2008 18:47:57 -0000       1.1
@@ -0,0 +1,33 @@
+#!/bin/sh
+######################################################
+#
+# Test that whatnow's "ls" copes with directory names
+# which have spaces and quotes in them (see bug #23319)
+#
+######################################################
+
+set -e
+SPDIR="$MH_TEST_DIR/foo's bar"
+rm -rf "$SPDIR"
+mkdir "$SPDIR"
+cd "$SPDIR"
+touch baz boz
+
+expectederr=$MH_TEST_DIR/$$.expectederr
+actualerr=$MH_TEST_DIR/$$.actualerr
+expected=$MH_TEST_DIR/$$.expected
+actual=$MH_TEST_DIR/$$.actual
+
+cat > $expected <<EOF
+baz
+boz
+EOF
+
+cat > $expectederr <<EOF
+EOF
+
+# NB use of sort as the order of output of ls is not guaranteed
+echo 'ls' | whatnow -noedit -prompt '' 2> "$actualerr" | sort > "$actual"
+
+diff -u $expectederr $actualerr
+diff -u $expected $actual