From 858942ce31e9324b9c57fcd5854f9e4f286e075b Mon Sep 17 00:00:00 2001
From: Caleb Hearon <caleb@chearon.net>
Date: Tue, 25 Apr 2023 21:31:37 -0400
Subject: [PATCH] src: avoid calling into C++ with a null this

Avoid calling into C++ with a null this when exceptions are off

PR-URL: https://github.com/nodejs/node-addon-api/pull/1313
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com
---
 napi-inl.h         | 14 +++++++-------
 test/objectwrap.js |  9 +++++++++
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/napi-inl.h b/napi-inl.h
index a9d1fec3f..b1657d5fe 100644
--- a/napi-inl.h
+++ b/napi-inl.h
@@ -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();
   });
 }
 
@@ -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;
   });
 }
@@ -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;
   });
 }
@@ -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();
   });
 }
 
@@ -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();
   });
 }
 
@@ -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;
   });
 }
@@ -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;
   });
 }
diff --git a/test/objectwrap.js b/test/objectwrap.js
index 58528e9e3..a0d278062 100644
--- a/test/objectwrap.js
+++ b/test/objectwrap.js
@@ -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
@@ -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
@@ -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) => {