[erlang-patches] fix for the win32 filename with space execution bug

[Blaine Whittle]

This patch is for $ER_TOP/erts/emulator/sys/win32 and fixes a bug affecting
os:cmd and open_port + spawn on winnt systems that prevents execution of
child processes containing spaces in the filename or path string.

It appears that the original code allowed for the file name and / or path to
be enclosed in double quotes.  For instance parse_command searches for the
first occurrence of a space outside an opening closing pair of double
quotes.   The location of that space is used to split the application and
its path from any command line arguments.   The actual bug occurs when the
applications' file name and path are passed to the windows API
GetFileAttributes with the quotes still intact in the function
ApplicationType.

Since ApplicationType does some file searching by appending .com, .exe, .bat
to the filename, I figured it would be best to strip the quotes from the
command string before calling ApplicationType, however the code could just
as easily be placed in ApplicationType.  On a side note, it appears that
GetFileAttributes is called in places and it's possible that the same kind
of quote scrubbing may be required in other source files.

While I'm sure there is a easier way to strip characters from a string, I
removed the characters using strtok within a loop using double quotes
character as the token delimited and then appended the results using strncat
into a new string.

As for a test case, I have Visual Studio 2005 installed at C:\Program
Files\Microsoft Visual Studio 8\ (its default location, however the actual
installed location can be found in the registry.)   In order to fetch the
build number for Visual Stuudio 2005,  execute the following command line
C:\Program Files\Microsoft Visual Studio 8\Common7\IDE\devenv.com /?
and grab the version number from the first line.

To execute devenv.com (assuming that it is NOT in your path) via
command.comthe application and file path need to be wrapped in double
quotes.
cmd /c "C:\Program Files\Microsoft Visual Studio 8\Common7\IDE\devenv.com"
/?
It's important that the command line options (in this case /?) are located
outside the double quotes.

Without this patch, the following erlang expression fails however should
work after patching and building
os:cmd("\"C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\IDE\\
devenv.com\" /?").

The following two erlang expressions should always fail (with or without
patch)

No double quotes -- C:\Program Files\Microsoft Visual Studio 8\Common7\IDE\
devenv.com /?
os:cmd("C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\IDE\\
devenv.com /?").

Command line parameters inside the quotes used for the filename -
"C:\Program Files\Microsoft Visual Studio 8\Common7\IDE\devenv.com/?"
os:cmd("\"C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\IDE\\
devenv.com /?\"").


--- sys-orig.c  2008-06-10 05:47:26.000000000 -0700
+++ sys.c       2008-08-11 15:17:56.052245700 -0700
@@ -1186,6 +1186,8 @@
     char execPath[MAX_PATH];
     int cmdlength;
     char* thecommand;
+    char* fixedcommand;
+    char * pch;
     HANDLE hProcess = GetCurrentProcess();

     siStartInfo.cb = sizeof(STARTUPINFO);
@@ -1200,13 +1202,29 @@
      */
     cmdlength = parse_command(origcmd);
     thecommand = (char *) erts_alloc(ERTS_ALC_T_TMP, cmdlength+1);
+
     strncpy(thecommand, origcmd, cmdlength);
     thecommand[cmdlength] = '\0';
-    DEBUGF(("spawn command: %s\n", thecommand));
+
+    fixedcommand = (char *) erts_alloc(ERTS_ALC_T_TMP, cmdlength+1);
+    fixedcommand[0] = '\0';
+    pch = strtok(thecommand,"\"");
+    if (pch == NULL) {
+        strncpy(fixedcommand, origcmd, cmdlength);
+        fixedcommand[cmdlength] = '\0';
+    }
+    else {
+        while (pch != NULL) {
+            strncat(fixedcommand, pch, strlen(pch));
+            pch = strtok(NULL, "\"");
+        }
+    }

-    applType = ApplicationType(thecommand, execPath);
-    DEBUGF(("ApplicationType returned for (%s) is %d\n", thecommand,
applType));
+    DEBUGF(("spawn command: %s\n", thecommand));
+    applType = ApplicationType(fixedcommand, execPath);
+    DEBUGF(( "ApplicationType returned for (%s) is %d\n", fixedcommand,
applType));
     erts_free(ERTS_ALC_T_TMP, (void *) thecommand);
+    erts_free(ERTS_ALC_T_TMP, (void *) fixedcommand);
     if (applType == APPL_NONE) {
        return FALSE;
     }
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20080812/9098f3de/attachment.htm>