From fd03278a6a119a8e8f2c1080319693ee4f0d1bd7 Mon Sep 17 00:00:00 2001 From: Sampson Gao Date: Mon, 11 Sep 2017 09:57:38 -0400 Subject: [PATCH] n-api: add optional string length parameters PR-URL: https://github.com/nodejs/node/pull/15343 Reviewed-By: Michael Dawson Reviewed-By: James M Snell Reviewed-By: Franziska Hinkelmann --- doc/api/n-api.md | 15 ++++++++++- src/node_api.cc | 26 ++++++++++++++++--- src/node_api.h | 6 ++++- test/addons-napi/4_object_factory/binding.c | 2 +- test/addons-napi/5_function_factory/binding.c | 6 ++--- test/addons-napi/6_object_wrap/myobject.cc | 4 +-- test/addons-napi/7_factory_wrap/binding.cc | 2 +- test/addons-napi/7_factory_wrap/myobject.cc | 4 +-- .../addons-napi/8_passing_wrapped/myobject.cc | 3 ++- test/addons-napi/test_constructor/binding.gyp | 4 +++ test/addons-napi/test_constructor/test2.js | 8 ++++++ .../test_constructor/test_constructor.c | 2 +- .../test_constructor/test_constructor_name.c | 23 ++++++++++++++++ .../test_env_sharing/compare_env.c | 2 +- test/addons-napi/test_fatal/test2.js | 18 +++++++++++++ test/addons-napi/test_fatal/test_fatal.c | 8 +++++- test/addons-napi/test_function/test.js | 11 +++++--- .../addons-napi/test_function/test_function.c | 23 +++++++++++++--- .../addons-napi/test_make_callback/binding.cc | 2 +- .../test_make_callback_recurse/binding.cc | 2 +- 20 files changed, 141 insertions(+), 30 deletions(-) create mode 100644 test/addons-napi/test_constructor/test2.js create mode 100644 test/addons-napi/test_constructor/test_constructor_name.c create mode 100644 test/addons-napi/test_fatal/test2.js diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 00bba55a4c..42db6e3a9e 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -552,11 +552,18 @@ thrown to immediately terminate the process. added: v8.2.0 --> ```C -NAPI_NO_RETURN void napi_fatal_error(const char* location, const char* message); +NAPI_NO_RETURN void napi_fatal_error(const char* location, + size_t location_len, + const char* message, + size_t message_len); ``` - `[in] location`: Optional location at which the error occurred. +- `[in] location_len`: The length of the location in bytes, or -1 if it is +null-terminated. - `[in] message`: The message associated with the error. +- `[in] message_len`: The length of the message in bytes, or -1 if it is +null-terminated. The function call does not return, the process will be terminated. @@ -1248,6 +1255,7 @@ added: v8.0.0 ```C napi_status napi_create_function(napi_env env, const char* utf8name, + size_t length, napi_callback cb, void* data, napi_value* result) @@ -1256,6 +1264,8 @@ napi_status napi_create_function(napi_env env, - `[in] env`: The environment that the API is invoked under. - `[in] utf8name`: A string representing the name of the function encoded as UTF8. +- `[in] length`: The length of the utf8name in bytes, or -1 if it is +null-terminated. - `[in] cb`: A function pointer to the native function to be invoked when the created function is invoked from JavaScript. - `[in] data`: Optional arbitrary context data to be passed into the native @@ -3026,6 +3036,7 @@ added: v8.0.0 ```C napi_status napi_define_class(napi_env env, const char* utf8name, + size_t length, napi_callback constructor, void* data, size_t property_count, @@ -3037,6 +3048,8 @@ napi_status napi_define_class(napi_env env, - `[in] utf8name`: Name of the JavaScript constructor function; this is not required to be the same as the C++ class name, though it is recommended for clarity. + - `[in] length`: The length of the utf8name in bytes, or -1 if it is +null-terminated. - `[in] constructor`: Callback function that handles constructing instances of the class. (This should be a static method on the class, not an actual C++ constructor function.) diff --git a/src/node_api.cc b/src/node_api.cc index 724d5c3ee9..cedf5ad7a4 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -933,12 +933,29 @@ napi_status napi_get_last_error_info(napi_env env, } NAPI_NO_RETURN void napi_fatal_error(const char* location, - const char* message) { - node::FatalError(location, message); + size_t location_len, + const char* message, + size_t message_len) { + char* location_string = const_cast(location); + char* message_string = const_cast(message); + if (location_len != -1) { + location_string = reinterpret_cast( + malloc(location_len * sizeof(char) + 1)); + strncpy(location_string, location, location_len); + location_string[location_len] = '\0'; + } + if (message_len != -1) { + message_string = reinterpret_cast( + malloc(message_len * sizeof(char) + 1)); + strncpy(message_string, message, message_len); + message_string[message_len] = '\0'; + } + node::FatalError(location_string, message_string); } napi_status napi_create_function(napi_env env, const char* utf8name, + size_t length, napi_callback cb, void* callback_data, napi_value* result) { @@ -965,7 +982,7 @@ napi_status napi_create_function(napi_env env, if (utf8name != nullptr) { v8::Local name_string; - CHECK_NEW_FROM_UTF8(env, name_string, utf8name); + CHECK_NEW_FROM_UTF8_LEN(env, name_string, utf8name, length); return_value->SetName(name_string); } @@ -976,6 +993,7 @@ napi_status napi_create_function(napi_env env, napi_status napi_define_class(napi_env env, const char* utf8name, + size_t length, napi_callback constructor, void* callback_data, size_t property_count, @@ -997,7 +1015,7 @@ napi_status napi_define_class(napi_env env, isolate, v8impl::FunctionCallbackWrapper::Invoke, cbdata); v8::Local name_string; - CHECK_NEW_FROM_UTF8(env, name_string, utf8name); + CHECK_NEW_FROM_UTF8_LEN(env, name_string, utf8name, length); tpl->SetClassName(name_string); size_t static_property_count = 0; diff --git a/src/node_api.h b/src/node_api.h index e1a40983a9..82f2e1900e 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -109,7 +109,9 @@ napi_get_last_error_info(napi_env env, const napi_extended_error_info** result); NAPI_EXTERN NAPI_NO_RETURN void napi_fatal_error(const char* location, - const char* message); + size_t location_len, + const char* message, + size_t message_len); // Getters for defined singletons NAPI_EXTERN napi_status napi_get_undefined(napi_env env, napi_value* result); @@ -154,6 +156,7 @@ NAPI_EXTERN napi_status napi_create_symbol(napi_env env, napi_value* result); NAPI_EXTERN napi_status napi_create_function(napi_env env, const char* utf8name, + size_t length, napi_callback cb, void* data, napi_value* result); @@ -336,6 +339,7 @@ NAPI_EXTERN napi_status napi_get_new_target(napi_env env, NAPI_EXTERN napi_status napi_define_class(napi_env env, const char* utf8name, + size_t length, napi_callback constructor, void* data, size_t property_count, diff --git a/test/addons-napi/4_object_factory/binding.c b/test/addons-napi/4_object_factory/binding.c index dedfd288f7..38b8ec8e1c 100644 --- a/test/addons-napi/4_object_factory/binding.c +++ b/test/addons-napi/4_object_factory/binding.c @@ -16,7 +16,7 @@ napi_value CreateObject(napi_env env, napi_callback_info info) { napi_value Init(napi_env env, napi_value exports) { NAPI_CALL(env, - napi_create_function(env, "exports", CreateObject, NULL, &exports)); + napi_create_function(env, "exports", -1, CreateObject, NULL, &exports)); return exports; } diff --git a/test/addons-napi/5_function_factory/binding.c b/test/addons-napi/5_function_factory/binding.c index c8751f1fa9..8cc41f6aac 100644 --- a/test/addons-napi/5_function_factory/binding.c +++ b/test/addons-napi/5_function_factory/binding.c @@ -4,21 +4,19 @@ napi_value MyFunction(napi_env env, napi_callback_info info) { napi_value str; NAPI_CALL(env, napi_create_string_utf8(env, "hello world", -1, &str)); - return str; } napi_value CreateFunction(napi_env env, napi_callback_info info) { napi_value fn; NAPI_CALL(env, - napi_create_function(env, "theFunction", MyFunction, NULL, &fn)); - + napi_create_function(env, "theFunction", -1, MyFunction, NULL, &fn)); return fn; } napi_value Init(napi_env env, napi_value exports) { NAPI_CALL(env, - napi_create_function(env, "exports", CreateFunction, NULL, &exports)); + napi_create_function(env, "exports", -1, CreateFunction, NULL, &exports)); return exports; } diff --git a/test/addons-napi/6_object_wrap/myobject.cc b/test/addons-napi/6_object_wrap/myobject.cc index 90815253ad..aca91877d3 100644 --- a/test/addons-napi/6_object_wrap/myobject.cc +++ b/test/addons-napi/6_object_wrap/myobject.cc @@ -22,8 +22,8 @@ void MyObject::Init(napi_env env, napi_value exports) { }; napi_value cons; - NAPI_CALL_RETURN_VOID(env, - napi_define_class(env, "MyObject", New, nullptr, 3, properties, &cons)); + NAPI_CALL_RETURN_VOID(env, napi_define_class( + env, "MyObject", -1, New, nullptr, 3, properties, &cons)); NAPI_CALL_RETURN_VOID(env, napi_create_reference(env, cons, 1, &constructor)); diff --git a/test/addons-napi/7_factory_wrap/binding.cc b/test/addons-napi/7_factory_wrap/binding.cc index 551bfef15d..c8fca4d536 100644 --- a/test/addons-napi/7_factory_wrap/binding.cc +++ b/test/addons-napi/7_factory_wrap/binding.cc @@ -16,7 +16,7 @@ napi_value Init(napi_env env, napi_value exports) { NAPI_CALL(env, MyObject::Init(env)); NAPI_CALL(env, - napi_create_function(env, "exports", CreateObject, NULL, &exports)); + napi_create_function(env, "exports", -1, CreateObject, NULL, &exports)); return exports; } diff --git a/test/addons-napi/7_factory_wrap/myobject.cc b/test/addons-napi/7_factory_wrap/myobject.cc index c6d538a7ce..4e1d79c1fe 100644 --- a/test/addons-napi/7_factory_wrap/myobject.cc +++ b/test/addons-napi/7_factory_wrap/myobject.cc @@ -21,8 +21,8 @@ napi_status MyObject::Init(napi_env env) { }; napi_value cons; - status = - napi_define_class(env, "MyObject", New, nullptr, 1, properties, &cons); + status = napi_define_class( + env, "MyObject", -1, New, nullptr, 1, properties, &cons); if (status != napi_ok) return status; status = napi_create_reference(env, cons, 1, &constructor); diff --git a/test/addons-napi/8_passing_wrapped/myobject.cc b/test/addons-napi/8_passing_wrapped/myobject.cc index 0c24d7696e..19cc7dd2a2 100644 --- a/test/addons-napi/8_passing_wrapped/myobject.cc +++ b/test/addons-napi/8_passing_wrapped/myobject.cc @@ -17,7 +17,8 @@ napi_status MyObject::Init(napi_env env) { napi_status status; napi_value cons; - status = napi_define_class(env, "MyObject", New, nullptr, 0, nullptr, &cons); + status = napi_define_class( + env, "MyObject", -1, New, nullptr, 0, nullptr, &cons); if (status != napi_ok) return status; status = napi_create_reference(env, cons, 1, &constructor); diff --git a/test/addons-napi/test_constructor/binding.gyp b/test/addons-napi/test_constructor/binding.gyp index 55140e7c37..1945a9fd5a 100644 --- a/test/addons-napi/test_constructor/binding.gyp +++ b/test/addons-napi/test_constructor/binding.gyp @@ -3,6 +3,10 @@ { "target_name": "test_constructor", "sources": [ "test_constructor.c" ] + }, + { + "target_name": "test_constructor_name", + "sources": [ "test_constructor_name.c" ] } ] } diff --git a/test/addons-napi/test_constructor/test2.js b/test/addons-napi/test_constructor/test2.js new file mode 100644 index 0000000000..64c03cbc68 --- /dev/null +++ b/test/addons-napi/test_constructor/test2.js @@ -0,0 +1,8 @@ +'use strict'; +const common = require('../../common'); +const assert = require('assert'); + +// Testing api calls for a constructor that defines properties +const TestConstructor = + require(`./build/${common.buildType}/test_constructor_name`); +assert.strictEqual(TestConstructor.name, 'MyObject'); diff --git a/test/addons-napi/test_constructor/test_constructor.c b/test/addons-napi/test_constructor/test_constructor.c index 73de5ad287..d742f2bcd1 100644 --- a/test/addons-napi/test_constructor/test_constructor.c +++ b/test/addons-napi/test_constructor/test_constructor.c @@ -77,7 +77,7 @@ napi_value Init(napi_env env, napi_value exports) { }; napi_value cons; - NAPI_CALL(env, napi_define_class(env, "MyObject", New, + NAPI_CALL(env, napi_define_class(env, "MyObject", -1, New, NULL, sizeof(properties)/sizeof(*properties), properties, &cons)); NAPI_CALL(env, napi_create_reference(env, cons, 1, &constructor_)); diff --git a/test/addons-napi/test_constructor/test_constructor_name.c b/test/addons-napi/test_constructor/test_constructor_name.c new file mode 100644 index 0000000000..b178e80f49 --- /dev/null +++ b/test/addons-napi/test_constructor/test_constructor_name.c @@ -0,0 +1,23 @@ +#include +#include "../common.h" + +napi_ref constructor_; + +napi_value New(napi_env env, napi_callback_info info) { + napi_value _this; + NAPI_CALL(env, napi_get_cb_info(env, info, NULL, NULL, &_this, NULL)); + + return _this; +} + +napi_value Init(napi_env env, napi_value exports) { + napi_value cons; + NAPI_CALL(env, napi_define_class( + env, "MyObject_Extra", 8, New, NULL, 0, NULL, &cons)); + + NAPI_CALL(env, + napi_create_reference(env, cons, 1, &constructor_)); + return cons; +} + +NAPI_MODULE(addon, Init) diff --git a/test/addons-napi/test_env_sharing/compare_env.c b/test/addons-napi/test_env_sharing/compare_env.c index b7e789ac49..da984b68f5 100644 --- a/test/addons-napi/test_env_sharing/compare_env.c +++ b/test/addons-napi/test_env_sharing/compare_env.c @@ -15,7 +15,7 @@ napi_value compare(napi_env env, napi_callback_info info) { } napi_value Init(napi_env env, napi_value exports) { - NAPI_CALL(env, napi_create_function(env, "exports", compare, NULL, &exports)); + NAPI_CALL(env, napi_create_function(env, "exports", -1, compare, NULL, &exports)); return exports; } diff --git a/test/addons-napi/test_fatal/test2.js b/test/addons-napi/test_fatal/test2.js new file mode 100644 index 0000000000..b9bde8f130 --- /dev/null +++ b/test/addons-napi/test_fatal/test2.js @@ -0,0 +1,18 @@ +'use strict'; +const common = require('../../common'); +const assert = require('assert'); +const child_process = require('child_process'); +const test_fatal = require(`./build/${common.buildType}/test_fatal`); + +// Test in a child process because the test code will trigger a fatal error +// that crashes the process. +if (process.argv[2] === 'child') { + test_fatal.TestStringLength(); + return; +} + +const p = child_process.spawnSync( + process.execPath, [ '--napi-modules', __filename, 'child' ]); +assert.ifError(p.error); +assert.ok(p.stderr.toString().includes( + 'FATAL ERROR: test_fatal::Test fatal message')); diff --git a/test/addons-napi/test_fatal/test_fatal.c b/test/addons-napi/test_fatal/test_fatal.c index 78d791cc91..6d8fa6ff86 100644 --- a/test/addons-napi/test_fatal/test_fatal.c +++ b/test/addons-napi/test_fatal/test_fatal.c @@ -2,13 +2,19 @@ #include "../common.h" napi_value Test(napi_env env, napi_callback_info info) { - napi_fatal_error("test_fatal::Test", "fatal message"); + napi_fatal_error("test_fatal::Test", -1, "fatal message", -1); + return NULL; +} + +napi_value TestStringLength(napi_env env, napi_callback_info info) { + napi_fatal_error("test_fatal::TestStringLength", 16, "fatal message", 13); return NULL; } napi_value Init(napi_env env, napi_value exports) { napi_property_descriptor properties[] = { DECLARE_NAPI_PROPERTY("Test", Test), + DECLARE_NAPI_PROPERTY("TestStringLength", TestStringLength), }; NAPI_CALL(env, napi_define_properties( diff --git a/test/addons-napi/test_function/test.js b/test/addons-napi/test_function/test.js index bdb9133adf..752e9965b2 100644 --- a/test/addons-napi/test_function/test.js +++ b/test/addons-napi/test_function/test.js @@ -9,20 +9,23 @@ const test_function = require(`./build/${common.buildType}/test_function`); function func1() { return 1; } -assert.strictEqual(test_function.Test(func1), 1); +assert.strictEqual(test_function.TestCall(func1), 1); function func2() { console.log('hello world!'); return null; } -assert.strictEqual(test_function.Test(func2), null); +assert.strictEqual(test_function.TestCall(func2), null); function func3(input) { return input + 1; } -assert.strictEqual(test_function.Test(func3, 1), 2); +assert.strictEqual(test_function.TestCall(func3, 1), 2); function func4(input) { return func3(input); } -assert.strictEqual(test_function.Test(func4, 1), 2); +assert.strictEqual(test_function.TestCall(func4, 1), 2); + +assert.strictEqual(test_function.TestName.name, 'Name'); +assert.strictEqual(test_function.TestNameShort.name, 'Name_'); diff --git a/test/addons-napi/test_function/test_function.c b/test/addons-napi/test_function/test_function.c index 76a8b05e03..99a699bfea 100644 --- a/test/addons-napi/test_function/test_function.c +++ b/test/addons-napi/test_function/test_function.c @@ -1,7 +1,7 @@ #include #include "../common.h" -napi_value Test(napi_env env, napi_callback_info info) { +napi_value TestCallFunction(napi_env env, napi_callback_info info) { size_t argc = 10; napi_value args[10]; NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); @@ -26,10 +26,25 @@ napi_value Test(napi_env env, napi_callback_info info) { return result; } +void TestFunctionName(napi_env env, napi_callback_info info) {} + napi_value Init(napi_env env, napi_value exports) { - napi_value fn; - NAPI_CALL(env, napi_create_function(env, NULL, Test, NULL, &fn)); - NAPI_CALL(env, napi_set_named_property(env, exports, "Test", fn)); + napi_value fn1; + NAPI_CALL(env, napi_create_function( + env, NULL, -1, TestCallFunction, NULL, &fn1)); + + napi_value fn2; + NAPI_CALL(env, napi_create_function( + env, "Name", -1, TestFunctionName, NULL, &fn2)); + + napi_value fn3; + NAPI_CALL(env, napi_create_function( + env, "Name_extra", 5, TestFunctionName, NULL, &fn3)); + + NAPI_CALL(env, napi_set_named_property(env, exports, "TestCall", fn1)); + NAPI_CALL(env, napi_set_named_property(env, exports, "TestName", fn2)); + NAPI_CALL(env, napi_set_named_property(env, exports, "TestNameShort", fn3)); + return exports; } diff --git a/test/addons-napi/test_make_callback/binding.cc b/test/addons-napi/test_make_callback/binding.cc index df2c4dc672..8a6d94985c 100644 --- a/test/addons-napi/test_make_callback/binding.cc +++ b/test/addons-napi/test_make_callback/binding.cc @@ -45,7 +45,7 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) { napi_value Init(napi_env env, napi_value exports) { napi_value fn; - NAPI_CALL(env, napi_create_function(env, NULL, MakeCallback, NULL, &fn)); + NAPI_CALL(env, napi_create_function(env, NULL, -1, MakeCallback, NULL, &fn)); NAPI_CALL(env, napi_set_named_property(env, exports, "makeCallback", fn)); return exports; } diff --git a/test/addons-napi/test_make_callback_recurse/binding.cc b/test/addons-napi/test_make_callback_recurse/binding.cc index 1e0c16c80e..212c9ec402 100644 --- a/test/addons-napi/test_make_callback_recurse/binding.cc +++ b/test/addons-napi/test_make_callback_recurse/binding.cc @@ -20,7 +20,7 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) { napi_value Init(napi_env env, napi_value exports) { napi_value fn; - NAPI_CALL(env, napi_create_function(env, NULL, MakeCallback, NULL, &fn)); + NAPI_CALL(env, napi_create_function(env, NULL, -1, MakeCallback, NULL, &fn)); NAPI_CALL(env, napi_set_named_property(env, exports, "makeCallback", fn)); return exports; }