Merge branch 'work-around-ucrt-spawn-env-issue' into 'main'

Work around an UCRT issue with _wspawn() functions taking an envp block

See merge request GNOME/glib!3289

glib/gspawn-win32.c

@@ -65,8 +65,10 @@
 #include <wchar.h>
 
 #ifdef _MSC_VER
+#ifdef HAVE_VCRUNTIME_H
 #include <vcruntime.h> /* for _UCRT */
 #endif
+#endif
 
 #ifndef GSPAWN_HELPER
 #ifdef G_SPAWN_WIN32_DEBUG
@@ -163,8 +165,85 @@ safe_wspawnvpe (int _Mode,
 
 #else
 
-#define safe_wspawnve _wspawnve
-#define safe_wspawnvpe _wspawnvpe
+/**< private >
+ * ensure_cmd_environment:
+ *
+ * Workaround for an issue in the universal C Runtime library (UCRT). This adds
+ * a custom environment variable to this process's environment block that looks
+ * like the cmd.exe's shell-related environment variables, i.e the name starts
+ * with an equal sign character: '='. This is needed because the UCRT may crash
+ * if those environment variables are missing from the calling process's block.
+ *
+ * Reference:
+ *
+ * https://developercommunity.visualstudio.com/t/UCRT-Crash-in-_wspawne-functions/10262748
+ */
+static void
+ensure_cmd_environment (void)
+{
+  static gsize initialization_value = 0;
+
+  if (g_once_init_enter (&initialization_value))
+    {
+      wchar_t *block = GetEnvironmentStringsW ();
+      gboolean have_cmd_environment = FALSE;
+
+      if (block)
+        {
+          const wchar_t *p = block;
+
+          while (*p != L'\0')
+            {
+              if (*p == L'=')
+                {
+                  have_cmd_environment = TRUE;
+                  break;
+                }
+
+              p += wcslen (p) + 1;
+            }
+
+          if (!FreeEnvironmentStringsW (block))
+            g_warning ("%s failed with error code %u",
+                       "FreeEnvironmentStrings",
+                       (guint) GetLastError ());
+        }
+
+      if (!have_cmd_environment)
+        {
+          if (!SetEnvironmentVariableW (L"=GLIB", L"GLIB"))
+            {
+              g_critical ("%s failed with error code %u",
+                          "SetEnvironmentVariable",
+                          (guint) GetLastError ());
+            }
+        }
+
+      g_once_init_leave (&initialization_value, 1);
+    }
+}
+
+static intptr_t
+safe_wspawnve (int                   _mode,
+               const wchar_t *       _filename,
+               const wchar_t *const *_args,
+               const wchar_t *const *_env)
+{
+  ensure_cmd_environment ();
+
+  return _wspawnve (_mode, _filename, _args, _env);;
+}
+
+static intptr_t
+safe_wspawnvpe (int                   _mode,
+                const wchar_t *       _filename,
+                const wchar_t *const *_args,
+                const wchar_t *const *_env)
+{
+  ensure_cmd_environment ();
+
+  return _wspawnvpe (_mode, _filename, _args, _env);
+}
 
 #endif /* _UCRT */
 
@@ -686,13 +765,6 @@ fork_exec (gint                  *exit_status,
 
   argc = protect_argv (argv, &protected_argv);
 
-  /*
-   * FIXME: Workaround broken spawnvpe functions that SEGV when "=X:="
-   * environment variables are missing. Calling chdir() will set the magic
-   * environment variable again.
-   */
-  _chdir (".");
-
   if (stdin_fd == -1 && stdout_fd == -1 && stderr_fd == -1 &&
       (flags & G_SPAWN_CHILD_INHERITS_STDIN) &&
       !(flags & G_SPAWN_STDOUT_TO_DEV_NULL) &&

glib/tests/spawn-test.c

@@ -59,7 +59,136 @@ get_system_directory (void)
 
   return path;
 }
-#endif
+
+static wchar_t *
+g_wcsdup (const wchar_t *wcs_string)
+{
+  size_t length = wcslen (wcs_string);
+
+  return g_memdup2 (wcs_string, (length + 1) * sizeof (wchar_t));
+}
+
+static wchar_t *
+g_wcsndup (const wchar_t *wcs_string,
+           size_t         length)
+{
+  wchar_t *result = NULL;
+
+  g_assert_true (length < SIZE_MAX);
+
+  result = g_new (wchar_t, length + 1);
+  memcpy (result, wcs_string, length * sizeof (wchar_t));
+  result[length] = L'\0';
+
+  return result;
+}
+
+/**
+ * parse_environment_string:
+ *
+ * @string: source environment string in the form <VARIABLE>=<VALUE>
+ *          (e.g as returned by GetEnvironmentStrings)
+ * @name: (out) (optional) (utf-16) name of the variable
+ * @value: (out) (optional) (utf-16) value of the variable
+ *
+ * Parse environment string in the form <VARIABLE>=<VALUE>, for example
+ * the strings in the environment block returned by GetEnvironmentStrings.
+ *
+ * Returns: %TRUE on success
+ */
+static gboolean
+parse_environment_string (const wchar_t  *string,
+                          wchar_t       **name,
+                          wchar_t       **value)
+{
+  const wchar_t *equal_sign;
+
+  g_assert_nonnull (string);
+  g_assert_true (name || value);
+
+  /* On Windows environment variables may have an equal-sign
+   * character as part of their name, but only as the first
+   * character */
+  equal_sign = wcschr (string[0] == L'=' ? (string + 1) : string, L'=');
+
+  if (name)
+    *name = equal_sign ? g_wcsndup (string, equal_sign - string) : NULL;
+
+  if (value)
+    *value = equal_sign ? g_wcsdup (equal_sign + 1) : NULL;
+
+  return (equal_sign != NULL);
+}
+
+/**
+ * find_cmd_shell_environment_variables:
+ *
+ * Finds all the environment variables related to cmd.exe, which are
+ * usually (but not always) present in a process environment block.
+ * Those environment variables are named "=X:", where X is a drive /
+ * volume letter and are used by cmd.exe to track per-drive current
+ * directories.
+ *
+ * See "What are these strange =C: environment variables?"
+ * https://devblogs.microsoft.com/oldnewthing/20100506-00/?p=14133
+ *
+ * This is used to test a work around for an UCRT issue
+ * https://developercommunity.visualstudio.com/t/UCRT-Crash-in-_wspawne-functions/10262748
+ */
+static GList *
+find_cmd_shell_environment_variables (void)
+{
+  wchar_t *block = NULL;
+  wchar_t *iter = NULL;
+  GList *variables = NULL;
+  size_t len = 0;
+
+  block = GetEnvironmentStringsW ();
+  if (!block)
+    {
+      DWORD code = GetLastError ();
+      g_error ("%s failed with error code %u",
+               "GetEnvironmentStrings", (unsigned int) code);
+    }
+
+  iter = block;
+
+  while ((len = wcslen (iter)))
+    {
+      if (iter[0] == L'=')
+        {
+          wchar_t *variable = NULL;
+
+          g_assert_true (parse_environment_string (iter, &variable, NULL));
+          g_assert_nonnull (variable);
+
+          variables = g_list_prepend (variables, variable);
+        }
+
+      iter += len + 1;
+    }
+
+  FreeEnvironmentStringsW (block);
+
+  return variables;
+}
+
+static void
+remove_environment_variables (GList *list)
+{
+  for (GList *l = list; l; l = l->next)
+    {
+      const wchar_t *variable = (const wchar_t*) l->data;
+
+      if (!SetEnvironmentVariableW (variable, NULL))
+        {
+          DWORD code = GetLastError ();
+          g_error ("%s failed with error code %u",
+                   "SetEnvironmentVariable", (unsigned int) code);
+        }
+    }
+}
+#endif /* G_OS_WIN32 */
 
 static void
 test_spawn_basics (void)
@@ -73,9 +202,11 @@ test_spawn_basics (void)
   char buf[100];
   int pipedown[2], pipeup[2];
   gchar **argv = NULL;
+  gchar **envp = g_get_environ ();
   gchar *system_directory;
   gchar spawn_binary[1000] = {0};
   gchar full_cmdline[1000] = {0};
+  GList *cmd_shell_env_vars = NULL;
   const LCID old_lcid = GetThreadUILanguage ();
   const unsigned int initial_cp = GetConsoleOutputCP ();
 
@@ -253,11 +384,29 @@ test_spawn_basics (void)
 
   buf[n] = '\0';
   g_assert_cmpstr (buf, ==, "See ya");
+
+  /* Test workaround for:
+   *
+   * https://developercommunity.visualstudio.com/t/UCRT-Crash-in-_wspawne-functions/10262748
+   */
+  cmd_shell_env_vars = find_cmd_shell_environment_variables ();
+  remove_environment_variables (cmd_shell_env_vars);
+
+  g_snprintf (full_cmdline, sizeof (full_cmdline),
+              "'%s\\sort.exe' non-existing-file.txt", system_directory);
+  g_assert_true (g_shell_parse_argv (full_cmdline, NULL, &argv, NULL));
+  g_assert_nonnull (argv);
+  g_spawn_sync (NULL, argv, envp, G_SPAWN_DEFAULT,
+                NULL, NULL, NULL, NULL, NULL, NULL);
+  g_free (argv);
+  argv = NULL;
 #endif
 
 #ifdef G_OS_WIN32
   SetThreadUILanguage (old_lcid);
   SetConsoleOutputCP (initial_cp); /* 437 means en-US codepage */
+  g_list_free_full (cmd_shell_env_vars, g_free);
+  g_strfreev (envp);
   g_free (system_directory);
 #endif
 }