From c1ff2936f97f4e103c89fe71d1dba7ab84d29cdb Mon Sep 17 00:00:00 2001 From: Luciano Martorella Date: Tue, 23 Oct 2018 09:43:47 +0200 Subject: [PATCH] src: fix missing void*data usage in PropertyDescriptors PR-URL: https://github.com/nodejs/node-addon-api/pull/374 Reviewed-By: Michael Dawson Reviewed-By: Gabriel Schulhof --- napi-inl.deprecated.h | 4 ++-- napi-inl.h | 19 +++++++++++-------- test/object/object.cc | 26 ++++++++++++++++++++++++++ test/object/object.js | 11 +++++++++++ 4 files changed, 50 insertions(+), 10 deletions(-) diff --git a/napi-inl.deprecated.h b/napi-inl.deprecated.h index d00174357..f19aca76b 100644 --- a/napi-inl.deprecated.h +++ b/napi-inl.deprecated.h @@ -73,7 +73,7 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(const char* utf8name, void* /*data*/) { typedef details::AccessorCallbackData CbData; // TODO: Delete when the function is destroyed - auto callbackData = new CbData({ getter, setter }); + auto callbackData = new CbData({ getter, setter, nullptr }); return PropertyDescriptor({ utf8name, @@ -104,7 +104,7 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(napi_value name, void* /*data*/) { typedef details::AccessorCallbackData CbData; // TODO: Delete when the function is destroyed - auto callbackData = new CbData({ getter, setter }); + auto callbackData = new CbData({ getter, setter, nullptr }); return PropertyDescriptor({ nullptr, diff --git a/napi-inl.h b/napi-inl.h index b831f34c9..261a261ee 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -176,6 +176,7 @@ struct AccessorCallbackData { CallbackInfo callbackInfo(env, info); AccessorCallbackData* callbackData = static_cast(callbackInfo.Data()); + callbackInfo.SetData(callbackData->data); return callbackData->getterCallback(callbackInfo); }); } @@ -186,6 +187,7 @@ struct AccessorCallbackData { CallbackInfo callbackInfo(env, info); AccessorCallbackData* callbackData = static_cast(callbackInfo.Data()); + callbackInfo.SetData(callbackData->data); callbackData->setterCallback(callbackInfo); return nullptr; }); @@ -193,6 +195,7 @@ struct AccessorCallbackData { Getter getterCallback; Setter setterCallback; + void* data; }; } // namespace details @@ -2603,9 +2606,9 @@ PropertyDescriptor::Accessor(Napi::Env env, const char* utf8name, Getter getter, napi_property_attributes attributes, - void* /*data*/) { + void* data) { typedef details::CallbackData CbData; - auto callbackData = new CbData({ getter, nullptr }); + auto callbackData = new CbData({ getter, data }); napi_status status = AttachData(env, object, callbackData); NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor()); @@ -2638,9 +2641,9 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(Napi::Env env, Name name, Getter getter, napi_property_attributes attributes, - void* /*data*/) { + void* data) { typedef details::CallbackData CbData; - auto callbackData = new CbData({ getter, nullptr }); + auto callbackData = new CbData({ getter, data }); napi_status status = AttachData(env, object, callbackData); NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor()); @@ -2664,9 +2667,9 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(Napi::Env env, Getter getter, Setter setter, napi_property_attributes attributes, - void* /*data*/) { + void* data) { typedef details::AccessorCallbackData CbData; - auto callbackData = new CbData({ getter, setter }); + auto callbackData = new CbData({ getter, setter, data }); napi_status status = AttachData(env, object, callbackData); NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor()); @@ -2701,9 +2704,9 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(Napi::Env env, Getter getter, Setter setter, napi_property_attributes attributes, - void* /*data*/) { + void* data) { typedef details::AccessorCallbackData CbData; - auto callbackData = new CbData({ getter, setter }); + auto callbackData = new CbData({ getter, setter, data }); napi_status status = AttachData(env, object, callbackData); NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor()); diff --git a/test/object/object.cc b/test/object/object.cc index 0dfc61818..c94a4215c 100644 --- a/test/object/object.cc +++ b/test/object/object.cc @@ -33,6 +33,10 @@ Value HasPropertyWithCStyleString(const CallbackInfo& info); Value HasPropertyWithCppStyleString(const CallbackInfo& info); static bool testValue = true; +// Used to test void* Data() integrity +struct UserDataHolder { + int32_t value; +}; Value TestGetter(const CallbackInfo& info) { return Boolean::New(info.Env(), testValue); @@ -42,6 +46,16 @@ void TestSetter(const CallbackInfo& info) { testValue = info[0].As(); } +Value TestGetterWithUserData(const CallbackInfo& info) { + const UserDataHolder* holder = reinterpret_cast(info.Data()); + return Number::New(info.Env(), holder->value); +} + +void TestSetterWithUserData(const CallbackInfo& info) { + UserDataHolder* holder = reinterpret_cast(info.Data()); + holder->value = info[0].As().Int32Value(); +} + Value TestFunction(const CallbackInfo& info) { return Boolean::New(info.Env(), true); } @@ -58,11 +72,15 @@ void DefineProperties(const CallbackInfo& info) { Env env = info.Env(); Boolean trueValue = Boolean::New(env, true); + UserDataHolder* holder = new UserDataHolder(); + holder->value = 1234; if (nameType.Utf8Value() == "literal") { obj.DefineProperties({ PropertyDescriptor::Accessor(env, obj, "readonlyAccessor", TestGetter), PropertyDescriptor::Accessor(env, obj, "readwriteAccessor", TestGetter, TestSetter), + PropertyDescriptor::Accessor(env, obj, "readonlyAccessorWithUserData", TestGetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast(holder)), + PropertyDescriptor::Accessor(env, obj, "readwriteAccessorWithUserData", TestGetterWithUserData, TestSetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast(holder)), PropertyDescriptor::Value("readonlyValue", trueValue), PropertyDescriptor::Value("readwriteValue", trueValue, napi_writable), PropertyDescriptor::Value("enumerableValue", trueValue, napi_enumerable), @@ -76,6 +94,8 @@ void DefineProperties(const CallbackInfo& info) { // work around the issue. std::string str1("readonlyAccessor"); std::string str2("readwriteAccessor"); + std::string str1a("readonlyAccessorWithUserData"); + std::string str2a("readwriteAccessorWithUserData"); std::string str3("readonlyValue"); std::string str4("readwriteValue"); std::string str5("enumerableValue"); @@ -85,6 +105,8 @@ void DefineProperties(const CallbackInfo& info) { obj.DefineProperties({ PropertyDescriptor::Accessor(env, obj, str1, TestGetter), PropertyDescriptor::Accessor(env, obj, str2, TestGetter, TestSetter), + PropertyDescriptor::Accessor(env, obj, str1a, TestGetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast(holder)), + PropertyDescriptor::Accessor(env, obj, str2a, TestGetterWithUserData, TestSetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast(holder)), PropertyDescriptor::Value(str3, trueValue), PropertyDescriptor::Value(str4, trueValue, napi_writable), PropertyDescriptor::Value(str5, trueValue, napi_enumerable), @@ -97,6 +119,10 @@ void DefineProperties(const CallbackInfo& info) { Napi::String::New(env, "readonlyAccessor"), TestGetter), PropertyDescriptor::Accessor(env, obj, Napi::String::New(env, "readwriteAccessor"), TestGetter, TestSetter), + PropertyDescriptor::Accessor(env, obj, + Napi::String::New(env, "readonlyAccessorWithUserData"), TestGetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast(holder)), + PropertyDescriptor::Accessor(env, obj, + Napi::String::New(env, "readwriteAccessorWithUserData"), TestGetterWithUserData, TestSetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast(holder)), PropertyDescriptor::Value( Napi::String::New(env, "readonlyValue"), trueValue), PropertyDescriptor::Value( diff --git a/test/object/object.js b/test/object/object.js index 6b5884519..b4fe57dff 100644 --- a/test/object/object.js +++ b/test/object/object.js @@ -26,6 +26,10 @@ function test(binding) { assertPropertyIsNot(obj, 'readonlyAccessor', 'configurable'); assert.strictEqual(obj.readonlyAccessor, true); + assertPropertyIsNot(obj, 'readonlyAccessorWithUserData', 'enumerable'); + assertPropertyIsNot(obj, 'readonlyAccessorWithUserData', 'configurable'); + assert.strictEqual(obj.readonlyAccessorWithUserData, 1234, nameType); + assertPropertyIsNot(obj, 'readwriteAccessor', 'enumerable'); assertPropertyIsNot(obj, 'readwriteAccessor', 'configurable'); obj.readwriteAccessor = false; @@ -33,6 +37,13 @@ function test(binding) { obj.readwriteAccessor = true; assert.strictEqual(obj.readwriteAccessor, true); + assertPropertyIsNot(obj, 'readwriteAccessorWithUserData', 'enumerable'); + assertPropertyIsNot(obj, 'readwriteAccessorWithUserData', 'configurable'); + obj.readwriteAccessorWithUserData = 2; + assert.strictEqual(obj.readwriteAccessorWithUserData, 2); + obj.readwriteAccessorWithUserData = -14; + assert.strictEqual(obj.readwriteAccessorWithUserData, -14); + assertPropertyIsNot(obj, 'readonlyValue', 'writable'); assertPropertyIsNot(obj, 'readonlyValue', 'enumerable'); assertPropertyIsNot(obj, 'readonlyValue', 'configurable');