src: improve error handling in buffer and dotenv

Replacing ToLocalChecked() PR-URL: #57189 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
nodejs · Feb 26, 2025 · e03af77 · e03af77
1 parent 6a19fde
commit e03af77
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 48 deletions.
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -567,9 +567,14 @@ void SlowCopy(const FunctionCallbackInfo<Value>& args) {
   ArrayBufferViewContents<char> source(args[0]);
   SPREAD_BUFFER_ARG(args[1].As<Object>(), target);
 
-  const auto target_start = args[2]->Uint32Value(env->context()).ToChecked();
-  const auto source_start = args[3]->Uint32Value(env->context()).ToChecked();
-  const auto to_copy = args[4]->Uint32Value(env->context()).ToChecked();
+  uint32_t target_start;
+  uint32_t source_start;
+  uint32_t to_copy;
+  if (!args[2]->Uint32Value(env->context()).To(&target_start) ||
+      !args[3]->Uint32Value(env->context()).To(&source_start) ||
+      !args[4]->Uint32Value(env->context()).To(&to_copy)) {
+    return;
+  }
 
   memmove(target_data + target_start, source.data() + source_start, to_copy);
   args.GetReturnValue().Set(to_copy);
@@ -634,7 +639,9 @@ void Fill(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
-  str_obj = args[1]->ToString(env->context()).ToLocalChecked();
+  if (!args[1]->ToString(env->context()).ToLocal(&str_obj)) {
+    return;
+  }
   enc = ParseEncoding(env->isolate(), args[4], UTF8);
 
   // Can't use StringBytes::Write() in all cases. For example if attempting
@@ -697,7 +704,10 @@ void StringWrite(const FunctionCallbackInfo<Value>& args) {
 
   THROW_AND_RETURN_IF_NOT_STRING(env, args[0], "argument");
 
-  Local<String> str = args[0]->ToString(env->context()).ToLocalChecked();
+  Local<String> str;
+  if (!args[0]->ToString(env->context()).ToLocal(&str)) {
+    return;
+  }
 
   size_t offset = 0;
   size_t max_length = 0;
@@ -1238,10 +1248,12 @@ void GetZeroFillToggle(const FunctionCallbackInfo<Value>& args) {
     ab = ArrayBuffer::New(env->isolate(), std::move(backing));
   }
 
-  ab->SetPrivate(
-      env->context(),
-      env->untransferable_object_private_symbol(),
-      True(env->isolate())).Check();
+  if (ab->SetPrivate(env->context(),
+                     env->untransferable_object_private_symbol(),
+                     True(env->isolate()))
+          .IsNothing()) {
+    return;
+  }
 
   args.GetReturnValue().Set(Uint32Array::New(ab, 0, 1));
 }
@@ -1252,7 +1264,9 @@ void DetachArrayBuffer(const FunctionCallbackInfo<Value>& args) {
     Local<ArrayBuffer> buf = args[0].As<ArrayBuffer>();
     if (buf->IsDetachable()) {
       std::shared_ptr<BackingStore> store = buf->GetBackingStore();
-      buf->Detach(Local<Value>()).Check();
+      if (buf->Detach(Local<Value>()).IsNothing()) {
+        return;
+      }
       args.GetReturnValue().Set(ArrayBuffer::New(env->isolate(), store));
     }
   }
@@ -1461,7 +1475,10 @@ void SlowWriteString(const FunctionCallbackInfo<Value>& args) {
 
   THROW_AND_RETURN_IF_NOT_STRING(env, args[1], "argument");
 
-  Local<String> str = args[1]->ToString(env->context()).ToLocalChecked();
+  Local<String> str;
+  if (!args[1]->ToString(env->context()).ToLocal(&str)) {
+    return;
+  }
 
   size_t offset = 0;
   size_t max_length = 0;

diff --git a/src/node_dotenv.cc b/src/node_dotenv.cc
@@ -6,10 +6,15 @@
 
 namespace node {
 
+using v8::EscapableHandleScope;
+using v8::JustVoid;
 using v8::Local;
-using v8::NewStringType;
+using v8::Maybe;
+using v8::MaybeLocal;
+using v8::Nothing;
 using v8::Object;
 using v8::String;
+using v8::Value;
 
 std::vector<Dotenv::env_file_data> Dotenv::GetDataFromArgs(
     const std::vector<std::string>& args) {
@@ -59,50 +64,42 @@ std::vector<Dotenv::env_file_data> Dotenv::GetDataFromArgs(
   return env_files;
 }
 
-void Dotenv::SetEnvironment(node::Environment* env) {
-  auto isolate = env->isolate();
+Maybe<void> Dotenv::SetEnvironment(node::Environment* env) {
+  Local<Value> name;
+  Local<Value> val;
+  auto context = env->context();
 
   for (const auto& entry : store_) {
-    auto key = entry.first;
-    auto value = entry.second;
-
-    auto existing = env->env_vars()->Get(key.data());
-
+    auto existing = env->env_vars()->Get(entry.first.data());
     if (!existing.has_value()) {
-      env->env_vars()->Set(
-          isolate,
-          v8::String::NewFromUtf8(
-              isolate, key.data(), NewStringType::kNormal, key.size())
-              .ToLocalChecked(),
-          v8::String::NewFromUtf8(
-              isolate, value.data(), NewStringType::kNormal, value.size())
-              .ToLocalChecked());
+      if (!ToV8Value(context, entry.first).ToLocal(&name) ||
+          !ToV8Value(context, entry.second).ToLocal(&val)) {
+        return Nothing<void>();
+      }
+      env->env_vars()->Set(env->isolate(), name.As<String>(), val.As<String>());
     }
   }
+
+  return JustVoid();
 }
 
-Local<Object> Dotenv::ToObject(Environment* env) const {
+MaybeLocal<Object> Dotenv::ToObject(Environment* env) const {
+  EscapableHandleScope scope(env->isolate());
   Local<Object> result = Object::New(env->isolate());
 
+  Local<Value> name;
+  Local<Value> val;
+  auto context = env->context();
+
   for (const auto& entry : store_) {
-    auto key = entry.first;
-    auto value = entry.second;
-
-    result
-        ->Set(
-            env->context(),
-            v8::String::NewFromUtf8(
-                env->isolate(), key.data(), NewStringType::kNormal, key.size())
-                .ToLocalChecked(),
-            v8::String::NewFromUtf8(env->isolate(),
-                                    value.data(),
-                                    NewStringType::kNormal,
-                                    value.size())
-                .ToLocalChecked())
-        .Check();
+    if (!ToV8Value(context, entry.first).ToLocal(&name) ||
+        !ToV8Value(context, entry.second).ToLocal(&val) ||
+        result->Set(context, name, val).IsNothing()) {
+      return MaybeLocal<Object>();
+    }
   }
 
-  return result;
+  return scope.Escape(result);
 }
 
 // Removes space characters (spaces, tabs and newlines) from

diff --git a/src/node_dotenv.h b/src/node_dotenv.h
@@ -28,8 +28,8 @@ class Dotenv {
   void ParseContent(const std::string_view content);
   ParseResult ParsePath(const std::string_view path);
   void AssignNodeOptionsIfAvailable(std::string* node_options) const;
-  void SetEnvironment(Environment* env);
-  v8::Local<v8::Object> ToObject(Environment* env) const;
+  v8::Maybe<void> SetEnvironment(Environment* env);
+  v8::MaybeLocal<v8::Object> ToObject(Environment* env) const;
 
   static std::vector<env_file_data> GetDataFromArgs(
       const std::vector<std::string>& args);

diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc
@@ -511,7 +511,7 @@ static void LoadEnvFile(const v8::FunctionCallbackInfo<v8::Value>& args) {
 
   switch (dotenv.ParsePath(path)) {
     case dotenv.ParseResult::Valid: {
-      dotenv.SetEnvironment(env);
+      USE(dotenv.SetEnvironment(env));
       break;
     }
     case dotenv.ParseResult::InvalidContent: {

diff --git a/src/node_util.cc b/src/node_util.cc
@@ -244,7 +244,10 @@ static void ParseEnv(const FunctionCallbackInfo<Value>& args) {
   Utf8Value content(env->isolate(), args[0]);
   Dotenv dotenv{};
   dotenv.ParseContent(content.ToStringView());
-  args.GetReturnValue().Set(dotenv.ToObject(env));
+  Local<Object> obj;
+  if (dotenv.ToObject(env).ToLocal(&obj)) {
+    args.GetReturnValue().Set(obj);
+  }
 }
 
 static void GetCallSites(const FunctionCallbackInfo<Value>& args) {