Fix property setters not sending correct D-Bus errors

igo95862 · igo95862 · commit bd80101fd8d2 · 2022-06-13T12:14:14.000+03:00
diff --git a/src/sdbus/sd_bus_internals_interface.c b/src/sdbus/sd_bus_internals_interface.c
@@ -18,7 +18,6 @@
     License along with this library; if not, write to the Free Software
     Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
 */
-#include <systemd/sd-bus.h>
 #include "sd_bus_internals.h"
 
 // TODO: adding interface to different buses, recalculating vtable
@@ -458,14 +457,21 @@ static int _SdBusInterface_property_set_callback(sd_bus* Py_UNUSED(bus),
                                                  const char* property,
                                                  sd_bus_message* value,
                                                  void* userdata,
-                                                 sd_bus_error* Py_UNUSED(ret_error)) {
+                                                 sd_bus_error* ret_error) {
         SdBusInterfaceObject* self = userdata;
-        PyObject* property_name_bytes CLEANUP_PY_OBJECT = PyBytes_FromString(property);
-        PyObject* set_call = CALL_PYTHON_CHECK_RETURN_NEG1(PyDict_GetItem(self->property_set_dict, property_name_bytes));
+        PyObject* property_name_bytes CLEANUP_PY_OBJECT = NULL;
+        PyObject* new_message CLEANUP_PY_OBJECT = NULL;
+        property_name_bytes = PyBytes_FromString(property);
+        if (NULL == property_name_bytes) {
+                goto fail;
+        }
+        PyObject* set_call = CALL_PYTHON_GOTO_FAIL(PyDict_GetItem(self->property_set_dict, property_name_bytes));
 
-        PyObject* new_message CLEANUP_PY_OBJECT = CALL_PYTHON_CHECK_RETURN_NEG1(SD_BUS_PY_CLASS_DUNDER_NEW(SdBusMessage_class));
+        new_message = CALL_PYTHON_GOTO_FAIL(SD_BUS_PY_CLASS_DUNDER_NEW(SdBusMessage_class));
         _SdBusMessage_set_messsage((SdBusMessageObject*)new_message, value);
 
-        Py_XDECREF(CALL_PYTHON_CHECK_RETURN_NEG1(PyObject_CallFunctionObjArgs(set_call, new_message, NULL)));
+        Py_XDECREF(CALL_PYTHON_GOTO_FAIL(PyObject_CallFunctionObjArgs(set_call, new_message, NULL)));
         return 0;
+fail:
+        return set_dbus_error_from_python_exception(ret_error);
 }
diff --git a/test/test_low_level_errors.py b/test/test_low_level_errors.py
@@ -43,6 +43,9 @@ class IndependentError(Exception):
     ...
 
 
+GOOD_STR = 'Good'
+
+
 class InterfaceWithErrors(
     DbusInterfaceCommonAsync,
     interface_name='org.example.test',
@@ -63,6 +66,22 @@ async def hello_error(self) -> str:
     async def hello_world(self) -> str:
         return HELLO_WORLD
 
+    @dbus_property_async('s')
+    def indep_err_setable(self) -> str:
+        return GOOD_STR
+
+    @indep_err_setable.setter
+    def indep_err_setter(self, new_value: str) -> None:
+        raise IndependentError
+
+    @dbus_property_async('s')
+    def derrive_err_settable(self) -> str:
+        return GOOD_STR
+
+    @derrive_err_settable.setter
+    def derrive_err_setter(self, new_value: str) -> None:
+        raise DbusDerivePropertydError
+
 
 class TestLowLevelErrors(IsolatedDbusTestCase):
     async def asyncSetUp(self) -> None:
@@ -100,3 +119,46 @@ async def test_property_getter_derived_error(self) -> None:
                 self.test_object_connection.derrive_err_getter.get_async(),
                 timeout=1,
             )
+
+        await self.test_object_connection.hello_world()
+
+    async def test_property_setter_independent_error(self) -> None:
+
+        self.assertEqual(
+            await wait_for(
+                self.test_object_connection.indep_err_setable.get_async(),
+                timeout=1,
+            ),
+            GOOD_STR,
+        )
+
+        with self.assertRaises(DbusFailedError) as cm:
+            await wait_for(
+                self.test_object_connection.
+                indep_err_setable.set_async('Test'),
+                timeout=1,
+            )
+
+        should_be_dbus_failed = cm.exception
+        self.assertIs(should_be_dbus_failed.__class__, DbusFailedError)
+
+        await self.test_object_connection.hello_world()
+
+    async def test_property_setter_derived_error(self) -> None:
+
+        self.assertEqual(
+            await wait_for(
+                self.test_object_connection.derrive_err_settable.get_async(),
+                timeout=1,
+            ),
+            GOOD_STR,
+        )
+
+        with self.assertRaises(DbusDerivePropertydError):
+            await wait_for(
+                self.test_object_connection.
+                derrive_err_settable.set_async('Test'),
+                timeout=1,
+            )
+
+        await self.test_object_connection.hello_world()
