Skip to content

Commit

Permalink
src: avoid calling into C++ with a null this
Browse files Browse the repository at this point in the history
Avoid calling into C++ with a null this when exceptions are off

PR-URL: #1313
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com
  • Loading branch information
chearon authored and mhdawson committed May 5, 2023
1 parent 64f6515 commit 858942c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 7 deletions.
14 changes: 7 additions & 7 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ napi_value TemplatedInstanceCallback(napi_env env,
return details::WrapCallback([&] {
CallbackInfo cbInfo(env, info);
T* instance = T::Unwrap(cbInfo.This().As<Object>());
return (instance->*UnwrapCallback)(cbInfo);
return instance ? (instance->*UnwrapCallback)(cbInfo) : Napi::Value();
});
}

Expand All @@ -175,7 +175,7 @@ napi_value TemplatedInstanceVoidCallback(napi_env env, napi_callback_info info)
return details::WrapCallback([&] {
CallbackInfo cbInfo(env, info);
T* instance = T::Unwrap(cbInfo.This().As<Object>());
(instance->*UnwrapCallback)(cbInfo);
if (instance) (instance->*UnwrapCallback)(cbInfo);
return nullptr;
});
}
Expand Down Expand Up @@ -4356,7 +4356,7 @@ inline napi_value InstanceWrap<T>::InstanceVoidMethodCallbackWrapper(
callbackInfo.SetData(callbackData->data);
T* instance = T::Unwrap(callbackInfo.This().As<Object>());
auto cb = callbackData->callback;
(instance->*cb)(callbackInfo);
if (instance) (instance->*cb)(callbackInfo);
return nullptr;
});
}
Expand All @@ -4371,7 +4371,7 @@ inline napi_value InstanceWrap<T>::InstanceMethodCallbackWrapper(
callbackInfo.SetData(callbackData->data);
T* instance = T::Unwrap(callbackInfo.This().As<Object>());
auto cb = callbackData->callback;
return (instance->*cb)(callbackInfo);
return instance ? (instance->*cb)(callbackInfo) : Napi::Value();
});
}

Expand All @@ -4385,7 +4385,7 @@ inline napi_value InstanceWrap<T>::InstanceGetterCallbackWrapper(
callbackInfo.SetData(callbackData->data);
T* instance = T::Unwrap(callbackInfo.This().As<Object>());
auto cb = callbackData->getterCallback;
return (instance->*cb)(callbackInfo);
return instance ? (instance->*cb)(callbackInfo) : Napi::Value();
});
}

Expand All @@ -4399,7 +4399,7 @@ inline napi_value InstanceWrap<T>::InstanceSetterCallbackWrapper(
callbackInfo.SetData(callbackData->data);
T* instance = T::Unwrap(callbackInfo.This().As<Object>());
auto cb = callbackData->setterCallback;
(instance->*cb)(callbackInfo, callbackInfo[0]);
if (instance) (instance->*cb)(callbackInfo, callbackInfo[0]);
return nullptr;
});
}
Expand All @@ -4411,7 +4411,7 @@ inline napi_value InstanceWrap<T>::WrappedMethod(
return details::WrapCallback([&] {
const CallbackInfo cbInfo(env, info);
T* instance = T::Unwrap(cbInfo.This().As<Object>());
(instance->*method)(cbInfo, cbInfo[0]);
if (instance) (instance->*method)(cbInfo, cbInfo[0]);
return nullptr;
});
}
Expand Down
9 changes: 9 additions & 0 deletions test/objectwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ async function test (binding) {
obj.testSetter = 'instance getter 2';
assert.strictEqual(obj.testGetter, 'instance getter 2');
assert.strictEqual(obj.testGetterT, 'instance getter 2');

assert.throws(() => clazz.prototype.testGetter, /Invalid argument/);
assert.throws(() => clazz.prototype.testGetterT, /Invalid argument/);
}

// read write-only
Expand Down Expand Up @@ -61,6 +64,9 @@ async function test (binding) {

obj.testGetSetT = 'instance getset 4';
assert.strictEqual(obj.testGetSetT, 'instance getset 4');

assert.throws(() => { clazz.prototype.testGetSet = 'instance getset'; }, /Invalid argument/);
assert.throws(() => { clazz.prototype.testGetSetT = 'instance getset'; }, /Invalid argument/);
}

// rw symbol
Expand Down Expand Up @@ -98,6 +104,9 @@ async function test (binding) {
assert.strictEqual(obj.testMethodT(), 'method<>(const char*)');
obj[clazz.kTestVoidMethodTInternal]('method<>(Symbol)');
assert.strictEqual(obj[clazz.kTestMethodTInternal](), 'method<>(Symbol)');
assert.throws(() => clazz.prototype.testMethod('method'));
assert.throws(() => clazz.prototype.testMethodT());
assert.throws(() => clazz.prototype.testVoidMethodT('method<>(const char*)'));
};

const testEnumerables = (obj, clazz) => {
Expand Down

0 comments on commit 858942c

Please sign in to comment.