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]>

Author: joyeecheung

src/debug_utils.cc

@@ -62,10 +62,9 @@ EnabledDebugList enabled_debug_list;
 using v8::Local;
 using v8::StackTrace;
 
-voidEnabledDebugList::Parse(std::shared_ptr<KVStore> env_vars,
- v8::Isolate* isolate){
+voidEnabledDebugList::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.
-voidParse(std::shared_ptr<KVStore> env_vars = nullptr,
- v8::Isolate* isolate = nullptr);
+voidParse(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.
 boolSafeGetenv(constchar* 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;
+returnfalse;
 #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());
-returntrue;
- }
-
-{
- 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;
-returntrue;
- }
+// 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:
-returnfalse;
+return env_vars->Get(key).To(text);
 }
 
 staticvoidSafeGetenv(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{
 namespacecredentials{
 boolSafeGetenv(constchar* key,
  std::string* text,
- std::shared_ptr<KVStore> env_vars = nullptr,
- v8::Isolate* isolate = nullptr);
+ std::shared_ptr<KVStore> env_vars = nullptr);
 } // namespace credentials
 
 voidDefineZlibConstants(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);