From f5cfa09ca4e631522419c3e931f4a1a4b99bd52a Mon Sep 17 00:00:00 2001 From: Jason Ginchereau Date: Tue, 18 Apr 2017 15:02:00 -0700 Subject: [PATCH] n-api: remove napi_get_value_string_length() This API doesn't serve much purpose, and is only likely to cause confusion and bugs. The intention was that this would return the number of characters in a string independent of encoding, but that's not generally useful. In almost all cases, one of the encoding-specific napi_get_value_string_* APIs is more correct. (Pass a null buffer if only the encoded length is desired.) Anyway the current implementation of napi_get_value_string_length() is technically wrong: it returns the number of 2-byte code units of the UTF-16 encoding, but there are actually some characters that are encoded as two UTF-16 code units. Note the JavaScript String.prototype.length property returns the number of UTF-16 code units, which may be different from the number of characters. So, getting the true character count is not common with JavaScript, and is probably best left to specialized internationalization libraries. Backport-PR-URL: https://github.com/nodejs/node/pull/19447 PR-URL: https://github.com/nodejs/node/pull/12496 Fixes: https://github.com/nodejs/abi-stable-node/issues/226 Reviewed-By: Michael Dawson Reviewed-By: Jeremiah Senkpiel Reviewed-By: James M Snell --- src/node_api.cc | 15 --------------- src/node_api.h | 5 ----- test/addons-napi/test_string/test.js | 14 +++++++------- test/addons-napi/test_string/test_string.c | 6 +++--- 4 files changed, 10 insertions(+), 30 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index 76cc4e9d80f7a6..421765e3f0e08a 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -1720,21 +1720,6 @@ napi_status napi_get_value_bool(napi_env env, napi_value value, bool* result) { return napi_ok; } -// Gets the number of CHARACTERS in the string. -napi_status napi_get_value_string_length(napi_env env, - napi_value value, - size_t* result) { - NAPI_PREAMBLE(env); - CHECK_ARG(env, result); - - v8::Local val = v8impl::V8LocalValueFromJsValue(value); - RETURN_STATUS_IF_FALSE(env, val->IsString(), napi_string_expected); - - *result = val.As()->Length(); - - return GET_RETURN_STATUS(env); -} - // Copies a JavaScript string into a LATIN-1 string buffer. The result is the // number of bytes (excluding the null terminator) copied into buf. // A sufficient buffer size should be greater than the length of string, diff --git a/src/node_api.h b/src/node_api.h index 50b7da64c60179..29c5c513f72f59 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -171,11 +171,6 @@ NAPI_EXTERN napi_status napi_get_value_bool(napi_env env, napi_value value, bool* result); -// Gets the number of CHARACTERS in the string. -NAPI_EXTERN napi_status napi_get_value_string_length(napi_env env, - napi_value value, - size_t* result); - // Copies LATIN-1 encoded bytes from a string into a buffer. NAPI_EXTERN napi_status napi_get_value_string_latin1(napi_env env, napi_value value, diff --git a/test/addons-napi/test_string/test.js b/test/addons-napi/test_string/test.js index de6f9d98baab49..6e4962d129a6a4 100644 --- a/test/addons-napi/test_string/test.js +++ b/test/addons-napi/test_string/test.js @@ -9,7 +9,7 @@ const empty = ''; assert.strictEqual(test_string.TestLatin1(empty), empty); assert.strictEqual(test_string.TestUtf8(empty), empty); assert.strictEqual(test_string.TestUtf16(empty), empty); -assert.strictEqual(test_string.Length(empty), 0); +assert.strictEqual(test_string.Utf16Length(empty), 0); assert.strictEqual(test_string.Utf8Length(empty), 0); const str1 = 'hello world'; @@ -19,7 +19,7 @@ assert.strictEqual(test_string.TestUtf16(str1), str1); assert.strictEqual(test_string.TestLatin1Insufficient(str1), str1.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str1), str1.slice(0, 3)); assert.strictEqual(test_string.TestUtf16Insufficient(str1), str1.slice(0, 3)); -assert.strictEqual(test_string.Length(str1), 11); +assert.strictEqual(test_string.Utf16Length(str1), 11); assert.strictEqual(test_string.Utf8Length(str1), 11); const str2 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; @@ -29,7 +29,7 @@ assert.strictEqual(test_string.TestUtf16(str2), str2); assert.strictEqual(test_string.TestLatin1Insufficient(str2), str2.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str2), str2.slice(0, 3)); assert.strictEqual(test_string.TestUtf16Insufficient(str2), str2.slice(0, 3)); -assert.strictEqual(test_string.Length(str2), 62); +assert.strictEqual(test_string.Utf16Length(str2), 62); assert.strictEqual(test_string.Utf8Length(str2), 62); const str3 = '?!@#$%^&*()_+-=[]{}/.,<>\'"\\'; @@ -39,7 +39,7 @@ assert.strictEqual(test_string.TestUtf16(str3), str3); assert.strictEqual(test_string.TestLatin1Insufficient(str3), str3.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str3), str3.slice(0, 3)); assert.strictEqual(test_string.TestUtf16Insufficient(str3), str3.slice(0, 3)); -assert.strictEqual(test_string.Length(str3), 27); +assert.strictEqual(test_string.Utf16Length(str3), 27); assert.strictEqual(test_string.Utf8Length(str3), 27); const str4 = '¡¢£¤¥¦§¨©ª«¬­®¯°±²³´µ¶·¸¹º»¼½¾¿'; @@ -49,7 +49,7 @@ assert.strictEqual(test_string.TestUtf16(str4), str4); assert.strictEqual(test_string.TestLatin1Insufficient(str4), str4.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str4), str4.slice(0, 1)); assert.strictEqual(test_string.TestUtf16Insufficient(str4), str4.slice(0, 3)); -assert.strictEqual(test_string.Length(str4), 31); +assert.strictEqual(test_string.Utf16Length(str4), 31); assert.strictEqual(test_string.Utf8Length(str4), 62); const str5 = 'ÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖ×ØÙÚÛÜÝÞßàáâãäåæçèéêëìíîïðñòóôõö÷øùúûüýþ'; @@ -59,7 +59,7 @@ assert.strictEqual(test_string.TestUtf16(str5), str5); assert.strictEqual(test_string.TestLatin1Insufficient(str5), str5.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str5), str5.slice(0, 1)); assert.strictEqual(test_string.TestUtf16Insufficient(str5), str5.slice(0, 3)); -assert.strictEqual(test_string.Length(str5), 63); +assert.strictEqual(test_string.Utf16Length(str5), 63); assert.strictEqual(test_string.Utf8Length(str5), 126); const str6 = '\u{2003}\u{2101}\u{2001}\u{202}\u{2011}'; @@ -67,5 +67,5 @@ assert.strictEqual(test_string.TestUtf8(str6), str6); assert.strictEqual(test_string.TestUtf16(str6), str6); assert.strictEqual(test_string.TestUtf8Insufficient(str6), str6.slice(0, 1)); assert.strictEqual(test_string.TestUtf16Insufficient(str6), str6.slice(0, 3)); -assert.strictEqual(test_string.Length(str6), 5); +assert.strictEqual(test_string.Utf16Length(str6), 5); assert.strictEqual(test_string.Utf8Length(str6), 14); diff --git a/test/addons-napi/test_string/test_string.c b/test/addons-napi/test_string/test_string.c index 593902edd58f9d..5cd6d413a679fb 100644 --- a/test/addons-napi/test_string/test_string.c +++ b/test/addons-napi/test_string/test_string.c @@ -157,7 +157,7 @@ napi_value TestUtf16Insufficient(napi_env env, napi_callback_info info) { return output; } -napi_value Length(napi_env env, napi_callback_info info) { +napi_value Utf16Length(napi_env env, napi_callback_info info) { size_t argc = 1; napi_value args[1]; NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); @@ -171,7 +171,7 @@ napi_value Length(napi_env env, napi_callback_info info) { "Wrong type of argment. Expects a string."); size_t length; - NAPI_CALL(env, napi_get_value_string_length(env, args[0], &length)); + NAPI_CALL(env, napi_get_value_string_utf16(env, args[0], NULL, 0, &length)); napi_value output; NAPI_CALL(env, napi_create_number(env, (double)length, &output)); @@ -209,7 +209,7 @@ void Init(napi_env env, napi_value exports, napi_value module, void* priv) { DECLARE_NAPI_PROPERTY("TestUtf8Insufficient", TestUtf8Insufficient), DECLARE_NAPI_PROPERTY("TestUtf16", TestUtf16), DECLARE_NAPI_PROPERTY("TestUtf16Insufficient", TestUtf16Insufficient), - DECLARE_NAPI_PROPERTY("Length", Length), + DECLARE_NAPI_PROPERTY("Utf16Length", Utf16Length), DECLARE_NAPI_PROPERTY("Utf8Length", Utf8Length), };