bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Noam Postavsky <npostavs@users.sourceforge.net>]

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]

On Wed, Oct 22, 2014 at 1:12 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> Thanks, the changes look reasonable.  If you didn't already, please
> run the test suite and make sure there are no regressions due to these
> changes.  Bonus points for adding specialized tests to test this
> specific issue.

The tests in test/automated? No regressions.

Here is a new patch with added test and formatting issues fixed.

[-- Attachment #2: bat-quote-v2.ChangeLog --]
[-- Type: application/octet-stream, Size: 401 bytes --]

2014-10-22  Noam Postavsky  <npostavs@users.sourceforget.net>

	Paper over MS Windows CreateProcess deficiencies (Bug#18745).

	* nt/cmdproxy.c (batch_file_p): New function.
	(spawn): If calling a quoted batch file pass NULL for progname.
	* src/w32proc.c (create_child): If calling a quoted batch file
	pass NULL for exe.
	* test/automated/process-tests.el (process-test-quoted-batfile):
	New test.


[-- Attachment #3: bat-quote-v2.patch --]
[-- Type: application/octet-stream, Size: 3180 bytes --]

--- org/nt/cmdproxy.c
+++ new/nt/cmdproxy.c
@@ -220,6 +220,28 @@
   return o - buf;
 }
 
+/* Return TRUE if PROGNAME is a batch file. */
+BOOL
+batch_file_p (const char *progname)
+{
+  const char *exts[] = {".bat", ".cmd"};
+  int n_exts = sizeof (exts) / sizeof (char *);
+  int i;
+
+  const char *ext = strrchr (progname, '.');
+
+  if (ext)
+    {
+      for (i = 0; i < n_exts; i++)
+        {
+          if (stricmp (ext, exts[i]) == 0)
+            return TRUE;
+        }
+    }
+
+  return FALSE;
+}
+
 /* Search for EXEC file in DIR.  If EXEC does not have an extension,
    DIR is searched for EXEC with the standard extensions appended.  */
 int
@@ -469,6 +491,13 @@
 
   memset (&start, 0, sizeof (start));
   start.cb = sizeof (start);
+
+  /* CreateProcess handles batch files as progname specially. This
+     special handling fails when both the batch file and arguments are
+     quoted.  We pass NULL as progname to avoid the special
+     handling. */
+  if (progname != NULL && cmdline[0] == '"' && batch_file_p (progname))
+      progname = NULL;
 
   if (CreateProcess (progname, cmdline, &sec_attrs, NULL, TRUE,
 		     0, envblock, dir, &start, &child))
--- org/src/w32proc.c
+++ new/src/w32proc.c
@@ -1078,6 +1078,7 @@
   DWORD flags;
   char dir[ MAX_PATH ];
   char *p;
+  const char *ext;
 
   if (cp == NULL) emacs_abort ();
 
@@ -1115,6 +1116,15 @@
   for (p = dir; *p; p = CharNextA (p))
     if (*p == '/')
       *p = '\\';
+
+  /* CreateProcess handles batch files as exe specially.  This special
+     handling fails when both the batch file and arguments are quoted.
+     We pass NULL as exe to avoid the special handling. */
+  if (exe && cmdline[0] == '"' &&
+      (ext = strrchr (exe, '.')) &&
+      (xstrcasecmp (ext, ".bat") == 0
+       || xstrcasecmp (ext, ".cmd") == 0))
+      exe = NULL;
 
   flags = (!NILP (Vw32_start_process_share_console)
 	   ? CREATE_NEW_PROCESS_GROUP
--- org/test/automated/process-tests.el
+++ new/test/automated/process-tests.el
@@ -50,4 +50,26 @@
   (should
    (process-test-sentinel-wait-function-working-p (lambda () (sit-for 0.01 t)))))
 
+(when (eq system-type 'windows-nt)
+  (ert-deftest process-test-quoted-batfile ()
+    "Check that Emacs hides CreateProcess deficiency (bug#18745)."
+    (let (batfile)
+      (unwind-protect
+          (progn
+            ;; CreateProcess will fail when both the bat file and 1st
+            ;; argument are quoted, so include spaces in both of those
+            ;; to force quoting.
+            (setq batfile (make-temp-file "echo args" nil ".bat"))
+            (with-temp-file batfile
+              (insert "@echo arg1 = %1, arg2 = %2\n"))
+            (with-temp-buffer
+              (call-process batfile nil '(t t) t "x &y")
+              (should (string= (buffer-string) "arg1 = \"x &y\", arg2 = \n")))
+            (with-temp-buffer
+              (call-process-shell-command
+               (mapconcat #'shell-quote-argument (list batfile "x &y") " ")
+               nil '(t t) t)
+              (should (string= (buffer-string) "arg1 = \"x &y\", arg2 = \n"))))
+        (when batfile (delete-file batfile))))))
+
 (provide 'process-tests)