Repository navigation
Missing napi_type_tag_object for Napi::External? #1293
Description
Activity
I can still call it but I need
external.As<Object>().CheckTypeTag()which means the API works for things that are not just
Objects.We discussed this in the 10 March Node.js Node-API meeting. The documentation for the underlying
napi_type_tag_objectcall says:[in] js_object: The JavaScript object to be marked.
This implies that the API is limited to objects and not externals. The team needs to clarify the semantics of this function because it may have implications on other engines that implement Node-API.
Path forward:
- Create a unit test in core that verifies that
napi_type_tag_objectandnapi_check_type_tagcan be used on externals. We expect this to work since a workaround was pointed out on Missing napi_type_tag_object for Napi::External? #1293 (comment) - If so, update the Node-API documentation for those two methods to say "The JavaScript object or external"
- Create a unit test in core that verifies that
Although I can see a usage of tagging for any javascript value (a bit like using the topmost bits of a c pointer), for externals it is a question of life of segfault.
I think the underlying api should be way more strict
- only create an external with a tag
- only extract the
void *if the tag matches
otherwise, how do I differentiate an external created by my library and one created elsewhere? and how do they do the same? probably a big security hole.
Opened nodejs/node#47141 for the core portion.
- added 3 commits that reference this issue
on Mar 21, 2023 @audetto type-tagging externals and wrapped objects is certainly a good practice. The problem is that type tags came along later than the wrapped objects and externals themselves. Thus, we can no longer go back and restrict their usage, because that would break all the code out there that already uses them, potentially unrestricted. We can certainly advocate for the usage of type tags so add-on authors eventually adopt the practice of type-tagging the pointers they expose to JS.
I full understand the constraints.
I think as well the lack of tags must be a source of many vulnerabilities.Reacted by Gabriel Schulhof- added a commit that references this issue
on Mar 27, 2023 Fixed this in #1298. If you think more work needs to be done, please reopen this issue!
- added a commit that references this issue
on Aug 11, 2023
#1261
This PR added support for
napi_type_tag_object, but it did so forNapi::Object.This API is mostly useful for
Napi::Externals which do no inherit fromNapi::Object.node-addon-api/napi.h
Lines 984 to 991 in b16c762
Should this api be moved down to
Napi::Value?