From: Ryan Lortie <desrt@desrt.ca>
Date: Mon, 1 Jul 2013 21:42:43 +0000 (-0400)
Subject: GDBusConnection: be more careful with async GetAll
X-Git-Tag: 2.37.4~8
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=cb4469600c5146a48501a31e9a3fb9bfc261477d;p=platform%2Fupstream%2Fglib.git

GDBusConnection: be more careful with async GetAll

It's possible to get a org.freedesktop.Properties.GetAll call even if we
have no readable properties in the introspection, in which case we
should return the empty list in the usual way.

We should certainly _not_ be dispatching to the method call handler of
an interface which has no properties (since it will not be expecting
this).

Add a check to make sure that there is at least one readable property
before assuming that a NULL get_property handler implies that we want to
handle properties asynchronously.

Add a testcase that was failing before the change and works after it.

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

diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index 8ea5c43..8a0748d 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -4538,6 +4538,21 @@ invoke_get_all_properties_in_idle_cb (gpointer _data)
   return FALSE;
 }
 
+static gboolean
+interface_has_readable_properties (GDBusInterfaceInfo *interface_info)
+{
+  gint i;
+
+  if (!interface_info->properties)
+    return FALSE;
+
+  for (i = 0; interface_info->properties[i]; i++)
+    if (interface_info->properties[i]->flags & G_DBUS_PROPERTY_INFO_FLAGS_READABLE)
+      return TRUE;
+
+  return FALSE;
+}
+
 /* called in any thread with connection's lock held */
 static gboolean
 validate_and_maybe_schedule_property_get_all (GDBusConnection            *connection,
@@ -4558,10 +4573,11 @@ validate_and_maybe_schedule_property_get_all (GDBusConnection            *connec
   if (vtable == NULL)
     goto out;
 
-  /* If the vtable pointer for get_property() is NULL, then dispatch the
-   * call via the method_call() handler.
+  /* If the vtable pointer for get_property() is NULL but we have a
+   * non-zero number of readable properties, then dispatch the call via
+   * the method_call() handler.
    */
-  if (vtable->get_property == NULL)
+  if (vtable->get_property == NULL && interface_has_readable_properties (interface_info))
     {
       schedule_method_call (connection, message, registration_id, subtree_registration_id,
                             interface_info, NULL, NULL, g_dbus_message_get_body (message),
diff --git a/gio/tests/gdbus-export.c b/gio/tests/gdbus-export.c
index 03e6c6c..a1a9bc4 100644
--- a/gio/tests/gdbus-export.c
+++ b/gio/tests/gdbus-export.c
@@ -122,6 +122,15 @@ static const GDBusInterfaceInfo foo_interface_info =
   NULL,
 };
 
+/* Foo2 is just Foo without the properties */
+static const GDBusInterfaceInfo foo2_interface_info =
+{
+  -1,
+  "org.example.Foo2",
+  (GDBusMethodInfo **) &foo_method_info_pointers,
+  (GDBusSignalInfo **) &foo_signal_info_pointers,
+};
+
 static void
 foo_method_call (GDBusConnection       *connection,
                  const gchar           *sender,
@@ -1547,6 +1556,12 @@ test_async_method_call (GDBusConnection       *connection,
 
   property = g_dbus_method_invocation_get_property_info (invocation);
 
+  /* We should never be seeing any property calls on the com.example.Bar
+   * interface because it doesn't export any properties.
+   *
+   * In each case below make sure the interface is org.example.Foo.
+   */
+
   /* Do a whole lot of asserts to make sure that invalid calls are still
    * getting properly rejected by GDBusConnection and that our
    * environment is as we expect it to be.
@@ -1652,7 +1667,7 @@ static void
 test_async_properties (void)
 {
   GError *error = NULL;
-  guint registration_id;
+  guint registration_id, registration_id2;
   static const GDBusInterfaceVTable vtable = {
     test_async_method_call, NULL, NULL
   };
@@ -1667,6 +1682,12 @@ test_async_properties (void)
                                                        &vtable, NULL, NULL, &error);
   g_assert_no_error (error);
   g_assert (registration_id);
+  registration_id2 = g_dbus_connection_register_object (c,
+                                                        "/foo",
+                                                        (GDBusInterfaceInfo *) &foo2_interface_info,
+                                                        &vtable, NULL, NULL, &error);
+  g_assert_no_error (error);
+  g_assert (registration_id);
 
   test_async_case (c, NULL, "random", "()");
 
@@ -1685,6 +1706,11 @@ test_async_properties (void)
   test_async_case (c, NULL, "GetAll", "(si)", "wrong signature", 5);
   test_async_case (c, NULL, "GetAll", "(s)", "org.example.WrongInterface");
 
+  /* Make sure that we get no unexpected async property calls for com.example.Foo2 */
+  test_async_case (c, NULL, "Get", "(ss)", "org.example.Foo2", "zzz");
+  test_async_case (c, NULL, "Set", "(ssv)", "org.example.Foo2", "zzz", g_variant_new_string (""));
+  test_async_case (c, "(@a{sv} {},)", "GetAll", "(s)", "org.example.Foo2");
+
   /* Now do the proper things */
   test_async_case (c, "(<'PropertyUno'>,)", "Get", "(ss)", "org.example.Foo", "PropertyUno");
   test_async_case (c, "(<'NotWritable'>,)", "Get", "(ss)", "org.example.Foo", "NotWritable");
@@ -1696,6 +1722,7 @@ test_async_properties (void)
     g_main_context_iteration (NULL, TRUE);
 
   g_dbus_connection_unregister_object (c, registration_id);
+  g_dbus_connection_unregister_object (c, registration_id2);
   g_object_unref (c);
 }