From b354d12599ea9e2b29e6b26dedae82b9b474d424 Mon Sep 17 00:00:00 2001 From: Devendra Satram Date: Fri, 19 Apr 2019 18:03:18 +0530 Subject: [PATCH] src: modified RealEnvStore methods to use libuv functions Modified RealEnvStore::Get, Set, Query and Delete methods to use libuv methods environment variables operations instead of using os specific logic and switches. Fixes: https://github.com/nodejs/node/issues/27211 Refs: http://docs.libuv.org/en/v1.x/misc.html PR-URL: https://github.com/nodejs/node/pull/27310 Reviewed-By: Gus Caplan Reviewed-By: Joyee Cheung Reviewed-By: Anna Henningsen --- src/env.h | 4 +- src/inspector_profiler.cc | 3 +- src/node_env_var.cc | 124 +++++++++++++++++--------------------- 3 files changed, 58 insertions(+), 73 deletions(-) diff --git a/src/env.h b/src/env.h index 87d71bbc9fb124..74d65f4fbb7b8a 100644 --- a/src/env.h +++ b/src/env.h @@ -610,8 +610,8 @@ class KVStore { KVStore(KVStore&&) = delete; KVStore& operator=(KVStore&&) = delete; - virtual v8::Local Get(v8::Isolate* isolate, - v8::Local key) const = 0; + virtual v8::MaybeLocal Get(v8::Isolate* isolate, + v8::Local key) const = 0; virtual void Set(v8::Isolate* isolate, v8::Local key, v8::Local value) = 0; diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc index e5e01dc3c9204c..867f145ff5907d 100644 --- a/src/inspector_profiler.cc +++ b/src/inspector_profiler.cc @@ -340,7 +340,8 @@ std::string GetCwd(Environment* env) { void StartProfilers(Environment* env) { Isolate* isolate = env->isolate(); Local coverage_str = env->env_vars()->Get( - isolate, FIXED_ONE_BYTE_STRING(isolate, "NODE_V8_COVERAGE")); + isolate, FIXED_ONE_BYTE_STRING(isolate, "NODE_V8_COVERAGE")) + .FromMaybe(Local()); if (!coverage_str.IsEmpty() && coverage_str->Length() > 0) { CHECK_NULL(env->coverage_connection()); env->set_coverage_connection(std::make_unique(env)); diff --git a/src/node_env_var.cc b/src/node_env_var.cc index ef2b78d664858e..2c1af65b867b0b 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -36,7 +36,7 @@ using v8::Value; class RealEnvStore final : public KVStore { public: - Local Get(Isolate* isolate, Local key) const override; + MaybeLocal Get(Isolate* isolate, Local key) const override; void Set(Isolate* isolate, Local key, Local value) override; int32_t Query(Isolate* isolate, Local key) const override; void Delete(Isolate* isolate, Local key) override; @@ -45,7 +45,7 @@ class RealEnvStore final : public KVStore { class MapKVStore final : public KVStore { public: - Local Get(Isolate* isolate, Local key) const override; + MaybeLocal Get(Isolate* isolate, Local key) const override; void Set(Isolate* isolate, Local key, Local value) override; int32_t Query(Isolate* isolate, Local key) const override; void Delete(Isolate* isolate, Local key) override; @@ -79,93 +79,73 @@ void DateTimeConfigurationChangeNotification(Isolate* isolate, const T& key) { } } -Local RealEnvStore::Get(Isolate* isolate, - Local property) const { +MaybeLocal RealEnvStore::Get(Isolate* isolate, + Local property) const { Mutex::ScopedLock lock(per_process::env_var_mutex); -#ifdef __POSIX__ + node::Utf8Value key(isolate, property); - const char* val = getenv(*key); - if (val) { - return String::NewFromUtf8(isolate, val, NewStringType::kNormal) - .ToLocalChecked(); + size_t init_sz = 256; + MaybeStackBuffer 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); } -#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(*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(buffer); - v8::MaybeLocal rc = String::NewFromTwoByte( - isolate, two_byte_buffer, NewStringType::kNormal); - if (rc.IsEmpty()) { - isolate->ThrowException(ERR_STRING_TOO_LONG(isolate)); - return Local(); - } - return rc.ToLocalChecked(); + + if (ret >= 0) { // Env key value fetch success. + MaybeLocal value_string = + String::NewFromUtf8(isolate, *val, NewStringType::kNormal, init_sz); + return value_string; } -#endif - return Local(); + + return MaybeLocal(); } void RealEnvStore::Set(Isolate* isolate, Local property, Local value) { Mutex::ScopedLock lock(per_process::env_var_mutex); -#ifdef __POSIX__ + node::Utf8Value key(isolate, property); node::Utf8Value val(isolate, value); - setenv(*key, *val, 1); -#else // _WIN32 - node::TwoByteValue key(isolate, property); - node::TwoByteValue val(isolate, value); - WCHAR* key_ptr = reinterpret_cast(*key); - // Environment variables that start with '=' are read-only. - if (key_ptr[0] != L'=') { - SetEnvironmentVariableW(key_ptr, reinterpret_cast(*val)); - } + +#ifdef _WIN32 + if (key[0] == L'=') return; #endif + uv_os_setenv(*key, *val); DateTimeConfigurationChangeNotification(isolate, key); } int32_t RealEnvStore::Query(Isolate* isolate, Local property) const { Mutex::ScopedLock lock(per_process::env_var_mutex); -#ifdef __POSIX__ + node::Utf8Value key(isolate, property); - if (getenv(*key)) return 0; -#else // _WIN32 - node::TwoByteValue key(isolate, property); - WCHAR* key_ptr = reinterpret_cast(*key); - SetLastError(ERROR_SUCCESS); - if (GetEnvironmentVariableW(key_ptr, nullptr, 0) > 0 || - GetLastError() == ERROR_SUCCESS) { - if (key_ptr[0] == L'=') { - // Environment variables that start with '=' are hidden and read-only. - return static_cast(v8::ReadOnly) | - static_cast(v8::DontDelete) | - static_cast(v8::DontEnum); - } - return 0; - } +#ifdef _WIN32 + if (key[0] == L'=') + return static_cast(v8::ReadOnly) | + static_cast(v8::DontDelete) | + static_cast(v8::DontEnum); #endif - return -1; + + char val[2]; + size_t init_sz = sizeof(val); + int ret = uv_os_getenv(*key, val, &init_sz); + + if (ret == UV_ENOENT) { + return -1; + } + + return 0; } void RealEnvStore::Delete(Isolate* isolate, Local property) { Mutex::ScopedLock lock(per_process::env_var_mutex); -#ifdef __POSIX__ + node::Utf8Value key(isolate, property); - unsetenv(*key); -#else - node::TwoByteValue key(isolate, property); - WCHAR* key_ptr = reinterpret_cast(*key); - SetEnvironmentVariableW(key_ptr, nullptr); -#endif + uv_os_unsetenv(*key); DateTimeConfigurationChangeNotification(isolate, key); } @@ -231,19 +211,20 @@ std::shared_ptr KVStore::Clone(v8::Isolate* isolate) const { for (uint32_t i = 0; i < keys_length; i++) { Local key = keys->Get(context, i).ToLocalChecked(); CHECK(key->IsString()); - copy->Set(isolate, key.As(), Get(isolate, key.As())); + copy->Set(isolate, + key.As(), + Get(isolate, key.As()).ToLocalChecked()); } return copy; } -Local MapKVStore::Get(Isolate* isolate, Local key) const { +MaybeLocal MapKVStore::Get(Isolate* isolate, Local key) const { Mutex::ScopedLock lock(mutex_); Utf8Value str(isolate, key); auto it = map_.find(std::string(*str, str.length())); if (it == map_.end()) return Local(); return String::NewFromUtf8(isolate, it->second.data(), - NewStringType::kNormal, it->second.size()) - .ToLocalChecked(); + NewStringType::kNormal, it->second.size()); } void MapKVStore::Set(Isolate* isolate, Local key, Local value) { @@ -323,8 +304,11 @@ static void EnvGetter(Local property, return info.GetReturnValue().SetUndefined(); } CHECK(property->IsString()); - info.GetReturnValue().Set( - env->env_vars()->Get(env->isolate(), property.As())); + MaybeLocal value_string = + env->env_vars()->Get(env->isolate(), property.As()); + if (!value_string.IsEmpty()) { + info.GetReturnValue().Set(value_string.ToLocalChecked()); + } } static void EnvSetter(Local property,