Skip to content

Commit

Permalink
src: improve error handling in encoding_binding.cc
Browse files Browse the repository at this point in the history 
PR-URL: #56915
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
  • Loading branch information
@jasnell
jasnell authored Feb 6, 2025
1 parent d54641c commit 269496e
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 73 deletions.
32 changes: 17 additions & 15 deletions src/encoding_binding.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ using v8::Context;
using v8::FunctionCallbackInfo;
using v8::Isolate;
using v8::Local;
using v8::MaybeLocal;
using v8::Object;
using v8::ObjectTemplate;
using v8::String;
Expand Down Expand Up @@ -139,8 +138,7 @@ void BindingData::EncodeUtf8String(const FunctionCallbackInfo<Value>& args) {
ab = ArrayBuffer::New(isolate, std::move(bs));
}

auto array = Uint8Array::New(ab, 0, length);
args.GetReturnValue().Set(array);
args.GetReturnValue().Set(Uint8Array::New(ab, 0, length));
}

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

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

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

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

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

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

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

Local<String> output =
String::NewFromUtf8(
env->isolate(), result.c_str(), v8::NewStringType::kNormal, written)
.ToLocalChecked();
args.GetReturnValue().Set(output);
std::string_view view(result.c_str(), written);

Local<Value> ret;
if (ToV8Value(env->context(), view, env->isolate()).ToLocal(&ret)) {
args.GetReturnValue().Set(ret);
}
}

} // namespace encoding_binding
Expand Down
21 changes: 12 additions & 9 deletions src/inspector_agent.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ using v8::HandleScope;
using v8::Isolate;
using v8::Local;
using v8::Message;
using v8::Name;
using v8::Object;
using v8::Value;
using v8_inspector::StringBuffer;
Expand Down Expand Up @@ -411,12 +412,12 @@ class SameThreadInspectorSession : public InspectorSession {
void NotifyClusterWorkersDebugEnabled(Environment* env) {
Isolate* isolate = env->isolate();
HandleScope handle_scope(isolate);
Local<Context> context = env->context();

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

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

const char* msg = "This Environment was initialized without a V8::Inspector";
Local<Value> exception =
v8::String::NewFromUtf8(env->isolate(), msg).ToLocalChecked();

env->isolate()->ThrowException(exception);
std::string_view msg =
"This Environment was initialized without a V8::Inspector";
Local<Value> exception;
if (ToV8Value(env->context(), msg, env->isolate()).ToLocal(&exception)) {
env->isolate()->ThrowException(exception);
}
// V8 will have scheduled a superseding error here.
}

} // namespace
Expand Down
11 changes: 6 additions & 5 deletions src/inspector_js_api.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,12 @@ void CallAndPauseOnStart(const FunctionCallbackInfo<v8::Value>& args) {
CHECK(args[0]->IsFunction());
SlicedArguments call_args(args, /* start */ 2);
env->inspector_agent()->PauseOnNextJavascriptStatement("Break on start");
v8::MaybeLocal<v8::Value> retval =
args[0].As<v8::Function>()->Call(env->context(), args[1],
call_args.length(), call_args.out());
if (!retval.IsEmpty()) {
args.GetReturnValue().Set(retval.ToLocalChecked());
Local<Value> ret;
if (args[0]
.As<v8::Function>()
->Call(env->context(), args[1], call_args.length(), call_args.out())
.ToLocal(&ret)) {
args.GetReturnValue().Set(ret);
}
}

Expand Down
83 changes: 50 additions & 33 deletions src/node_contextify.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,8 @@ using v8::Value;
namespace {

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

} // anonymous namespace
Expand Down Expand Up @@ -852,8 +851,11 @@ Intercepted ContextifyContext::IndexedPropertyQueryCallback(
return Intercepted::kNo;
}

return ContextifyContext::PropertyQueryCallback(
Uint32ToName(ctx->context(), index), args);
Local<String> name;
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
return ContextifyContext::PropertyQueryCallback(name, args);
}
return Intercepted::kNo;
}

// static
Expand All @@ -866,8 +868,11 @@ Intercepted ContextifyContext::IndexedPropertyGetterCallback(
return Intercepted::kNo;
}

return ContextifyContext::PropertyGetterCallback(
Uint32ToName(ctx->context(), index), args);
Local<String> name;
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
return ContextifyContext::PropertyGetterCallback(name, args);
}
return Intercepted::kNo;
}

Intercepted ContextifyContext::IndexedPropertySetterCallback(
Expand All @@ -881,8 +886,11 @@ Intercepted ContextifyContext::IndexedPropertySetterCallback(
return Intercepted::kNo;
}

return ContextifyContext::PropertySetterCallback(
Uint32ToName(ctx->context(), index), value, args);
Local<String> name;
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
return ContextifyContext::PropertySetterCallback(name, value, args);
}
return Intercepted::kNo;
}

// static
Expand All @@ -895,8 +903,11 @@ Intercepted ContextifyContext::IndexedPropertyDescriptorCallback(
return Intercepted::kNo;
}

return ContextifyContext::PropertyDescriptorCallback(
Uint32ToName(ctx->context(), index), args);
Local<String> name;
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
return ContextifyContext::PropertyDescriptorCallback(name, args);
}
return Intercepted::kNo;
}

Intercepted ContextifyContext::IndexedPropertyDefinerCallback(
Expand All @@ -910,8 +921,11 @@ Intercepted ContextifyContext::IndexedPropertyDefinerCallback(
return Intercepted::kNo;
}

return ContextifyContext::PropertyDefinerCallback(
Uint32ToName(ctx->context(), index), desc, args);
Local<String> name;
if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
return ContextifyContext::PropertyDefinerCallback(name, desc, args);
}
return Intercepted::kNo;
}

// static
Expand Down Expand Up @@ -1130,22 +1144,20 @@ Maybe<void> StoreCodeCacheResult(
if (produce_cached_data) {
bool cached_data_produced = new_cached_data != nullptr;
if (cached_data_produced) {
MaybeLocal<Object> buf =
Buffer::Copy(env,
reinterpret_cast<const char*>(new_cached_data->data),
new_cached_data->length);
if (target->Set(context, env->cached_data_string(), buf.ToLocalChecked())
Local<Object> buf;
if (!Buffer::Copy(env,
reinterpret_cast<const char*>(new_cached_data->data),
new_cached_data->length)
.ToLocal(&buf) ||
target->Set(context, env->cached_data_string(), buf).IsNothing() ||
target
->Set(context,
env->cached_data_produced_string(),
Boolean::New(env->isolate(), cached_data_produced))
.IsNothing()) {
return Nothing<void>();
}
}
if (target
->Set(context,
env->cached_data_produced_string(),
Boolean::New(env->isolate(), cached_data_produced))
.IsNothing()) {
return Nothing<void>();
}
}
return JustVoid();
}
Expand Down Expand Up @@ -1179,14 +1191,19 @@ void ContextifyScript::CreateCachedData(
ASSIGN_OR_RETURN_UNWRAP_CPPGC(&wrapped_script, args.This());
std::unique_ptr<ScriptCompiler::CachedData> cached_data(
ScriptCompiler::CreateCodeCache(wrapped_script->unbound_script()));
if (!cached_data) {
args.GetReturnValue().Set(Buffer::New(env, 0).ToLocalChecked());
} else {
MaybeLocal<Object> buf = Buffer::Copy(
env,
reinterpret_cast<const char*>(cached_data->data),
cached_data->length);
args.GetReturnValue().Set(buf.ToLocalChecked());

auto maybeRet = ([&] {
if (!cached_data) {
return Buffer::New(env, 0);
}
return Buffer::Copy(env,
reinterpret_cast<const char*>(cached_data->data),
cached_data->length);
})();

Local<Object> ret;
if (maybeRet.ToLocal(&ret)) {
args.GetReturnValue().Set(ret);
}
}

Expand Down
29 changes: 18 additions & 11 deletions src/node_credentials.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ using v8::Context;
using v8::FunctionCallbackInfo;
using v8::Isolate;
using v8::Local;
using v8::MaybeLocal;
using v8::Object;
using v8::Uint32;
using v8::Value;
Expand Down Expand Up @@ -111,9 +110,10 @@ static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
Utf8Value strenvtag(isolate, args[0]);
std::string text;
if (!SafeGetenv(*strenvtag, &text, env)) return;
Local<Value> result =
ToV8Value(isolate->GetCurrentContext(), text).ToLocalChecked();
args.GetReturnValue().Set(result);
Local<Value> result;
if (ToV8Value(isolate->GetCurrentContext(), text).ToLocal(&result)) {
args.GetReturnValue().Set(result);
}
}

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

args.GetReturnValue().Set(
ToV8Value(isolate->GetCurrentContext(), dir).ToLocalChecked());
Local<Value> result;
if (ToV8Value(isolate->GetCurrentContext(), dir).ToLocal(&result)) {
args.GetReturnValue().Set(result);
}
}

#ifdef NODE_IMPLEMENTS_POSIX_CREDENTIALS
Expand Down Expand Up @@ -385,9 +387,10 @@ static void GetGroups(const FunctionCallbackInfo<Value>& args) {
gid_t egid = getegid();
if (std::find(groups.begin(), groups.end(), egid) == groups.end())
groups.push_back(egid);
MaybeLocal<Value> array = ToV8Value(env->context(), groups);
if (!array.IsEmpty())
args.GetReturnValue().Set(array.ToLocalChecked());
Local<Value> result;
if (ToV8Value(env->context(), groups).ToLocal(&result)) {
args.GetReturnValue().Set(result);
}
}

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

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

if (gid == gid_not_found) {
// Tells JS to throw ERR_INVALID_CREDENTIAL
Expand Down

0 comments on commit 269496e

Please sign in to comment.