Merge branch 'fix-getenv-win32' into 'main'

Rework Windows implementation of g_getenv()

See merge request GNOME/glib!4637

glib/genviron.c

@@ -448,81 +448,114 @@ g_get_environ (void)
 }
 
 /* Win32 implementation {{{1 */
-#else   /* G_OS_WIN32 */
+
+#else /* G_OS_WIN32 */
+
+static wchar_t *
+expand_environment_string (const wchar_t *string_utf16)
+{
+  wchar_t *expanded = NULL;
+  DWORD previous_wchars_count = 0;
+  DWORD wchars_count = 0;
+
+  do
+    {
+      previous_wchars_count = wchars_count;
+      expanded = g_renew (wchar_t, expanded, wchars_count);
+
+      /* Note: ExpandEnvironmentStrings is 1-pass only. In addition, placeholders
+       * referring to non-existing variables are kept as-is, they aren't removed.
+       * That differs e.g from CMD. */
+      wchars_count = ExpandEnvironmentStrings (string_utf16, expanded, wchars_count);
+    }
+  while (wchars_count > previous_wchars_count);
+
+  if (wchars_count == 0)
+    {
+      g_warning ("%s failed with error code %u",
+                 "ExpandEnvironmentStrings", (unsigned int) GetLastError ());
+      g_clear_pointer (&expanded, g_free);
+    }
+
+  return expanded;
+}
 
 const gchar *
 g_getenv (const gchar *variable)
 {
-  GQuark quark;
+  wchar_t *name_utf16 = NULL;
-  gchar *value;
+  wchar_t *value_utf16 = NULL;
-  wchar_t dummy[2], *wname, *wvalue;
+  DWORD previous_wchars_count = 0;
-  DWORD len;
+  DWORD wchars_count = 0;
+  char *value_utf8 = NULL;
+  GQuark quark = 0;
 
   g_return_val_if_fail (variable != NULL, NULL);
   g_return_val_if_fail (g_utf8_validate (variable, -1, NULL), NULL);
 
-  /* On Windows NT, it is relatively typical that environment
+  name_utf16 = g_utf8_to_utf16 (variable, -1, NULL, NULL, NULL);
-   * variables contain references to other environment variables. If
+  g_assert (name_utf16);
-   * so, use ExpandEnvironmentStrings(). (In an ideal world, such
-   * environment variables would be stored in the Registry as
-   * REG_EXPAND_SZ type values, and would then get automatically
-   * expanded before a program sees them. But there is broken software
-   * that stores environment variables as REG_SZ values even if they
-   * contain references to other environment variables.)
-   */
 
-  wname = g_utf8_to_utf16 (variable, -1, NULL, NULL, NULL);
+  do
-
-  len = GetEnvironmentVariableW (wname, dummy, 2);
-
-  if (len == 0)
     {
-      g_free (wname);
+      value_utf16 = g_renew (wchar_t, value_utf16, wchars_count);
-      if (GetLastError () == ERROR_ENVVAR_NOT_FOUND)
-        return NULL;
 
-      quark = g_quark_from_static_string ("");
+      previous_wchars_count = wchars_count;
-      return g_quark_to_string (quark);
+
+      SetLastError (ERROR_SUCCESS);
+      wchars_count = GetEnvironmentVariable (name_utf16, value_utf16, wchars_count);
     }
-  else if (len == 1)
+  while (wchars_count > previous_wchars_count);
-    len = 2;
 
-  wvalue = g_new (wchar_t, len);
+  if (wchars_count == 0)
-
-  if (GetEnvironmentVariableW (wname, wvalue, len) != len - 1)
     {
-      g_free (wname);
+      DWORD code = GetLastError ();
-      g_free (wvalue);
-      return NULL;
-    }
 
-  if (wcschr (wvalue, L'%') != NULL)
+      if (code == ERROR_SUCCESS)
-    {
-      wchar_t *tem = wvalue;
-
-      len = ExpandEnvironmentStringsW (wvalue, dummy, 2);
-
-      if (len > 0)
         {
-          wvalue = g_new (wchar_t, len);
+          /* Value is an empty string */
+          quark = g_quark_from_static_string ("");
+        }
+      else if (code == ERROR_ENVVAR_NOT_FOUND)
+        {
+          /* The variable doesn't exist */
+        }
+      else
+        {
+          g_warning ("%s failed with error code %u",
+                     "GetEnvironmentVariable", (unsigned int) GetLastError ());
+        }
+    }
+  else
+    {
+      g_assert (wchars_count != previous_wchars_count);
+      g_assert (value_utf16[wchars_count] == L'\0');
 
-          if (ExpandEnvironmentStringsW (tem, wvalue, len) != len)
+      /* On Windows NT, it is relatively typical that environment
-            {
+       * variables contain references to other environment variables.
-              g_free (wvalue);
+       * If so, use ExpandEnvironmentStrings() */
-              wvalue = tem;
+      if (wcschr (value_utf16, L'%'))
-            }
+        {
+          wchar_t *expanded = expand_environment_string (value_utf16);
+
+          g_free (value_utf16);
+          value_utf16 = expanded;
+        }
+
+      if (value_utf16)
+        {
+          value_utf8 = g_utf16_to_utf8 (value_utf16, -1, NULL, NULL, NULL);
+
+          if (value_utf8 == NULL)
+            g_warning ("Environment variable `%s' contains invalid UTF-16", variable);
           else
-            g_free (tem);
+            quark = g_quark_from_string (value_utf8);
         }
     }
 
-  value = g_utf16_to_utf8 (wvalue, -1, NULL, NULL, NULL);
+  g_free (name_utf16);
-
+  g_free (value_utf16);
-  g_free (wname);
+  g_free (value_utf8);
-  g_free (wvalue);
-
-  quark = g_quark_from_string (value);
-  g_free (value);
 
   return g_quark_to_string (quark);
 }

glib/tests/environment.c

@@ -19,6 +19,10 @@
 
 #include <glib.h>
 
+#ifdef G_OS_WIN32
+#include <windows.h>
+#endif
+
 static void
 test_listenv (void)
 {
@@ -148,25 +152,60 @@ test_getenv (void)
 
   g_unsetenv ("foo");
   g_assert_null (g_getenv ("foo"));
+
+  g_assert_null (g_getenv (""));
 }
 
 static void
 test_setenv (void)
 {
-  const gchar *var, *value;
+  if (g_test_subprocess ())
+    {
+      const char *var, *value;
 
-  var = "NOSUCHENVVAR";
+      var = "NOSUCHENVVAR";
-  value = "value1";
+      value = "value1";
 
-  g_assert_null (g_getenv (var));
+      g_assert_null (g_getenv (var));
-  g_setenv (var, value, FALSE);
+      g_setenv (var, value, FALSE);
-  g_assert_cmpstr (g_getenv (var), ==, value);
+      g_assert_cmpstr (g_getenv (var), ==, value);
-  g_assert_true (g_setenv (var, "value2", FALSE));
+      g_assert_true (g_setenv (var, "value2", FALSE));
-  g_assert_cmpstr (g_getenv (var), ==, value);
+      g_assert_cmpstr (g_getenv (var), ==, value);
-  g_assert_true (g_setenv (var, "value2", TRUE));
+      g_assert_true (g_setenv (var, "value2", TRUE));
-  g_assert_cmpstr (g_getenv (var), ==, "value2");
+      g_assert_cmpstr (g_getenv (var), ==, "value2");
-  g_unsetenv (var);
+      g_unsetenv (var);
-  g_assert_null (g_getenv (var));
+      g_assert_null (g_getenv (var));
+
+      g_assert_true (g_setenv ("EMPTY_VAR", "", TRUE));
+      g_assert_cmpstr (g_getenv ("EMPTY_VAR"), ==, "");
+
+      g_assert_false (g_setenv ("", "value", TRUE));
+
+#ifdef G_OS_WIN32
+      {
+        wchar_t invalid_utf16[] = {
+          L't', L'e', L's', L't',
+          LOW_SURROGATE_START,
+          HIGH_SURROGATE_START,
+          L'\0'
+        };
+        if (!SetEnvironmentVariable (L"INVALID_UTF16_VAR", invalid_utf16))
+          {
+            g_error ("%s failed with error code %u",
+                     "SetEnvironmentVariable", (unsigned int) GetLastError ());
+          }
+      }
+
+      g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_WARNING, "*invalid UTF-16*");
+      g_assert_null (g_getenv ("INVALID_UTF16_VAR"));
+      g_test_assert_expected_messages ();
+#endif
+
+      return;
+    }
+
+  g_test_trap_subprocess (NULL, 0, G_TEST_SUBPROCESS_DEFAULT);
+  g_test_trap_assert_passed ();
 }
 
 static void
@@ -309,6 +348,52 @@ test_environ_case (void)
   g_strfreev (env);
 }
 
+static void
+test_expansion (void)
+{
+  g_test_summary ("Test expansion of environment variable references.");
+
+#ifdef G_OS_WIN32
+  if (g_test_subprocess ())
+    {
+      const struct
+      {
+        const wchar_t *name;
+        const wchar_t *value;
+      } tests[] = {
+        { L"EMPTY_VAR", L"" },
+        { L"NON_EXISTING_VAR", NULL },
+        { L"HELLO_VAR", L"HELLO" },
+        { L"TO_EXPAND_VAR1", L"%HELLO_VAR% WORLD" },
+        { L"TO_EXPAND_VAR2", L"%EMPTY_VAR%" },
+        { L"TO_EXPAND_VAR3", L"%NON_EXISTING_VAR%" },
+        { L"VAR1", L"%VAR2%" },
+        { L"VAR2", L"%VAR1%" },
+      };
+
+      for (size_t i = 0; i < G_N_ELEMENTS (tests); i++)
+        if (!SetEnvironmentVariable (tests[i].name, tests[i].value))
+          {
+            g_error ("%s failed with error code %u",
+                     "SetEnvironmentVariable", (unsigned int) GetLastError ());
+          }
+
+      g_assert_cmpstr (g_getenv ("TO_EXPAND_VAR1"), ==, "HELLO WORLD");
+      g_assert_cmpstr (g_getenv ("TO_EXPAND_VAR2"), ==, "");
+      g_assert_cmpstr (g_getenv ("TO_EXPAND_VAR3"), ==, "%NON_EXISTING_VAR%");
+
+      g_assert_nonnull (g_getenv ("VAR1"));
+
+      return;
+    }
+
+  g_test_trap_subprocess (NULL, 0, G_TEST_SUBPROCESS_DEFAULT);
+  g_test_trap_assert_passed ();
+#else
+  g_test_skip ("Environment variable expansion is only supported on Windows");
+#endif
+}
+
 int
 main (int argc, char **argv)
 {
@@ -320,6 +405,7 @@ main (int argc, char **argv)
   g_test_add_func ("/environ/array", test_environ_array);
   g_test_add_func ("/environ/null", test_environ_null);
   g_test_add_func ("/environ/case", test_environ_case);
+  g_test_add_func ("/environ/expansion", test_expansion);
 
   return g_test_run ();
 }