object: Add GObject properties to the correct prototype

Whenever successfully resolving a property, GJS is mistakenly setting
the property in the prototype of the prototype, instead of just the
prototype. When that happens, it ends up breaking any properties of
subclasses. Suppose the following scenario:

      GObject
         |________________
         |                |
  GCharsetConverter   Subclass2
         |
     Subclass1

In this scenario, if 'Subclass1' accesses any possible property from
GCharsetConverter, GJS resolves that but ends up setting this property
in the prototype of the prototype.

If 'Subclass2' then tries to subsequently access that property, the
resolving process will accidentally call the wrong getter. This leads
to an immediate crash.

Fix that by using the correct object prototype when resolving the
properties. A reproducer was added to the test suite in addition to
the fix to this problem, as a protective measure.

Fixes #171
parent 8ea9cff7
Pipeline #18818 passed with stages
in 8 minutes and 15 seconds
......@@ -827,11 +827,8 @@ bool ObjectPrototype::resolve_impl(JSContext* context, JS::HandleObject obj,
/* If the name refers to a GObject property or field, lazily define the
* property in JS, on the prototype. */
if (is_gobject_property_name(m_info, name)) {
JS::RootedObject proto(context);
JS_GetPrototype(context, obj, &proto);
bool found = false;
if (!JS_AlreadyHasOwnPropertyById(context, proto, id, &found))
if (!JS_AlreadyHasOwnPropertyById(context, obj, id, &found))
return false;
if (found) {
/* Already defined, so *resolved = false because we didn't just
......@@ -844,7 +841,7 @@ bool ObjectPrototype::resolve_impl(JSContext* context, JS::HandleObject obj,
JS::RootedValue private_id(context, JS::StringValue(JSID_TO_STRING(id)));
if (!gjs_define_property_dynamic(
context, proto, name, "gobject_prop", &ObjectBase::prop_getter,
context, obj, name, "gobject_prop", &ObjectBase::prop_getter,
&ObjectBase::prop_setter, private_id, GJS_MODULE_PROP_FLAGS))
return false;
......@@ -854,11 +851,8 @@ bool ObjectPrototype::resolve_impl(JSContext* context, JS::HandleObject obj,
GjsAutoInfo<GIFieldInfo> field_info = lookup_field_info(m_info, name);
if (field_info) {
JS::RootedObject proto(context);
JS_GetPrototype(context, obj, &proto);
bool found = false;
if (!JS_AlreadyHasOwnPropertyById(context, proto, id, &found))
if (!JS_AlreadyHasOwnPropertyById(context, obj, id, &found))
return false;
if (found) {
*resolved = false;
......@@ -872,7 +866,7 @@ bool ObjectPrototype::resolve_impl(JSContext* context, JS::HandleObject obj,
flags |= JSPROP_READONLY;
JS::RootedValue private_id(context, JS::StringValue(JSID_TO_STRING(id)));
if (!gjs_define_property_dynamic(context, proto, name, "gobject_field",
if (!gjs_define_property_dynamic(context, obj, name, "gobject_field",
&ObjectBase::field_getter,
&ObjectBase::field_setter, private_id,
flags))
......
......@@ -334,4 +334,22 @@ describe('GObject class with decorator', function () {
},
}, class BadOverride extends GObject.Object {})).toThrow();
});
it('does not pollute the wrong prototype with GObject properties', function () {
const MyCustomCharset = GObject.registerClass(class MyCustomCharset extends Gio.CharsetConverter {
_init() {
super._init();
void this.from_charset;
}
});
const MySecondCustomCharset = GObject.registerClass(class MySecondCustomCharset extends GObject.Object {
_init() {
super._init();
this.from_charset = 'another value';
}
});
expect (() => new MyCustomCharset() && new MySecondCustomCharset()).not.toThrow();
});
});
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment