Skip to content

Commit 3d957d1

Browse files
jasnelltargos
jasnell
authored and
targos
committed
src: improve error handling in encoding_binding.cc
PR-URL: #56915 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Xuguang Mei <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
1 parent 9e9ac3c commit 3d957d1

File tree

5 files changed

+103
-73
lines changed

5 files changed

+103
-73
lines changed

‎src/encoding_binding.cc‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ using v8::Context;
2020
using v8::FunctionCallbackInfo;
2121
using v8::Isolate;
2222
using v8::Local;
23-
using v8::MaybeLocal;
2423
using v8::Object;
2524
using v8::ObjectTemplate;
2625
using v8::String;
@@ -139,8 +138,7 @@ void BindingData::EncodeUtf8String(const FunctionCallbackInfo<Value>& args) {
139138
ab = ArrayBuffer::New(isolate, std::move(bs));
140139
}
141140

142-
auto array = Uint8Array::New(ab, 0, length);
143-
args.GetReturnValue().Set(array);
141+
args.GetReturnValue().Set(Uint8Array::New(ab, 0, length));
144142
}
145143

146144
// Convert the input into an encoded string
@@ -184,11 +182,10 @@ void BindingData::DecodeUTF8(const FunctionCallbackInfo<Value>& args) {
184182
if (length == 0) return args.GetReturnValue().SetEmptyString();
185183

186184
Local<Value> error;
187-
MaybeLocal<Value> maybe_ret =
188-
StringBytes::Encode(env->isolate(), data, length, UTF8, &error);
189185
Local<Value> ret;
190186

191-
if (!maybe_ret.ToLocal(&ret)) {
187+
if (!StringBytes::Encode(env->isolate(), data, length, UTF8, &error)
188+
.ToLocal(&ret)) {
192189
CHECK(!error.IsEmpty());
193190
env->isolate()->ThrowException(error);
194191
return;
@@ -204,8 +201,10 @@ void BindingData::ToASCII(const v8::FunctionCallbackInfo<v8::Value>& args) {
204201

205202
Utf8Value input(env->isolate(), args[0]);
206203
auto out = ada::idna::to_ascii(input.ToStringView());
207-
args.GetReturnValue().Set(
208-
String::NewFromUtf8(env->isolate(), out.c_str()).ToLocalChecked());
204+
Local<Value> ret;
205+
if (ToV8Value(env->context(), out, env->isolate()).ToLocal(&ret)) {
206+
args.GetReturnValue().Set(ret);
207+
}
209208
}
210209

211210
void BindingData::ToUnicode(const v8::FunctionCallbackInfo<v8::Value>& args) {
@@ -215,8 +214,10 @@ void BindingData::ToUnicode(const v8::FunctionCallbackInfo<v8::Value>& args) {
215214

216215
Utf8Value input(env->isolate(), args[0]);
217216
auto out = ada::idna::to_unicode(input.ToStringView());
218-
args.GetReturnValue().Set(
219-
String::NewFromUtf8(env->isolate(), out.c_str()).ToLocalChecked());
217+
Local<Value> ret;
218+
if (ToV8Value(env->context(), out, env->isolate()).ToLocal(&ret)) {
219+
args.GetReturnValue().Set(ret);
220+
}
220221
}
221222

222223
void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data,
@@ -286,11 +287,12 @@ void BindingData::DecodeLatin1(const FunctionCallbackInfo<Value>& args) {
286287
env->isolate(), "The encoded data was not valid for encoding latin1");
287288
}
288289

289-
Local<String> output =
290-
String::NewFromUtf8(
291-
env->isolate(), result.c_str(), v8::NewStringType::kNormal, written)
292-
.ToLocalChecked();
293-
args.GetReturnValue().Set(output);
290+
std::string_view view(result.c_str(), written);
291+
292+
Local<Value> ret;
293+
if (ToV8Value(env->context(), view, env->isolate()).ToLocal(&ret)) {
294+
args.GetReturnValue().Set(ret);
295+
}
294296
}
295297

296298
} // namespace encoding_binding

‎src/inspector_agent.cc‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ using v8::HandleScope;
5151
using v8::Isolate;
5252
using v8::Local;
5353
using v8::Message;
54+
using v8::Name;
5455
using v8::Object;
5556
using v8::Value;
5657
using v8_inspector::StringBuffer;
@@ -411,12 +412,12 @@ class SameThreadInspectorSession : public InspectorSession {
411412
void NotifyClusterWorkersDebugEnabled(Environment* env) {
412413
Isolate* isolate = env->isolate();
413414
HandleScope handle_scope(isolate);
414-
Local<Context> context = env->context();
415415

416416
// Send message to enable debug in cluster workers
417-
Local<Object> message = Object::New(isolate);
418-
message->Set(context, FIXED_ONE_BYTE_STRING(isolate, "cmd"),
419-
FIXED_ONE_BYTE_STRING(isolate, "NODE_DEBUG_ENABLED")).Check();
417+
Local<Name> name = FIXED_ONE_BYTE_STRING(isolate, "cmd");
418+
Local<Value> value = FIXED_ONE_BYTE_STRING(isolate, "NODE_DEBUG_ENABLED");
419+
Local<Object> message =
420+
Object::New(isolate, Object::New(isolate), &name, &value, 1);
420421
ProcessEmit(env, "internalMessage", message);
421422
}
422423

@@ -441,11 +442,13 @@ bool IsFilePath(const std::string& path) {
441442
void ThrowUninitializedInspectorError(Environment* env) {
442443
HandleScope scope(env->isolate());
443444

444-
const char* msg = "This Environment was initialized without a V8::Inspector";
445-
Local<Value> exception =
446-
v8::String::NewFromUtf8(env->isolate(), msg).ToLocalChecked();
447-
448-
env->isolate()->ThrowException(exception);
445+
std::string_view msg =
446+
"This Environment was initialized without a V8::Inspector";
447+
Local<Value> exception;
448+
if (ToV8Value(env->context(), msg, env->isolate()).ToLocal(&exception)) {
449+
env->isolate()->ThrowException(exception);
450+
}
451+
// V8 will have scheduled a superseding error here.
449452
}
450453

451454
} // namespace

‎src/inspector_js_api.cc‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,11 +188,12 @@ void CallAndPauseOnStart(const FunctionCallbackInfo<v8::Value>& args) {
188188
CHECK(args[0]->IsFunction());
189189
SlicedArguments call_args(args, /* start */ 2);
190190
env->inspector_agent()->PauseOnNextJavascriptStatement("Break on start");
191-
v8::MaybeLocal<v8::Value> retval =
192-
args[0].As<v8::Function>()->Call(env->context(), args[1],
193-
call_args.length(), call_args.out());
194-
if (!retval.IsEmpty()) {
195-
args.GetReturnValue().Set(retval.ToLocalChecked());
191+
Local<Value> ret;
192+
if (args[0]
193+
.As<v8::Function>()
194+
->Call(env->context(), args[1], call_args.length(), call_args.out())
195+
.ToLocal(&ret)) {
196+
args.GetReturnValue().Set(ret);
196197
}
197198
}
198199

‎src/node_contextify.cc‎

Lines changed: 50 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,8 @@ using v8::Value;
111111
namespace {
112112

113113
// Convert an int to a V8 Name (String or Symbol).
114-
Local<Name> Uint32ToName(Local<Context> context, uint32_t index) {
115-
return Uint32::New(context->GetIsolate(), index)->ToString(context)
116-
.ToLocalChecked();
114+
MaybeLocal<String> Uint32ToName(Local<Context> context, uint32_t index) {
115+
return Uint32::New(context->GetIsolate(), index)->ToString(context);
117116
}
118117

119118
} // anonymous namespace
@@ -852,8 +851,11 @@ Intercepted ContextifyContext::IndexedPropertyQueryCallback(
852851
return Intercepted::kNo;
853852
}
854853

855-
return ContextifyContext::PropertyQueryCallback(
856-
Uint32ToName(ctx->context(), index), args);
854+
Local<String> name;
855+
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
856+
return ContextifyContext::PropertyQueryCallback(name, args);
857+
}
858+
return Intercepted::kNo;
857859
}
858860

859861
// static
@@ -866,8 +868,11 @@ Intercepted ContextifyContext::IndexedPropertyGetterCallback(
866868
return Intercepted::kNo;
867869
}
868870

869-
return ContextifyContext::PropertyGetterCallback(
870-
Uint32ToName(ctx->context(), index), args);
871+
Local<String> name;
872+
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
873+
return ContextifyContext::PropertyGetterCallback(name, args);
874+
}
875+
return Intercepted::kNo;
871876
}
872877

873878
Intercepted ContextifyContext::IndexedPropertySetterCallback(
@@ -881,8 +886,11 @@ Intercepted ContextifyContext::IndexedPropertySetterCallback(
881886
return Intercepted::kNo;
882887
}
883888

884-
return ContextifyContext::PropertySetterCallback(
885-
Uint32ToName(ctx->context(), index), value, args);
889+
Local<String> name;
890+
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
891+
return ContextifyContext::PropertySetterCallback(name, value, args);
892+
}
893+
return Intercepted::kNo;
886894
}
887895

888896
// static
@@ -895,8 +903,11 @@ Intercepted ContextifyContext::IndexedPropertyDescriptorCallback(
895903
return Intercepted::kNo;
896904
}
897905

898-
return ContextifyContext::PropertyDescriptorCallback(
899-
Uint32ToName(ctx->context(), index), args);
906+
Local<String> name;
907+
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
908+
return ContextifyContext::PropertyDescriptorCallback(name, args);
909+
}
910+
return Intercepted::kNo;
900911
}
901912

902913
Intercepted ContextifyContext::IndexedPropertyDefinerCallback(
@@ -910,8 +921,11 @@ Intercepted ContextifyContext::IndexedPropertyDefinerCallback(
910921
return Intercepted::kNo;
911922
}
912923

913-
return ContextifyContext::PropertyDefinerCallback(
914-
Uint32ToName(ctx->context(), index), desc, args);
924+
Local<String> name;
925+
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
926+
return ContextifyContext::PropertyDefinerCallback(name, desc, args);
927+
}
928+
return Intercepted::kNo;
915929
}
916930

917931
// static
@@ -1130,22 +1144,20 @@ Maybe<void> StoreCodeCacheResult(
11301144
if (produce_cached_data) {
11311145
bool cached_data_produced = new_cached_data != nullptr;
11321146
if (cached_data_produced) {
1133-
MaybeLocal<Object> buf =
1134-
Buffer::Copy(env,
1135-
reinterpret_cast<const char*>(new_cached_data->data),
1136-
new_cached_data->length);
1137-
if (target->Set(context, env->cached_data_string(), buf.ToLocalChecked())
1147+
Local<Object> buf;
1148+
if (!Buffer::Copy(env,
1149+
reinterpret_cast<const char*>(new_cached_data->data),
1150+
new_cached_data->length)
1151+
.ToLocal(&buf) ||
1152+
target->Set(context, env->cached_data_string(), buf).IsNothing() ||
1153+
target
1154+
->Set(context,
1155+
env->cached_data_produced_string(),
1156+
Boolean::New(env->isolate(), cached_data_produced))
11381157
.IsNothing()) {
11391158
return Nothing<void>();
11401159
}
11411160
}
1142-
if (target
1143-
->Set(context,
1144-
env->cached_data_produced_string(),
1145-
Boolean::New(env->isolate(), cached_data_produced))
1146-
.IsNothing()) {
1147-
return Nothing<void>();
1148-
}
11491161
}
11501162
return JustVoid();
11511163
}
@@ -1179,14 +1191,19 @@ void ContextifyScript::CreateCachedData(
11791191
ASSIGN_OR_RETURN_UNWRAP_CPPGC(&wrapped_script, args.This());
11801192
std::unique_ptr<ScriptCompiler::CachedData> cached_data(
11811193
ScriptCompiler::CreateCodeCache(wrapped_script->unbound_script()));
1182-
if (!cached_data) {
1183-
args.GetReturnValue().Set(Buffer::New(env, 0).ToLocalChecked());
1184-
} else {
1185-
MaybeLocal<Object> buf = Buffer::Copy(
1186-
env,
1187-
reinterpret_cast<const char*>(cached_data->data),
1188-
cached_data->length);
1189-
args.GetReturnValue().Set(buf.ToLocalChecked());
1194+
1195+
auto maybeRet = ([&] {
1196+
if (!cached_data) {
1197+
return Buffer::New(env, 0);
1198+
}
1199+
return Buffer::Copy(env,
1200+
reinterpret_cast<const char*>(cached_data->data),
1201+
cached_data->length);
1202+
})();
1203+
1204+
Local<Object> ret;
1205+
if (maybeRet.ToLocal(&ret)) {
1206+
args.GetReturnValue().Set(ret);
11901207
}
11911208
}
11921209

‎src/node_credentials.cc‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ using v8::Context;
2626
using v8::FunctionCallbackInfo;
2727
using v8::Isolate;
2828
using v8::Local;
29-
using v8::MaybeLocal;
3029
using v8::Object;
3130
using v8::Uint32;
3231
using v8::Value;
@@ -111,9 +110,10 @@ static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
111110
Utf8Value strenvtag(isolate, args[0]);
112111
std::string text;
113112
if (!SafeGetenv(*strenvtag, &text, env)) return;
114-
Local<Value> result =
115-
ToV8Value(isolate->GetCurrentContext(), text).ToLocalChecked();
116-
args.GetReturnValue().Set(result);
113+
Local<Value> result;
114+
if (ToV8Value(isolate->GetCurrentContext(), text).ToLocal(&result)) {
115+
args.GetReturnValue().Set(result);
116+
}
117117
}
118118

119119
static void GetTempDir(const FunctionCallbackInfo<Value>& args) {
@@ -137,8 +137,10 @@ static void GetTempDir(const FunctionCallbackInfo<Value>& args) {
137137
dir.pop_back();
138138
}
139139

140-
args.GetReturnValue().Set(
141-
ToV8Value(isolate->GetCurrentContext(), dir).ToLocalChecked());
140+
Local<Value> result;
141+
if (ToV8Value(isolate->GetCurrentContext(), dir).ToLocal(&result)) {
142+
args.GetReturnValue().Set(result);
143+
}
142144
}
143145

144146
#ifdef NODE_IMPLEMENTS_POSIX_CREDENTIALS
@@ -385,9 +387,10 @@ static void GetGroups(const FunctionCallbackInfo<Value>& args) {
385387
gid_t egid = getegid();
386388
if (std::find(groups.begin(), groups.end(), egid) == groups.end())
387389
groups.push_back(egid);
388-
MaybeLocal<Value> array = ToV8Value(env->context(), groups);
389-
if (!array.IsEmpty())
390-
args.GetReturnValue().Set(array.ToLocalChecked());
390+
Local<Value> result;
391+
if (ToV8Value(env->context(), groups).ToLocal(&result)) {
392+
args.GetReturnValue().Set(result);
393+
}
391394
}
392395

393396
static void SetGroups(const FunctionCallbackInfo<Value>& args) {
@@ -403,8 +406,12 @@ static void SetGroups(const FunctionCallbackInfo<Value>& args) {
403406
MaybeStackBuffer<gid_t, 64> groups(size);
404407

405408
for (size_t i = 0; i < size; i++) {
406-
gid_t gid = gid_by_name(
407-
env->isolate(), groups_list->Get(env->context(), i).ToLocalChecked());
409+
Local<Value> val;
410+
if (!groups_list->Get(env->context(), i).ToLocal(&val)) {
411+
// V8 will have scheduled an error to be thrown.
412+
return;
413+
}
414+
gid_t gid = gid_by_name(env->isolate(), val);
408415

409416
if (gid == gid_not_found) {
410417
// Tells JS to throw ERR_INVALID_CREDENTIAL

0 commit comments

Comments
 (0)