diff --git a/glib/gspawn-win32-helper.c b/glib/gspawn-win32-helper.c index 045d90f71..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; @@ -179,6 +190,7 @@ main (int ignored_argc, char **ignored_argv) { int child_err_report_fd = -1; int helper_sync_fd = -1; + int saved_stderr_fd = -1; int i; int fd; int mode; @@ -280,6 +292,7 @@ main (int ignored_argc, char **ignored_argv) } } + saved_stderr_fd = reopen_noninherited (dup (2), _O_WRONLY); if (argv[ARG_STDERR][0] == '-') ; /* Nothing */ else if (argv[ARG_STDERR][0] == 'z') @@ -315,15 +328,15 @@ main (int ignored_argc, char **ignored_argv) */ if (argv[ARG_CLOSE_DESCRIPTORS][0] == 'y') for (i = 3; i < 1000; i++) /* FIXME real limit? */ - if (i != child_err_report_fd && i != helper_sync_fd) + if (i != child_err_report_fd && i != helper_sync_fd && i != saved_stderr_fd) if (_get_osfhandle (i) != -1) close (i); /* We don't want our child to inherit the error report and * helper sync fds. */ - child_err_report_fd = dup_noninherited (child_err_report_fd, _O_WRONLY); - helper_sync_fd = dup_noninherited (helper_sync_fd, _O_RDONLY); + child_err_report_fd = reopen_noninherited (child_err_report_fd, _O_WRONLY); + helper_sync_fd = reopen_noninherited (helper_sync_fd, _O_RDONLY); /* argv[ARG_WAIT] is "w" to wait for the program to exit */ if (argv[ARG_WAIT][0] == 'w') @@ -351,6 +364,11 @@ main (int ignored_argc, char **ignored_argv) saved_errno = errno; + /* Some coverage warnings may be printed on stderr during this process exit. + * Remove redirection so that they would go to original stderr + * instead of being treated as part of stderr of child process. + */ + dup2 (saved_stderr_fd, 2); if (handle == -1 && saved_errno != 0) { int ec = (saved_errno == ENOENT) diff --git a/glib/gspawn-win32.c b/glib/gspawn-win32.c index b0cf5ab7a..8f998d927 100644 --- a/glib/gspawn-win32.c +++ b/glib/gspawn-win32.c @@ -105,8 +105,8 @@ enum { }; static int -dup_noninherited (int fd, - int mode) +reopen_noninherited (int fd, + int mode) { HANDLE filehandle; @@ -125,28 +125,24 @@ dup_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; @@ -606,7 +617,7 @@ do_spawn_with_fds (gint *exit_status, * helper process, and the started actual user process. As such that * shouldn't harm, but it is unnecessary. */ - child_err_report_pipe[0] = dup_noninherited (child_err_report_pipe[0], _O_RDONLY); + child_err_report_pipe[0] = reopen_noninherited (child_err_report_pipe[0], _O_RDONLY); if (flags & G_SPAWN_FILE_AND_ARGV_ZERO) { @@ -625,7 +636,7 @@ do_spawn_with_fds (gint *exit_status, * process won't read but won't get any EOF either, as it has the * write end open itself. */ - helper_sync_pipe[1] = dup_noninherited (helper_sync_pipe[1], _O_WRONLY); + helper_sync_pipe[1] = reopen_noninherited (helper_sync_pipe[1], _O_WRONLY); if (stdin_fd != -1) { @@ -657,7 +668,7 @@ do_spawn_with_fds (gint *exit_status, new_argv[ARG_STDOUT] = "-"; } - if (stdout_fd != -1) + if (stderr_fd != -1) { _g_sprintf (args[ARG_STDERR], "%d", stderr_fd); new_argv[ARG_STDERR] = args[ARG_STDERR]; diff --git a/glib/tests/spawn-singlethread.c b/glib/tests/spawn-singlethread.c index 909f702fc..15f03a0c7 100644 --- a/glib/tests/spawn-singlethread.c +++ b/glib/tests/spawn-singlethread.c @@ -312,24 +312,40 @@ test_spawn_sync (void) { int tnum = 1; GError *error = NULL; - GPtrArray *argv; - char *arg; + char *arg = g_strdup_printf ("thread %d", tnum); + /* Include arguments with special symbols to test that they are correctly passed to child. + * This is tested on all platforms, but the most prone to failure is win32, + * where args are specially escaped during spawning. + */ + const char * const argv[] = { + echo_prog_path, + arg, + "doublequotes\\\"after\\\\\"\"backslashes", /* this would be special escaped on win32 */ + "\\\"\"doublequotes spaced after backslashes\\\\\"", /* this would be special escaped on win32 */ + "even$$dollars", + "even%%percents", + "even\"\"doublequotes", + "even''singlequotes", + "even\\\\backslashes", + "even//slashes", + "$odd spaced$dollars$", + "%odd spaced%spercents%", + "\"odd spaced\"doublequotes\"", + "'odd spaced'singlequotes'", + "\\odd spaced\\backslashes\\", /* this wasn't handled correctly on win32 in glib <=2.58 */ + "/odd spaced/slashes/", + NULL + }; + char *joined_args_str = g_strjoinv ("", (char**)argv + 1); char *stdout_str; int estatus; - arg = g_strdup_printf ("thread %d", tnum); - - argv = g_ptr_array_new (); - g_ptr_array_add (argv, echo_prog_path); - g_ptr_array_add (argv, arg); - g_ptr_array_add (argv, NULL); - - g_spawn_sync (NULL, (char**)argv->pdata, NULL, 0, NULL, NULL, &stdout_str, NULL, &estatus, &error); + g_spawn_sync (NULL, (char**)argv, NULL, 0, NULL, NULL, &stdout_str, NULL, &estatus, &error); g_assert_no_error (error); - g_assert_cmpstr (arg, ==, stdout_str); + g_assert_cmpstr (joined_args_str, ==, stdout_str); g_free (arg); g_free (stdout_str); - g_ptr_array_free (argv, TRUE); + g_free (joined_args_str); } /* Like test_spawn_sync but uses spawn flags that trigger the optimized diff --git a/tests/meson.build b/tests/meson.build index 8dcb712e4..1d53db288 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -38,6 +38,7 @@ tests = { 'dependencies' : [libgthread_dep], }, 'sources' : {}, + 'spawn-test' : {}, 'thread-test' : {}, 'threadpool-test' : {'suite' : ['slow']}, 'type-test' : {}, @@ -68,9 +69,12 @@ test_extra_programs = { if host_machine.system() != 'windows' tests += { 'timeloop' : {}, - 'spawn-test' : {}, 'iochannel-test' : {}, } +else + test_extra_programs += { + 'spawn-test-win32-gui' : {'gui_app' : true} + } endif if installed_tests_enabled @@ -139,5 +143,6 @@ foreach program_name, extra_args : test_extra_programs dependencies : common_deps + extra_args.get('dependencies', []), install_dir : installed_tests_execdir, install : install, + gui_app : extra_args.get('gui_app', false), ) endforeach diff --git a/tests/spawn-test-win32-gui.c b/tests/spawn-test-win32-gui.c index 2798ce6da..34945f524 100644 --- a/tests/spawn-test-win32-gui.c +++ b/tests/spawn-test-win32-gui.c @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -19,24 +20,15 @@ WinMain (struct HINSTANCE__ *hInstance, char *lpszCmdLine, int nCmdShow) { - char buf[100]; - - if (__argc >= 2 && strcmp (__argv[1], "nop") == 0) + if (__argc >= 2 && strcmp (__argv[1], "print_argv0") == 0) { - sprintf (buf, "spawn-test-win32-gui: argv[0]=\"%s\"", __argv[0]); - MessageBox (NULL, buf, lpszCmdLine, MB_ICONINFORMATION|MB_SYSTEMMODAL); + printf ("%s", __argv[0]); } else if (__argc <= 2) { - MessageBox (NULL, "spawn-test-win32-gui: Will write to stdout", - lpszCmdLine, MB_ICONINFORMATION|MB_SYSTEMMODAL); - printf ("This is stdout\n"); fflush (stdout); - MessageBox (NULL, "spawn-test-win32-gui: Will write to stderr", - lpszCmdLine, MB_ICONINFORMATION|MB_SYSTEMMODAL); - fprintf (stderr, "This is stderr\n"); fflush (stderr); } @@ -45,70 +37,52 @@ WinMain (struct HINSTANCE__ *hInstance, int infd = atoi (__argv[2]); int outfd = atoi (__argv[3]); int k, n; + char buf[100] = {0}; if (infd < 0 || outfd < 0) { - MessageBox (NULL, "spawn-test-win32-gui: illegal fds on command line", - lpszCmdLine, MB_ICONERROR|MB_SYSTEMMODAL); + printf ("spawn-test-win32-gui: illegal fds on command line %s", + lpszCmdLine); exit (1); } - MessageBox (NULL, "spawn-test-win32-gui: Will write to parent", - lpszCmdLine, MB_ICONINFORMATION|MB_SYSTEMMODAL); - n = strlen ("Hello there"); if (write (outfd, &n, sizeof (n)) == -1 || write (outfd, "Hello there", n) == -1) { int errsv = errno; - sprintf (buf, "spawn-test-win32-gui: Write: %s", strerror (errsv)); - MessageBox (NULL, buf, lpszCmdLine, MB_ICONERROR|MB_SYSTEMMODAL); + printf ("spawn-test-win32-gui: Write error: %s", strerror (errsv)); exit (1); } - MessageBox (NULL, "spawn-test-win32-gui: Will read from parent", - lpszCmdLine, MB_ICONINFORMATION|MB_SYSTEMMODAL); - if ((k = read (infd, &n, sizeof (n))) != sizeof (n)) { - sprintf (buf, "spawn-test-win32-gui: Got only %d bytes, wanted %d", - k, sizeof (n)); - MessageBox (NULL, buf, lpszCmdLine, MB_ICONERROR|MB_SYSTEMMODAL); + printf ("spawn-test-win32-gui: Got only %d bytes, wanted %d", + k, (int)sizeof (n)); exit (1); } - sprintf (buf, "spawn-test-win32-gui: Parent says %d bytes to read", n); - MessageBox (NULL, buf, lpszCmdLine, MB_ICONINFORMATION|MB_SYSTEMMODAL); + printf ("spawn-test-win32-gui: Parent says %d bytes to read", n); if ((k = read (infd, buf, n)) != n) { int errsv = errno; if (k == -1) - sprintf (buf, "spawn-test-win32-gui: Read: %s", strerror (errsv)); + printf ("spawn-test-win32-gui: Read error: %s", strerror (errsv)); else - sprintf (buf, "spawn-test-win32-gui: Got only %d bytes", k); - MessageBox (NULL, buf, lpszCmdLine, MB_ICONERROR|MB_SYSTEMMODAL); + printf ("spawn-test-win32-gui: Got only %d bytes", k); exit (1); } - MessageBox (NULL, "spawn-test-win32-gui: Will write more to parent", - lpszCmdLine, MB_ICONINFORMATION|MB_SYSTEMMODAL); - n = strlen ("See ya"); if (write (outfd, &n, sizeof (n)) == -1 || write (outfd, "See ya", n) == -1) { int errsv = errno; - sprintf (buf, "spawn-test-win32-gui: Write: %s", strerror (errsv)); - MessageBox (NULL, buf, lpszCmdLine, MB_ICONERROR|MB_SYSTEMMODAL); + printf ("spawn-test-win32-gui: Write error: %s", strerror (errsv)); exit (1); } } - Sleep (2000); - - MessageBox (NULL, "spawn-test-win32-gui: Done, exiting.", - lpszCmdLine, MB_ICONINFORMATION|MB_SYSTEMMODAL); - return 0; } diff --git a/tests/spawn-test.c b/tests/spawn-test.c index 05cce17f8..78a7e7c7d 100644 --- a/tests/spawn-test.c +++ b/tests/spawn-test.c @@ -26,6 +26,7 @@ #undef G_LOG_DOMAIN #include +#include #include #include #include @@ -38,15 +39,20 @@ static void -run_tests (void) +run_tests (const gchar* argv0) { - GError *err; + GError *err = NULL; gchar *output = NULL; -#ifdef G_OS_WIN32 gchar *erroutput = NULL; + gchar *dirname = g_path_get_dirname (argv0); +#ifdef G_OS_WIN32 int pipedown[2], pipeup[2]; gchar **argv = 0; + gchar spawn_binary[1000] = {0}; + gchar full_cmdline[1000] = {0}; + g_snprintf (spawn_binary, sizeof (spawn_binary), "%s\\spawn-test-win32-gui.exe", dirname); #endif + g_free (dirname); err = NULL; if (!g_spawn_command_line_sync ("nonexistent_application foo 'bar baz' blah blah", @@ -96,12 +102,15 @@ run_tests (void) } g_free (output); + output = NULL; } -#else -#ifdef G_OS_WIN32 - printf ("Running netstat synchronously, collecting its output\n"); - - if (!g_spawn_command_line_sync ("netstat -n", +#endif + /* Running sort synchronously, collecting its output. 'sort' command is selected + * because it is non-builtin command on both unix and win32 with well-defined stdout behaviour. + */ + g_file_set_contents ("spawn-test-created-file.txt", "line first\nline 2\nline last\n", -1, &err); + g_assert_no_error(err); + if (!g_spawn_command_line_sync ("sort spawn-test-created-file.txt", &output, &erroutput, NULL, &err)) { @@ -114,9 +123,9 @@ run_tests (void) g_assert (output != NULL); g_assert (erroutput != NULL); - if (strstr (output, "Active Connections") == 0) + if (strstr (output, "\nline first") == 0) { - printf ("output was '%s', should have contained 'Active Connections'\n", + printf ("output was '%s', should have contained 'line first' in second line\n", output); exit (1); @@ -132,13 +141,36 @@ run_tests (void) output = NULL; g_free (erroutput); erroutput = NULL; + g_unlink ("spawn-test-created-file.txt"); } - printf ("Running spawn-test-win32-gui in various ways. Click on the OK buttons.\n"); + if (!g_spawn_command_line_sync ("sort non-existing-file.txt", + NULL, &erroutput, NULL, + &err)) + { + fprintf (stderr, "Error: %s\n", err->message); + g_error_free (err); + exit (1); + } + else + { + g_assert (erroutput != NULL); + + if (erroutput[0] == '\0') + { + printf ("erroutput was empty, expected contain error message about non-existing-file.txt\n"); + exit (1); + } + g_free (erroutput); + erroutput = NULL; + } + +#ifdef G_OS_WIN32 + printf ("Running spawn-test-win32-gui in various ways.\n"); printf ("First asynchronously (without wait).\n"); - - if (!g_spawn_command_line_async ("'.\\spawn-test-win32-gui.exe' 1", &err)) + g_snprintf (full_cmdline, sizeof (full_cmdline), "'%s' 1", spawn_binary); + if (!g_spawn_command_line_async (full_cmdline, &err)) { fprintf (stderr, "Error: %s\n", err->message); g_error_free (err); @@ -146,7 +178,8 @@ run_tests (void) } printf ("Now synchronously, collecting its output.\n"); - if (!g_spawn_command_line_sync ("'.\\spawn-test-win32-gui.exe' 2", + g_snprintf (full_cmdline, sizeof (full_cmdline), "'%s' 2", spawn_binary); + if (!g_spawn_command_line_sync (full_cmdline, &output, &erroutput, NULL, &err)) { @@ -174,27 +207,39 @@ run_tests (void) } g_free (output); + output = NULL; g_free (erroutput); + erroutput = NULL; } printf ("Now with G_SPAWN_FILE_AND_ARGV_ZERO.\n"); - - if (!g_shell_parse_argv ("'.\\spawn-test-win32-gui.exe' this-should-be-argv-zero nop", NULL, &argv, &err)) + g_snprintf (full_cmdline, sizeof (full_cmdline), "'%s' this-should-be-argv-zero print_argv0", spawn_binary); + if (!g_shell_parse_argv (full_cmdline, NULL, &argv, &err)) { fprintf (stderr, "Error parsing command line? %s\n", err->message); g_error_free (err); exit (1); } - if (!g_spawn_async (NULL, argv, NULL, + if (!g_spawn_sync (NULL, argv, NULL, G_SPAWN_FILE_AND_ARGV_ZERO, - NULL, NULL, NULL, + NULL, NULL, &output, NULL, NULL, &err)) { fprintf (stderr, "Error: %s\n", err->message); g_error_free (err); exit (1); } + else + { + if (strcmp (output, "this-should-be-argv-zero") != 0) + { + printf ("output was '%s', should have been 'this-should-be-argv-zero'\n", output); + exit (1); + } + g_free (output); + output = NULL; + } printf ("Now talking to it through pipes.\n"); @@ -205,8 +250,8 @@ run_tests (void) exit (1); } - if (!g_shell_parse_argv (g_strdup_printf ("'.\\spawn-test-win32-gui.exe' pipes %d %d", - pipedown[0], pipeup[1]), + g_snprintf (full_cmdline, sizeof (full_cmdline), "'%s' pipes %d %d", spawn_binary, pipedown[0], pipeup[1]); + if (!g_shell_parse_argv (full_cmdline, NULL, &argv, &err)) { @@ -237,7 +282,7 @@ run_tests (void) fprintf (stderr, "Read error: %s\n", g_strerror (errsv)); else fprintf (stderr, "Wanted to read %d bytes, got %d\n", - sizeof (n), k); + (int)sizeof (n), k); exit (1); } @@ -268,7 +313,12 @@ run_tests (void) fprintf (stderr, "Read error: %s\n", g_strerror (errsv)); else fprintf (stderr, "Wanted to read %d bytes, got %d\n", - sizeof (n), k); + (int)sizeof (n), k); + exit (1); + } + if (n != strlen ("See ya")) + { + printf ("child wrote %d bytes, expected %d", n, (int) strlen ("See ya")); exit (1); } @@ -282,16 +332,21 @@ run_tests (void) n, k); exit (1); } + buf[n] = '\0'; + if (strcmp (buf, "See ya") != 0) + { + printf ("output was '%s', should have been 'See ya'\n", buf); + exit (1); + } } #endif -#endif } int main (int argc, char *argv[]) { - run_tests (); + run_tests (argv[0]); return 0; }