From ca9ea3a534e92b456949e5cb130e7d4710c4f6c9 Mon Sep 17 00:00:00 2001 From: Anthony Tuininga Date: Mon, 4 Mar 2019 15:02:27 -0700 Subject: [PATCH 1/4] Improve performance creating strings using N-API by ensuring that the strings are not internalized (see Node issue #26437). --- src/js_native_api_v8.cc | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 022ce104eb545c..b68be95b0284c1 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -1320,7 +1320,7 @@ napi_status napi_create_string_latin1(napi_env env, auto str_maybe = v8::String::NewFromOneByte(isolate, reinterpret_cast(str), - v8::NewStringType::kInternalized, + v8::NewStringType::kNormal, length); CHECK_MAYBE_EMPTY(env, str_maybe, napi_generic_failure); @@ -1335,10 +1335,14 @@ napi_status napi_create_string_utf8(napi_env env, CHECK_ENV(env); CHECK_ARG(env, result); - v8::Local s; - CHECK_NEW_FROM_UTF8_LEN(env, s, str, length); - - *result = v8impl::JsValueFromV8LocalValue(s); + auto isolate = env->isolate; + auto str_maybe = + v8::String::NewFromUtf8(isolate, + str, + v8::NewStringType::kNormal, + static_cast(length)); + CHECK_MAYBE_EMPTY(env, str_maybe, napi_generic_failure); + *result = v8impl::JsValueFromV8LocalValue(str_maybe.ToLocalChecked()); return napi_clear_last_error(env); } @@ -1353,7 +1357,7 @@ napi_status napi_create_string_utf16(napi_env env, auto str_maybe = v8::String::NewFromTwoByte(isolate, reinterpret_cast(str), - v8::NewStringType::kInternalized, + v8::NewStringType::kNormal, length); CHECK_MAYBE_EMPTY(env, str_maybe, napi_generic_failure); From b94b312757b4c8f83f2566504f132f2ade5778ac Mon Sep 17 00:00:00 2001 From: Anthony Tuininga Date: Mon, 4 Mar 2019 15:36:59 -0700 Subject: [PATCH 2/4] Add missing check. --- src/js_native_api_v8.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index b68be95b0284c1..8ce06261823b41 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -1335,6 +1335,9 @@ napi_status napi_create_string_utf8(napi_env env, CHECK_ENV(env); CHECK_ARG(env, result); + RETURN_STATUS_IF_FALSE(env, + (length == NAPI_AUTO_LENGTH) || length <= INT_MAX, + napi_invalid_arg); auto isolate = env->isolate; auto str_maybe = v8::String::NewFromUtf8(isolate, From 64171a93fc1495c6e6e7b40b7378ad7e1d211faf Mon Sep 17 00:00:00 2001 From: Anthony Tuininga Date: Mon, 4 Mar 2019 17:27:33 -0700 Subject: [PATCH 3/4] Add checks on length to other two string creation methods. --- src/js_native_api_v8.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 8ce06261823b41..8df89f9a38991f 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -1315,6 +1315,9 @@ napi_status napi_create_string_latin1(napi_env env, napi_value* result) { CHECK_ENV(env); CHECK_ARG(env, result); + RETURN_STATUS_IF_FALSE(env, + (length == NAPI_AUTO_LENGTH) || length <= INT_MAX, + napi_invalid_arg); auto isolate = env->isolate; auto str_maybe = @@ -1334,10 +1337,10 @@ napi_status napi_create_string_utf8(napi_env env, napi_value* result) { CHECK_ENV(env); CHECK_ARG(env, result); - RETURN_STATUS_IF_FALSE(env, (length == NAPI_AUTO_LENGTH) || length <= INT_MAX, napi_invalid_arg); + auto isolate = env->isolate; auto str_maybe = v8::String::NewFromUtf8(isolate, @@ -1355,6 +1358,9 @@ napi_status napi_create_string_utf16(napi_env env, napi_value* result) { CHECK_ENV(env); CHECK_ARG(env, result); + RETURN_STATUS_IF_FALSE(env, + (length == NAPI_AUTO_LENGTH) || length <= INT_MAX, + napi_invalid_arg); auto isolate = env->isolate; auto str_maybe = From ed195b6eeb859e4dcdca1a66e8ec8d179ac17e90 Mon Sep 17 00:00:00 2001 From: Anthony Tuininga Date: Mon, 4 Mar 2019 18:11:12 -0700 Subject: [PATCH 4/4] Added test cases for verifying additional checks on latin-1 and utf-16 string sizes. --- test/js-native-api/test_string/test.js | 8 ++++++ test/js-native-api/test_string/test_string.c | 28 ++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/test/js-native-api/test_string/test.js b/test/js-native-api/test_string/test.js index 5ce3d739c7a941..86fdc1640602bc 100644 --- a/test/js-native-api/test_string/test.js +++ b/test/js-native-api/test_string/test.js @@ -73,3 +73,11 @@ assert.strictEqual(test_string.Utf8Length(str6), 14); assert.throws(() => { test_string.TestLargeUtf8(); }, /^Error: Invalid argument$/); + +assert.throws(() => { + test_string.TestLargeLatin1(); +}, /^Error: Invalid argument$/); + +assert.throws(() => { + test_string.TestLargeUtf16(); +}, /^Error: Invalid argument$/); diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index 5b62d9de6cbcec..1f08559140e2a4 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -216,6 +216,32 @@ static napi_value TestLargeUtf8(napi_env env, napi_callback_info info) { return output; } +static napi_value TestLargeLatin1(napi_env env, napi_callback_info info) { + napi_value output; + if (SIZE_MAX > INT_MAX) { + NAPI_CALL(env, napi_create_string_latin1(env, "", ((size_t)INT_MAX) + 1, &output)); + } else { + // just throw the expected error as there is nothing to test + // in this case since we can't overflow + NAPI_CALL(env, napi_throw_error(env, NULL, "Invalid argument")); + } + + return output; +} + +static napi_value TestLargeUtf16(napi_env env, napi_callback_info info) { + napi_value output; + if (SIZE_MAX > INT_MAX) { + NAPI_CALL(env, napi_create_string_utf16(env, "", ((size_t)INT_MAX) + 1, &output)); + } else { + // just throw the expected error as there is nothing to test + // in this case since we can't overflow + NAPI_CALL(env, napi_throw_error(env, NULL, "Invalid argument")); + } + + return output; +} + EXTERN_C_START napi_value Init(napi_env env, napi_value exports) { napi_property_descriptor properties[] = { @@ -228,6 +254,8 @@ napi_value Init(napi_env env, napi_value exports) { DECLARE_NAPI_PROPERTY("Utf16Length", Utf16Length), DECLARE_NAPI_PROPERTY("Utf8Length", Utf8Length), DECLARE_NAPI_PROPERTY("TestLargeUtf8", TestLargeUtf8), + DECLARE_NAPI_PROPERTY("TestLargeLatin1", TestLargeLatin1), + DECLARE_NAPI_PROPERTY("TestLargeUtf16", TestLargeUtf16), }; NAPI_CALL(env, napi_define_properties(