process: move environment variable proxy code into node_env_var.cc

Instead of exposing all the NamedPropertyHandlerConfiguration()
parameters in node_internals, simply expose a CreateEnvVarProxy()
method that returns a Local<Value> that implements process.env,
and mark all the property handlers static in node_env_var.cc.
This makes the code more encapsulated.

PR-URL: https://github.com/nodejs/node/pull/25067
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

node.gyp

@@ -352,6 +352,7 @@
         'src/node_contextify.cc',
         'src/node_domain.cc',
         'src/node_encoding.cc',
+        'src/node_env_var.cc',
         'src/node_errors.cc',
         'src/node_file.cc',
         'src/node_http_parser_llhttp.cc',

src/node.cc

@@ -132,7 +132,6 @@ using v8::Maybe;
 using v8::MaybeLocal;
 using v8::Message;
 using v8::MicrotasksPolicy;
-using v8::NamedPropertyHandlerConfiguration;
 using v8::NewStringType;
 using v8::None;
 using v8::Nothing;
@@ -987,21 +986,11 @@ void SetupProcessObject(Environment* env,
                exec_arguments).FromJust();
 
   // create process.env
-  Local<ObjectTemplate> process_env_template =
-      ObjectTemplate::New(env->isolate());
-  process_env_template->SetHandler(NamedPropertyHandlerConfiguration(
-          EnvGetter,
-          EnvSetter,
-          EnvQuery,
-          EnvDeleter,
-          EnvEnumerator,
-          env->as_external()));
-
-  Local<Object> process_env =
-      process_env_template->NewInstance(env->context()).ToLocalChecked();
-  process->Set(env->context(),
-               FIXED_ONE_BYTE_STRING(env->isolate(), "env"),
-               process_env).FromJust();
+  process
+      ->Set(env->context(),
+            FIXED_ONE_BYTE_STRING(env->isolate(), "env"),
+            CreateEnvVarProxy(context, isolate, env->as_external()))
+      .FromJust();
 
   READONLY_PROPERTY(process, "pid",
                     Integer::New(env->isolate(), uv_os_getpid()));

src/node_env_var.cc

@@ -0,0 +1,214 @@
+#include "node_internals.h"
+#include "node_errors.h"
+
+#ifdef __APPLE__
+#include <crt_externs.h>
+#define environ (*_NSGetEnviron())
+#elif !defined(_MSC_VER)
+extern char** environ;
+#endif
+
+namespace node {
+using v8::Array;
+using v8::Boolean;
+using v8::Context;
+using v8::EscapableHandleScope;
+using v8::Integer;
+using v8::Isolate;
+using v8::Local;
+using v8::Name;
+using v8::NamedPropertyHandlerConfiguration;
+using v8::NewStringType;
+using v8::Object;
+using v8::ObjectTemplate;
+using v8::PropertyCallbackInfo;
+using v8::String;
+using v8::Value;
+
+static void EnvGetter(Local<Name> property,
+                      const PropertyCallbackInfo<Value>& info) {
+  Isolate* isolate = info.GetIsolate();
+  if (property->IsSymbol()) {
+    return info.GetReturnValue().SetUndefined();
+  }
+  Mutex::ScopedLock lock(environ_mutex);
+#ifdef __POSIX__
+  node::Utf8Value key(isolate, property);
+  const char* val = getenv(*key);
+  if (val) {
+    return info.GetReturnValue().Set(
+        String::NewFromUtf8(isolate, val, NewStringType::kNormal)
+            .ToLocalChecked());
+  }
+#else  // _WIN32
+  node::TwoByteValue key(isolate, property);
+  WCHAR buffer[32767];  // The maximum size allowed for environment variables.
+  SetLastError(ERROR_SUCCESS);
+  DWORD result = GetEnvironmentVariableW(
+      reinterpret_cast<WCHAR*>(*key), buffer, arraysize(buffer));
+  // If result >= sizeof buffer the buffer was too small. That should never
+  // happen. If result == 0 and result != ERROR_SUCCESS the variable was not
+  // found.
+  if ((result > 0 || GetLastError() == ERROR_SUCCESS) &&
+      result < arraysize(buffer)) {
+    const uint16_t* two_byte_buffer = reinterpret_cast<const uint16_t*>(buffer);
+    v8::MaybeLocal<String> rc = String::NewFromTwoByte(
+        isolate, two_byte_buffer, NewStringType::kNormal);
+    if (rc.IsEmpty()) {
+      isolate->ThrowException(ERR_STRING_TOO_LONG(isolate));
+      return;
+    }
+    return info.GetReturnValue().Set(rc.ToLocalChecked());
+  }
+#endif
+}
+
+static void EnvSetter(Local<Name> property,
+                      Local<Value> value,
+                      const PropertyCallbackInfo<Value>& info) {
+  Environment* env = Environment::GetCurrent(info);
+  if (env->options()->pending_deprecation && env->EmitProcessEnvWarning() &&
+      !value->IsString() && !value->IsNumber() && !value->IsBoolean()) {
+    if (ProcessEmitDeprecationWarning(
+            env,
+            "Assigning any value other than a string, number, or boolean to a "
+            "process.env property is deprecated. Please make sure to convert "
+            "the "
+            "value to a string before setting process.env with it.",
+            "DEP0104")
+            .IsNothing())
+      return;
+  }
+
+  Mutex::ScopedLock lock(environ_mutex);
+#ifdef __POSIX__
+  node::Utf8Value key(info.GetIsolate(), property);
+  node::Utf8Value val(info.GetIsolate(), value);
+  setenv(*key, *val, 1);
+#else  // _WIN32
+  node::TwoByteValue key(info.GetIsolate(), property);
+  node::TwoByteValue val(info.GetIsolate(), value);
+  WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
+  // Environment variables that start with '=' are read-only.
+  if (key_ptr[0] != L'=') {
+    SetEnvironmentVariableW(key_ptr, reinterpret_cast<WCHAR*>(*val));
+  }
+#endif
+  // Whether it worked or not, always return value.
+  info.GetReturnValue().Set(value);
+}
+
+static void EnvQuery(Local<Name> property,
+                     const PropertyCallbackInfo<Integer>& info) {
+  Mutex::ScopedLock lock(environ_mutex);
+  int32_t rc = -1;  // Not found unless proven otherwise.
+  if (property->IsString()) {
+#ifdef __POSIX__
+    node::Utf8Value key(info.GetIsolate(), property);
+    if (getenv(*key)) rc = 0;
+#else  // _WIN32
+    node::TwoByteValue key(info.GetIsolate(), property);
+    WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
+    SetLastError(ERROR_SUCCESS);
+    if (GetEnvironmentVariableW(key_ptr, nullptr, 0) > 0 ||
+        GetLastError() == ERROR_SUCCESS) {
+      rc = 0;
+      if (key_ptr[0] == L'=') {
+        // Environment variables that start with '=' are hidden and read-only.
+        rc = static_cast<int32_t>(v8::ReadOnly) |
+             static_cast<int32_t>(v8::DontDelete) |
+             static_cast<int32_t>(v8::DontEnum);
+      }
+    }
+#endif
+  }
+  if (rc != -1) info.GetReturnValue().Set(rc);
+}
+
+static void EnvDeleter(Local<Name> property,
+                       const PropertyCallbackInfo<Boolean>& info) {
+  Mutex::ScopedLock lock(environ_mutex);
+  if (property->IsString()) {
+#ifdef __POSIX__
+    node::Utf8Value key(info.GetIsolate(), property);
+    unsetenv(*key);
+#else
+    node::TwoByteValue key(info.GetIsolate(), property);
+    WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
+    SetEnvironmentVariableW(key_ptr, nullptr);
+#endif
+  }
+
+  // process.env never has non-configurable properties, so always
+  // return true like the tc39 delete operator.
+  info.GetReturnValue().Set(true);
+}
+
+static void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
+  Environment* env = Environment::GetCurrent(info);
+  Isolate* isolate = env->isolate();
+
+  Mutex::ScopedLock lock(environ_mutex);
+  Local<Array> envarr;
+  int env_size = 0;
+#ifdef __POSIX__
+  while (environ[env_size]) {
+    env_size++;
+  }
+  std::vector<Local<Value>> env_v(env_size);
+
+  for (int i = 0; i < env_size; ++i) {
+    const char* var = environ[i];
+    const char* s = strchr(var, '=');
+    const int length = s ? s - var : strlen(var);
+    env_v[i] = String::NewFromUtf8(isolate, var, NewStringType::kNormal, length)
+                   .ToLocalChecked();
+  }
+#else  // _WIN32
+  std::vector<Local<Value>> env_v;
+  WCHAR* environment = GetEnvironmentStringsW();
+  if (environment == nullptr) return;  // This should not happen.
+  WCHAR* p = environment;
+  while (*p) {
+    WCHAR* s;
+    if (*p == L'=') {
+      // If the key starts with '=' it is a hidden environment variable.
+      p += wcslen(p) + 1;
+      continue;
+    } else {
+      s = wcschr(p, L'=');
+    }
+    if (!s) {
+      s = p + wcslen(p);
+    }
+    const uint16_t* two_byte_buffer = reinterpret_cast<const uint16_t*>(p);
+    const size_t two_byte_buffer_len = s - p;
+    v8::MaybeLocal<String> rc = String::NewFromTwoByte(
+        isolate, two_byte_buffer, NewStringType::kNormal, two_byte_buffer_len);
+    if (rc.IsEmpty()) {
+      isolate->ThrowException(ERR_STRING_TOO_LONG(isolate));
+      FreeEnvironmentStringsW(environment);
+      return;
+    }
+    env_v.push_back(rc.ToLocalChecked());
+    p = s + wcslen(s) + 1;
+  }
+  FreeEnvironmentStringsW(environment);
+#endif
+
+  envarr = Array::New(isolate, env_v.data(), env_v.size());
+  info.GetReturnValue().Set(envarr);
+}
+
+Local<Object> CreateEnvVarProxy(Local<Context> context,
+                                Isolate* isolate,
+                                Local<Value> data) {
+  EscapableHandleScope scope(isolate);
+  Local<ObjectTemplate> env_proxy_template = ObjectTemplate::New(isolate);
+  env_proxy_template->SetHandler(NamedPropertyHandlerConfiguration(
+      EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, data));
+  Local<Object> env_proxy =
+      env_proxy_template->NewInstance(context).ToLocalChecked();
+  return scope.Escape(env_proxy);
+}
+}  // namespace node

src/node_internals.h

@@ -128,6 +128,9 @@ void RegisterSignalHandler(int signal,
 #endif
 
 bool SafeGetenv(const char* key, std::string* text);
+v8::Local<v8::Object> CreateEnvVarProxy(v8::Local<v8::Context> context,
+                                        v8::Isolate* isolate,
+                                        v8::Local<v8::Value> data);
 
 std::string GetHumanReadableProcessName();
 void GetHumanReadableProcessName(char (*name)[1024]);
@@ -716,16 +719,6 @@ void StopProfilerIdleNotifier(const v8::FunctionCallbackInfo<v8::Value>& args);
 void Umask(const v8::FunctionCallbackInfo<v8::Value>& args);
 void Uptime(const v8::FunctionCallbackInfo<v8::Value>& args);
 
-void EnvDeleter(v8::Local<v8::Name> property,
-                const v8::PropertyCallbackInfo<v8::Boolean>& info);
-void EnvGetter(v8::Local<v8::Name> property,
-               const v8::PropertyCallbackInfo<v8::Value>& info);
-void EnvSetter(v8::Local<v8::Name> property,
-               v8::Local<v8::Value> value,
-               const v8::PropertyCallbackInfo<v8::Value>& info);
-void EnvQuery(v8::Local<v8::Name> property,
-              const v8::PropertyCallbackInfo<v8::Integer>& info);
-void EnvEnumerator(const v8::PropertyCallbackInfo<v8::Array>& info);
 void DebugPortGetter(v8::Local<v8::Name> property,
                      const v8::PropertyCallbackInfo<v8::Value>& info);
 void DebugPortSetter(v8::Local<v8::Name> property,

src/node_process.cc

@@ -32,19 +32,11 @@ typedef int mode_t;
 #include <grp.h>  // getgrnam()
 #endif
 
-#ifdef __APPLE__
-#include <crt_externs.h>
-#define environ (*_NSGetEnviron())
-#elif !defined(_MSC_VER)
-extern char **environ;
-#endif
-
 namespace node {
 
 using v8::Array;
 using v8::ArrayBuffer;
 using v8::BigUint64Array;
-using v8::Boolean;
 using v8::Context;
 using v8::Float64Array;
 using v8::Function;
@@ -605,189 +597,6 @@ void ProcessTitleSetter(Local<Name> property,
   uv_set_process_title(*title);
 }
 
-void EnvGetter(Local<Name> property,
-               const PropertyCallbackInfo<Value>& info) {
-  Isolate* isolate = info.GetIsolate();
-  if (property->IsSymbol()) {
-    return info.GetReturnValue().SetUndefined();
-  }
-  Mutex::ScopedLock lock(environ_mutex);
-#ifdef __POSIX__
-  node::Utf8Value key(isolate, property);
-  const char* val = getenv(*key);
-  if (val) {
-    return info.GetReturnValue().Set(String::NewFromUtf8(isolate, val,
-        NewStringType::kNormal).ToLocalChecked());
-  }
-#else  // _WIN32
-  node::TwoByteValue key(isolate, property);
-  WCHAR buffer[32767];  // The maximum size allowed for environment variables.
-  SetLastError(ERROR_SUCCESS);
-  DWORD result = GetEnvironmentVariableW(reinterpret_cast<WCHAR*>(*key),
-                                         buffer,
-                                         arraysize(buffer));
-  // If result >= sizeof buffer the buffer was too small. That should never
-  // happen. If result == 0 and result != ERROR_SUCCESS the variable was not
-  // found.
-  if ((result > 0 || GetLastError() == ERROR_SUCCESS) &&
-      result < arraysize(buffer)) {
-    const uint16_t* two_byte_buffer = reinterpret_cast<const uint16_t*>(buffer);
-    v8::MaybeLocal<String> rc = String::NewFromTwoByte(
-        isolate, two_byte_buffer, NewStringType::kNormal);
-    if (rc.IsEmpty()) {
-      isolate->ThrowException(ERR_STRING_TOO_LONG(isolate));
-      return;
-    }
-    return info.GetReturnValue().Set(rc.ToLocalChecked());
-  }
-#endif
-}
-
-
-void EnvSetter(Local<Name> property,
-               Local<Value> value,
-               const PropertyCallbackInfo<Value>& info) {
-  Environment* env = Environment::GetCurrent(info);
-  if (env->options()->pending_deprecation && env->EmitProcessEnvWarning() &&
-      !value->IsString() && !value->IsNumber() && !value->IsBoolean()) {
-    if (ProcessEmitDeprecationWarning(
-          env,
-          "Assigning any value other than a string, number, or boolean to a "
-          "process.env property is deprecated. Please make sure to convert the "
-          "value to a string before setting process.env with it.",
-          "DEP0104").IsNothing())
-      return;
-  }
-
-  Mutex::ScopedLock lock(environ_mutex);
-#ifdef __POSIX__
-  node::Utf8Value key(info.GetIsolate(), property);
-  node::Utf8Value val(info.GetIsolate(), value);
-  setenv(*key, *val, 1);
-#else  // _WIN32
-  node::TwoByteValue key(info.GetIsolate(), property);
-  node::TwoByteValue val(info.GetIsolate(), value);
-  WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
-  // Environment variables that start with '=' are read-only.
-  if (key_ptr[0] != L'=') {
-    SetEnvironmentVariableW(key_ptr, reinterpret_cast<WCHAR*>(*val));
-  }
-#endif
-  // Whether it worked or not, always return value.
-  info.GetReturnValue().Set(value);
-}
-
-
-void EnvQuery(Local<Name> property, const PropertyCallbackInfo<Integer>& info) {
-  Mutex::ScopedLock lock(environ_mutex);
-  int32_t rc = -1;  // Not found unless proven otherwise.
-  if (property->IsString()) {
-#ifdef __POSIX__
-    node::Utf8Value key(info.GetIsolate(), property);
-    if (getenv(*key))
-      rc = 0;
-#else  // _WIN32
-    node::TwoByteValue key(info.GetIsolate(), property);
-    WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
-    SetLastError(ERROR_SUCCESS);
-    if (GetEnvironmentVariableW(key_ptr, nullptr, 0) > 0 ||
-        GetLastError() == ERROR_SUCCESS) {
-      rc = 0;
-      if (key_ptr[0] == L'=') {
-        // Environment variables that start with '=' are hidden and read-only.
-        rc = static_cast<int32_t>(v8::ReadOnly) |
-             static_cast<int32_t>(v8::DontDelete) |
-             static_cast<int32_t>(v8::DontEnum);
-      }
-    }
-#endif
-  }
-  if (rc != -1)
-    info.GetReturnValue().Set(rc);
-}
-
-
-void EnvDeleter(Local<Name> property,
-                const PropertyCallbackInfo<Boolean>& info) {
-  Mutex::ScopedLock lock(environ_mutex);
-  if (property->IsString()) {
-#ifdef __POSIX__
-    node::Utf8Value key(info.GetIsolate(), property);
-    unsetenv(*key);
-#else
-    node::TwoByteValue key(info.GetIsolate(), property);
-    WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
-    SetEnvironmentVariableW(key_ptr, nullptr);
-#endif
-  }
-
-  // process.env never has non-configurable properties, so always
-  // return true like the tc39 delete operator.
-  info.GetReturnValue().Set(true);
-}
-
-
-void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
-  Environment* env = Environment::GetCurrent(info);
-  Isolate* isolate = env->isolate();
-
-  Mutex::ScopedLock lock(environ_mutex);
-  Local<Array> envarr;
-  int env_size = 0;
-#ifdef __POSIX__
-  while (environ[env_size]) {
-    env_size++;
-  }
-  std::vector<Local<Value>> env_v(env_size);
-
-  for (int i = 0; i < env_size; ++i) {
-    const char* var = environ[i];
-    const char* s = strchr(var, '=');
-    const int length = s ? s - var : strlen(var);
-    env_v[i] =
-        String::NewFromUtf8(isolate, var, NewStringType::kNormal, length)
-            .ToLocalChecked();
-  }
-#else  // _WIN32
-  std::vector<Local<Value>> env_v;
-  WCHAR* environment = GetEnvironmentStringsW();
-  if (environment == nullptr)
-    return;  // This should not happen.
-  WCHAR* p = environment;
-  while (*p) {
-    WCHAR* s;
-    if (*p == L'=') {
-      // If the key starts with '=' it is a hidden environment variable.
-      p += wcslen(p) + 1;
-      continue;
-    } else {
-      s = wcschr(p, L'=');
-    }
-    if (!s) {
-      s = p + wcslen(p);
-    }
-    const uint16_t* two_byte_buffer = reinterpret_cast<const uint16_t*>(p);
-    const size_t two_byte_buffer_len = s - p;
-    v8::MaybeLocal<String> rc =
-        String::NewFromTwoByte(isolate,
-                               two_byte_buffer,
-                               NewStringType::kNormal,
-                               two_byte_buffer_len);
-    if (rc.IsEmpty()) {
-      isolate->ThrowException(ERR_STRING_TOO_LONG(isolate));
-      FreeEnvironmentStringsW(environment);
-      return;
-    }
-    env_v.push_back(rc.ToLocalChecked());
-    p = s + wcslen(s) + 1;
-  }
-  FreeEnvironmentStringsW(environment);
-#endif
-
-  envarr = Array::New(isolate, env_v.data(), env_v.size());
-  info.GetReturnValue().Set(envarr);
-}
-
 void GetParentProcessId(Local<Name> property,
                         const PropertyCallbackInfo<Value>& info) {
   info.GetReturnValue().Set(uv_os_getppid());