Skip to content

Commit c1ff293

Browse files
lmartorellaGabriel Schulhof
lmartorella
authored and
Gabriel Schulhof
committed
src: fix missing void*data usage in PropertyDescriptors
PR-URL: #374 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
1 parent d8e9c22 commit c1ff293

File tree

4 files changed

+50
-10
lines changed

4 files changed

+50
-10
lines changed

‎napi-inl.deprecated.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(const char* utf8name,
7373
void* /*data*/) {
7474
typedef details::AccessorCallbackData<Getter, Setter> CbData;
7575
// TODO: Delete when the function is destroyed
76-
auto callbackData = new CbData({ getter, setter });
76+
auto callbackData = new CbData({ getter, setter, nullptr });
7777

7878
return PropertyDescriptor({
7979
utf8name,
@@ -104,7 +104,7 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(napi_value name,
104104
void* /*data*/) {
105105
typedef details::AccessorCallbackData<Getter, Setter> CbData;
106106
// TODO: Delete when the function is destroyed
107-
auto callbackData = new CbData({ getter, setter });
107+
auto callbackData = new CbData({ getter, setter, nullptr });
108108

109109
return PropertyDescriptor({
110110
nullptr,

‎napi-inl.h‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ struct AccessorCallbackData {
176176
CallbackInfo callbackInfo(env, info);
177177
AccessorCallbackData* callbackData =
178178
static_cast<AccessorCallbackData*>(callbackInfo.Data());
179+
callbackInfo.SetData(callbackData->data);
179180
return callbackData->getterCallback(callbackInfo);
180181
});
181182
}
@@ -186,13 +187,15 @@ struct AccessorCallbackData {
186187
CallbackInfo callbackInfo(env, info);
187188
AccessorCallbackData* callbackData =
188189
static_cast<AccessorCallbackData*>(callbackInfo.Data());
190+
callbackInfo.SetData(callbackData->data);
189191
callbackData->setterCallback(callbackInfo);
190192
return nullptr;
191193
});
192194
}
193195

194196
Getter getterCallback;
195197
Setter setterCallback;
198+
void* data;
196199
};
197200

198201
} // namespace details
@@ -2603,9 +2606,9 @@ PropertyDescriptor::Accessor(Napi::Env env,
26032606
const char* utf8name,
26042607
Getter getter,
26052608
napi_property_attributes attributes,
2606-
void* /*data*/) {
2609+
void* data) {
26072610
typedef details::CallbackData<Getter, Napi::Value> CbData;
2608-
auto callbackData = new CbData({ getter, nullptr });
2611+
auto callbackData = new CbData({ getter, data });
26092612

26102613
napi_status status = AttachData(env, object, callbackData);
26112614
NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor());
@@ -2638,9 +2641,9 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(Napi::Env env,
26382641
Name name,
26392642
Getter getter,
26402643
napi_property_attributes attributes,
2641-
void* /*data*/) {
2644+
void* data) {
26422645
typedef details::CallbackData<Getter, Napi::Value> CbData;
2643-
auto callbackData = new CbData({ getter, nullptr });
2646+
auto callbackData = new CbData({ getter, data });
26442647

26452648
napi_status status = AttachData(env, object, callbackData);
26462649
NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor());
@@ -2664,9 +2667,9 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(Napi::Env env,
26642667
Getter getter,
26652668
Setter setter,
26662669
napi_property_attributes attributes,
2667-
void* /*data*/) {
2670+
void* data) {
26682671
typedef details::AccessorCallbackData<Getter, Setter> CbData;
2669-
auto callbackData = new CbData({ getter, setter });
2672+
auto callbackData = new CbData({ getter, setter, data });
26702673

26712674
napi_status status = AttachData(env, object, callbackData);
26722675
NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor());
@@ -2701,9 +2704,9 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(Napi::Env env,
27012704
Getter getter,
27022705
Setter setter,
27032706
napi_property_attributes attributes,
2704-
void* /*data*/) {
2707+
void* data) {
27052708
typedef details::AccessorCallbackData<Getter, Setter> CbData;
2706-
auto callbackData = new CbData({ getter, setter });
2709+
auto callbackData = new CbData({ getter, setter, data });
27072710

27082711
napi_status status = AttachData(env, object, callbackData);
27092712
NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor());

‎test/object/object.cc‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ Value HasPropertyWithCStyleString(const CallbackInfo& info);
3333
Value HasPropertyWithCppStyleString(const CallbackInfo& info);
3434

3535
static bool testValue = true;
36+
// Used to test void* Data() integrity
37+
struct UserDataHolder {
38+
int32_t value;
39+
};
3640

3741
Value TestGetter(const CallbackInfo& info) {
3842
return Boolean::New(info.Env(), testValue);
@@ -42,6 +46,16 @@ void TestSetter(const CallbackInfo& info) {
4246
testValue = info[0].As<Boolean>();
4347
}
4448

49+
Value TestGetterWithUserData(const CallbackInfo& info) {
50+
const UserDataHolder* holder = reinterpret_cast<UserDataHolder*>(info.Data());
51+
return Number::New(info.Env(), holder->value);
52+
}
53+
54+
void TestSetterWithUserData(const CallbackInfo& info) {
55+
UserDataHolder* holder = reinterpret_cast<UserDataHolder*>(info.Data());
56+
holder->value = info[0].As<Number>().Int32Value();
57+
}
58+
4559
Value TestFunction(const CallbackInfo& info) {
4660
return Boolean::New(info.Env(), true);
4761
}
@@ -58,11 +72,15 @@ void DefineProperties(const CallbackInfo& info) {
5872
Env env = info.Env();
5973

6074
Boolean trueValue = Boolean::New(env, true);
75+
UserDataHolder* holder = new UserDataHolder();
76+
holder->value = 1234;
6177

6278
if (nameType.Utf8Value() == "literal") {
6379
obj.DefineProperties({
6480
PropertyDescriptor::Accessor(env, obj, "readonlyAccessor", TestGetter),
6581
PropertyDescriptor::Accessor(env, obj, "readwriteAccessor", TestGetter, TestSetter),
82+
PropertyDescriptor::Accessor(env, obj, "readonlyAccessorWithUserData", TestGetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast<void*>(holder)),
83+
PropertyDescriptor::Accessor(env, obj, "readwriteAccessorWithUserData", TestGetterWithUserData, TestSetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast<void*>(holder)),
6684
PropertyDescriptor::Value("readonlyValue", trueValue),
6785
PropertyDescriptor::Value("readwriteValue", trueValue, napi_writable),
6886
PropertyDescriptor::Value("enumerableValue", trueValue, napi_enumerable),
@@ -76,6 +94,8 @@ void DefineProperties(const CallbackInfo& info) {
7694
// work around the issue.
7795
std::string str1("readonlyAccessor");
7896
std::string str2("readwriteAccessor");
97+
std::string str1a("readonlyAccessorWithUserData");
98+
std::string str2a("readwriteAccessorWithUserData");
7999
std::string str3("readonlyValue");
80100
std::string str4("readwriteValue");
81101
std::string str5("enumerableValue");
@@ -85,6 +105,8 @@ void DefineProperties(const CallbackInfo& info) {
85105
obj.DefineProperties({
86106
PropertyDescriptor::Accessor(env, obj, str1, TestGetter),
87107
PropertyDescriptor::Accessor(env, obj, str2, TestGetter, TestSetter),
108+
PropertyDescriptor::Accessor(env, obj, str1a, TestGetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast<void*>(holder)),
109+
PropertyDescriptor::Accessor(env, obj, str2a, TestGetterWithUserData, TestSetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast<void*>(holder)),
88110
PropertyDescriptor::Value(str3, trueValue),
89111
PropertyDescriptor::Value(str4, trueValue, napi_writable),
90112
PropertyDescriptor::Value(str5, trueValue, napi_enumerable),
@@ -97,6 +119,10 @@ void DefineProperties(const CallbackInfo& info) {
97119
Napi::String::New(env, "readonlyAccessor"), TestGetter),
98120
PropertyDescriptor::Accessor(env, obj,
99121
Napi::String::New(env, "readwriteAccessor"), TestGetter, TestSetter),
122+
PropertyDescriptor::Accessor(env, obj,
123+
Napi::String::New(env, "readonlyAccessorWithUserData"), TestGetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast<void*>(holder)),
124+
PropertyDescriptor::Accessor(env, obj,
125+
Napi::String::New(env, "readwriteAccessorWithUserData"), TestGetterWithUserData, TestSetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast<void*>(holder)),
100126
PropertyDescriptor::Value(
101127
Napi::String::New(env, "readonlyValue"), trueValue),
102128
PropertyDescriptor::Value(

‎test/object/object.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,24 @@ function test(binding) {
2626
assertPropertyIsNot(obj, 'readonlyAccessor', 'configurable');
2727
assert.strictEqual(obj.readonlyAccessor, true);
2828

29+
assertPropertyIsNot(obj, 'readonlyAccessorWithUserData', 'enumerable');
30+
assertPropertyIsNot(obj, 'readonlyAccessorWithUserData', 'configurable');
31+
assert.strictEqual(obj.readonlyAccessorWithUserData, 1234, nameType);
32+
2933
assertPropertyIsNot(obj, 'readwriteAccessor', 'enumerable');
3034
assertPropertyIsNot(obj, 'readwriteAccessor', 'configurable');
3135
obj.readwriteAccessor = false;
3236
assert.strictEqual(obj.readwriteAccessor, false);
3337
obj.readwriteAccessor = true;
3438
assert.strictEqual(obj.readwriteAccessor, true);
3539

40+
assertPropertyIsNot(obj, 'readwriteAccessorWithUserData', 'enumerable');
41+
assertPropertyIsNot(obj, 'readwriteAccessorWithUserData', 'configurable');
42+
obj.readwriteAccessorWithUserData = 2;
43+
assert.strictEqual(obj.readwriteAccessorWithUserData, 2);
44+
obj.readwriteAccessorWithUserData = -14;
45+
assert.strictEqual(obj.readwriteAccessorWithUserData, -14);
46+
3647
assertPropertyIsNot(obj, 'readonlyValue', 'writable');
3748
assertPropertyIsNot(obj, 'readonlyValue', 'enumerable');
3849
assertPropertyIsNot(obj, 'readonlyValue', 'configurable');

0 commit comments

Comments
 (0)