src: improve error handling in node_url

Eliminate uses of USE and ToLocalChecked to properly
propagate errors

PR-URL: #56886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>

Author: jasnell

src/node_url.cc

@@ -26,17 +26,17 @@ using v8::FunctionCallbackInfo;
 using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
-using v8::NewStringType;
 using v8::Object;
 using v8::ObjectTemplate;
+using v8::SnapshotCreator;
 using v8::String;
 using v8::Value;
 
 void BindingData::MemoryInfo(MemoryTracker* tracker) const {
   tracker->TrackField("url_components_buffer", url_components_buffer_);
 }
 
-BindingData::BindingData(Realm* realm, v8::Local<v8::Object> object)
+BindingData::BindingData(Realm* realm, Local<Object> object)
     : SnapshotableObject(realm, object, type_int),
       url_components_buffer_(realm->isolate(), kURLComponentsLength) {
   object
@@ -47,8 +47,8 @@ BindingData::BindingData(Realm* realm, v8::Local<v8::Object> object)
   url_components_buffer_.MakeWeak();
 }
 
-bool BindingData::PrepareForSerialization(v8::Local<v8::Context> context,
-                                          v8::SnapshotCreator* creator) {
+bool BindingData::PrepareForSerialization(Local<Context> context,
+                                          SnapshotCreator* creator) {
   // We'll just re-initialize the buffers in the constructor since their
   // contents can be thrown away once consumed in the previous call.
   url_components_buffer_.Release();
@@ -64,12 +64,12 @@ InternalFieldInfoBase* BindingData::Serialize(int index) {
   return info;
 }
 
-void BindingData::Deserialize(v8::Local<v8::Context> context,
-                              v8::Local<v8::Object> holder,
+void BindingData::Deserialize(Local<Context> context,
+                              Local<Object> holder,
                               int index,
                               InternalFieldInfoBase* info) {
   DCHECK_IS_SNAPSHOT_SLOT(index);
-  v8::HandleScope scope(context->GetIsolate());
+  HandleScope scope(context->GetIsolate());
   Realm* realm = Realm::GetCurrent(context);
   BindingData* binding = realm->AddBindingData<BindingData>(holder);
   CHECK_NOT_NULL(binding);
@@ -173,8 +173,11 @@ void BindingData::PathToFileURL(const FunctionCallbackInfo<Value>& args) {
 
   binding_data->UpdateComponents(out->get_components(), out->type);
 
-  args.GetReturnValue().Set(
-      ToV8Value(realm->context(), out->get_href(), isolate).ToLocalChecked());
+  Local<Value> ret;
+  if (ToV8Value(realm->context(), out->get_href(), isolate).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::DomainToASCII(const FunctionCallbackInfo<Value>& args) {
@@ -196,8 +199,12 @@ void BindingData::DomainToASCII(const FunctionCallbackInfo<Value>& args) {
     return args.GetReturnValue().Set(String::Empty(env->isolate()));
   }
   std::string host = out->get_hostname();
-  args.GetReturnValue().Set(
-      String::NewFromUtf8(env->isolate(), host.c_str()).ToLocalChecked());
+
+  Local<Value> ret;
+  if (ToV8Value(env->context(), host, env->isolate()).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::DomainToUnicode(const FunctionCallbackInfo<Value>& args) {
@@ -220,14 +227,14 @@ void BindingData::DomainToUnicode(const FunctionCallbackInfo<Value>& args) {
   }
   std::string result = ada::idna::to_unicode(out->get_hostname());
 
-  args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
-                                                result.c_str(),
-                                                NewStringType::kNormal,
-                                                result.length())
-                                .ToLocalChecked());
+  Local<Value> ret;
+  if (ToV8Value(env->context(), result, env->isolate()).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
-void BindingData::GetOrigin(const v8::FunctionCallbackInfo<Value>& args) {
+void BindingData::GetOrigin(const FunctionCallbackInfo<Value>& args) {
   CHECK_GE(args.Length(), 1);
   CHECK(args[0]->IsString());  // input
 
@@ -244,11 +251,12 @@ void BindingData::GetOrigin(const v8::FunctionCallbackInfo<Value>& args) {
   }
 
   std::string origin = out->get_origin();
-  args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
-                                                origin.data(),
-                                                NewStringType::kNormal,
-                                                origin.length())
-                                .ToLocalChecked());
+
+  Local<Value> ret;
+  if (ToV8Value(env->context(), origin, env->isolate()).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::CanParse(const FunctionCallbackInfo<Value>& args) {
@@ -328,11 +336,12 @@ void BindingData::Format(const FunctionCallbackInfo<Value>& args) {
   }
 
   std::string result = out->get_href();
-  args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
-                                                result.data(),
-                                                NewStringType::kNormal,
-                                                result.length())
-                                .ToLocalChecked());
+
+  Local<Value> ret;
+  if (ToV8Value(env->context(), result, env->isolate()).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::Parse(const FunctionCallbackInfo<Value>& args) {
@@ -372,8 +381,11 @@ void BindingData::Parse(const FunctionCallbackInfo<Value>& args) {
 
   binding_data->UpdateComponents(out->get_components(), out->type);
 
-  args.GetReturnValue().Set(
-      ToV8Value(realm->context(), out->get_href(), isolate).ToLocalChecked());
+  Local<Value> ret;
+  if (ToV8Value(realm->context(), out->get_href(), isolate).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::Update(const FunctionCallbackInfo<Value>& args) {
@@ -446,8 +458,12 @@ void BindingData::Update(const FunctionCallbackInfo<Value>& args) {
   }
 
   binding_data->UpdateComponents(out->get_components(), out->type);
-  args.GetReturnValue().Set(
-      ToV8Value(realm->context(), out->get_href(), isolate).ToLocalChecked());
+
+  Local<Value> ret;
+  if (ToV8Value(realm->context(), out->get_href(), isolate).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::UpdateComponents(const ada::url_components& components,
@@ -513,22 +529,21 @@ void ThrowInvalidURL(node::Environment* env,
 
   auto err_object = err.As<Object>();
 
-  USE(err_object->Set(env->context(),
-                      env->input_string(),
-                      v8::String::NewFromUtf8(env->isolate(),
-                                              input.data(),
-                                              v8::NewStringType::kNormal,
-                                              input.size())
-                          .ToLocalChecked()));
+  Local<Value> tmp;
+  if (!ToV8Value(env->context(), input, env->isolate()).ToLocal(&tmp) ||
+      err_object->Set(env->context(), env->input_string(), tmp).IsNothing())
+      [[unlikely]] {
+    // A superseding error has been thrown.
+    return;
+  }
 
   if (base.has_value()) {
-    USE(err_object->Set(env->context(),
-                        env->base_string(),
-                        v8::String::NewFromUtf8(env->isolate(),
-                                                base.value().c_str(),
-                                                v8::NewStringType::kNormal,
-                                                base.value().size())
-                            .ToLocalChecked()));
+    if (!ToV8Value(env->context(), base.value(), env->isolate())
+             .ToLocal(&tmp) ||
+        err_object->Set(env->context(), env->base_string(), tmp).IsNothing())
+        [[unlikely]] {
+      return;
+    }
   }
 
   env->isolate()->ThrowException(err);