Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 7543e77

Browse filesBrowse files
joyeecheungmarco-ippolito
joyeecheung
authored and
marco-ippolito
committed
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 <yagiz.nizipli@sentry.io> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 2cdfe35 commit 7543e77
Copy full SHA for 7543e77

File tree

Expand file treeCollapse file tree

8 files changed

+19
-62
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+19
-62
lines changed
Open diff view settings
Collapse file

‎src/debug_utils.cc‎

Copy file name to clipboardExpand all lines: src/debug_utils.cc
+2-3Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,9 @@ EnabledDebugList enabled_debug_list;
6262
using v8::Local;
6363
using v8::StackTrace;
6464

65-
void EnabledDebugList::Parse(std::shared_ptr<KVStore> env_vars,
66-
v8::Isolate* isolate) {
65+
void EnabledDebugList::Parse(std::shared_ptr<KVStore> env_vars) {
6766
std::string cats;
68-
credentials::SafeGetenv("NODE_DEBUG_NATIVE", &cats, env_vars, isolate);
67+
credentials::SafeGetenv("NODE_DEBUG_NATIVE", &cats, env_vars);
6968
Parse(cats);
7069
}
7170

Collapse file

‎src/debug_utils.h‎

Copy file name to clipboardExpand all lines: src/debug_utils.h
+1-2Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,7 @@ class NODE_EXTERN_PRIVATE EnabledDebugList {
7575
// Uses NODE_DEBUG_NATIVE to initialize the categories. The env_vars variable
7676
// is parsed if it is not a nullptr, otherwise the system environment
7777
// variables are parsed.
78-
void Parse(std::shared_ptr<KVStore> env_vars = nullptr,
79-
v8::Isolate* isolate = nullptr);
78+
void Parse(std::shared_ptr<KVStore> env_vars);
8079

8180
private:
8281
// Enable all categories matching cats.
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,7 +848,7 @@ Environment::Environment(IsolateData* isolate_data,
848848
}
849849

850850
set_env_vars(per_process::system_environment);
851-
enabled_debug_list_.Parse(env_vars(), isolate);
851+
enabled_debug_list_.Parse(env_vars());
852852

853853
// We create new copies of the per-Environment option sets, so that it is
854854
// easier to modify them after Environment creation. The defaults are
Collapse file

‎src/inspector_profiler.cc‎

Copy file name to clipboardExpand all lines: src/inspector_profiler.cc
+3-6Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -418,12 +418,9 @@ void StartProfilers(Environment* env) {
418418
EndStartedProfilers(static_cast<Environment*>(env));
419419
}, env);
420420

421-
Isolate* isolate = env->isolate();
422-
Local<String> coverage_str = env->env_vars()->Get(
423-
isolate, FIXED_ONE_BYTE_STRING(isolate, "NODE_V8_COVERAGE"))
424-
.FromMaybe(Local<String>());
425-
if ((!coverage_str.IsEmpty() && coverage_str->Length() > 0) ||
426-
env->options()->test_runner_coverage) {
421+
std::string coverage_str =
422+
env->env_vars()->Get("NODE_V8_COVERAGE").FromMaybe(std::string());
423+
if (!coverage_str.empty() || env->options()->test_runner_coverage) {
427424
CHECK_NULL(env->coverage_connection());
428425
env->set_coverage_connection(std::make_unique<V8CoverageConnection>(env));
429426
env->coverage_connection()->Start();
Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1030,7 +1030,7 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
10301030
if (!(flags & ProcessInitializationFlags::kNoParseGlobalDebugVariables)) {
10311031
// Initialized the enabled list for Debug() calls with system
10321032
// environment variables.
1033-
per_process::enabled_debug_list.Parse();
1033+
per_process::enabled_debug_list.Parse(per_process::system_environment);
10341034
}
10351035

10361036
PlatformInit(flags);
Collapse file

‎src/node_credentials.cc‎

Copy file name to clipboardExpand all lines: src/node_credentials.cc
+8-41Lines changed: 8 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,10 @@ namespace node {
2424
using v8::Array;
2525
using v8::Context;
2626
using v8::FunctionCallbackInfo;
27-
using v8::HandleScope;
2827
using v8::Isolate;
2928
using v8::Local;
3029
using v8::MaybeLocal;
3130
using v8::Object;
32-
using v8::String;
33-
using v8::TryCatch;
3431
using v8::Uint32;
3532
using v8::Value;
3633

@@ -77,54 +74,24 @@ static bool HasOnly(int capability) {
7774
// setuid root then lookup will not be allowed.
7875
bool SafeGetenv(const char* key,
7976
std::string* text,
80-
std::shared_ptr<KVStore> env_vars,
81-
v8::Isolate* isolate) {
77+
std::shared_ptr<KVStore> env_vars) {
8278
#if !defined(__CloudABI__) && !defined(_WIN32)
8379
#if defined(__linux__)
8480
if ((!HasOnly(CAP_NET_BIND_SERVICE) && linux_at_secure()) ||
8581
getuid() != geteuid() || getgid() != getegid())
8682
#else
8783
if (linux_at_secure() || getuid() != geteuid() || getgid() != getegid())
8884
#endif
89-
goto fail;
85+
return false;
9086
#endif
9187

92-
if (env_vars != nullptr) {
93-
DCHECK_NOT_NULL(isolate);
94-
HandleScope handle_scope(isolate);
95-
TryCatch ignore_errors(isolate);
96-
MaybeLocal<String> maybe_value = env_vars->Get(
97-
isolate, String::NewFromUtf8(isolate, key).ToLocalChecked());
98-
Local<String> value;
99-
if (!maybe_value.ToLocal(&value)) goto fail;
100-
String::Utf8Value utf8_value(isolate, value);
101-
if (*utf8_value == nullptr) goto fail;
102-
*text = std::string(*utf8_value, utf8_value.length());
103-
return true;
104-
}
105-
106-
{
107-
Mutex::ScopedLock lock(per_process::env_var_mutex);
108-
109-
size_t init_sz = 256;
110-
MaybeStackBuffer<char, 256> val;
111-
int ret = uv_os_getenv(key, *val, &init_sz);
112-
113-
if (ret == UV_ENOBUFS) {
114-
// Buffer is not large enough, reallocate to the updated init_sz
115-
// and fetch env value again.
116-
val.AllocateSufficientStorage(init_sz);
117-
ret = uv_os_getenv(key, *val, &init_sz);
118-
}
119-
120-
if (ret == 0) { // Env key value fetch success.
121-
*text = *val;
122-
return true;
123-
}
88+
// Fallback to system environment which reads the real environment variable
89+
// through uv_os_getenv.
90+
if (env_vars == nullptr) {
91+
env_vars = per_process::system_environment;
12492
}
12593

126-
fail:
127-
return false;
94+
return env_vars->Get(key).To(text);
12895
}
12996

13097
static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
@@ -133,7 +100,7 @@ static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
133100
Isolate* isolate = env->isolate();
134101
Utf8Value strenvtag(isolate, args[0]);
135102
std::string text;
136-
if (!SafeGetenv(*strenvtag, &text, env->env_vars(), isolate)) return;
103+
if (!SafeGetenv(*strenvtag, &text, env->env_vars())) return;
137104
Local<Value> result =
138105
ToV8Value(isolate->GetCurrentContext(), text).ToLocalChecked();
139106
args.GetReturnValue().Set(result);
Collapse file

‎src/node_internals.h‎

Copy file name to clipboardExpand all lines: src/node_internals.h
+1-2Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -308,8 +308,7 @@ class ThreadPoolWork {
308308
namespace credentials {
309309
bool SafeGetenv(const char* key,
310310
std::string* text,
311-
std::shared_ptr<KVStore> env_vars = nullptr,
312-
v8::Isolate* isolate = nullptr);
311+
std::shared_ptr<KVStore> env_vars = nullptr);
313312
} // namespace credentials
314313

315314
void DefineZlibConstants(v8::Local<v8::Object> target);
Collapse file

‎src/node_worker.cc‎

Copy file name to clipboardExpand all lines: src/node_worker.cc
+2-6Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ using v8::Isolate;
3232
using v8::Local;
3333
using v8::Locker;
3434
using v8::Maybe;
35-
using v8::MaybeLocal;
3635
using v8::Null;
3736
using v8::Number;
3837
using v8::Object;
@@ -537,11 +536,8 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
537536
});
538537

539538
#ifndef NODE_WITHOUT_NODE_OPTIONS
540-
MaybeLocal<String> maybe_node_opts =
541-
env_vars->Get(isolate, OneByteString(isolate, "NODE_OPTIONS"));
542-
Local<String> node_opts;
543-
if (maybe_node_opts.ToLocal(&node_opts)) {
544-
std::string node_options(*String::Utf8Value(isolate, node_opts));
539+
std::string node_options;
540+
if (env_vars->Get("NODE_OPTIONS").To(&node_options)) {
545541
std::vector<std::string> errors{};
546542
std::vector<std::string> env_argv =
547543
ParseNodeOptionsEnvVar(node_options, &errors);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.