diff --git a/src/node_env_var.cc b/src/node_env_var.cc index 70ff9da2537e93..fe0896aaf50900 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -26,7 +26,6 @@ using v8::Maybe; using v8::MaybeLocal; using v8::Name; using v8::NamedPropertyHandlerConfiguration; -using v8::NewStringType; using v8::Nothing; using v8::Object; using v8::ObjectTemplate; @@ -45,7 +44,7 @@ class RealEnvStore final : public KVStore { int32_t Query(Isolate* isolate, Local key) const override; int32_t Query(const char* key) const override; void Delete(Isolate* isolate, Local key) override; - Local Enumerate(Isolate* isolate) const override; + MaybeLocal Enumerate(Isolate* isolate) const override; }; class MapKVStore final : public KVStore { @@ -56,7 +55,7 @@ class MapKVStore final : public KVStore { int32_t Query(Isolate* isolate, Local key) const override; int32_t Query(const char* key) const override; void Delete(Isolate* isolate, Local key) override; - Local Enumerate(Isolate* isolate) const override; + MaybeLocal Enumerate(Isolate* isolate) const override; std::shared_ptr Clone(Isolate* isolate) const override; @@ -131,8 +130,12 @@ MaybeLocal RealEnvStore::Get(Isolate* isolate, if (value.has_value()) { std::string val = value.value(); - return String::NewFromUtf8( - isolate, val.data(), NewStringType::kNormal, val.size()); + Local ret; + if (!ToV8Value(isolate->GetCurrentContext(), val).ToLocal(&ret)) { + return {}; + } + DCHECK(ret->IsString()); + return ret.As(); } return MaybeLocal(); @@ -188,7 +191,7 @@ void RealEnvStore::Delete(Isolate* isolate, Local property) { DateTimeConfigurationChangeNotification(isolate, key); } -Local RealEnvStore::Enumerate(Isolate* isolate) const { +MaybeLocal RealEnvStore::Enumerate(Isolate* isolate) const { Mutex::ScopedLock lock(per_process::env_var_mutex); uv_env_item_t* items; int count; @@ -203,12 +206,12 @@ Local RealEnvStore::Enumerate(Isolate* isolate) const { // If the key starts with '=' it is a hidden environment variable. if (items[i].name[0] == '=') continue; #endif - MaybeLocal str = String::NewFromUtf8(isolate, items[i].name); - if (str.IsEmpty()) { + Local str; + if (!String::NewFromUtf8(isolate, items[i].name).ToLocal(&str)) { isolate->ThrowException(ERR_STRING_TOO_LONG(isolate)); - return Local(); + return {}; } - env_v[env_v_index++] = str.ToLocalChecked(); + env_v[env_v_index++] = str; } return Array::New(isolate, env_v.out(), env_v_index); @@ -219,14 +222,22 @@ std::shared_ptr KVStore::Clone(Isolate* isolate) const { Local context = isolate->GetCurrentContext(); std::shared_ptr copy = KVStore::CreateMapKVStore(); - Local keys = Enumerate(isolate); + Local keys; + if (!Enumerate(isolate).ToLocal(&keys)) { + return nullptr; + } uint32_t keys_length = keys->Length(); for (uint32_t i = 0; i < keys_length; i++) { - Local key = keys->Get(context, i).ToLocalChecked(); + Local key; + Local value; + if (!keys->Get(context, i).ToLocal(&key)) { + return nullptr; + } CHECK(key->IsString()); - copy->Set(isolate, - key.As(), - Get(isolate, key.As()).ToLocalChecked()); + if (!Get(isolate, key.As()).ToLocal(&value)) { + return nullptr; + } + copy->Set(isolate, key.As(), value.As()); } return copy; } @@ -242,8 +253,12 @@ MaybeLocal MapKVStore::Get(Isolate* isolate, Local key) const { std::optional value = Get(*str); if (!value.has_value()) return MaybeLocal(); std::string val = value.value(); - return String::NewFromUtf8( - isolate, val.data(), NewStringType::kNormal, val.size()); + Local ret; + if (!ToV8Value(isolate->GetCurrentContext(), val).ToLocal(&ret)) { + return {}; + } + DCHECK(ret->IsString()); + return ret.As(); } void MapKVStore::Set(Isolate* isolate, Local key, Local value) { @@ -272,15 +287,16 @@ void MapKVStore::Delete(Isolate* isolate, Local key) { map_.erase(std::string(*str, str.length())); } -Local MapKVStore::Enumerate(Isolate* isolate) const { +MaybeLocal MapKVStore::Enumerate(Isolate* isolate) const { Mutex::ScopedLock lock(mutex_); LocalVector values(isolate); values.reserve(map_.size()); for (const auto& pair : map_) { - values.emplace_back( - String::NewFromUtf8(isolate, pair.first.data(), - NewStringType::kNormal, pair.first.size()) - .ToLocalChecked()); + Local val; + if (!ToV8Value(isolate->GetCurrentContext(), pair.first).ToLocal(&val)) { + return {}; + } + values.emplace_back(val); } return Array::New(isolate, values.data(), values.size()); } @@ -324,7 +340,10 @@ Maybe KVStore::AssignToObject(v8::Isolate* isolate, v8::Local context, v8::Local object) { HandleScope scope(isolate); - Local keys = Enumerate(isolate); + Local keys; + if (!Enumerate(isolate).ToLocal(&keys)) { + return Nothing(); + } uint32_t keys_length = keys->Length(); for (uint32_t i = 0; i < keys_length; i++) { Local key; @@ -421,6 +440,7 @@ static Intercepted EnvGetter(Local property, TraceEnvVar(env, "get", property.As()); if (has_env) { + // ToLocalChecked here is ok since we check IsEmpty above. info.GetReturnValue().Set(value_string.ToLocalChecked()); return Intercepted::kYes; } @@ -502,8 +522,10 @@ static void EnvEnumerator(const PropertyCallbackInfo& info) { TraceEnvVar(env, "enumerate environment variables"); - info.GetReturnValue().Set( - env->env_vars()->Enumerate(env->isolate())); + Local ret; + if (env->env_vars()->Enumerate(env->isolate()).ToLocal(&ret)) { + info.GetReturnValue().Set(ret); + } } static Intercepted EnvDefiner(Local property, diff --git a/src/node_worker.cc b/src/node_worker.cc index edb43bc796c320..71870b8d3e21b3 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -545,6 +545,10 @@ void Worker::New(const FunctionCallbackInfo& args) { env_vars = env->env_vars(); } + if (!env_vars) { + THROW_ERR_OPERATION_FAILED(env, "Failed to copy environment variables"); + } + if (args[1]->IsObject() || args[2]->IsArray()) { per_isolate_opts.reset(new PerIsolateOptions()); diff --git a/src/util.h b/src/util.h index 6b414a79bbb613..57d25d7a32b52e 100644 --- a/src/util.h +++ b/src/util.h @@ -315,7 +315,7 @@ class KVStore { v8::Local key) const = 0; virtual int32_t Query(const char* key) const = 0; virtual void Delete(v8::Isolate* isolate, v8::Local key) = 0; - virtual v8::Local Enumerate(v8::Isolate* isolate) const = 0; + virtual v8::MaybeLocal Enumerate(v8::Isolate* isolate) const = 0; virtual std::shared_ptr Clone(v8::Isolate* isolate) const; virtual v8::Maybe AssignFromObject(v8::Local context,