Skip to content

Commit 6697c51

Browse files
src,test: fix up null char * exception thrown
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: #1019 Reviewed-By: Nicola Del Gobbo <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
1 parent e02e8a4 commit 6697c51

File tree

3 files changed

+21
-8
lines changed

3 files changed

+21
-8
lines changed

napi-inl.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -900,10 +900,11 @@ inline String String::New(napi_env env, const std::u16string& val) {
900900
}
901901

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

914915
inline String String::New(napi_env env, const char16_t* val) {
915916
napi_value value;
917+
// TODO(@gabrielschulhof) Remove if-statement when core's error handling is
918+
// available in all supported versions.
919+
if (val == nullptr) {
920+
// Throw an error that looks like it came from core.
921+
NAPI_THROW_IF_FAILED(env, napi_invalid_arg, String());
922+
}
916923
napi_status status = napi_create_string_utf16(env, val, std::u16string(val).size(), &value);
917924
NAPI_THROW_IF_FAILED(env, status, String());
918925
return String(env, value);

test/name.cc

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,18 +82,24 @@ Value CheckSymbol(const CallbackInfo& info) {
8282
return Boolean::New(info.Env(), info[0].Type() == napi_symbol);
8383
}
8484

85-
void AssertErrorThrownWhenPassedNullptr(const CallbackInfo& info) {
85+
void NullStringShouldThrow(const CallbackInfo& info) {
8686
const char* nullStr = nullptr;
8787
String::New(info.Env(), nullStr);
8888
}
8989

90+
void NullString16ShouldThrow(const CallbackInfo& info) {
91+
const char16_t* nullStr = nullptr;
92+
String::New(info.Env(), nullStr);
93+
}
94+
9095
Object InitName(Env env) {
9196
Object exports = Object::New(env);
9297

9398
exports["echoString"] = Function::New(env, EchoString);
9499
exports["createString"] = Function::New(env, CreateString);
95-
exports["nullStringShouldThrow"] =
96-
Function::New(env, AssertErrorThrownWhenPassedNullptr);
100+
exports["nullStringShouldThrow"] = Function::New(env, NullStringShouldThrow);
101+
exports["nullString16ShouldThrow"] =
102+
Function::New(env, NullString16ShouldThrow);
97103
exports["checkString"] = Function::New(env, CheckString);
98104
exports["createSymbol"] = Function::New(env, CreateSymbol);
99105
exports["checkSymbol"] = Function::New(env, CheckSymbol);

test/name.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ function test(binding) {
99

1010

1111
assert.throws(binding.name.nullStringShouldThrow, {
12-
name: 'TypeError',
13-
message: 'String::New received a nullpointer as a value',
12+
name: 'Error',
13+
message: 'Error in native callback',
1414
});
1515
assert.ok(binding.name.checkString(expected, 'utf8'));
1616
assert.ok(binding.name.checkString(expected, 'utf16'));

0 commit comments

Comments
 (0)