Skip to content

Commit

Permalink
src: improve error handling in node_url
Browse files Browse the repository at this point in the history 
Eliminate uses of USE and ToLocalChecked to properly
propagate errors
  • Loading branch information
@jasnell
jasnell committed Feb 3, 2025
1 parent cadc4ed commit a38dcfd
Showing 1 changed file with 60 additions and 45 deletions.
105 changes: 60 additions & 45 deletions src/node_url.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit a38dcfd

Please sign in to comment.