From 06b353a1d037a8fb140c187d948f4c2ef5d1bea1 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Mon, 27 Nov 2017 15:51:15 +0000
Subject: [PATCH 4/9] squash! Add a test for header fields

---

Use g_test_slow() instead of reinventing it. Do some limited OOM
testing even in quick mode (it takes about a second per test on my
laptop, so hopefully no more than a minute even when run on a potato).

Send a method call that expects a reply, and stop waiting for it to be
delivered to the other side if we get an error reply. Otherwise, we'll
wait forever in the case where our attempt to send the message is the
thing that runs out of memory.

Correct copypasta in comment.
---
 test/header-fields.c | 79 ++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 61 insertions(+), 18 deletions(-)

diff --git a/test/header-fields.c b/test/header-fields.c
index f33ce0df..150a02ac 100644
--- a/test/header-fields.c
+++ b/test/header-fields.c
@@ -1,4 +1,4 @@
-/* Targeted unit tests for OOM paths in DBusMessage
+/* Unit tests for detailed header field manipulation
  *
  * Copyright © 2017 Collabora Ltd.
  *
@@ -118,6 +118,18 @@ string_overwrite_n (DBusString *str,
   memcpy (data, bytes, len);
 }
 
+static void
+steal_reply_cb (DBusPendingCall *pc,
+                void *data)
+{
+  DBusMessage **message_p = data;
+
+  g_assert (message_p != NULL);
+  g_assert (*message_p == NULL);
+  *message_p = dbus_pending_call_steal_reply (pc);
+  g_assert (*message_p != NULL);
+}
+
 /*
  * Test the handling of unknown header fields.
  *
@@ -136,6 +148,8 @@ test_weird_header_field (void        *user_data,
   DBusMessage *modified = NULL;
   DBusMessage *filtered = NULL;
   DBusMessage *relayed = NULL;
+  DBusMessage *reply = NULL;
+  DBusPendingCall *pc = NULL;
   char *blob = NULL;
   int blob_len;
   DBusString modified_blob = _DBUS_STRING_INIT_INVALID;
@@ -215,9 +229,6 @@ test_weird_header_field (void        *user_data,
   /* Messages with serial number 0 can't be demarshalled. */
   dbus_message_set_serial (original, 42);
 
-  /* We aren't going to wait for a reply anyway. */
-  dbus_message_set_no_reply (original, TRUE);
-
   if (!dbus_message_marshal (original, &blob, &blob_len))
     {
       g_assert_false (have_memory);
@@ -521,15 +532,36 @@ test_weird_header_field (void        *user_data,
    * unknown header fields, but it should not.
    * https://bugs.freedesktop.org/show_bug.cgi?id=100317 */
 
-  if (!dbus_connection_send (f->left_conn, modified, NULL))
+  /* We have to use send_with_reply because if we don't, we won't know
+   * if the message was dropped on the floor due to out-of-memory. */
+  if (!dbus_connection_send_with_reply (f->left_conn, modified, &pc, -1))
     {
       g_assert_false (have_memory);
       goto out;
     }
 
-  while (g_queue_get_length (&f->held_messages) < 1)
+  if (dbus_pending_call_get_completed (pc))
+    {
+      steal_reply_cb (pc, &reply);
+    }
+  else if (!dbus_pending_call_set_notify (pc, steal_reply_cb, &reply, NULL))
+    {
+      dbus_pending_call_cancel (pc);
+      g_assert_false (have_memory);
+      goto out;
+    }
+
+  while (g_queue_get_length (&f->held_messages) < 1 && reply == NULL)
     test_main_context_iterate (f->ctx, TRUE);
 
+  if (reply != NULL)
+    {
+      dbus_set_error_from_message (&error, reply);
+      g_assert_cmpstr (error.name, ==, DBUS_ERROR_NO_MEMORY);
+      g_assert_false (have_memory);
+      goto out;
+    }
+
   relayed = g_queue_pop_head (&f->held_messages);
   g_assert_cmpint (g_queue_get_length (&f->held_messages), ==, 0);
 
@@ -600,10 +632,19 @@ test_weird_header_field (void        *user_data,
       _dbus_type_reader_next (&array);
     }
 
+  /* On success, we don't actually reply: it's one more thing that
+   * could hit OOM */
+
 out:
   g_clear_error (&gerror);
   _dbus_string_free (&modified_blob);
   dbus_free (blob);
+
+  if (pc != NULL)
+    dbus_pending_call_cancel (pc);
+
+  dbus_clear_pending_call (&pc);
+  dbus_clear_message (&reply);
   dbus_clear_message (&relayed);
   dbus_clear_message (&filtered);
   dbus_clear_message (&modified);
@@ -655,21 +696,20 @@ test_oom_wrapper (Fixture *f,
 
   f->mode = test->mode;
 
-  if (_dbus_getenv ("DBUS_TEST_SLOW") != NULL &&
-      atoi (_dbus_getenv ("DBUS_TEST_SLOW")) >= 2)
+  if (g_test_slow ())
     {
       /* When we say slow, we mean it. */
       test_timeout_reset (30);
-
-      if (!_dbus_test_oom_handling (test->name, test->function, f))
-        {
-          g_test_message ("OOM test failed");
-          g_test_fail ();
-        }
     }
   else
     {
-      test->function (f, TRUE);
+      test_timeout_reset (1);
+    }
+
+  if (!_dbus_test_oom_handling (test->name, test->function, f))
+    {
+      g_test_message ("OOM test failed");
+      g_test_fail ();
     }
 }
 
@@ -699,12 +739,15 @@ main (int argc,
 {
   int ret;
 
+  test_init (&argc, &argv);
+
   /* Normally we test up to 4 consecutive malloc failures, but that's
    * painfully slow here. */
   if (g_getenv ("DBUS_TEST_MALLOC_FAILURES") == NULL)
-    g_setenv ("DBUS_TEST_MALLOC_FAILURES", "3", TRUE);
-
-  test_init (&argc, &argv);
+    {
+      if (!g_test_slow ())
+        g_setenv ("DBUS_TEST_MALLOC_FAILURES", "2", TRUE);
+    }
 
   test_cases_to_free = g_queue_new ();
   add_oom_test ("/message/weird-header-field/none", test_weird_header_field,
-- 
2.15.1