Skip to content

Commit c9a99df

Browse files
jasnelltargos
jasnell
authored and
targos
committed
src: improve error handling in node_url
Eliminate uses of USE and ToLocalChecked to properly propagate errors PR-URL: #56886 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 582b922 commit c9a99df

File tree

1 file changed

+60
-45
lines changed

1 file changed

+60
-45
lines changed

‎src/node_url.cc‎

Lines changed: 60 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,17 @@ using v8::FunctionCallbackInfo;
2626
using v8::HandleScope;
2727
using v8::Isolate;
2828
using v8::Local;
29-
using v8::NewStringType;
3029
using v8::Object;
3130
using v8::ObjectTemplate;
31+
using v8::SnapshotCreator;
3232
using v8::String;
3333
using v8::Value;
3434

3535
void BindingData::MemoryInfo(MemoryTracker* tracker) const {
3636
tracker->TrackField("url_components_buffer", url_components_buffer_);
3737
}
3838

39-
BindingData::BindingData(Realm* realm, v8::Local<v8::Object> object)
39+
BindingData::BindingData(Realm* realm, Local<Object> object)
4040
: SnapshotableObject(realm, object, type_int),
4141
url_components_buffer_(realm->isolate(), kURLComponentsLength) {
4242
object
@@ -47,8 +47,8 @@ BindingData::BindingData(Realm* realm, v8::Local<v8::Object> object)
4747
url_components_buffer_.MakeWeak();
4848
}
4949

50-
bool BindingData::PrepareForSerialization(v8::Local<v8::Context> context,
51-
v8::SnapshotCreator* creator) {
50+
bool BindingData::PrepareForSerialization(Local<Context> context,
51+
SnapshotCreator* creator) {
5252
// We'll just re-initialize the buffers in the constructor since their
5353
// contents can be thrown away once consumed in the previous call.
5454
url_components_buffer_.Release();
@@ -64,12 +64,12 @@ InternalFieldInfoBase* BindingData::Serialize(int index) {
6464
return info;
6565
}
6666

67-
void BindingData::Deserialize(v8::Local<v8::Context> context,
68-
v8::Local<v8::Object> holder,
67+
void BindingData::Deserialize(Local<Context> context,
68+
Local<Object> holder,
6969
int index,
7070
InternalFieldInfoBase* info) {
7171
DCHECK_IS_SNAPSHOT_SLOT(index);
72-
v8::HandleScope scope(context->GetIsolate());
72+
HandleScope scope(context->GetIsolate());
7373
Realm* realm = Realm::GetCurrent(context);
7474
BindingData* binding = realm->AddBindingData<BindingData>(holder);
7575
CHECK_NOT_NULL(binding);
@@ -173,8 +173,11 @@ void BindingData::PathToFileURL(const FunctionCallbackInfo<Value>& args) {
173173

174174
binding_data->UpdateComponents(out->get_components(), out->type);
175175

176-
args.GetReturnValue().Set(
177-
ToV8Value(realm->context(), out->get_href(), isolate).ToLocalChecked());
176+
Local<Value> ret;
177+
if (ToV8Value(realm->context(), out->get_href(), isolate).ToLocal(&ret))
178+
[[likely]] {
179+
args.GetReturnValue().Set(ret);
180+
}
178181
}
179182

180183
void BindingData::DomainToASCII(const FunctionCallbackInfo<Value>& args) {
@@ -196,8 +199,12 @@ void BindingData::DomainToASCII(const FunctionCallbackInfo<Value>& args) {
196199
return args.GetReturnValue().Set(String::Empty(env->isolate()));
197200
}
198201
std::string host = out->get_hostname();
199-
args.GetReturnValue().Set(
200-
String::NewFromUtf8(env->isolate(), host.c_str()).ToLocalChecked());
202+
203+
Local<Value> ret;
204+
if (ToV8Value(env->context(), host, env->isolate()).ToLocal(&ret))
205+
[[likely]] {
206+
args.GetReturnValue().Set(ret);
207+
}
201208
}
202209

203210
void BindingData::DomainToUnicode(const FunctionCallbackInfo<Value>& args) {
@@ -220,14 +227,14 @@ void BindingData::DomainToUnicode(const FunctionCallbackInfo<Value>& args) {
220227
}
221228
std::string result = ada::idna::to_unicode(out->get_hostname());
222229

223-
args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
224-
result.c_str(),
225-
NewStringType::kNormal,
226-
result.length())
227-
.ToLocalChecked());
230+
Local<Value> ret;
231+
if (ToV8Value(env->context(), result, env->isolate()).ToLocal(&ret))
232+
[[likely]] {
233+
args.GetReturnValue().Set(ret);
234+
}
228235
}
229236

230-
void BindingData::GetOrigin(const v8::FunctionCallbackInfo<Value>& args) {
237+
void BindingData::GetOrigin(const FunctionCallbackInfo<Value>& args) {
231238
CHECK_GE(args.Length(), 1);
232239
CHECK(args[0]->IsString()); // input
233240

@@ -244,11 +251,12 @@ void BindingData::GetOrigin(const v8::FunctionCallbackInfo<Value>& args) {
244251
}
245252

246253
std::string origin = out->get_origin();
247-
args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
248-
origin.data(),
249-
NewStringType::kNormal,
250-
origin.length())
251-
.ToLocalChecked());
254+
255+
Local<Value> ret;
256+
if (ToV8Value(env->context(), origin, env->isolate()).ToLocal(&ret))
257+
[[likely]] {
258+
args.GetReturnValue().Set(ret);
259+
}
252260
}
253261

254262
void BindingData::CanParse(const FunctionCallbackInfo<Value>& args) {
@@ -328,11 +336,12 @@ void BindingData::Format(const FunctionCallbackInfo<Value>& args) {
328336
}
329337

330338
std::string result = out->get_href();
331-
args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
332-
result.data(),
333-
NewStringType::kNormal,
334-
result.length())
335-
.ToLocalChecked());
339+
340+
Local<Value> ret;
341+
if (ToV8Value(env->context(), result, env->isolate()).ToLocal(&ret))
342+
[[likely]] {
343+
args.GetReturnValue().Set(ret);
344+
}
336345
}
337346

338347
void BindingData::Parse(const FunctionCallbackInfo<Value>& args) {
@@ -372,8 +381,11 @@ void BindingData::Parse(const FunctionCallbackInfo<Value>& args) {
372381

373382
binding_data->UpdateComponents(out->get_components(), out->type);
374383

375-
args.GetReturnValue().Set(
376-
ToV8Value(realm->context(), out->get_href(), isolate).ToLocalChecked());
384+
Local<Value> ret;
385+
if (ToV8Value(realm->context(), out->get_href(), isolate).ToLocal(&ret))
386+
[[likely]] {
387+
args.GetReturnValue().Set(ret);
388+
}
377389
}
378390

379391
void BindingData::Update(const FunctionCallbackInfo<Value>& args) {
@@ -446,8 +458,12 @@ void BindingData::Update(const FunctionCallbackInfo<Value>& args) {
446458
}
447459

448460
binding_data->UpdateComponents(out->get_components(), out->type);
449-
args.GetReturnValue().Set(
450-
ToV8Value(realm->context(), out->get_href(), isolate).ToLocalChecked());
461+
462+
Local<Value> ret;
463+
if (ToV8Value(realm->context(), out->get_href(), isolate).ToLocal(&ret))
464+
[[likely]] {
465+
args.GetReturnValue().Set(ret);
466+
}
451467
}
452468

453469
void BindingData::UpdateComponents(const ada::url_components& components,
@@ -513,22 +529,21 @@ void ThrowInvalidURL(node::Environment* env,
513529

514530
auto err_object = err.As<Object>();
515531

516-
USE(err_object->Set(env->context(),
517-
env->input_string(),
518-
v8::String::NewFromUtf8(env->isolate(),
519-
input.data(),
520-
v8::NewStringType::kNormal,
521-
input.size())
522-
.ToLocalChecked()));
532+
Local<Value> tmp;
533+
if (!ToV8Value(env->context(), input, env->isolate()).ToLocal(&tmp) ||
534+
err_object->Set(env->context(), env->input_string(), tmp).IsNothing())
535+
[[unlikely]] {
536+
// A superseding error has been thrown.
537+
return;
538+
}
523539

524540
if (base.has_value()) {
525-
USE(err_object->Set(env->context(),
526-
env->base_string(),
527-
v8::String::NewFromUtf8(env->isolate(),
528-
base.value().c_str(),
529-
v8::NewStringType::kNormal,
530-
base.value().size())
531-
.ToLocalChecked()));
541+
if (!ToV8Value(env->context(), base.value(), env->isolate())
542+
.ToLocal(&tmp) ||
543+
err_object->Set(env->context(), env->base_string(), tmp).IsNothing())
544+
[[unlikely]] {
545+
return;
546+
}
532547
}
533548

534549
env->isolate()->ThrowException(err);

0 commit comments

Comments
 (0)