Skip to content

Commit

Permalink
src,test: fix up null char * exception thrown
Browse files Browse the repository at this point in the history
Throw an exception when receiving a null pointer for the `char *` and
`char16_t *` overloads of `String::New` that looks identical to an
error that core would have thrown under the circumstances
(`napi_invalid_arg`).

Also, rename the test methods to conform with our naming convention.

PR-URL: nodejs#1019
Reviewed-By: Nicola Del Gobbo <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
  • Loading branch information
gabrielschulhof authored and Deepak Rajamohan committed Sep 23, 2021
1 parent e937efb commit fe950f6
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 8 deletions.
13 changes: 10 additions & 3 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -900,10 +900,11 @@ inline String String::New(napi_env env, const std::u16string& val) {
}

inline String String::New(napi_env env, const char* val) {
// TODO(@gabrielschulhof) Remove if-statement when core's error handling is
// available in all supported versions.
if (val == nullptr) {
NAPI_THROW(
TypeError::New(env, "String::New received a nullpointer as a value"),
Napi::String());
// Throw an error that looks like it came from core.
NAPI_THROW_IF_FAILED(env, napi_invalid_arg, String());
}
napi_value value;
napi_status status = napi_create_string_utf8(env, val, std::strlen(val), &value);
Expand All @@ -913,6 +914,12 @@ inline String String::New(napi_env env, const char* val) {

inline String String::New(napi_env env, const char16_t* val) {
napi_value value;
// TODO(@gabrielschulhof) Remove if-statement when core's error handling is
// available in all supported versions.
if (val == nullptr) {
// Throw an error that looks like it came from core.
NAPI_THROW_IF_FAILED(env, napi_invalid_arg, String());
}
napi_status status = napi_create_string_utf16(env, val, std::u16string(val).size(), &value);
NAPI_THROW_IF_FAILED(env, status, String());
return String(env, value);
Expand Down
12 changes: 9 additions & 3 deletions test/name.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,24 @@ Value CheckSymbol(const CallbackInfo& info) {
return Boolean::New(info.Env(), info[0].Type() == napi_symbol);
}

void AssertErrorThrownWhenPassedNullptr(const CallbackInfo& info) {
void NullStringShouldThrow(const CallbackInfo& info) {
const char* nullStr = nullptr;
String::New(info.Env(), nullStr);
}

void NullString16ShouldThrow(const CallbackInfo& info) {
const char16_t* nullStr = nullptr;
String::New(info.Env(), nullStr);
}

Object InitName(Env env) {
Object exports = Object::New(env);

exports["echoString"] = Function::New(env, EchoString);
exports["createString"] = Function::New(env, CreateString);
exports["nullStringShouldThrow"] =
Function::New(env, AssertErrorThrownWhenPassedNullptr);
exports["nullStringShouldThrow"] = Function::New(env, NullStringShouldThrow);
exports["nullString16ShouldThrow"] =
Function::New(env, NullString16ShouldThrow);
exports["checkString"] = Function::New(env, CheckString);
exports["createSymbol"] = Function::New(env, CreateSymbol);
exports["checkSymbol"] = Function::New(env, CheckSymbol);
Expand Down
4 changes: 2 additions & 2 deletions test/name.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ function test(binding) {


assert.throws(binding.name.nullStringShouldThrow, {
name: 'TypeError',
message: 'String::New received a nullpointer as a value',
name: 'Error',
message: 'Error in native callback',
});
assert.ok(binding.name.checkString(expected, 'utf8'));
assert.ok(binding.name.checkString(expected, 'utf16'));
Expand Down

0 comments on commit fe950f6

Please sign in to comment.