From 73b5614c5c84e395a4f1c8e29f5fb28e70b0d41e Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 5 Apr 2011 15:42:29 +0100 Subject: [PATCH 06/15] object_registration_message: handle remote caller errors better If the remote process sends us a wrong message, that's its fault, not ours; we should send back a comprehensible D-Bus error, and not spam to stderr. Previously, in each of these cases libdbus would have sent back NoReply, because we declined to handle the method call. One case that's still wrong is passing extra arguments to Get, GetAll or Set, like so: Get("com.example.Iface", "MyProperty", "extra") Set("com.example.Iface", "MyProperty", Variant("foo"), "extra") GetAll("com.example.Iface", "extra") dbus-glib has historically warned, ignored the extra argument(s) and sent back a reply as if they hadn't been there, whereas a stricter implementation (like telepathy-glib's TpDBusPropertiesMixin) would have sent back an error reply and done nothing. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=35766 --- dbus/dbus-gobject.c | 20 ++++++++++++++------ 1 files changed, 14 insertions(+), 6 deletions(-) diff --git a/dbus/dbus-gobject.c b/dbus/dbus-gobject.c index e4121b6..fbe3740 100644 --- a/dbus/dbus-gobject.c +++ b/dbus/dbus-gobject.c @@ -2096,8 +2096,9 @@ object_registration_message (DBusConnection *connection, if (dbus_message_iter_get_arg_type (&iter) != DBUS_TYPE_STRING) { - g_warning ("Property get or set does not have an interface string as first arg\n"); - return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; + ret = error_or_die (message, DBUS_ERROR_INVALID_ARGS, + "First argument to Get(), GetAll() or Set() must be an interface string"); + goto out; } dbus_message_iter_get_basic (&iter, &wincaps_propiface); @@ -2115,8 +2116,9 @@ object_registration_message (DBusConnection *connection, { if (dbus_message_iter_get_arg_type (&iter) != DBUS_TYPE_STRING) { - g_warning ("Property get or set does not have a property name string as second arg\n"); - return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; + ret = error_or_die (message, DBUS_ERROR_INVALID_ARGS, + "Second argument to Get() or Set() must be a property name string"); + goto out; } dbus_message_iter_get_basic (&iter, &requested_propname); dbus_message_iter_next (&iter); @@ -2140,8 +2142,9 @@ object_registration_message (DBusConnection *connection, { if (dbus_message_iter_get_arg_type (&iter) != DBUS_TYPE_VARIANT) { - g_warning ("Property set does not have a variant value as third arg\n"); - return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; + ret = error_or_die (message, DBUS_ERROR_INVALID_ARGS, + "Third argument to Set() must be a variant"); + goto out; } ret = set_object_property (connection, message, &iter, @@ -2169,9 +2172,14 @@ object_registration_message (DBusConnection *connection, g_assert (ret != NULL); + /* FIXME: this should be returned as a D-Bus error, not spammed out + * as a warning. This is too late to do that, though - we've already + * had any side-effects we were going to have - and it would break + * anything that's relying on ability to give us too many arguments. */ if (dbus_message_iter_get_arg_type (&iter) != DBUS_TYPE_INVALID) g_warning ("Property get, set or set all had too many arguments\n"); +out: dbus_connection_send (connection, ret, NULL); dbus_message_unref (ret); return DBUS_HANDLER_RESULT_HANDLED; -- 1.7.4.1