diff --git a/napi-inl.h b/napi-inl.h index 7e4d158db..e29fa6ef5 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -3167,12 +3167,29 @@ inline ObjectWrap::~ObjectWrap() { } } +namespace details { + // This type resolution fails, if the T is not convertible to ObjectWrap + template + using ObjectWrapSaferType = typename std::enable_if< + std::is_convertible*>::value, T>::type; + + template +#ifndef NAPI_UNSAFER_UNWRAP + ObjectWrapSaferType* +#else + T* +#endif + SaferUnwrap(Object wrapper) { + T* unwrapped; + napi_status status = napi_unwrap(wrapper.Env(), wrapper, reinterpret_cast(&unwrapped)); + NAPI_THROW_IF_FAILED(wrapper.Env(), status, nullptr); + return unwrapped; + } +} + template inline T* ObjectWrap::Unwrap(Object wrapper) { - T* unwrapped; - napi_status status = napi_unwrap(wrapper.Env(), wrapper, reinterpret_cast(&unwrapped)); - NAPI_THROW_IF_FAILED(wrapper.Env(), status, nullptr); - return unwrapped; + return details::SaferUnwrap(wrapper); } template diff --git a/test/binding.cc b/test/binding.cc index ea1094638..3e32c447a 100644 --- a/test/binding.cc +++ b/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)); diff --git a/test/binding.gyp b/test/binding.gyp index 8dafe13fc..95ed0b73a 100644 --- a/test/binding.gyp +++ b/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', diff --git a/test/objectwrap-saferunwrap.cc b/test/objectwrap-saferunwrap.cc new file mode 100644 index 000000000..8ea8130b9 --- /dev/null +++ b/test/objectwrap-saferunwrap.cc @@ -0,0 +1,54 @@ +#define NAPI_UNSAFER_UNWRAP +#include + +class TestSub { +public: + virtual ~TestSub() {} + virtual int Get1() { return 1; } +}; + +class Test : public Napi::ObjectWrap, public TestSub { +public: + Test(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) { + + } + + static void Initialize(Napi::Env env, Napi::Object exports) { + exports.Set("Test", DefineClass(env, "Test", {})); + } +}; + +class TestReceiver : public Napi::ObjectWrap { +public: + TestReceiver(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) { + + } + + Napi::Value Unwrap(const Napi::CallbackInfo& info) { + + // This is okay => reinterpret_cast of the same type is valid + int n = Napi::ObjectWrap::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::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; +}