src: simplify direct queries of env vars in C++ land

In many cases we can query the environment variable directly instead
of encoding it into UTF8 first and then decoding it again from
UTF8 via an intermediate V8 string.

Drive-by: pass per_process::system_environment explicitly when
the real environment variables are supposed to be used instead of
relying on fallback with defaults.

PR-URL: #51829
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>

src/debug_utils.cc

@@ -62,10 +62,9 @@ EnabledDebugList enabled_debug_list;
 using v8::Local;
 using v8::StackTrace;
 
-void EnabledDebugList::Parse(std::shared_ptr<KVStore> env_vars,
-                             v8::Isolate* isolate) {
+void EnabledDebugList::Parse(std::shared_ptr<KVStore> env_vars) {
   std::string cats;
-  credentials::SafeGetenv("NODE_DEBUG_NATIVE", &cats, env_vars, isolate);
+  credentials::SafeGetenv("NODE_DEBUG_NATIVE", &cats, env_vars);
   Parse(cats);
 }
 

src/debug_utils.h

@@ -74,8 +74,7 @@ class NODE_EXTERN_PRIVATE EnabledDebugList {
   // Uses NODE_DEBUG_NATIVE to initialize the categories. The env_vars variable
   // is parsed if it is not a nullptr, otherwise the system environment
   // variables are parsed.
-  void Parse(std::shared_ptr<KVStore> env_vars = nullptr,
-             v8::Isolate* isolate = nullptr);
+  void Parse(std::shared_ptr<KVStore> env_vars);
 
  private:
   // Enable all categories matching cats.

src/env.cc

@@ -848,7 +848,7 @@ Environment::Environment(IsolateData* isolate_data,
   }
 
   set_env_vars(per_process::system_environment);
-  enabled_debug_list_.Parse(env_vars(), isolate);
+  enabled_debug_list_.Parse(env_vars());
 
   // We create new copies of the per-Environment option sets, so that it is
   // easier to modify them after Environment creation. The defaults are

src/inspector_profiler.cc

@@ -418,12 +418,9 @@ void StartProfilers(Environment* env) {
     EndStartedProfilers(static_cast<Environment*>(env));
   }, env);
 
-  Isolate* isolate = env->isolate();
-  Local<String> coverage_str = env->env_vars()->Get(
-      isolate, FIXED_ONE_BYTE_STRING(isolate, "NODE_V8_COVERAGE"))
-      .FromMaybe(Local<String>());
-  if ((!coverage_str.IsEmpty() && coverage_str->Length() > 0) ||
-      env->options()->test_runner_coverage) {
+  std::string coverage_str =
+      env->env_vars()->Get("NODE_V8_COVERAGE").FromMaybe(std::string());
+  if (!coverage_str.empty() || env->options()->test_runner_coverage) {
     CHECK_NULL(env->coverage_connection());
     env->set_coverage_connection(std::make_unique<V8CoverageConnection>(env));
     env->coverage_connection()->Start();

src/node.cc

@@ -1030,7 +1030,7 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
   if (!(flags & ProcessInitializationFlags::kNoParseGlobalDebugVariables)) {
     // Initialized the enabled list for Debug() calls with system
     // environment variables.
-    per_process::enabled_debug_list.Parse();
+    per_process::enabled_debug_list.Parse(per_process::system_environment);
   }
 
   PlatformInit(flags);

src/node_credentials.cc

@@ -24,13 +24,10 @@ namespace node {
 using v8::Array;
 using v8::Context;
 using v8::FunctionCallbackInfo;
-using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
 using v8::MaybeLocal;
 using v8::Object;
-using v8::String;
-using v8::TryCatch;
 using v8::Uint32;
 using v8::Value;
 
@@ -77,54 +74,24 @@ static bool HasOnly(int capability) {
 // setuid root then lookup will not be allowed.
 bool SafeGetenv(const char* key,
                 std::string* text,
-                std::shared_ptr<KVStore> env_vars,
-                v8::Isolate* isolate) {
+                std::shared_ptr<KVStore> env_vars) {
 #if !defined(__CloudABI__) && !defined(_WIN32)
 #if defined(__linux__)
   if ((!HasOnly(CAP_NET_BIND_SERVICE) && linux_at_secure()) ||
       getuid() != geteuid() || getgid() != getegid())
 #else
   if (linux_at_secure() || getuid() != geteuid() || getgid() != getegid())
 #endif
-    goto fail;
+    return false;
 #endif
 
-  if (env_vars != nullptr) {
-    DCHECK_NOT_NULL(isolate);
-    HandleScope handle_scope(isolate);
-    TryCatch ignore_errors(isolate);
-    MaybeLocal<String> maybe_value = env_vars->Get(
-        isolate, String::NewFromUtf8(isolate, key).ToLocalChecked());
-    Local<String> value;
-    if (!maybe_value.ToLocal(&value)) goto fail;
-    String::Utf8Value utf8_value(isolate, value);
-    if (*utf8_value == nullptr) goto fail;
-    *text = std::string(*utf8_value, utf8_value.length());
-    return true;
-  }
-
-  {
-    Mutex::ScopedLock lock(per_process::env_var_mutex);
-
-    size_t init_sz = 256;
-    MaybeStackBuffer<char, 256> val;
-    int ret = uv_os_getenv(key, *val, &init_sz);
-
-    if (ret == UV_ENOBUFS) {
-      // Buffer is not large enough, reallocate to the updated init_sz
-      // and fetch env value again.
-      val.AllocateSufficientStorage(init_sz);
-      ret = uv_os_getenv(key, *val, &init_sz);
-    }
-
-    if (ret == 0) {  // Env key value fetch success.
-      *text = *val;
-      return true;
-    }
+  // Fallback to system environment which reads the real environment variable
+  // through uv_os_getenv.
+  if (env_vars == nullptr) {
+    env_vars = per_process::system_environment;
   }
 
-fail:
-  return false;
+  return env_vars->Get(key).To(text);
 }
 
 static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
@@ -133,7 +100,7 @@ static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
   Isolate* isolate = env->isolate();
   Utf8Value strenvtag(isolate, args[0]);
   std::string text;
-  if (!SafeGetenv(*strenvtag, &text, env->env_vars(), isolate)) return;
+  if (!SafeGetenv(*strenvtag, &text, env->env_vars())) return;
   Local<Value> result =
       ToV8Value(isolate->GetCurrentContext(), text).ToLocalChecked();
   args.GetReturnValue().Set(result);

src/node_internals.h

@@ -308,8 +308,7 @@ class ThreadPoolWork {
 namespace credentials {
 bool SafeGetenv(const char* key,
                 std::string* text,
-                std::shared_ptr<KVStore> env_vars = nullptr,
-                v8::Isolate* isolate = nullptr);
+                std::shared_ptr<KVStore> env_vars = nullptr);
 }  // namespace credentials
 
 void DefineZlibConstants(v8::Local<v8::Object> target);

src/node_worker.cc

@@ -32,7 +32,6 @@ using v8::Isolate;
 using v8::Local;
 using v8::Locker;
 using v8::Maybe;
-using v8::MaybeLocal;
 using v8::Null;
 using v8::Number;
 using v8::Object;
@@ -537,11 +536,8 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
     });
 
 #ifndef NODE_WITHOUT_NODE_OPTIONS
-    MaybeLocal<String> maybe_node_opts =
-        env_vars->Get(isolate, OneByteString(isolate, "NODE_OPTIONS"));
-    Local<String> node_opts;
-    if (maybe_node_opts.ToLocal(&node_opts)) {
-      std::string node_options(*String::Utf8Value(isolate, node_opts));
+    std::string node_options;
+    if (env_vars->Get("NODE_OPTIONS").To(&node_options)) {
       std::vector<std::string> errors{};
       std::vector<std::string> env_argv =
           ParseNodeOptionsEnvVar(node_options, &errors);