Skip to content

Commit a1f9289

Browse files
jasnellRafaelGSS
jasnell
authored and
RafaelGSS
committed
src: improve error handling in encoding_binding.cc
PR-URL: #56915 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Xuguang Mei <meixuguang@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 6067bea commit a1f9289

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
@@ -19,7 +19,6 @@ using v8::Context;
1919
using v8::FunctionCallbackInfo;
2020
using v8::Isolate;
2121
using v8::Local;
22-
using v8::MaybeLocal;
2322
using v8::Object;
2423
using v8::ObjectTemplate;
2524
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;
@@ -410,12 +411,12 @@ class SameThreadInspectorSession : public InspectorSession {
410411
void NotifyClusterWorkersDebugEnabled(Environment* env) {
411412
Isolate* isolate = env->isolate();
412413
HandleScope handle_scope(isolate);
413-
Local<Context> context = env->context();
414414

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

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

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

450453
} // 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
@@ -110,9 +110,8 @@ using v8::Value;
110110
namespace {
111111

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

118117
} // anonymous namespace
@@ -854,8 +853,11 @@ Intercepted ContextifyContext::IndexedPropertyQueryCallback(
854853
return Intercepted::kNo;
855854
}
856855

857-
return ContextifyContext::PropertyQueryCallback(
858-
Uint32ToName(ctx->context(), index), args);
856+
Local<String> name;
857+
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
858+
return ContextifyContext::PropertyQueryCallback(name, args);
859+
}
860+
return Intercepted::kNo;
859861
}
860862

861863
// static
@@ -868,8 +870,11 @@ Intercepted ContextifyContext::IndexedPropertyGetterCallback(
868870
return Intercepted::kNo;
869871
}
870872

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

875880
Intercepted ContextifyContext::IndexedPropertySetterCallback(
@@ -883,8 +888,11 @@ Intercepted ContextifyContext::IndexedPropertySetterCallback(
883888
return Intercepted::kNo;
884889
}
885890

886-
return ContextifyContext::PropertySetterCallback(
887-
Uint32ToName(ctx->context(), index), value, args);
891+
Local<String> name;
892+
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
893+
return ContextifyContext::PropertySetterCallback(name, value, args);
894+
}
895+
return Intercepted::kNo;
888896
}
889897

890898
// static
@@ -897,8 +905,11 @@ Intercepted ContextifyContext::IndexedPropertyDescriptorCallback(
897905
return Intercepted::kNo;
898906
}
899907

900-
return ContextifyContext::PropertyDescriptorCallback(
901-
Uint32ToName(ctx->context(), index), args);
908+
Local<String> name;
909+
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
910+
return ContextifyContext::PropertyDescriptorCallback(name, args);
911+
}
912+
return Intercepted::kNo;
902913
}
903914

904915
Intercepted ContextifyContext::IndexedPropertyDefinerCallback(
@@ -912,8 +923,11 @@ Intercepted ContextifyContext::IndexedPropertyDefinerCallback(
912923
return Intercepted::kNo;
913924
}
914925

915-
return ContextifyContext::PropertyDefinerCallback(
916-
Uint32ToName(ctx->context(), index), desc, args);
926+
Local<String> name;
927+
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
928+
return ContextifyContext::PropertyDefinerCallback(name, desc, args);
929+
}
930+
return Intercepted::kNo;
917931
}
918932

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

‎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)