After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 676603 - setting a property using the wrong type segfaults
setting a property using the wrong type segfaults
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: introspection
3.2.x
Other Linux
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
Depends on:
Blocks:
 
 
Reported: 2012-05-22 23:02 UTC by Marien Zwart
Modified: 2012-06-05 10:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
attempt at a patch (1.64 KB, patch)
2012-05-22 23:40 UTC, Marien Zwart
none Details | Review
more appropriately formatted attempt at a patch, against git (1.72 KB, patch)
2012-05-22 23:56 UTC, Marien Zwart
none Details | Review

Description Marien Zwart 2012-05-22 23:02:44 UTC
Using pygobject-3.2.2, libsoup-2.38.1 and python 2.7 (on debian sid or a gentoo system), the following code:

from gi.repository import Soup
msg = Soup.Message()
msg.props.uri = 'http://www.gnome.org/'

segfaults with the following backtrace (truncated when it hits cpython):

  • #0 soup_uri_copy
    at soup-uri.c line 523
  • #1 g_boxed_copy
    at gboxed.c line 337
  • #2 value_set_boxed_internal
    at gboxed.c line 464
  • #3 g_value_set_boxed
    at gboxed.c line 481
  • #4 pygi_set_property_value_real
    at pygi-property.c line 300
  • #5 pygi_set_property_value
    at ../../gi/pygi.h line 133
  • #6 PyGProps_setattro
    at pygobject.c line 338
  • #7 PyObject_SetAttr
    at Objects/object.c line 1247

Note the "uri" property is documented to be of type SoupUri, not string. So this probably *should* fail, but preferably with a python TypeError, not a segfault.
Comment 1 Jasper St. Pierre (not reading bugmail) 2012-05-22 23:08:42 UTC
We need to check that it's actually a PygBoxed. We should also check to make sure that the object matches introspection info.
Comment 2 Marien Zwart 2012-05-22 23:40:20 UTC
Created attachment 214715 [details] [review]
attempt at a patch

Not hindered by any knowledge of pygobject I attempted to patch this. It looks like there's two bugs:

- pygi_set_property_value_real does not check if _pygi_argument_from_object fails (set an exception). Added a PyErr_Occurred check.

- _pygi_argument_from_object assumes the python object is a GBoxed if its target type is. Added what I *hope* is the correct pyg_boxed_check call, but someone who actually knows what they're doing should check :)

With this patch applied everything seems much happier:

>>> from gi.repository import Soup
>>> msg = Soup.Message()
>>> msg.props.uri = 'http://www.gnome.org/'
Traceback (most recent call last):
  • File "<stdin>", line 1 in <module>
TypeError: wrong boxed type
>>> uri = Soup.URI()                                                                      
>>> msg.props.uri = uri
(process:1624): libsoup-WARNING **: (soup-uri.c:523):soup_uri_copy: runtime check failed: (SOUP_URI_IS_VALID (uri))
>>> cookie = Soup.Cookie()
>>> msg.props.uri = cookie
Traceback (most recent call last):
  • File "<stdin>", line 1 in <module>
TypeError: wrong boxed type

Comment 3 Marien Zwart 2012-05-22 23:56:57 UTC
Created attachment 214716 [details] [review]
more appropriately formatted attempt at a patch, against git

It has been pointed out to me that the quick-and-dirty patch I previously attached violates the rules of common decency for patch submission. Let me attempt to rectify that:

The attached patch against git master adds a necessary type check to _pygi_argument_from_object, which previously just assumed its python argument was a GBoxed if that's what it was asked to produce.

The other issue fixed by my previous patch (missing PyErr_Occurred check) was already fixed in master.

This could probably still do with a unit test, but I'm hoping someone will beat me to writing that one.
Comment 4 Martin Pitt 2012-06-05 10:02:15 UTC
> Not hindered by any knowledge of pygobject I attempted to patch this.

*chuckle*

Thanks for this!

I added a boxed struct property to GIMarshallingTests:

http://git.gnome.org/browse/gobject-introspection/commit/?id=5b8f63cb767424008a0549a28177574eb7538be9

and use that to add a test case for this bug. Committed that with your patch:

http://git.gnome.org/browse/pygobject/commit/?id=635a7d1b48d99ddd1ea123797c493b18b0cdfd45