From 060c95a3b1b1d26396df7cf6d19472a8bcaad9e8 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Tue, 30 Jun 2020 13:58:00 -0700 Subject: [PATCH] test: add n-api null checks for conversions Add assertions that conversion and coercion N-APIs return appropriate error statuses when given `NULL`s for parameters they expect to not be `NULL`. For `napi_get_value_string_*` this also checks that it returns `napi_string_expected` when passed a `napi_value` not containing a string. PR-URL: https://github.com/nodejs/node/pull/34142 Reviewed-By: Anna Henningsen Reviewed-By: Michael Dawson --- .../test_conversions/binding.gyp | 4 +- test/js-native-api/test_conversions/test.js | 78 ++++++++++++++ .../test_conversions/test_conversions.c | 3 + .../test_conversions/test_null.c | 102 ++++++++++++++++++ .../test_conversions/test_null.h | 8 ++ 5 files changed, 194 insertions(+), 1 deletion(-) create mode 100644 test/js-native-api/test_conversions/test_null.c create mode 100644 test/js-native-api/test_conversions/test_null.h diff --git a/test/js-native-api/test_conversions/binding.gyp b/test/js-native-api/test_conversions/binding.gyp index e4b1889fecb728..f1640c6638e41e 100644 --- a/test/js-native-api/test_conversions/binding.gyp +++ b/test/js-native-api/test_conversions/binding.gyp @@ -4,7 +4,9 @@ "target_name": "test_conversions", "sources": [ "../entry_point.c", - "test_conversions.c" + "../common.c", + "test_conversions.c", + "test_null.c", ] } ] diff --git a/test/js-native-api/test_conversions/test.js b/test/js-native-api/test_conversions/test.js index b9e43da0c5ed20..2fd6ace593840e 100644 --- a/test/js-native-api/test_conversions/test.js +++ b/test/js-native-api/test_conversions/test.js @@ -138,3 +138,81 @@ assert.strictEqual(test.toString({ toString: () => 'test' }), 'test'); assert.strictEqual(test.toString([]), ''); assert.strictEqual(test.toString([ 1, 2, 3 ]), '1,2,3'); assert.throws(() => test.toString(testSym), TypeError); + +assert.deepStrictEqual(test.testNull.getValueBool(), { + envIsNull: 'Invalid argument', + valueIsNull: 'Invalid argument', + resultIsNull: 'Invalid argument', + inputTypeCheck: 'A boolean was expected' +}); + +assert.deepStrictEqual(test.testNull.getValueInt32(), { + envIsNull: 'Invalid argument', + valueIsNull: 'Invalid argument', + resultIsNull: 'Invalid argument', + inputTypeCheck: 'A number was expected' +}); + +assert.deepStrictEqual(test.testNull.getValueUint32(), { + envIsNull: 'Invalid argument', + valueIsNull: 'Invalid argument', + resultIsNull: 'Invalid argument', + inputTypeCheck: 'A number was expected' +}); + +assert.deepStrictEqual(test.testNull.getValueInt64(), { + envIsNull: 'Invalid argument', + valueIsNull: 'Invalid argument', + resultIsNull: 'Invalid argument', + inputTypeCheck: 'A number was expected' +}); + + +assert.deepStrictEqual(test.testNull.getValueDouble(), { + envIsNull: 'Invalid argument', + valueIsNull: 'Invalid argument', + resultIsNull: 'Invalid argument', + inputTypeCheck: 'A number was expected' +}); + +assert.deepStrictEqual(test.testNull.coerceToBool(), { + envIsNull: 'Invalid argument', + valueIsNull: 'Invalid argument', + resultIsNull: 'Invalid argument', + inputTypeCheck: 'napi_ok' +}); + +assert.deepStrictEqual(test.testNull.coerceToObject(), { + envIsNull: 'Invalid argument', + valueIsNull: 'Invalid argument', + resultIsNull: 'Invalid argument', + inputTypeCheck: 'napi_ok' +}); + +assert.deepStrictEqual(test.testNull.coerceToString(), { + envIsNull: 'Invalid argument', + valueIsNull: 'Invalid argument', + resultIsNull: 'Invalid argument', + inputTypeCheck: 'napi_ok' +}); + +assert.deepStrictEqual(test.testNull.getValueStringUtf8(), { + envIsNull: 'Invalid argument', + valueIsNull: 'Invalid argument', + wrongTypeIn: 'A string was expected', + bufAndOutLengthIsNull: 'Invalid argument' +}); + +assert.deepStrictEqual(test.testNull.getValueStringLatin1(), { + envIsNull: 'Invalid argument', + valueIsNull: 'Invalid argument', + wrongTypeIn: 'A string was expected', + bufAndOutLengthIsNull: 'Invalid argument' +}); + +assert.deepStrictEqual(test.testNull.getValueStringUtf16(), { + envIsNull: 'Invalid argument', + valueIsNull: 'Invalid argument', + wrongTypeIn: 'A string was expected', + bufAndOutLengthIsNull: 'Invalid argument' +}); diff --git a/test/js-native-api/test_conversions/test_conversions.c b/test/js-native-api/test_conversions/test_conversions.c index 85f7783d627aae..4f8561a9acf13a 100644 --- a/test/js-native-api/test_conversions/test_conversions.c +++ b/test/js-native-api/test_conversions/test_conversions.c @@ -1,5 +1,6 @@ #include #include "../common.h" +#include "test_null.h" static napi_value AsBool(napi_env env, napi_callback_info info) { size_t argc = 1; @@ -149,6 +150,8 @@ napi_value Init(napi_env env, napi_value exports) { NAPI_CALL(env, napi_define_properties( env, exports, sizeof(descriptors) / sizeof(*descriptors), descriptors)); + init_test_null(env, exports); + return exports; } EXTERN_C_END diff --git a/test/js-native-api/test_conversions/test_null.c b/test/js-native-api/test_conversions/test_null.c new file mode 100644 index 00000000000000..9cf73bc2d4980f --- /dev/null +++ b/test/js-native-api/test_conversions/test_null.c @@ -0,0 +1,102 @@ +#include + +#include "../common.h" +#include "test_null.h" + +#define GEN_NULL_CHECK_BINDING(binding_name, output_type, api) \ + static napi_value binding_name(napi_env env, napi_callback_info info) { \ + napi_value return_value; \ + output_type result; \ + NAPI_CALL(env, napi_create_object(env, &return_value)); \ + add_returned_status(env, \ + "envIsNull", \ + return_value, \ + "Invalid argument", \ + napi_invalid_arg, \ + api(NULL, return_value, &result)); \ + api(env, NULL, &result); \ + add_last_status(env, "valueIsNull", return_value); \ + api(env, return_value, NULL); \ + add_last_status(env, "resultIsNull", return_value); \ + api(env, return_value, &result); \ + add_last_status(env, "inputTypeCheck", return_value); \ + return return_value; \ + } + +GEN_NULL_CHECK_BINDING(GetValueBool, bool, napi_get_value_bool) +GEN_NULL_CHECK_BINDING(GetValueInt32, int32_t, napi_get_value_int32) +GEN_NULL_CHECK_BINDING(GetValueUint32, uint32_t, napi_get_value_uint32) +GEN_NULL_CHECK_BINDING(GetValueInt64, int64_t, napi_get_value_int64) +GEN_NULL_CHECK_BINDING(GetValueDouble, double, napi_get_value_double) +GEN_NULL_CHECK_BINDING(CoerceToBool, napi_value, napi_coerce_to_bool) +GEN_NULL_CHECK_BINDING(CoerceToObject, napi_value, napi_coerce_to_object) +GEN_NULL_CHECK_BINDING(CoerceToString, napi_value, napi_coerce_to_string) + +#define GEN_NULL_CHECK_STRING_BINDING(binding_name, arg_type, api) \ + static napi_value binding_name(napi_env env, napi_callback_info info) { \ + napi_value return_value; \ + NAPI_CALL(env, napi_create_object(env, &return_value)); \ + arg_type buf1[4]; \ + size_t length1 = 3; \ + add_returned_status(env, \ + "envIsNull", \ + return_value, \ + "Invalid argument", \ + napi_invalid_arg, \ + api(NULL, return_value, buf1, length1, &length1)); \ + arg_type buf2[4]; \ + size_t length2 = 3; \ + api(env, NULL, buf2, length2, &length2); \ + add_last_status(env, "valueIsNull", return_value); \ + api(env, return_value, NULL, 3, NULL); \ + add_last_status(env, "wrongTypeIn", return_value); \ + napi_value string; \ + NAPI_CALL(env, \ + napi_create_string_utf8(env, \ + "Something", \ + NAPI_AUTO_LENGTH, \ + &string)); \ + api(env, string, NULL, 3, NULL); \ + add_last_status(env, "bufAndOutLengthIsNull", return_value); \ + return return_value; \ + } + +GEN_NULL_CHECK_STRING_BINDING(GetValueStringUtf8, + char, + napi_get_value_string_utf8) +GEN_NULL_CHECK_STRING_BINDING(GetValueStringLatin1, + char, + napi_get_value_string_latin1) +GEN_NULL_CHECK_STRING_BINDING(GetValueStringUtf16, + char16_t, + napi_get_value_string_utf16) + +void init_test_null(napi_env env, napi_value exports) { + napi_value test_null; + + const napi_property_descriptor test_null_props[] = { + DECLARE_NAPI_PROPERTY("getValueBool", GetValueBool), + DECLARE_NAPI_PROPERTY("getValueInt32", GetValueInt32), + DECLARE_NAPI_PROPERTY("getValueUint32", GetValueUint32), + DECLARE_NAPI_PROPERTY("getValueInt64", GetValueInt64), + DECLARE_NAPI_PROPERTY("getValueDouble", GetValueDouble), + DECLARE_NAPI_PROPERTY("coerceToBool", CoerceToBool), + DECLARE_NAPI_PROPERTY("coerceToObject", CoerceToObject), + DECLARE_NAPI_PROPERTY("coerceToString", CoerceToString), + DECLARE_NAPI_PROPERTY("getValueStringUtf8", GetValueStringUtf8), + DECLARE_NAPI_PROPERTY("getValueStringLatin1", GetValueStringLatin1), + DECLARE_NAPI_PROPERTY("getValueStringUtf16", GetValueStringUtf16), + }; + + NAPI_CALL_RETURN_VOID(env, napi_create_object(env, &test_null)); + NAPI_CALL_RETURN_VOID(env, napi_define_properties( + env, test_null, sizeof(test_null_props) / sizeof(*test_null_props), + test_null_props)); + + const napi_property_descriptor test_null_set = { + "testNull", NULL, NULL, NULL, NULL, test_null, napi_enumerable, NULL + }; + + NAPI_CALL_RETURN_VOID(env, + napi_define_properties(env, exports, 1, &test_null_set)); +} diff --git a/test/js-native-api/test_conversions/test_null.h b/test/js-native-api/test_conversions/test_null.h new file mode 100644 index 00000000000000..977efdd534c4ae --- /dev/null +++ b/test/js-native-api/test_conversions/test_null.h @@ -0,0 +1,8 @@ +#ifndef TEST_JS_NATIVE_API_TEST_CONVERSIONS_TEST_NULL_H_ +#define TEST_JS_NATIVE_API_TEST_CONVERSIONS_TEST_NULL_H_ + +#include + +void init_test_null(napi_env env, napi_value exports); + +#endif // TEST_JS_NATIVE_API_TEST_CONVERSIONS_TEST_NULL_H_