Include important Windows environmental variables even when a blank/custom environment is specified.

src/win/process.c

@@ -28,6 +28,16 @@
 #include <stdlib.h>
 #include <windows.h>
 
+typedef struct env_var {
+  const char* narrow;
+  const wchar_t* wide;
+  int len; /* including null or '=' */
+  int supplied;
+  int value_len;
+} env_var_t;
+
+#define E_V(str) { ##str"=", L##str, sizeof(##str), 0, 0 }
+
 #define UTF8_TO_UTF16(s, t)                               \
   size = uv_utf8_to_utf16(s, NULL, 0) * sizeof(wchar_t);  \
   t = (wchar_t*)malloc(size);                             \
@@ -361,6 +371,7 @@ static wchar_t* search_path(const wchar_t *file,
   return result;
 }
 
+
 /*
  * Quotes command line arguments
  * Returns a pointer to the end (next char to be written) of the buffer
@@ -437,6 +448,7 @@ wchar_t* quote_cmd_arg(const wchar_t *source, wchar_t *target) {
   return target;
 }
 
+
 wchar_t* make_program_args(char** args, int verbatim_arguments) {
   wchar_t* dst;
   wchar_t* ptr;
@@ -494,23 +506,69 @@ error:
   return NULL;
 }
 
+
 /*
-  * The way windows takes environment variables is different than what C does;
+ * If we learn that people are passing in huge environment blocks
-  * Windows wants a contiguous block of null-terminated strings, terminated
+ * then we should probably qsort() the array and then bsearch()
-  * with an additional null.
+ * to see if it contains this variable. But there are ownership
-  */
+ * issues associated with that solution; this is the caller's 
+ * char**, and modifying it is rude.
+ */
+static void check_required_vars_contains_var(env_var_t* required, int size, const char* var) {
+  int i;
+  for (i = 0; i < size; ++i) {
+    if (_strnicmp(required[i].narrow, var, required[i].len) == 0) {
+      required[i].supplied =  1;
+      return;
+    }
+  }
+}
+
+
+/*
+ * The way windows takes environment variables is different than what C does;
+ * Windows wants a contiguous block of null-terminated strings, terminated
+ * with an additional null.
+ * 
+ * Windows has a few "essential" environment variables. winsock will fail
+ * to initialize if SYSTEMROOT is not defined; some APIs make reference to
+ * TEMP. SYSTEMDRIVE is probably also important. We therefore ensure that
+ * these get defined if the input environment block does not contain any
+ * values for them.
+ */
 wchar_t* make_program_env(char** env_block) {
   wchar_t* dst;
   wchar_t* ptr;
   char** env;
   int env_len = 1 * sizeof(wchar_t); /* room for closing null */
   int len;
+  int i;
+  DWORD var_size;
+
+  env_var_t required_vars[] = {
+    E_V("SYSTEMROOT"),
+    E_V("SYSTEMDRIVE"),
+    E_V("TEMP"),
+  };
 
   for (env = env_block; *env; env++) {
+    check_required_vars_contains_var(required_vars, COUNTOF(required_vars), *env);
     env_len += (uv_utf8_to_utf16(*env, NULL, 0) * sizeof(wchar_t));
   }
 
-  dst = (wchar_t*)malloc(env_len);
+  for (i = 0; i < COUNTOF(required_vars); ++i) {
+    if (!required_vars[i].supplied) {
+      env_len += required_vars[i].len * sizeof(wchar_t);
+      var_size = GetEnvironmentVariableW(required_vars[i].wide, NULL, 0);
+      if (var_size == 0) {
+        uv_fatal_error(GetLastError(), "GetEnvironmentVariableW");
+      }
+      required_vars[i].value_len = (int)var_size;
+      env_len += (int)var_size * sizeof(wchar_t);
+    }
+  }
+
+  dst = malloc(env_len);
   if (!dst) {
     uv_fatal_error(ERROR_OUTOFMEMORY, "malloc");
   }
@@ -525,6 +583,19 @@ wchar_t* make_program_env(char** env_block) {
     }
   }
 
+  for (i = 0; i < COUNTOF(required_vars); ++i) {
+    if (!required_vars[i].supplied) {
+      wcscpy(ptr, required_vars[i].wide);
+      ptr += required_vars[i].len - 1;
+      *ptr++ = L'=';
+      var_size = GetEnvironmentVariableW(required_vars[i].wide, ptr, required_vars[i].value_len);
+      if (var_size == 0) {
+        uv_fatal_error(GetLastError(), "GetEnvironmentVariableW");
+      }
+      ptr += required_vars[i].value_len;
+    }
+  }
+
   *ptr = L'\0';
   return dst;
 }

test/test-list.h

@@ -70,6 +70,7 @@ TEST_DECLARE   (spawn_and_kill)
 #ifdef _WIN32
 TEST_DECLARE   (spawn_detect_pipe_name_collisions_on_windows)
 TEST_DECLARE   (argument_escaping)
+TEST_DECLARE   (environment_creation)
 #endif
 HELPER_DECLARE (tcp4_echo_server)
 HELPER_DECLARE (tcp6_echo_server)
@@ -155,6 +156,7 @@ TASK_LIST_START
 #ifdef _WIN32
   TEST_ENTRY  (spawn_detect_pipe_name_collisions_on_windows)
   TEST_ENTRY  (argument_escaping)
+  TEST_ENTRY  (environment_creation)
 #endif
 
 #if 0

test/test-spawn.c

@@ -340,6 +340,53 @@ TEST_IMPL(argument_escaping) {
   ASSERT(wcscmp(verbatim_output, L"cmd.exe /c c:\\path\\to\\node.exe --eval \"require('c:\\\\path\\\\to\\\\test.js')\"") == 0);
   ASSERT(wcscmp(non_verbatim_output, L"cmd.exe /c \"c:\\path\\to\\node.exe --eval \\\"require('c:\\\\path\\\\to\\\\test.js')\\\"\"") == 0);
 
+  free(verbatim_output);
+  free(non_verbatim_output);
+
   return 0;
 }
-#endif
+
+wchar_t* make_program_env(char** env_block);
+
+TEST_IMPL(environment_creation) {
+  int i;
+  char* environment[] = {
+    "FOO=BAR",
+    "SYSTEM=ROOT", /* substring of a supplied var name */
+    "SYSTEMROOTED=OMG", /* supplied var name is a substring */
+    "TEMP=C:\\Temp",
+    "BAZ=QUX",
+    NULL
+  };
+
+  wchar_t expected[512];
+  wchar_t* ptr = expected;
+  wchar_t* result;
+  wchar_t* str;
+
+  for (i = 0; i < sizeof(environment) / sizeof(environment[0]) - 1; i++) {
+    ptr += uv_utf8_to_utf16(environment[i], ptr, expected + sizeof(expected) - ptr);
+  }
+
+  memcpy(ptr, L"SYSTEMROOT=", sizeof(L"SYSTEMROOT="));
+  ptr += sizeof(L"SYSTEMROOT=")/sizeof(wchar_t) - 1;
+  ptr += GetEnvironmentVariableW(L"SYSTEMROOT", ptr, expected + sizeof(expected) - ptr);
+  ++ptr;
+
+  memcpy(ptr, L"SYSTEMDRIVE=", sizeof(L"SYSTEMDRIVE="));
+  ptr += sizeof(L"SYSTEMDRIVE=")/sizeof(wchar_t) - 1;
+  ptr += GetEnvironmentVariableW(L"SYSTEMDRIVE", ptr, expected + sizeof(expected) - ptr);
+  ++ptr;
+  *ptr = '\0';
+  
+  result = make_program_env(environment);
+
+  for (str = result; *str; str += wcslen(str) + 1) {
+    wprintf(L"%s\n", str);
+  }
+
+  ASSERT(wcscmp(expected, result) == 0);
+ 
+  return 0;
+}
+#endif