[glib: 5/6] gspawn, win32: quoted args - escape end backslash

[Philip Withnall <pwithnall src gnome org>]

commit 22e875f71041bacff0e4d4a5745ceb753b16642c
Author: Vasily Galkin <galkin-vv yandex ru>
Date:   Thu Dec 27 00:06:58 2018 +0300

    gspawn, win32: quoted args - escape end backslash
    
    According to msdn documentation last backslash(es) of quoted argument
    in a win32 cmdline need to be escaped, since they are
    directly preceding quote in the resulting string:
    https://docs.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments
    
    Glib <=2.58.0 passed children arguments like C:\Program Files\
    without escaping last backslash(es).
    So it had been passed as "C:\Program Files\"
    windows command line parsing treated this as escaped quote,
    and later text was treated as argument continuation instead of separate
    arguments.
    
    Existing implementation wasn't easily adoptable to fix this problem,
    so escaping logic was rewritten.
    Since the resulting length need to be increased due to extra escaping
    it was rewritten too. Now the calculated length assumes that all
    escapable chars would be escaped in a resulting string,
    so the length may be a bit bigger than actually needed,
    since backslashes not preceding quotes are not escaped.
    
    This fixes the glib/tests/spawn-singlethread.c test
    (which introduced testing for special chars to make this problem
    testable).
    The problem itself was found during investigations about fixing
    related https://gitlab.gnome.org/GNOME/glib/issues/1566
    
    The logic is duplicated in protect_argv_string() and protect_wargv() funcs.
    However there is no single obvious way to get rid of duplication -
    https://gitlab.gnome.org/GNOME/glib/merge_requests/419#note_371483
    
    So by now adding a note referencing protect_wargv from protect_argv_string,
    the other direction is already referenced.

 glib/gspawn-win32-helper.c | 47 +++++++++++++++++++++++++++-----------------
 glib/gspawn-win32.c        | 49 ++++++++++++++++++++++++++++------------------
 2 files changed, 59 insertions(+), 37 deletions(-)
---
diff --git a/glib/gspawn-win32-helper.c b/glib/gspawn-win32-helper.c
index bbb6161ef..d082de279 100644
--- a/glib/gspawn-win32-helper.c
+++ b/glib/gspawn-win32-helper.c
@@ -92,22 +92,17 @@ protect_wargv (gint       argc,
       wchar_t *p = wargv[i];
       wchar_t *q;
       gint len = 0;
+      gint pre_bslash = 0;
       gboolean need_dblquotes = FALSE;
       while (*p)
        {
          if (*p == ' ' || *p == '\t')
            need_dblquotes = TRUE;
-         else if (*p == '"')
-           len++;
-         else if (*p == '\\')
-           {
-             wchar_t *pp = p;
-             while (*pp && *pp == '\\')
-               pp++;
-             if (*pp == '"')
-               len++;
-           }
-         len++;
+         /* estimate max len, assuming that all escapable chracters will be escaped */
+         if (*p == '"' || *p == '\\')
+           len += 2;
+         else
+           len += 1;
          p++;
        }
 
@@ -117,24 +112,40 @@ protect_wargv (gint       argc,
       if (need_dblquotes)
        *q++ = '"';
 
+      /* Only quotes and backslashes preceeding quotes are escaped:
+       * see "Parsing C Command-Line Arguments" at
+       * https://docs.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments
+       */
       while (*p)
        {
          if (*p == '"')
-           *q++ = '\\';
-         else if (*p == '\\')
            {
-             wchar_t *pp = p;
-             while (*pp && *pp == '\\')
-               pp++;
-             if (*pp == '"')
+             /* Add backslash for escaping quote itself */
+             *q++ = '\\';
+             /* Add backslash for every preceeding backslash for escaping it */
+             for (;pre_bslash > 0; --pre_bslash)
                *q++ = '\\';
            }
+
+         /* Count length of continuous sequence of preceeding backslashes. */
+         if (*p == '\\')
+           ++pre_bslash;
+         else
+           pre_bslash = 0;
+
          *q++ = *p;
          p++;
        }
 
       if (need_dblquotes)
-       *q++ = '"';
+       {
+         /* Add backslash for every preceeding backslash for escaping it,
+          * do NOT escape quote itself.
+          */
+         for (;pre_bslash > 0; --pre_bslash)
+           *q++ = '\\';
+         *q++ = '"';
+       }
       *q++ = '\0';
     }
   (*new_wargv)[argc] = NULL;
diff --git a/glib/gspawn-win32.c b/glib/gspawn-win32.c
index c4cc8593b..8f998d927 100644
--- a/glib/gspawn-win32.c
+++ b/glib/gspawn-win32.c
@@ -125,28 +125,24 @@ reopen_noninherited (int fd,
 #define HELPER_PROCESS "gspawn-win32-helper"
 #endif
 
+/* This logic has a copy for wchar_t in gspawn-win32-helper.c, protect_wargv() */
 static gchar *
 protect_argv_string (const gchar *string)
 {
   const gchar *p = string;
   gchar *retval, *q;
   gint len = 0;
+  gint pre_bslash = 0;
   gboolean need_dblquotes = FALSE;
   while (*p)
     {
       if (*p == ' ' || *p == '\t')
        need_dblquotes = TRUE;
-      else if (*p == '"')
-       len++;
-      else if (*p == '\\')
-       {
-         const gchar *pp = p;
-         while (*pp && *pp == '\\')
-           pp++;
-         if (*pp == '"')
-           len++;
-       }
-      len++;
+      /* estimate max len, assuming that all escapable chracters will be escaped */
+      if (*p == '"' || *p == '\\')
+       len += 2;
+      else
+       len += 1;
       p++;
     }
   
@@ -155,25 +151,40 @@ protect_argv_string (const gchar *string)
 
   if (need_dblquotes)
     *q++ = '"';
-  
+  /* Only quotes and backslashes preceeding quotes are escaped:
+   * see "Parsing C Command-Line Arguments" at
+   * https://docs.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments
+   */
   while (*p)
     {
       if (*p == '"')
-       *q++ = '\\';
-      else if (*p == '\\')
        {
-         const gchar *pp = p;
-         while (*pp && *pp == '\\')
-           pp++;
-         if (*pp == '"')
+         /* Add backslash for escaping quote itself */
+         *q++ = '\\';
+         /* Add backslash for every preceeding backslash for escaping it */
+         for (;pre_bslash > 0; --pre_bslash)
            *q++ = '\\';
        }
+
+      /* Count length of continuous sequence of preceeding backslashes. */
+      if (*p == '\\')
+       ++pre_bslash;
+      else
+       pre_bslash = 0;
+
       *q++ = *p;
       p++;
     }
   
   if (need_dblquotes)
-    *q++ = '"';
+    {
+      /* Add backslash for every preceeding backslash for escaping it,
+       * do NOT escape quote itself.
+       */
+      for (;pre_bslash > 0; --pre_bslash)
+       *q++ = '\\';
+      *q++ = '"';
+    }
   *q++ = '\0';
 
   return retval;