monitor test: Reproduce dbus/dbus#457

The exact failure mode reported in dbus/dbus#457 is quite difficult
to achieve in a reliable way in a unit test, because we'd have to send
enough messages to a client to fill up its queue, then stop that client
from draining its queue, while still triggering a message that gets a
reply from the bus driver. However, we can trigger the same crash in a
slightly different way by not allowing the client to receive a
particular message. I chose NameAcquired.

Signed-off-by: Simon McVittie <smcv@collabora.com>

diff --git a/test/data/valid-config-files/forbidding.conf.in b/test/data/valid-config-files/forbidding.conf.in
index d145613c06811104fd7577e6b2e403cf244a4719..58b3cc6a7f88eb5cac18ead92981177063febaad 100644 (file)
--- a/test/data/valid-config-files/forbidding.conf.in
+++ b/test/data/valid-config-files/forbidding.conf.in
@@ -24,5 +24,8 @@
     <allow send_interface="com.example.CannotUnicast2" send_broadcast="true"/>
 
     <deny receive_interface="com.example.CannotReceive"/>
+
+    <!-- Used to reproduce dbus#457 -->
+    <deny receive_interface="org.freedesktop.DBus" receive_member="NameAcquired"/>
   </policy>
 </busconfig>

diff --git a/test/monitor.c b/test/monitor.c
index d5a54b002bcd9b87110cbba999810cc49305ce28..846a980c7e8f86d4684bcd90e479eea7dd2114fd 100644 (file)
--- a/test/monitor.c
+++ b/test/monitor.c
@@ -155,6 +155,21 @@ static Config side_effects_config = {
     TRUE
 };
 
+static dbus_bool_t
+config_forbids_name_acquired_signal (const Config *config)
+{
+  if (config == NULL)
+    return FALSE;
+
+  if (config->config_file == NULL)
+    return FALSE;
+
+  if (strcmp (config->config_file, forbidding_config.config_file) == 0)
+    return TRUE;
+
+  return FALSE;
+}
+
 static inline const char *
 not_null2 (const char *x,
     const char *fallback)
@@ -253,9 +268,6 @@ do { \
 
 #define assert_name_acquired(m) \
 do { \
-  DBusError _e = DBUS_ERROR_INIT; \
-  const char *_s; \
-    \
   g_assert_cmpstr (dbus_message_type_to_string (dbus_message_get_type (m)), \
       ==, dbus_message_type_to_string (DBUS_MESSAGE_TYPE_SIGNAL)); \
   g_assert_cmpstr (dbus_message_get_sender (m), ==, DBUS_SERVICE_DBUS); \
@@ -265,7 +277,14 @@ do { \
   g_assert_cmpstr (dbus_message_get_signature (m), ==, "s"); \
   g_assert_cmpint (dbus_message_get_serial (m), !=, 0); \
   g_assert_cmpint (dbus_message_get_reply_serial (m), ==, 0); \
+} while (0)
+
+#define assert_unique_name_acquired(m) \
+do { \
+  DBusError _e = DBUS_ERROR_INIT; \
+  const char *_s; \
     \
+  assert_name_acquired (m); \
   dbus_message_get_args (m, &_e, \
         DBUS_TYPE_STRING, &_s, \
         DBUS_TYPE_INVALID); \
@@ -333,6 +352,21 @@ do { \
   g_assert_cmpint (dbus_message_get_reply_serial (m), !=, 0); \
 } while (0)
 
+/* forbidding.conf does not allow receiving NameAcquired, so if we are in
+ * that configuration, then dbus-daemon synthesizes an error reply to itself
+ * and sends that to monitors */
+#define expect_name_acquired_error(queue, in_reply_to) \
+do { \
+  DBusMessage *message; \
+  \
+  message = g_queue_pop_head (queue); \
+  assert_error_reply (message, DBUS_SERVICE_DBUS, DBUS_SERVICE_DBUS, \
+                      DBUS_ERROR_ACCESS_DENIED); \
+  g_assert_cmpint (dbus_message_get_reply_serial (message), ==, \
+                   dbus_message_get_serial (in_reply_to)); \
+  dbus_message_unref (message); \
+} while (0)
+
 /* This is called after processing pending replies to our own method
  * calls, but before anything else.
  */
@@ -727,6 +761,11 @@ test_become_monitor (Fixture *f,
   test_assert_no_error (&f->e);
   g_assert_cmpint (ret, ==, DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER);
 
+  /* If the policy forbids receiving NameAcquired, then we'll never
+   * receive it, so behave as though we had */
+  if (config_forbids_name_acquired_signal (f->config))
+    got_unique = got_a = got_b = got_c = TRUE;
+
   while (!got_unique || !got_a || !got_b || !got_c)
     {
       if (g_queue_is_empty (&f->monitored))
@@ -1378,6 +1417,7 @@ test_dbus_daemon (Fixture *f,
 {
   DBusMessage *m;
   int res;
+  size_t n_expected;
 
   if (f->address == NULL)
     return;
@@ -1393,7 +1433,12 @@ test_dbus_daemon (Fixture *f,
   test_assert_no_error (&f->e);
   g_assert_cmpint (res, ==, DBUS_RELEASE_NAME_REPLY_RELEASED);
 
-  while (g_queue_get_length (&f->monitored) < 8)
+  n_expected = 8;
+
+  if (config_forbids_name_acquired_signal (context))
+    n_expected += 1;
+
+  while (g_queue_get_length (&f->monitored) < n_expected)
     test_main_context_iterate (f->ctx, TRUE);
 
   m = g_queue_pop_head (&f->monitored);
@@ -1406,10 +1451,12 @@ test_dbus_daemon (Fixture *f,
       "NameOwnerChanged", "sss", NULL);
   dbus_message_unref (m);
 
-  /* FIXME: should we get this? */
   m = g_queue_pop_head (&f->monitored);
-  assert_signal (m, DBUS_SERVICE_DBUS, DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS,
-      "NameAcquired", "s", f->sender_name);
+  assert_name_acquired (m);
+
+  if (config_forbids_name_acquired_signal (f->config))
+    expect_name_acquired_error (&f->monitored, m);
+
   dbus_message_unref (m);
 
   m = g_queue_pop_head (&f->monitored);
@@ -1631,8 +1678,14 @@ static void
 expect_new_connection (Fixture *f)
 {
   DBusMessage *m;
+  size_t n_expected;
 
-  while (g_queue_get_length (&f->monitored) < 4)
+  n_expected = 4;
+
+  if (config_forbids_name_acquired_signal (f->config))
+    n_expected += 1;
+
+  while (g_queue_get_length (&f->monitored) < n_expected)
     test_main_context_iterate (f->ctx, TRUE);
 
   m = g_queue_pop_head (&f->monitored);
@@ -1649,7 +1702,11 @@ expect_new_connection (Fixture *f)
   dbus_message_unref (m);
 
   m = g_queue_pop_head (&f->monitored);
-  assert_name_acquired (m);
+  assert_unique_name_acquired (m);
+
+  if (config_forbids_name_acquired_signal (f->config))
+    expect_name_acquired_error (&f->monitored, m);
+
   dbus_message_unref (m);
 }
 
@@ -1988,6 +2045,8 @@ main (int argc,
       setup, test_method_call, teardown);
   g_test_add ("/monitor/forbidden-method", Fixture, &forbidding_config,
       setup, test_forbidden_method_call, teardown);
+  g_test_add ("/monitor/forbidden-reply", Fixture, &forbidding_config,
+      setup, test_dbus_daemon, teardown);
   g_test_add ("/monitor/dbus-daemon", Fixture, NULL,
       setup, test_dbus_daemon, teardown);
   g_test_add ("/monitor/selective", Fixture, &selective_config,