WIP: add safer unwrap

napi-inl.h

@@ -3167,12 +3167,29 @@ inline ObjectWrap<T>::~ObjectWrap() {
   }
 }
 
+namespace details {
+  // This type resolution fails, if the T is not convertible to ObjectWrap<T>
+  template<class T>
+  using ObjectWrapSaferType = typename std::enable_if<
+    std::is_convertible<T*, ObjectWrap<T>*>::value, T>::type;
+
+  template<class T>
+#ifndef NAPI_UNSAFER_UNWRAP
+  ObjectWrapSaferType<T>*
+#else
+  T*
+#endif
+  SaferUnwrap(Object wrapper) {
+    T* unwrapped;
+    napi_status status = napi_unwrap(wrapper.Env(), wrapper, reinterpret_cast<void**>(&unwrapped));
+    NAPI_THROW_IF_FAILED(wrapper.Env(), status, nullptr);
+    return unwrapped;
+  }
+}
+
 template<typename T>
 inline T* ObjectWrap<T>::Unwrap(Object wrapper) {
-  T* unwrapped;
-  napi_status status = napi_unwrap(wrapper.Env(), wrapper, reinterpret_cast<void**>(&unwrapped));
-  NAPI_THROW_IF_FAILED(wrapper.Env(), status, nullptr);
-  return unwrapped;
+  return details::SaferUnwrap<T>(wrapper);
 }
 
 template <typename T>

test/binding.cc

@@ -51,6 +51,7 @@ Object InitThreadSafeFunction(Env env);
 #endif
 Object InitTypedArray(Env env);
 Object InitObjectWrap(Env env);
+Object InitObjectWrapSaferUnwrap(Env env);
 Object InitObjectWrapConstructorException(Env env);
 Object InitObjectWrapRemoveWrap(Env env);
 Object InitObjectReference(Env env);
@@ -108,6 +109,7 @@ Object Init(Env env, Object exports) {
 #endif
   exports.Set("typedarray", InitTypedArray(env));
   exports.Set("objectwrap", InitObjectWrap(env));
+  exports.Set("objectwrapSaferUnwrap", InitObjectWrapSaferUnwrap(env));
   exports.Set("objectwrapConstructorException",
       InitObjectWrapConstructorException(env));
   exports.Set("objectwrap_removewrap", InitObjectWrapRemoveWrap(env));

test/binding.gyp

@@ -45,6 +45,7 @@
         'objectwrap.cc',
         'objectwrap_constructor_exception.cc',
         'objectwrap-removewrap.cc',
+        'objectwrap-saferunwrap.cc',
         'objectreference.cc',
         'version_management.cc',
         'thunking_manual.cc',

test/objectwrap-saferunwrap.cc

@@ -0,0 +1,54 @@
+#define NAPI_UNSAFER_UNWRAP
+#include <napi.h>
+
+class TestSub {
+public:
+  virtual ~TestSub() {}
+  virtual int Get1() { return 1; }
+};
+
+class Test : public Napi::ObjectWrap<Test>, public TestSub {
+public:
+  Test(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Test>(info) {
+
+  }
+
+  static void Initialize(Napi::Env env, Napi::Object exports) {
+    exports.Set("Test", DefineClass(env, "Test", {}));
+  }
+};
+
+class TestReceiver : public Napi::ObjectWrap<TestReceiver> {
+public:
+  TestReceiver(const Napi::CallbackInfo& info) : Napi::ObjectWrap<TestReceiver>(info) {
+
+  }
+
+  Napi::Value Unwrap(const Napi::CallbackInfo& info) {
+
+    // This is okay => reinterpret_cast of the same type is valid
+    int n = Napi::ObjectWrap<Test>::Unwrap(info[0].ToObject())->Get1();
+
+    // This should fail => reinterpret_cast of a subclass (or any other type) is possible but NOT allowed
+    // If you're lucky you get a segfault. If not, you may get a stacktrace invoking `->A()`, but invoked `->B()` or any other method.
+    n = Napi::ObjectWrap<TestSub>::Unwrap(info[0].ToObject())->Get1();
+
+    // => do we have "should not compile" tests?
+    // without the fix, this will compile and you'll spend lot of time debugging :(
+
+    return Napi::Number::New(info.Env(), n);
+  }
+
+  static void Initialize(Napi::Env env, Napi::Object exports) {
+    exports.Set("TestReceiver", DefineClass(env, "TestReceiver", {
+      InstanceMethod("unwrap", &TestReceiver::Unwrap),
+    }));
+  }
+};
+
+
+Napi::Object InitObjectWrapSaferUnwrap(Napi::Env env) {
+  Napi::Object exports = Napi::Object::New(env);
+  Test::Initialize(env, exports);
+  return exports;
+}