Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/jsc/bindings/napi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2076,20 +2076,31 @@ extern "C" napi_status napi_get_all_property_names(
JSArray* filteredKeys = JSArray::create(JSC::getVM(globalObject), globalObject->originalArrayStructureForIndexingType(ArrayWithContiguous), 0);
for (unsigned i = 0; i < exportKeys->getArrayLength(); i++) {
JSValue key = exportKeys->get(globalObject, i);
NAPI_RETURN_IF_EXCEPTION(env);
auto propertyKey = key.toPropertyKey(globalObject);
NAPI_RETURN_IF_EXCEPTION(env);
PropertyDescriptor desc;

if (key_mode == napi_key_include_prototypes) {
// Climb up the prototype chain to find inherited properties
JSObject* current_object = object;
while (!current_object->getOwnPropertyDescriptor(globalObject, key.toPropertyKey(globalObject), desc)) {
JSObject* proto = current_object->getPrototype(globalObject).getObject();
while (true) {
bool found = current_object->getOwnPropertyDescriptor(globalObject, propertyKey, desc);
NAPI_RETURN_IF_EXCEPTION(env);
if (found) {
break;
}
JSValue protoValue = current_object->getPrototype(globalObject);
NAPI_RETURN_IF_EXCEPTION(env);
JSObject* proto = protoValue.getObject();
if (!proto) {
break;
}
current_object = proto;
}
} else {
object->getOwnPropertyDescriptor(globalObject, key.toPropertyKey(globalObject), desc);
object->getOwnPropertyDescriptor(globalObject, propertyKey, desc);
NAPI_RETURN_IF_EXCEPTION(env);
}

bool include = true;
Expand All @@ -2107,6 +2118,7 @@ extern "C" napi_status napi_get_all_property_names(

if (include) {
filteredKeys->push(globalObject, key);
NAPI_RETURN_IF_EXCEPTION(env);
}
}
exportKeys = filteredKeys;
Expand Down
25 changes: 25 additions & 0 deletions test/napi/napi-app/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -1329,4 +1329,29 @@ nativeTests.test_threadsafe_function_microtask_order = async () => {
}
};

// napi_get_all_property_names with a key filter (enumerable/writable/configurable)
// walks property descriptors; when the target is a Proxy whose
// getOwnPropertyDescriptor trap throws inside that loop, the call must return
// napi_pending_exception rather than napi_ok.
nativeTests.test_get_all_property_names_throwing_proxy = () => {
// ownKeys succeeds so key collection passes and we enter the descriptor
// filter loop; getOwnPropertyDescriptor then throws inside it.
const throwingDescriptor = new Proxy(
{},
{
ownKeys() {
return ["a", "b"];
},
getOwnPropertyDescriptor() {
throw new Error("getOwnPropertyDescriptor trap threw");
},
},
);

// napi_key_own_only
nativeTests.test_napi_get_all_property_names_throws(undefined, throwingDescriptor, 1);
// napi_key_include_prototypes
nativeTests.test_napi_get_all_property_names_throws(undefined, throwingDescriptor, 0);
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.

module.exports = nativeTests;
43 changes: 43 additions & 0 deletions test/napi/napi-app/standalone_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3450,6 +3450,48 @@ test_node_api_sharedarraybuffer(const Napi::CallbackInfo &info) {
return ok(env);
}

// napi_get_all_property_names must propagate exceptions thrown by Proxy traps
// that run inside its descriptor filter loop. info[1] is the target object,
// info[2] is the napi_key_collection_mode (0 = include_prototypes,
// 1 = own_only).
static napi_value
test_napi_get_all_property_names_throws(const Napi::CallbackInfo &info) {
napi_env env = info.Env();

napi_value target = info[1];
int mode = info[2].As<Napi::Number>().Int32Value();
napi_key_collection_mode key_mode =
mode == 0 ? napi_key_include_prototypes : napi_key_own_only;

napi_value result = nullptr;
napi_status status = napi_get_all_property_names(
env, target, key_mode, napi_key_enumerable, napi_key_keep_numbers,
&result);
printf("napi_get_all_property_names(mode=%d) status -> %d\n", mode,
(int)status);

bool is_exception_pending = false;
NODE_API_CALL(env, napi_is_exception_pending(env, &is_exception_pending));
printf("napi_is_exception_pending -> %s\n",
is_exception_pending ? "true" : "false");

if (is_exception_pending) {
napi_value exception;
NODE_API_CALL(env, napi_get_and_clear_last_exception(env, &exception));
napi_value message_val;
if (napi_get_named_property(env, exception, "message", &message_val) ==
napi_ok) {
char message[256] = {0};
size_t message_len = 0;
napi_get_value_string_utf8(env, message_val, message,
sizeof(message) - 1, &message_len);
printf("exception message: %s\n", message);
}
}

return ok(env);
}

void register_standalone_tests(Napi::Env env, Napi::Object exports) {
REGISTER_FUNCTION(env, exports, test_typedarray_info_byte_offset);
REGISTER_FUNCTION(env, exports, test_dataview_info_byte_offset);
Expand Down Expand Up @@ -3525,6 +3567,7 @@ void register_standalone_tests(Napi::Env env, Napi::Object exports) {
REGISTER_FUNCTION(env, exports, test_node_api_set_prototype);
REGISTER_FUNCTION(env, exports, test_node_api_create_object_with_properties);
REGISTER_FUNCTION(env, exports, test_node_api_sharedarraybuffer);
REGISTER_FUNCTION(env, exports, test_napi_get_all_property_names_throws);
}

} // namespace napitests
7 changes: 7 additions & 0 deletions test/napi/napi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,13 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => {
expect(result).toContain(`own_only + skip_symbols|enumerable: status=0 keys=["x"]`);
expect(result).toContain(`own_only + skip_strings|enumerable: status=0 keys=[Symbol(s)]`);
});

it("returns napi_pending_exception when a Proxy trap throws during descriptor filtering", async () => {
const output = await checkSameOutput("test_get_all_property_names_throwing_proxy", []);
expect(output).toContain("napi_get_all_property_names(mode=1) status -> 10");
expect(output).toContain("napi_get_all_property_names(mode=0) status -> 10");
expect(output).toContain("exception message: getOwnPropertyDescriptor trap threw");
});
});

describe("napi_ref", () => {
Expand Down
Loading