Fix crash with type checking invalid GObject arguments

Ensure we have a valid GObject before attempting to call g_type_is_a.
Swap conditional blocks to make if condition more readable.

https://bugzilla.gnome.org/show_bug.cgi?id=727604

2 files changed, 45 insertions, 9 deletions

diff --git a/gi/pygi-object.c b/gi/pygi-object.c
index 29cd518a..7a05fdf4 100644
--- a/gi/pygi-object.c
+++ b/gi/pygi-object.c
@@ -128,8 +128,6 @@ _pygi_marshal_from_py_interface_object (PyGIInvokeState   *state,
                                         GIArgument        *arg,
                                         gpointer          *cleanup_data)
 {
-    gboolean res = FALSE;
-    GObject *gobj = NULL;
     PyGIInterfaceCache *iface_cache = (PyGIInterfaceCache *)arg_cache;
 
     if (py_arg == Py_None) {
@@ -137,9 +135,16 @@ _pygi_marshal_from_py_interface_object (PyGIInvokeState   *state,
         return TRUE;
     }
 
-    gobj = pygobject_get (py_arg);
-    if (!PyObject_IsInstance (py_arg, iface_cache->py_type) &&
-            !g_type_is_a (G_OBJECT_TYPE (gobj), iface_cache->g_type)) {
+    if (PyObject_IsInstance (py_arg, iface_cache->py_type) ||
+            (pygobject_check (py_arg, &PyGObject_Type) &&
+             g_type_is_a (G_OBJECT_TYPE (pygobject_get (py_arg)), iface_cache->g_type))) {
+
+        gboolean res;
+        res = _pygi_marshal_from_py_gobject (py_arg, arg, arg_cache->transfer);
+        *cleanup_data = arg->v_pointer;
+        return res;
+
+    } else {
         PyObject *module = PyObject_GetAttrString(py_arg, "__module__");
 
         PyErr_Format (PyExc_TypeError, "argument %s: Expected %s, but got %s%s%s",
@@ -152,10 +157,6 @@ _pygi_marshal_from_py_interface_object (PyGIInvokeState   *state,
             Py_DECREF (module);
         return FALSE;
     }
-
-    res = _pygi_marshal_from_py_gobject (py_arg, arg, arg_cache->transfer);
-    *cleanup_data = arg->v_pointer;
-    return res;
 }
 
 static void
diff --git a/tests/test_object_marshaling.py b/tests/test_object_marshaling.py
index 624ed9d0..ae0727e9 100644
--- a/tests/test_object_marshaling.py
+++ b/tests/test_object_marshaling.py
@@ -10,6 +10,11 @@ import warnings
 from gi.repository import GObject
 from gi.repository import GIMarshallingTests
 
+try:
+    from gi.repository import Regress
+except ImportError:
+    Regress = None
+
 
 class StrongRef(object):
     # A class that behaves like weakref.ref but holds a strong reference.
@@ -601,3 +606,33 @@ class TestPropertyHoldingObject(unittest.TestCase):
     def test_set_object_property_to_invalid_type(self):
         obj = GIMarshallingTests.PropertiesObject()
         self.assertRaises(TypeError, obj.set_property, 'some-object', 'not_an_object')
+
+
+@unittest.skipIf(Regress is None, 'Regress is required')
+class TestArgumentTypeErrors(unittest.TestCase):
+    def test_object_argument_type_error(self):
+        # ensure TypeError is raised for things which are not GObjects
+        obj = Regress.TestObj()
+        obj.set_bare(GObject.Object())
+        obj.set_bare(None)
+
+        self.assertRaises(TypeError, obj.set_bare, object())
+        self.assertRaises(TypeError, obj.set_bare, 42)
+        self.assertRaises(TypeError, obj.set_bare, 'not an object')
+
+    def test_instance_argument_error(self):
+        # ensure TypeError is raised for non Regress.TestObj instances.
+        obj = Regress.TestObj()
+        self.assertEqual(Regress.TestObj.instance_method(obj), -1)
+        self.assertRaises(TypeError, Regress.TestObj.instance_method, object())
+        self.assertRaises(TypeError, Regress.TestObj.instance_method, GObject.Object())
+        self.assertRaises(TypeError, Regress.TestObj.instance_method, 42)
+        self.assertRaises(TypeError, Regress.TestObj.instance_method, 'not an object')
+
+    def test_instance_argument_base_type_error(self):
+        # ensure TypeError is raised when a base type is passed to something
+        # expecting a derived type
+        obj = Regress.TestSubObj()
+        self.assertEqual(Regress.TestSubObj.instance_method(obj), 0)
+        self.assertRaises(TypeError, Regress.TestSubObj.instance_method, GObject.Object())
+        self.assertRaises(TypeError, Regress.TestSubObj.instance_method, Regress.TestObj())