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
Reviewed-by: Cosimo Alfarano <cosimo.alfarano@collabora.co.uk>

1 files changed, 14 insertions, 6 deletions

diff --git a/dbus/dbus-gobject.c b/dbus/dbus-gobject.c
index 80ec5c4..18cab14 100644
--- a/dbus/dbus-gobject.c
+++ b/dbus/dbus-gobject.c
@@ -2123,8 +2123,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);
@@ -2142,8 +2143,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);
@@ -2167,8 +2169,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,
@@ -2196,9 +2199,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;