[Emacs-diffs] /srv/bzr/emacs/trunk r104014: Improve Windows quoting robustness

[Daniel Colascione]

------------------------------------------------------------
revno: 104014
committer: Daniel Colascione <address@hidden>
branch nick: trunk
timestamp: Tue 2011-04-26 03:44:03 -0700
message:
  Improve Windows quoting robustness
modified:
  lisp/ChangeLog
  lisp/subr.el
  nt/ChangeLog
  nt/cmdproxy.c

=== modified file 'lisp/ChangeLog'
--- a/lisp/ChangeLog    2011-04-25 18:17:17 +0000
+++ b/lisp/ChangeLog    2011-04-26 10:44:03 +0000
@@ -1,3 +1,7 @@
+2011-04-26  Daniel Colascione <address@hidden>
+
+       * subr.el (shell-quote-argument): Escape correctly under Windows.
+
 2011-04-25  Stefan Monnier  <address@hidden>
 
        * emulation/cua-base.el (cua-selection-mode): Make it toggle again.
@@ -50,6 +54,7 @@
        * net/network-stream.el (network-stream-open-starttls): Give host
        parameter to `gnutls-negotiate'.
        (gnutls-negotiate): Adjust `gnutls-negotiate' declaration.
+       * subr.el (shell-quote-argument): Escape correctly under Windows.
 
 2011-04-24  Daniel Colascione <address@hidden>
 

=== modified file 'lisp/subr.el'
--- a/lisp/subr.el      2011-04-19 13:44:55 +0000
+++ b/lisp/subr.el      2011-04-26 10:44:03 +0000
@@ -2505,27 +2505,63 @@
 
 (defun shell-quote-argument (argument)
   "Quote ARGUMENT for passing as argument to an inferior shell."
-  (if (or (eq system-type 'ms-dos)
-          (and (eq system-type 'windows-nt) (w32-shell-dos-semantics)))
-      ;; Quote using double quotes, but escape any existing quotes in
-      ;; the argument with backslashes.
-      (let ((result "")
-           (start 0)
-           end)
-       (if (or (null (string-match "[^\"]" argument))
-               (< (match-end 0) (length argument)))
-           (while (string-match "[\"]" argument start)
-             (setq end (match-beginning 0)
-                   result (concat result (substring argument start end)
-                                  "\\" (substring argument end (1+ end)))
-                   start (1+ end))))
-       (concat "\"" result (substring argument start) "\""))
+  (cond
+   ((eq system-type 'ms-dos)
+    ;; Quote using double quotes, but escape any existing quotes in
+    ;; the argument with backslashes.
+    (let ((result "")
+          (start 0)
+          end)
+      (if (or (null (string-match "[^\"]" argument))
+              (< (match-end 0) (length argument)))
+          (while (string-match "[\"]" argument start)
+            (setq end (match-beginning 0)
+                  result (concat result (substring argument start end)
+                                 "\\" (substring argument end (1+ end)))
+                  start (1+ end))))
+      (concat "\"" result (substring argument start) "\"")))
+
+   ((and (eq system-type 'windows-nt) (w32-shell-dos-semantics))
+    
+    ;; First, quote argument so that CommandLineToArgvW will
+    ;; understand it.  See
+    ;; http://msdn.microsoft.com/en-us/library/17w5ykft%28v=vs.85%29.aspx
+    ;; After we perform that level of quoting, escape shell
+    ;; metacharacters so that cmd won't mangle our argument.  If the
+    ;; argument contains no double quote characters, we can just
+    ;; surround it with double quotes.  Otherwise, we need to prefix
+    ;; each shell metacharacter with a caret.
+
+    (setq argument
+          ;; escape backslashes at end of string
+          (replace-regexp-in-string
+           "\\(\\\\*\\)$"
+           "\\1\\1"
+           ;; escape backslashes and quotes in string body
+           (replace-regexp-in-string
+            "\\(\\\\*\\)\""
+            "\\1\\1\\\\\""
+            argument)))
+
+    (if (string-match "\"" argument)
+        (concat
+         "^\""
+         (replace-regexp-in-string
+          "\\([%!()\"<>&|^]\\)"
+          "^\\1"
+          argument)
+         "^\"")
+      (concat "\"" argument "\"")))
+
+   (t
     (if (equal argument "")
         "''"
       ;; Quote everything except POSIX filename characters.
       ;; This should be safe enough even for really weird shells.
-      (replace-regexp-in-string "\n" "'\n'"
-       (replace-regexp-in-string "[^-0-9a-zA-Z_./\n]" "\\\\\\&" argument)))))
+      (replace-regexp-in-string
+       "\n" "'\n'"
+       (replace-regexp-in-string "[^-0-9a-zA-Z_./\n]" "\\\\\\&" argument))))
+   ))
 
 (defun string-or-null-p (object)
   "Return t if OBJECT is a string or nil.

=== modified file 'nt/ChangeLog'
--- a/nt/ChangeLog      2011-04-25 01:29:31 +0000
+++ b/nt/ChangeLog      2011-04-26 10:44:03 +0000
@@ -1,3 +1,8 @@
+2011-04-26  Daniel Colascione <address@hidden>
+
+       * cmdproxy.c (try_dequote_cmdline): New function.
+       (main): Use it.
+
 2011-04-24  Teodor Zlatanov  <address@hidden>
 
        * configure.bat: New options --without-gnutls and --lib, new build

=== modified file 'nt/cmdproxy.c'
--- a/nt/cmdproxy.c     2011-03-23 16:25:38 +0000
+++ b/nt/cmdproxy.c     2011-04-26 10:44:03 +0000
@@ -309,6 +309,74 @@
   return NULL;
 }
 
+/* Try to decode the given command line the way cmd would do it.  On
+   success, return 1 with cmdline dequoted.  Otherwise, when we've
+   found constructs only cmd can properly interpret, return 0 and
+   leave cmdline unchanged.  */
+int
+try_dequote_cmdline (char* cmdline)
+{
+  /* Dequoting can only subtract characters, so the length of the
+     original command line is a bound on the amount of scratch space
+     we need.  This length, in turn, is bounded by the 32k
+     CreateProces limit.  */
+  char * old_pos = cmdline;
+  char * new_cmdline = alloca (strlen(cmdline));
+  char * new_pos = new_cmdline;
+  char c;
+
+  enum {
+    NORMAL,
+    AFTER_CARET,
+    INSIDE_QUOTE
+  } state = NORMAL;
+
+  while ((c = *old_pos++))
+    {
+      switch (state)
+        {
+        case NORMAL:
+          switch(c)
+            {
+            case '"':
+              *new_pos++ = c;
+              state = INSIDE_QUOTE;
+              break;
+            case '^':
+              state = AFTER_CARET;
+              break;
+            case '<': case '>':
+            case '&': case '|':
+            case '(': case ')':
+            case '%': case '!':
+              /* We saw an unquoted shell metacharacter and we don't
+                 understand it. Bail out.  */
+              return 0;
+            default:
+              *new_pos++ = c;
+              break;
+            }
+          break;
+        case AFTER_CARET:
+          *new_pos++ = c;
+          state = NORMAL;
+          break;
+        case INSIDE_QUOTE:
+          *new_pos++ = c;
+          if (c == '"')
+            state = NORMAL;
+          
+          break;
+        }
+    }
+
+  /* We were able to dequote the entire string.  Copy our scratch
+     buffer on top of the original buffer and return success.  */
+  memcpy (cmdline, new_cmdline, new_pos - new_cmdline);
+  cmdline[new_pos - new_cmdline] = '\0';
+  return 1;
+}
+
 /*****************************************************************/
 
 #if 0
@@ -574,30 +642,26 @@
      execute the command directly ourself.  */
   if (cmdline)
     {
-      /* If no redirection or piping, and if program can be found, then
-        run program directly.  Otherwise invoke a real shell. */
-
-      static char copout_chars[] = "|<>&";
-
-      if (strpbrk (cmdline, copout_chars) == NULL)
-       {
-         const char *args;
-
-         /* The program name is the first token of cmdline.  Since
-            filenames cannot legally contain embedded quotes, the value
-            of escape_char doesn't matter.  */
-         args = cmdline;
-         if (!get_next_token (path, &args))
-           fail ("error: no program name specified.\n");
-
-         canon_filename (path);
-         progname = make_absolute (path);
-
-         /* If we found the program, run it directly (if not found it
-             might be an internal shell command, so don't fail).  */
-         if (progname != NULL)
-           need_shell = FALSE;
-       }
+      const char *args;
+
+      /* The program name is the first token of cmdline.  Since
+         filenames cannot legally contain embedded quotes, the value
+         of escape_char doesn't matter.  */
+      args = cmdline;
+      if (!get_next_token (path, &args))
+        fail ("error: no program name specified.\n");
+
+      canon_filename (path);
+      progname = make_absolute (path);
+
+      /* If we found the program and the rest of the command line does
+         not contain unquoted shell metacharacters, run the program
+         directly (if not found it might be an internal shell command,
+         so don't fail).  */
+      if (progname != NULL && try_dequote_cmdline (cmdline))
+        need_shell = FALSE;
+      else
+        progname = NULL;
     }
 
  pass_to_shell: