node-api: return napi_exception_pending on proxy handlers

Accessing JS objects can be trapped with proxy handlers. These
handlers can throw when no node-api errors occur. In such cases,
`napi_pending_exception` should be returned.

PR-URL: https://github.com/nodejs/node/pull/48607
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
This commit is contained in:
Chengzhong Wu 2023-06-15 20:02:58 +08:00 committed by Michael Dawson
parent 91a21a2cf6
commit 52fcf14258
5 changed files with 155 additions and 41 deletions

View File

@ -1105,7 +1105,8 @@ napi_status NAPI_CDECL napi_set_property(napi_env env,
v8::Maybe<bool> set_maybe = obj->Set(context, k, val);
RETURN_STATUS_IF_FALSE(env, set_maybe.FromMaybe(false), napi_generic_failure);
RETURN_STATUS_IF_FALSE_WITH_PREAMBLE(
env, set_maybe.FromMaybe(false), napi_generic_failure);
return GET_RETURN_STATUS(env);
}
@ -1125,7 +1126,7 @@ napi_status NAPI_CDECL napi_has_property(napi_env env,
v8::Local<v8::Value> k = v8impl::V8LocalValueFromJsValue(key);
v8::Maybe<bool> has_maybe = obj->Has(context, k);
CHECK_MAYBE_NOTHING(env, has_maybe, napi_generic_failure);
CHECK_MAYBE_NOTHING_WITH_PREAMBLE(env, has_maybe, napi_generic_failure);
*result = has_maybe.FromMaybe(false);
return GET_RETURN_STATUS(env);
@ -1147,7 +1148,7 @@ napi_status NAPI_CDECL napi_get_property(napi_env env,
auto get_maybe = obj->Get(context, k);
CHECK_MAYBE_EMPTY(env, get_maybe, napi_generic_failure);
CHECK_MAYBE_EMPTY_WITH_PREAMBLE(env, get_maybe, napi_generic_failure);
v8::Local<v8::Value> val = get_maybe.ToLocalChecked();
*result = v8impl::JsValueFromV8LocalValue(val);
@ -1167,7 +1168,7 @@ napi_status NAPI_CDECL napi_delete_property(napi_env env,
CHECK_TO_OBJECT(env, context, obj, object);
v8::Maybe<bool> delete_maybe = obj->Delete(context, k);
CHECK_MAYBE_NOTHING(env, delete_maybe, napi_generic_failure);
CHECK_MAYBE_NOTHING_WITH_PREAMBLE(env, delete_maybe, napi_generic_failure);
if (result != nullptr) *result = delete_maybe.FromMaybe(false);
@ -1189,7 +1190,7 @@ napi_status NAPI_CDECL napi_has_own_property(napi_env env,
v8::Local<v8::Value> k = v8impl::V8LocalValueFromJsValue(key);
RETURN_STATUS_IF_FALSE(env, k->IsName(), napi_name_expected);
v8::Maybe<bool> has_maybe = obj->HasOwnProperty(context, k.As<v8::Name>());
CHECK_MAYBE_NOTHING(env, has_maybe, napi_generic_failure);
CHECK_MAYBE_NOTHING_WITH_PREAMBLE(env, has_maybe, napi_generic_failure);
*result = has_maybe.FromMaybe(false);
return GET_RETURN_STATUS(env);
@ -1214,7 +1215,8 @@ napi_status NAPI_CDECL napi_set_named_property(napi_env env,
v8::Maybe<bool> set_maybe = obj->Set(context, key, val);
RETURN_STATUS_IF_FALSE(env, set_maybe.FromMaybe(false), napi_generic_failure);
RETURN_STATUS_IF_FALSE_WITH_PREAMBLE(
env, set_maybe.FromMaybe(false), napi_generic_failure);
return GET_RETURN_STATUS(env);
}
@ -1235,7 +1237,7 @@ napi_status NAPI_CDECL napi_has_named_property(napi_env env,
v8::Maybe<bool> has_maybe = obj->Has(context, key);
CHECK_MAYBE_NOTHING(env, has_maybe, napi_generic_failure);
CHECK_MAYBE_NOTHING_WITH_PREAMBLE(env, has_maybe, napi_generic_failure);
*result = has_maybe.FromMaybe(false);
return GET_RETURN_STATUS(env);
@ -1259,7 +1261,7 @@ napi_status NAPI_CDECL napi_get_named_property(napi_env env,
auto get_maybe = obj->Get(context, key);
CHECK_MAYBE_EMPTY(env, get_maybe, napi_generic_failure);
CHECK_MAYBE_EMPTY_WITH_PREAMBLE(env, get_maybe, napi_generic_failure);
v8::Local<v8::Value> val = get_maybe.ToLocalChecked();
*result = v8impl::JsValueFromV8LocalValue(val);
@ -1281,7 +1283,8 @@ napi_status NAPI_CDECL napi_set_element(napi_env env,
v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
auto set_maybe = obj->Set(context, index, val);
RETURN_STATUS_IF_FALSE(env, set_maybe.FromMaybe(false), napi_generic_failure);
RETURN_STATUS_IF_FALSE_WITH_PREAMBLE(
env, set_maybe.FromMaybe(false), napi_generic_failure);
return GET_RETURN_STATUS(env);
}
@ -1300,7 +1303,7 @@ napi_status NAPI_CDECL napi_has_element(napi_env env,
v8::Maybe<bool> has_maybe = obj->Has(context, index);
CHECK_MAYBE_NOTHING(env, has_maybe, napi_generic_failure);
CHECK_MAYBE_NOTHING_WITH_PREAMBLE(env, has_maybe, napi_generic_failure);
*result = has_maybe.FromMaybe(false);
return GET_RETURN_STATUS(env);
@ -1320,7 +1323,7 @@ napi_status NAPI_CDECL napi_get_element(napi_env env,
auto get_maybe = obj->Get(context, index);
CHECK_MAYBE_EMPTY(env, get_maybe, napi_generic_failure);
CHECK_MAYBE_EMPTY_WITH_PREAMBLE(env, get_maybe, napi_generic_failure);
*result = v8impl::JsValueFromV8LocalValue(get_maybe.ToLocalChecked());
return GET_RETURN_STATUS(env);
@ -1337,7 +1340,7 @@ napi_status NAPI_CDECL napi_delete_element(napi_env env,
CHECK_TO_OBJECT(env, context, obj, object);
v8::Maybe<bool> delete_maybe = obj->Delete(context, index);
CHECK_MAYBE_NOTHING(env, delete_maybe, napi_generic_failure);
CHECK_MAYBE_NOTHING_WITH_PREAMBLE(env, delete_maybe, napi_generic_failure);
if (result != nullptr) *result = delete_maybe.FromMaybe(false);
@ -1385,9 +1388,8 @@ napi_define_properties(napi_env env,
auto define_maybe =
obj->DefineProperty(context, property_name, descriptor);
if (!define_maybe.FromMaybe(false)) {
return napi_set_last_error(env, napi_invalid_arg);
}
RETURN_STATUS_IF_FALSE_WITH_PREAMBLE(
env, define_maybe.FromMaybe(false), napi_invalid_arg);
} else if (p->method != nullptr) {
v8::Local<v8::Function> method;
STATUS_CALL(v8impl::FunctionCallbackWrapper::NewFunction(
@ -1400,34 +1402,28 @@ napi_define_properties(napi_env env,
auto define_maybe =
obj->DefineProperty(context, property_name, descriptor);
if (!define_maybe.FromMaybe(false)) {
return napi_set_last_error(env, napi_generic_failure);
}
RETURN_STATUS_IF_FALSE_WITH_PREAMBLE(
env, define_maybe.FromMaybe(false), napi_generic_failure);
} else {
v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(p->value);
bool defined_successfully = false;
v8::Maybe<bool> define_maybe = v8::Just(false);
if ((p->attributes & napi_enumerable) &&
(p->attributes & napi_writable) &&
(p->attributes & napi_configurable)) {
// Use a fast path for this type of data property.
auto define_maybe =
obj->CreateDataProperty(context, property_name, value);
defined_successfully = define_maybe.FromMaybe(false);
define_maybe = obj->CreateDataProperty(context, property_name, value);
} else {
v8::PropertyDescriptor descriptor(value,
(p->attributes & napi_writable) != 0);
descriptor.set_enumerable((p->attributes & napi_enumerable) != 0);
descriptor.set_configurable((p->attributes & napi_configurable) != 0);
auto define_maybe =
obj->DefineProperty(context, property_name, descriptor);
defined_successfully = define_maybe.FromMaybe(false);
define_maybe = obj->DefineProperty(context, property_name, descriptor);
}
if (!defined_successfully) {
return napi_set_last_error(env, napi_invalid_arg);
}
RETURN_STATUS_IF_FALSE_WITH_PREAMBLE(
env, define_maybe.FromMaybe(false), napi_invalid_arg);
}
}
@ -1525,6 +1521,7 @@ napi_status NAPI_CDECL napi_get_prototype(napi_env env,
v8::Local<v8::Object> obj;
CHECK_TO_OBJECT(env, context, obj, object);
// This doesn't invokes Proxy's [[GetPrototypeOf]] handler.
v8::Local<v8::Value> val = obj->GetPrototype();
*result = v8impl::JsValueFromV8LocalValue(val);
return GET_RETURN_STATUS(env);
@ -2058,15 +2055,11 @@ napi_status NAPI_CDECL napi_call_function(napi_env env,
argc,
reinterpret_cast<v8::Local<v8::Value>*>(const_cast<napi_value*>(argv)));
if (try_catch.HasCaught()) {
return napi_set_last_error(env, napi_pending_exception);
} else {
if (result != nullptr) {
CHECK_MAYBE_EMPTY(env, maybe, napi_generic_failure);
*result = v8impl::JsValueFromV8LocalValue(maybe.ToLocalChecked());
}
return napi_clear_last_error(env);
CHECK_MAYBE_EMPTY_WITH_PREAMBLE(env, maybe, napi_generic_failure);
if (result != nullptr) {
*result = v8impl::JsValueFromV8LocalValue(maybe.ToLocalChecked());
}
return napi_clear_last_error(env);
}
napi_status NAPI_CDECL napi_get_global(napi_env env, napi_value* result) {

View File

@ -38,17 +38,32 @@ inline void add_last_status(napi_env env,
const char* key,
napi_value return_value) {
napi_value prop_value;
napi_value exception;
const napi_extended_error_info* p_last_error;
NODE_API_CALL_RETURN_VOID(env, napi_get_last_error_info(env, &p_last_error));
// Content of p_last_error can be updated in subsequent node-api calls.
// Retrieve it immediately.
const char* error_message = p_last_error->error_message == NULL
? "napi_ok"
: p_last_error->error_message;
bool is_exception_pending;
NODE_API_CALL_RETURN_VOID(
env, napi_is_exception_pending(env, &is_exception_pending));
if (is_exception_pending) {
NODE_API_CALL_RETURN_VOID(
env, napi_get_and_clear_last_exception(env, &exception));
char exception_key[50];
snprintf(exception_key, sizeof(exception_key), "%s%s", key, "Exception");
NODE_API_CALL_RETURN_VOID(
env,
napi_set_named_property(env, return_value, exception_key, exception));
}
NODE_API_CALL_RETURN_VOID(
env,
napi_create_string_utf8(
env,
(p_last_error->error_message == NULL ? "napi_ok"
: p_last_error->error_message),
NAPI_AUTO_LENGTH,
&prop_value));
env, error_message, NAPI_AUTO_LENGTH, &prop_value));
NODE_API_CALL_RETURN_VOID(
env, napi_set_named_property(env, return_value, key, prop_value));
}

View File

@ -6,6 +6,12 @@
"test_null.c",
"test_object.c"
]
},
{
"target_name": "test_exceptions",
"sources": [
"test_exceptions.c",
]
}
]
}

View File

@ -0,0 +1,82 @@
#include <assert.h>
#include <js_native_api.h>
#include <string.h>
#include "../common.h"
#include "../entry_point.h"
static napi_value TestExceptions(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
napi_value target = args[0];
napi_value exception, key, value;
napi_status status;
bool is_exception_pending;
bool bool_result;
NODE_API_CALL(env,
napi_create_string_utf8(env, "key", NAPI_AUTO_LENGTH, &key));
NODE_API_CALL(
env, napi_create_string_utf8(env, "value", NAPI_AUTO_LENGTH, &value));
#define PROCEDURE(call) \
{ \
status = (call); \
NODE_API_ASSERT( \
env, status == napi_pending_exception, "expect exception pending"); \
NODE_API_CALL(env, napi_is_exception_pending(env, &is_exception_pending)); \
NODE_API_ASSERT(env, is_exception_pending, "expect exception pending"); \
NODE_API_CALL(env, napi_get_and_clear_last_exception(env, &exception)); \
}
// discard the exception values.
// properties
PROCEDURE(napi_set_property(env, target, key, value));
PROCEDURE(napi_set_named_property(env, target, "key", value));
PROCEDURE(napi_has_property(env, target, key, &bool_result));
PROCEDURE(napi_has_own_property(env, target, key, &bool_result));
PROCEDURE(napi_has_named_property(env, target, "key", &bool_result));
PROCEDURE(napi_get_property(env, target, key, &value));
PROCEDURE(napi_get_named_property(env, target, "key", &value));
PROCEDURE(napi_delete_property(env, target, key, &bool_result));
// elements
PROCEDURE(napi_set_element(env, target, 0, value));
PROCEDURE(napi_has_element(env, target, 0, &bool_result));
PROCEDURE(napi_get_element(env, target, 0, &value));
PROCEDURE(napi_delete_element(env, target, 0, &bool_result));
napi_property_descriptor descriptors[] = {
DECLARE_NODE_API_PROPERTY_VALUE("key", value),
};
PROCEDURE(napi_define_properties(
env, target, sizeof(descriptors) / sizeof(*descriptors), descriptors));
PROCEDURE(napi_get_all_property_names(env,
target,
napi_key_own_only,
napi_key_enumerable,
napi_key_keep_numbers,
&value));
PROCEDURE(napi_get_property_names(env, target, &value));
return NULL;
}
EXTERN_C_START
napi_value Init(napi_env env, napi_value exports) {
napi_property_descriptor descriptors[] = {
DECLARE_NODE_API_PROPERTY("testExceptions", TestExceptions),
};
NODE_API_CALL(
env,
napi_define_properties(env,
exports,
sizeof(descriptors) / sizeof(*descriptors),
descriptors));
return exports;
}
EXTERN_C_END

View File

@ -0,0 +1,18 @@
'use strict';
const common = require('../../common');
// Test
const { testExceptions } = require(`./build/${common.buildType}/test_exceptions`);
function throws() {
throw new Error('foobar');
}
testExceptions(new Proxy({}, {
get: common.mustCallAtLeast(throws, 1),
getOwnPropertyDescriptor: common.mustCallAtLeast(throws, 1),
defineProperty: common.mustCallAtLeast(throws, 1),
deleteProperty: common.mustCallAtLeast(throws, 1),
has: common.mustCallAtLeast(throws, 1),
set: common.mustCallAtLeast(throws, 1),
ownKeys: common.mustCallAtLeast(throws, 1),
}));