Skip to content
Permalink
Browse files
[GLIB] jsc_value_object_define_property_accessor() throws an exceptio…
…n when called on a value without a wrapper instance

https://bugs.webkit.org/show_bug.cgi?id=233253

Reviewed by Michael Catanzaro.

Source/JavaScriptCore:

We assumed that getter and setter were always methods, so we always try to set the initial parameter as the
instance. When called with a value not having an instance we get an exception because the expected instance is
nullptr. This patch changes the behavior of jsc_value_object_define_property_accessor() to call the getter and
setter as functions, but keeping the behavior of jsc_class_add_property() in which case they are still called as
methods.

* API/glib/JSCClass.cpp:
(jsc_class_add_property): Use jscValueAddPropertyAccessor().
* API/glib/JSCValue.cpp:
(jsObjectCall): Remove useless break after return.
(jscValueObjectDefinePropertyAccessor): Helper to define the property accessor using the given function type for
the getter and setter.
(jsc_value_object_define_property_accessor): Call jscValueObjectDefinePropertyAccessor() with function as
function type.
(jscValueAddPropertyAccessor): Call jscValueObjectDefinePropertyAccessor() with method as function type.
* API/glib/JSCValuePrivate.h:

Tools:

Add unit tests to check jsc_value_object_define_property_accessor().

* TestWebKitAPI/Tests/JavaScriptCore/glib/TestJSC.cpp:
(getIntProperty):
(setIntProperty):
(testJSCObject):


Canonical link: https://commits.webkit.org/244385@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@285988 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
carlosgcampos committed Nov 18, 2021
1 parent 1172e24 commit 33f2261c9ebd0dabe0e7de1a0fb61ba17d029409
Showing 6 changed files with 240 additions and 36 deletions.
@@ -859,5 +859,5 @@ void jsc_class_add_property(JSCClass* jscClass, const char* name, GType property

auto context = jscContextGetOrCreate(priv->context);
GRefPtr<JSCValue> prototype = jscContextGetOrCreateValue(context.get(), toRef(priv->prototype.get()));
jsc_value_object_define_property_accessor(prototype.get(), name, JSC_VALUE_PROPERTY_CONFIGURABLE, propertyType, getter, setter, userData, destroyNotify);
jscValueAddPropertyAccessor(prototype.get(), name, propertyType, getter, setter, userData, destroyNotify);
}
@@ -867,13 +867,11 @@ static JSValueRef jsObjectCall(JSGlobalContextRef jsContext, JSObjectRef functio
switch (functionType) {
case JSC::JSCCallbackFunction::Type::Constructor:
return JSObjectCallAsConstructor(jsContext, function, arguments.size(), arguments.data(), exception);
break;
case JSC::JSCCallbackFunction::Type::Method:
ASSERT(thisObject);
FALLTHROUGH;
case JSC::JSCCallbackFunction::Type::Function:
return JSObjectCallAsFunction(jsContext, function, thisObject, arguments.size(), arguments.data(), exception);
break;
}
RELEASE_ASSERT_NOT_REACHED();
}
@@ -1074,35 +1072,8 @@ void jsc_value_object_define_property_data(JSCValue* value, const char* property
}
}

/**
* jsc_value_object_define_property_accessor:
* @value: a #JSCValue
* @property_name: the name of the property to define
* @flags: #JSCValuePropertyFlags
* @property_type: the #GType of the property
* @getter: (scope async) (nullable): a #GCallback to be called to get the property value
* @setter: (scope async) (nullable): a #GCallback to be called to set the property value
* @user_data: (closure): user data to pass to @getter and @setter
* @destroy_notify: (nullable): destroy notifier for @user_data
*
* Define or modify a property with @property_name in object referenced by @value. When the
* property value needs to be getted or set, @getter and @setter callbacks will be called.
* When the property is cleared in the #JSCClass context, @destroy_notify is called with
* @user_data as parameter. This is equivalent to JavaScript <function>Object.defineProperty()</function>
* when used with an accessor descriptor.
*
* Note that the value returned by @getter must be fully transferred. In case of boxed types, you could use
* %G_TYPE_POINTER instead of the actual boxed #GType to ensure that the instance owned by #JSCClass is used.
* If you really want to return a new copy of the boxed type, use #JSC_TYPE_VALUE and return a #JSCValue created
* with jsc_value_new_object() that receives the copy as instance parameter.
*/
void jsc_value_object_define_property_accessor(JSCValue* value, const char* propertyName, JSCValuePropertyFlags flags, GType propertyType, GCallback getter, GCallback setter, gpointer userData, GDestroyNotify destroyNotify)
static void jscValueObjectDefinePropertyAccessor(JSCValue* value, const char* propertyName, JSCValuePropertyFlags flags, GType propertyType, JSC::JSCCallbackFunction::Type functionType, GCallback getter, GCallback setter, gpointer userData, GDestroyNotify destroyNotify)
{
g_return_if_fail(JSC_IS_VALUE(value));
g_return_if_fail(propertyName);
g_return_if_fail(propertyType != G_TYPE_INVALID && propertyType != G_TYPE_NONE);
g_return_if_fail(getter || setter);

JSCValuePrivate* priv = value->priv;
auto* jsContext = jscContextGetJSContext(priv->context.get());
JSC::JSGlobalObject* globalObject = toJS(jsContext);
@@ -1126,15 +1097,17 @@ void jsc_value_object_define_property_accessor(JSCValue* value, const char* prop
descriptor.setEnumerable(flags & JSC_VALUE_PROPERTY_ENUMERABLE);
descriptor.setConfigurable(flags & JSC_VALUE_PROPERTY_CONFIGURABLE);
if (getter) {
GRefPtr<GClosure> closure = adoptGRef(g_cclosure_new(getter, userData, reinterpret_cast<GClosureNotify>(reinterpret_cast<GCallback>(destroyNotify))));
auto function = JSC::JSCCallbackFunction::create(vm, globalObject, "get"_s,
JSC::JSCCallbackFunction::Type::Method, nullptr, WTFMove(closure), propertyType, Vector<GType> { });
GRefPtr<GClosure> closure;
if (functionType == JSC::JSCCallbackFunction::Type::Function && userData)
closure = adoptGRef(g_cclosure_new_swap(getter, userData, reinterpret_cast<GClosureNotify>(reinterpret_cast<GCallback>(destroyNotify))));
else
closure = adoptGRef(g_cclosure_new(getter, userData, reinterpret_cast<GClosureNotify>(reinterpret_cast<GCallback>(destroyNotify))));
auto function = JSC::JSCCallbackFunction::create(vm, globalObject, "get"_s, functionType, nullptr, WTFMove(closure), propertyType, Vector<GType> { });
descriptor.setGetter(function);
}
if (setter) {
GRefPtr<GClosure> closure = adoptGRef(g_cclosure_new(setter, userData, getter ? nullptr : reinterpret_cast<GClosureNotify>(reinterpret_cast<GCallback>(destroyNotify))));
auto function = JSC::JSCCallbackFunction::create(vm, globalObject, "set"_s,
JSC::JSCCallbackFunction::Type::Method, nullptr, WTFMove(closure), G_TYPE_NONE, Vector<GType> { propertyType });
auto function = JSC::JSCCallbackFunction::create(vm, globalObject, "set"_s, functionType, nullptr, WTFMove(closure), G_TYPE_NONE, Vector<GType> { propertyType });
descriptor.setSetter(function);
}
object->methodTable(vm)->defineOwnProperty(object, globalObject, name->identifier(&vm), descriptor, true);
@@ -1144,6 +1117,46 @@ void jsc_value_object_define_property_accessor(JSCValue* value, const char* prop
}
}

/**
* jsc_value_object_define_property_accessor:
* @value: a #JSCValue
* @property_name: the name of the property to define
* @flags: #JSCValuePropertyFlags
* @property_type: the #GType of the property
* @getter: (scope async) (nullable): a #GCallback to be called to get the property value
* @setter: (scope async) (nullable): a #GCallback to be called to set the property value
* @user_data: (closure): user data to pass to @getter and @setter
* @destroy_notify: (nullable): destroy notifier for @user_data
*
* Define or modify a property with @property_name in object referenced by @value. When the
* property value needs to be getted or set, @getter and @setter callbacks will be called.
* When the property is cleared in the #JSCClass context, @destroy_notify is called with
* @user_data as parameter. This is equivalent to JavaScript <function>Object.defineProperty()</function>
* when used with an accessor descriptor.
*
* Note that the value returned by @getter must be fully transferred. In case of boxed types, you could use
* %G_TYPE_POINTER instead of the actual boxed #GType to ensure that the instance owned by #JSCClass is used.
* If you really want to return a new copy of the boxed type, use #JSC_TYPE_VALUE and return a #JSCValue created
* with jsc_value_new_object() that receives the copy as instance parameter.
*
* Note that @getter and @setter are called as functions and not methods, so they don't receive an instance as
* first parameter. Use jsc_class_add_property() if you want to add property accessor invoked as a method.
*/
void jsc_value_object_define_property_accessor(JSCValue* value, const char* propertyName, JSCValuePropertyFlags flags, GType propertyType, GCallback getter, GCallback setter, gpointer userData, GDestroyNotify destroyNotify)
{
g_return_if_fail(JSC_IS_VALUE(value));
g_return_if_fail(propertyName);
g_return_if_fail(propertyType != G_TYPE_INVALID && propertyType != G_TYPE_NONE);
g_return_if_fail(getter || setter);

jscValueObjectDefinePropertyAccessor(value, propertyName, flags, propertyType, JSC::JSCCallbackFunction::Type::Function, getter, setter, userData, destroyNotify);
}

void jscValueAddPropertyAccessor(JSCValue* value, const char* propertyName, GType propertyType, GCallback getter, GCallback setter, gpointer userData, GDestroyNotify destroyNotify)
{
jscValueObjectDefinePropertyAccessor(value, propertyName, JSC_VALUE_PROPERTY_CONFIGURABLE, propertyType, JSC::JSCCallbackFunction::Type::Method, getter, setter, userData, destroyNotify);
}

static GRefPtr<JSCValue> jscValueFunctionCreate(JSCContext* context, const char* name, GCallback callback, gpointer userData, GDestroyNotify destroyNotify, GType returnType, std::optional<Vector<GType>>&& parameters)
{
GRefPtr<GClosure> closure;
@@ -23,3 +23,4 @@

JS_EXPORT_PRIVATE JSValueRef jscValueGetJSValue(JSCValue*);
JSCValue* jscValueCreate(JSCContext*, JSValueRef);
void jscValueAddPropertyAccessor(JSCValue*, const char*, GType, GCallback, GCallback, gpointer, GDestroyNotify);
@@ -1,3 +1,27 @@
2021-11-18 Carlos Garcia Campos <cgarcia@igalia.com>

[GLIB] jsc_value_object_define_property_accessor() throws an exception when called on a value without a wrapper instance
https://bugs.webkit.org/show_bug.cgi?id=233253

Reviewed by Michael Catanzaro.

We assumed that getter and setter were always methods, so we always try to set the initial parameter as the
instance. When called with a value not having an instance we get an exception because the expected instance is
nullptr. This patch changes the behavior of jsc_value_object_define_property_accessor() to call the getter and
setter as functions, but keeping the behavior of jsc_class_add_property() in which case they are still called as
methods.

* API/glib/JSCClass.cpp:
(jsc_class_add_property): Use jscValueAddPropertyAccessor().
* API/glib/JSCValue.cpp:
(jsObjectCall): Remove useless break after return.
(jscValueObjectDefinePropertyAccessor): Helper to define the property accessor using the given function type for
the getter and setter.
(jsc_value_object_define_property_accessor): Call jscValueObjectDefinePropertyAccessor() with function as
function type.
(jscValueAddPropertyAccessor): Call jscValueObjectDefinePropertyAccessor() with method as function type.
* API/glib/JSCValuePrivate.h:

2021-11-17 Yusuke Suzuki <ysuzuki@apple.com>

[JSC] TypedArray GetArrayLength should not use Reuse
@@ -1,3 +1,17 @@
2021-11-18 Carlos Garcia Campos <cgarcia@igalia.com>

[GLIB] jsc_value_object_define_property_accessor() throws an exception when called on a value without a wrapper instance
https://bugs.webkit.org/show_bug.cgi?id=233253

Reviewed by Michael Catanzaro.

Add unit tests to check jsc_value_object_define_property_accessor().

* TestWebKitAPI/Tests/JavaScriptCore/glib/TestJSC.cpp:
(getIntProperty):
(setIntProperty):
(testJSCObject):

2021-11-17 Alex Christensen <achristensen@webkit.org>

Implement most of redirect and modify-headers action types
@@ -1164,6 +1164,16 @@ static void testJSCFunction()
}
}

static int getIntProperty(int* property)
{
return *property;
}

static void setIntProperty(int value, int* property)
{
*property = value;
}

static void testJSCObject()
{
{
@@ -1412,6 +1422,148 @@ static void testJSCObject()
g_assert_did_throw(exceptionHandler, didThrow);
}

{
LeakChecker checker;
GRefPtr<JSCContext> context = adoptGRef(jsc_context_new());
checker.watch(context.get());
ExceptionHandler exceptionHandler(context.get());

GRefPtr<JSCValue> object = adoptGRef(jsc_value_new_object(context.get(), nullptr, nullptr));
checker.watch(object.get());
g_assert_true(jsc_value_is_object(object.get()));
g_assert_true(jsc_value_object_is_instance_of(object.get(), "Object"));

GUniquePtr<char*> properties(jsc_value_object_enumerate_properties(object.get()));
g_assert_null(properties.get());

int property = 25;
g_assert_false(jsc_value_object_has_property(object.get(), "val"));
jsc_value_object_define_property_accessor(object.get(), "val", static_cast<JSCValuePropertyFlags>(0), G_TYPE_INT, G_CALLBACK(getIntProperty), nullptr, &property, nullptr);
g_assert_true(jsc_value_object_has_property(object.get(), "val"));
properties.reset(jsc_value_object_enumerate_properties(object.get()));
g_assert_null(properties.get());
jsc_context_set_value(context.get(), "f", object.get());

GRefPtr<JSCValue> value = adoptGRef(jsc_context_evaluate(context.get(), "f.val;", -1));
checker.watch(value.get());
g_assert_true(jsc_value_is_number(value.get()));
g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 25);

bool didThrow = false;
g_assert_throw_begin(exceptionHandler, didThrow);
value = adoptGRef(jsc_context_evaluate(context.get(), "'use strict'; f.val = 32;", -1));
checker.watch(value.get());
g_assert_true(jsc_value_is_undefined(value.get()));
g_assert_did_throw(exceptionHandler, didThrow);

value = adoptGRef(jsc_context_evaluate(context.get(), "f.propertyIsEnumerable('val');", -1));
checker.watch(value.get());
g_assert_true(jsc_value_is_boolean(value.get()));
g_assert_true(jsc_value_to_boolean(value.get()) == FALSE);

value = adoptGRef(jsc_context_evaluate(context.get(), "delete f.val;", -1));
checker.watch(value.get());
g_assert_true(jsc_value_object_has_property(object.get(), "val"));
value = adoptGRef(jsc_context_evaluate(context.get(), "f.val;", -1));
checker.watch(value.get());
g_assert_true(jsc_value_is_number(value.get()));
g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 25);

g_assert_false(jsc_value_object_delete_property(object.get(), "val"));
g_assert_true(jsc_value_object_has_property(object.get(), "val"));

g_assert_throw_begin(exceptionHandler, didThrow);
jsc_value_object_define_property_accessor(object.get(), "val", static_cast<JSCValuePropertyFlags>(0), G_TYPE_INT, G_CALLBACK(getIntProperty), nullptr, &property, nullptr);
g_assert_did_throw(exceptionHandler, didThrow);

property = 32;
g_assert_false(jsc_value_object_has_property(object.get(), "val2"));
jsc_value_object_define_property_accessor(object.get(), "val2", JSC_VALUE_PROPERTY_ENUMERABLE, G_TYPE_INT, G_CALLBACK(getIntProperty), G_CALLBACK(setIntProperty), &property, nullptr);
g_assert_true(jsc_value_object_has_property(object.get(), "val2"));
value = adoptGRef(jsc_context_evaluate(context.get(), "f.val2;", -1));
checker.watch(value.get());
g_assert_true(jsc_value_is_number(value.get()));
g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 32);

properties.reset(jsc_value_object_enumerate_properties(object.get()));
g_assert_cmpuint(g_strv_length(properties.get()), ==, 1);
g_assert_cmpstr(properties.get()[0], ==, "val2");
g_assert_null(properties.get()[1]);

value = adoptGRef(jsc_context_evaluate(context.get(), "'use strict'; f.val2 = 45;", -1));
checker.watch(value.get());
value = adoptGRef(jsc_context_evaluate(context.get(), "f.val2;", -1));
checker.watch(value.get());
g_assert_true(jsc_value_is_number(value.get()));
g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 45);

value = adoptGRef(jsc_context_evaluate(context.get(), "f.propertyIsEnumerable('val2');", -1));
checker.watch(value.get());
g_assert_true(jsc_value_is_boolean(value.get()));
g_assert_true(jsc_value_to_boolean(value.get()) == TRUE);

g_assert_false(jsc_value_object_delete_property(object.get(), "val2"));
g_assert_true(jsc_value_object_has_property(object.get(), "val2"));

property = 125;
g_assert_false(jsc_value_object_has_property(object.get(), "val3"));
jsc_value_object_define_property_accessor(object.get(), "val3", JSC_VALUE_PROPERTY_CONFIGURABLE, G_TYPE_INT, G_CALLBACK(getIntProperty), G_CALLBACK(setIntProperty), &property, nullptr);
g_assert_true(jsc_value_object_has_property(object.get(), "val3"));
value = adoptGRef(jsc_context_evaluate(context.get(), "f.val3;", -1));
checker.watch(value.get());
g_assert_true(jsc_value_is_number(value.get()));
g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 125);

properties.reset(jsc_value_object_enumerate_properties(object.get()));
g_assert_cmpuint(g_strv_length(properties.get()), ==, 1);
g_assert_cmpstr(properties.get()[0], ==, "val2");
g_assert_null(properties.get()[1]);

property = 150;
jsc_value_object_define_property_accessor(object.get(), "val3", JSC_VALUE_PROPERTY_CONFIGURABLE, G_TYPE_INT, G_CALLBACK(getIntProperty), nullptr, &property, nullptr);
g_assert_true(jsc_value_object_has_property(object.get(), "val3"));
value = adoptGRef(jsc_context_evaluate(context.get(), "f.val3;", -1));
checker.watch(value.get());
g_assert_true(jsc_value_is_number(value.get()));
g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 150);

properties.reset(jsc_value_object_enumerate_properties(object.get()));
g_assert_cmpuint(g_strv_length(properties.get()), ==, 1);
g_assert_cmpstr(properties.get()[0], ==, "val2");
g_assert_null(properties.get()[1]);

value = adoptGRef(jsc_context_evaluate(context.get(), "delete f.val3;", -1));
checker.watch(value.get());
g_assert_false(jsc_value_object_has_property(object.get(), "val3"));
value = adoptGRef(jsc_context_evaluate(context.get(), "f.val3;", -1));
checker.watch(value.get());
g_assert_true(jsc_value_is_undefined(value.get()));

property = 250;
g_assert_false(jsc_value_object_has_property(object.get(), "val4"));
jsc_value_object_define_property_accessor(object.get(), "val4", static_cast<JSCValuePropertyFlags>(JSC_VALUE_PROPERTY_CONFIGURABLE | JSC_VALUE_PROPERTY_ENUMERABLE),
G_TYPE_INT, G_CALLBACK(getIntProperty), nullptr, &property, nullptr);
g_assert_true(jsc_value_object_has_property(object.get(), "val4"));
value = adoptGRef(jsc_context_evaluate(context.get(), "f.val4;", -1));
checker.watch(value.get());
g_assert_true(jsc_value_is_number(value.get()));
g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 250);

properties.reset(jsc_value_object_enumerate_properties(object.get()));
g_assert_cmpuint(g_strv_length(properties.get()), ==, 2);
g_assert_cmpstr(properties.get()[0], ==, "val2");
g_assert_cmpstr(properties.get()[1], ==, "val4");
g_assert_null(properties.get()[2]);

g_assert_true(jsc_value_object_delete_property(object.get(), "val4"));
g_assert_false(jsc_value_object_has_property(object.get(), "val4"));

properties.reset(jsc_value_object_enumerate_properties(object.get()));
g_assert_cmpuint(g_strv_length(properties.get()), ==, 1);
g_assert_cmpstr(properties.get()[0], ==, "val2");
g_assert_null(properties.get()[1]);
}

{
LeakChecker checker;
GRefPtr<JSCContext> context = adoptGRef(jsc_context_new());

0 comments on commit 33f2261

Please sign in to comment.