Attachment 136100 Details for Bug 100317 – [3/7] Add a test for header fields

[patch] [3/7] Add a test for header fields

0003-Add-a-test-for-header-fields.patch (text/plain), 29.17 KB, created by Simon McVittie on 2017-12-12 15:24:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2017-12-12 15:24:30 UTC

Size: 29.17 KB

patch

obsolete

>From f26cc51cd12aea2169b6015805e2f9bab7537639 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <smcv@collabora.com>
>Date: Mon, 27 Nov 2017 15:51:15 +0000
>Subject: [PATCH 3/7] Add a test for header fields
>
>Signed-off-by: Simon McVittie <smcv@collabora.com>
>---
> dbus/dbus-marshal-header.c    |   3 +
> dbus/dbus-marshal-recursive.h |   1 +
> dbus/dbus-string.h            |   3 +
> test/Makefile.am              |  10 +
> test/header-fields.c          | 723 ++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 740 insertions(+)
> create mode 100644 test/header-fields.c
>
>diff --git a/dbus/dbus-marshal-header.c b/dbus/dbus-marshal-header.c
>index 1984499e..a3d73f89 100644
>--- a/dbus/dbus-marshal-header.c
>+++ b/dbus/dbus-marshal-header.c
>@@ -612,6 +612,9 @@ _dbus_header_create (DBusHeader  *header,
>         goto oom;
>     }
> 
>+  /* Note that test/message.c relies on this being in the middle of the
>+   * message: if you change the order of serialization here (but why
>+   * would you?), please find some other way to retain test coverage. */
>   if (interface != NULL)
>     {
>       if (!write_basic_field (&array,
>diff --git a/dbus/dbus-marshal-recursive.h b/dbus/dbus-marshal-recursive.h
>index fa1d1efa..c20bb913 100644
>--- a/dbus/dbus-marshal-recursive.h
>+++ b/dbus/dbus-marshal-recursive.h
>@@ -111,6 +111,7 @@ DBUS_PRIVATE_EXPORT
> int         _dbus_type_reader_get_current_type          (const DBusTypeReader  *reader);
> DBUS_PRIVATE_EXPORT
> int         _dbus_type_reader_get_element_type          (const DBusTypeReader  *reader);
>+DBUS_PRIVATE_EXPORT
> int         _dbus_type_reader_get_value_pos             (const DBusTypeReader  *reader);
> DBUS_PRIVATE_EXPORT
> void        _dbus_type_reader_read_basic                (const DBusTypeReader  *reader,
>diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h
>index 5e2d945e..5fff1044 100644
>--- a/dbus/dbus-string.h
>+++ b/dbus/dbus-string.h
>@@ -92,6 +92,7 @@ DBUS_PRIVATE_EXPORT
> void          _dbus_string_init_const_len        (DBusString        *str,
>                                                   const char        *value,
>                                                   int                len);
>+DBUS_PRIVATE_EXPORT
> dbus_bool_t   _dbus_string_init_preallocated     (DBusString        *str,
>                                                   int                allocate_size);
> 
>@@ -112,6 +113,7 @@ char*         _dbus_string_get_data              (DBusString        *str);
> DBUS_PRIVATE_EXPORT
> const char*   _dbus_string_get_const_data        (const DBusString  *str);
> #endif /* _dbus_string_get_const_data */
>+DBUS_PRIVATE_EXPORT
> char*         _dbus_string_get_data_len          (DBusString        *str,
>                                                   int                start,
>                                                   int                len);
>@@ -226,6 +228,7 @@ dbus_bool_t   _dbus_string_insert_2_aligned      (DBusString        *str,
> dbus_bool_t   _dbus_string_insert_4_aligned      (DBusString        *str,
>                                                   int                insert_at,
>                                                   const unsigned char octets[4]);
>+DBUS_PRIVATE_EXPORT
> dbus_bool_t   _dbus_string_insert_8_aligned      (DBusString        *str,
>                                                   int                insert_at,
>                                                   const unsigned char octets[8]);
>diff --git a/test/Makefile.am b/test/Makefile.am
>index 57342034..bd1b3000 100644
>--- a/test/Makefile.am
>+++ b/test/Makefile.am
>@@ -161,6 +161,7 @@ installable_tests += \
> 	test-dbus-daemon \
> 	test-dbus-daemon-eavesdrop \
> 	test-fdpass \
>+	test-header-fields \
> 	test-message \
> 	test-monitor \
> 	test-loopback \
>@@ -317,6 +318,15 @@ test_dbus_daemon_eavesdrop_LDADD = \
>     $(GLIB_LIBS) \
>     $(NULL)
> 
>+test_header_fields_SOURCES = \
>+    header-fields.c \
>+    $(NULL)
>+test_header_fields_LDADD = \
>+    libdbus-testutils.la \
>+    $(top_builddir)/dbus/libdbus-internal.la \
>+    $(GLIB_LIBS) \
>+    $(NULL)
>+
> if DBUS_UNIX
> test_sd_activation_SOURCES = \
>     sd-activation.c \
>diff --git a/test/header-fields.c b/test/header-fields.c
>new file mode 100644
>index 00000000..f33ce0df
>--- /dev/null
>+++ b/test/header-fields.c
>@@ -0,0 +1,723 @@
>+/* Targeted unit tests for OOM paths in DBusMessage
>+ *
>+ * Copyright Â© 2017 Collabora Ltd.
>+ *
>+ * Permission is hereby granted, free of charge, to any person
>+ * obtaining a copy of this software and associated documentation files
>+ * (the "Software"), to deal in the Software without restriction,
>+ * including without limitation the rights to use, copy, modify, merge,
>+ * publish, distribute, sublicense, and/or sell copies of the Software,
>+ * and to permit persons to whom the Software is furnished to do so,
>+ * subject to the following conditions:
>+ *
>+ * The above copyright notice and this permission notice shall be
>+ * included in all copies or substantial portions of the Software.
>+ *
>+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>+ * SOFTWARE.
>+ */
>+
>+#include <config.h>
>+
>+#include <glib.h>
>+
>+#include <dbus/dbus.h>
>+#include "dbus/dbus-internals.h"
>+#include "dbus/dbus-marshal-recursive.h"
>+#include "dbus/dbus-message-private.h"
>+#include "dbus/dbus-string.h"
>+#include "dbus/dbus-test-tap.h"
>+#include "test-utils-glib.h"
>+
>+typedef struct {
>+    const gchar *mode;
>+    TestMainContext *ctx;
>+    DBusConnection *left_conn;
>+    DBusConnection *right_conn;
>+    GPid daemon_pid;
>+    gchar *address;
>+    GQueue held_messages;
>+    gboolean skip;
>+} Fixture;
>+
>+static DBusHandlerResult
>+hold_filter (DBusConnection *connection,
>+             DBusMessage *message,
>+             void *user_data)
>+{
>+  Fixture *f = user_data;
>+
>+  if (dbus_message_get_type (message) != DBUS_MESSAGE_TYPE_METHOD_CALL)
>+    return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
>+
>+  g_queue_push_tail (&f->held_messages, dbus_message_ref (message));
>+
>+  return DBUS_HANDLER_RESULT_HANDLED;
>+}
>+
>+static void
>+setup_dbus_daemon (Fixture *f,
>+                   gconstpointer context G_GNUC_UNUSED)
>+{
>+  f->address = test_get_dbus_daemon (NULL, TEST_USER_ME, NULL, &f->daemon_pid);
>+
>+  if (f->address == NULL)
>+    {
>+      f->skip = TRUE;
>+      return;
>+    }
>+}
>+
>+static void
>+teardown_dbus_daemon (Fixture *f,
>+                      gconstpointer context G_GNUC_UNUSED)
>+{
>+  if (f->daemon_pid != 0)
>+    {
>+      test_kill_pid (f->daemon_pid);
>+      g_spawn_close_pid (f->daemon_pid);
>+      f->daemon_pid = 0;
>+    }
>+
>+  g_clear_pointer (&f->address, g_free);
>+}
>+
>+/* Offset to byte order from start of header */
>+#define BYTE_ORDER_OFFSET    0
>+/* Offset to version from start of header */
>+#define VERSION_OFFSET       3
>+/* Offset to fields array length from start of header, in protocol v1 */
>+#define FIELDS_ARRAY_LENGTH_OFFSET 12
>+/** Offset to first field in header, in protocol v1 */
>+#define FIRST_FIELD_OFFSET 16
>+
>+/* Offset from start of _dbus_header_signature_str to the signature of
>+ * the fields array */
>+#define FIELDS_ARRAY_SIGNATURE_OFFSET 6
>+
>+/* A byte that is not a DBUS_HEADER_FIELD_* */
>+#define NOT_A_HEADER_FIELD 123
>+
>+_DBUS_STRING_DEFINE_STATIC(_dbus_header_signature_str, DBUS_HEADER_SIGNATURE);
>+
>+static void
>+string_overwrite_n (DBusString *str,
>+                    int         start,
>+                    const void *bytes,
>+                    int         len)
>+{
>+  unsigned char *data = _dbus_string_get_udata_len (str, start, len);
>+
>+  g_assert (data != NULL);
>+  memcpy (data, bytes, len);
>+}
>+
>+/*
>+ * Test the handling of unknown header fields.
>+ *
>+ * Return TRUE if the right thing happens, but the right thing might include
>+ * OOM.
>+ */
>+static dbus_bool_t
>+test_weird_header_field (void        *user_data,
>+                         dbus_bool_t  have_memory)
>+{
>+  Fixture *f = user_data;
>+  const char *body = "hello";
>+  const char *new_body = NULL;
>+  DBusError error = DBUS_ERROR_INIT;
>+  DBusMessage *original = NULL;
>+  DBusMessage *modified = NULL;
>+  DBusMessage *filtered = NULL;
>+  DBusMessage *relayed = NULL;
>+  char *blob = NULL;
>+  int blob_len;
>+  DBusString modified_blob = _DBUS_STRING_INIT_INVALID;
>+  /* This is the serialization of a struct (uv), assumed to be at an
>+   * 8-byte boundary. */
>+  const unsigned char weird_header[8] = {
>+      NOT_A_HEADER_FIELD,   /*< type code */
>+      1,                    /*< length of signature */
>+      'u',                  /*< signature: uint32 */
>+      '\0',                 /*< end of signature */
>+      /* no padding required */
>+      '\x12',               /*< uint32 0x12345678 (BE) or 0x78563412 (LE) */
>+      '\x34',
>+      '\x56',
>+      '\x78'
>+  };
>+  int bytes_needed;
>+  DBusTypeReader reader;
>+  DBusTypeReader array;
>+  gboolean added_hold_filter = FALSE;
>+  GError *gerror = NULL;
>+
>+  if (f->skip)
>+    return TRUE;
>+
>+  /* We'd normally do this in setup_dbus_daemon(), but then we couldn't
>+   * allocate memory that we want freed by dbus_shutdown(). */
>+  g_assert_cmpint (_dbus_get_malloc_blocks_outstanding (), ==, 0);
>+  f->ctx = test_main_context_try_get ();
>+
>+  if (f->ctx == NULL)
>+    {
>+      g_assert_false (have_memory);
>+      goto out;
>+    }
>+
>+  f->left_conn = test_try_connect_to_bus (f->ctx, f->address, &gerror);
>+
>+  if (f->left_conn == NULL)
>+    {
>+      g_assert_error (gerror, G_DBUS_ERROR, G_DBUS_ERROR_NO_MEMORY);
>+      g_assert_false (have_memory);
>+      goto out;
>+    }
>+
>+  f->right_conn = test_try_connect_to_bus (f->ctx, f->address, &gerror);
>+
>+  if (f->right_conn == NULL)
>+    {
>+      g_assert_error (gerror, G_DBUS_ERROR, G_DBUS_ERROR_NO_MEMORY);
>+      g_assert_false (have_memory);
>+      goto out;
>+    }
>+
>+  if (!dbus_connection_add_filter (f->right_conn, hold_filter, f, NULL))
>+    {
>+      g_assert_false (have_memory);
>+      goto out;
>+    }
>+
>+  added_hold_filter = TRUE;
>+
>+  original = dbus_message_new_method_call (dbus_bus_get_unique_name (f->right_conn),
>+                                           "/com/example/Path",
>+                                           "com.example.Interface",
>+                                           "Method");
>+
>+  if (original == NULL ||
>+      !dbus_message_append_args (original,
>+                                 DBUS_TYPE_STRING, &body,
>+                                 DBUS_TYPE_INVALID))
>+    {
>+      g_assert_false (have_memory);
>+      goto out;
>+    }
>+
>+  /* 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);
>+      goto out;
>+    }
>+
>+  /* We will add up to 8 bytes, so preallocate that much. */
>+  if (!_dbus_string_init_preallocated (&modified_blob, blob_len + 8) ||
>+      !_dbus_string_append_len (&modified_blob, blob, blob_len))
>+    {
>+      g_assert_false (have_memory);
>+      goto out;
>+    }
>+
>+  /* If these are false then we need to change our byte-twiddling */
>+  g_assert_cmpint (blob_len, >, FIRST_FIELD_OFFSET);
>+  g_assert_cmpint (blob[VERSION_OFFSET], ==, 1);
>+  g_assert_cmpint (blob[BYTE_ORDER_OFFSET], ==, DBUS_COMPILER_BYTE_ORDER);
>+
>+  if (f->mode == NULL)
>+    {
>+      /* Do nothing: don't insert a weird header field at all */
>+    }
>+  else if (g_str_equal (f->mode, "change"))
>+    {
>+      /* Replace the interface (which is optional anyway) with the
>+       * weird header field */
>+
>+      _dbus_type_reader_init (&reader, DBUS_COMPILER_BYTE_ORDER,
>+                              &_dbus_header_signature_str,
>+                              FIELDS_ARRAY_SIGNATURE_OFFSET,
>+                              &modified_blob,
>+                              FIELDS_ARRAY_LENGTH_OFFSET);
>+      _dbus_type_reader_recurse (&reader, &array);
>+
>+      while (_dbus_type_reader_get_current_type (&array) !=
>+             DBUS_TYPE_INVALID)
>+        {
>+          DBusTypeReader sub;
>+          unsigned char field_code;
>+
>+          _dbus_type_reader_recurse (&array, &sub);
>+
>+          g_assert_cmpint (_dbus_type_reader_get_current_type (&sub),
>+                           ==, DBUS_TYPE_BYTE);
>+          _dbus_type_reader_read_basic (&sub, &field_code);
>+
>+          if (field_code == DBUS_HEADER_FIELD_INTERFACE)
>+            {
>+              _dbus_string_set_byte (&modified_blob,
>+                                     _dbus_type_reader_get_value_pos (&sub),
>+                                     NOT_A_HEADER_FIELD);
>+              break;
>+            }
>+
>+          _dbus_type_reader_next (&array);
>+        }
>+    }
>+  else if (g_str_equal (f->mode, "prepend"))
>+    {
>+      dbus_uint32_t header_fields_length;
>+
>+      memcpy (&header_fields_length, &blob[FIELDS_ARRAY_LENGTH_OFFSET], 4);
>+
>+      /* Insert a weird header field at the beginning of the fields
>+       * array. We do this by byte manipulation rather than by using a
>+       * DBusTypeReader, because we eventually want to get rid of the
>+       * counterintuitive ability for a DBusTypeReader to write to the
>+       * message: https://bugs.freedesktop.org/show_bug.cgi?id=38288 */
>+      if (!_dbus_string_insert_8_aligned (&modified_blob,
>+                                          FIRST_FIELD_OFFSET,
>+                                          weird_header))
>+        {
>+          g_assert_false (have_memory);
>+          goto out;
>+        }
>+
>+      header_fields_length += 8;
>+      string_overwrite_n (&modified_blob, FIELDS_ARRAY_LENGTH_OFFSET,
>+                          &header_fields_length, 4);
>+    }
>+  else if (g_str_equal (f->mode, "append"))
>+    {
>+      dbus_uint32_t header_fields_length;
>+
>+      memcpy (&header_fields_length, &blob[FIELDS_ARRAY_LENGTH_OFFSET], 4);
>+
>+      /* Insert a weird header field at the end of the fields
>+       * array, after the padding (which was previously the padding between
>+       * header and body, and is now the padding between the last-but-one
>+       * header field and the new header field). For simplicity, we've
>+       * used a weird header field that does not ever need to be followed
>+       * by padding itself.
>+       *
>+       * Old:
>+       *  | 0   | 1   | 2   | 3   | 4   | 5   | 6   | 7    |
>+       *  | ... 16-byte fixed-length part ...              |
>+       *  | ... 16-byte fixed-length part ...              |
>+       * [A] first header field ...                        |
>+       *  | last header field    [B] padding              [C]
>+       * [C] body...
>+       *
>+       * New:
>+       *  | 0   | 1   | 2   | 3   | 4   | 5   | 6   | 7    |
>+       *  | ... 16-byte fixed-length part ...              |
>+       *  | ... 16-byte fixed-length part ...              |
>+       * [A] first header field ...                        |
>+       *  | previously-last field [B] padding             [C]
>+       *  | weird header field, exactly 8 bytes long    [B'=C']
>+       * [B'=C'] body...
>+       *
>+       */
>+      header_fields_length = _DBUS_ALIGN_VALUE (header_fields_length, 8);
>+      g_assert_cmpint (header_fields_length % 8, ==, 0);
>+
>+      if (!_dbus_string_insert_8_aligned (&modified_blob,
>+                                          (FIRST_FIELD_OFFSET +
>+                                           header_fields_length),
>+                                          weird_header))
>+        {
>+          g_assert_false (have_memory);
>+          goto out;
>+        }
>+
>+      header_fields_length += 8;
>+      string_overwrite_n (&modified_blob, FIELDS_ARRAY_LENGTH_OFFSET,
>+                          &header_fields_length, 4);
>+    }
>+  else
>+    {
>+      g_assert_not_reached ();
>+    }
>+
>+  /* OK, now we've hacked up the message, compare it with the original. */
>+  bytes_needed = dbus_message_demarshal_bytes_needed (_dbus_string_get_const_data (&modified_blob),
>+                                                      _dbus_string_get_length (&modified_blob));
>+
>+  if (f->mode == NULL || g_str_equal (f->mode, "change"))
>+    {
>+      /* We edited the message in-place so its effective length didn't
>+       * change */
>+      g_assert_cmpint (bytes_needed, ==, blob_len);
>+    }
>+  else
>+    {
>+      g_assert_cmpint (bytes_needed, ==, blob_len + 8);
>+    }
>+
>+  g_assert_cmpint (_dbus_string_get_length (&modified_blob), ==,
>+                   bytes_needed);
>+  modified = dbus_message_demarshal (_dbus_string_get_const_data (&modified_blob),
>+                                     _dbus_string_get_length (&modified_blob),
>+                                     &error);
>+
>+  if (modified == NULL)
>+    {
>+      g_assert_cmpstr (error.name, ==, DBUS_ERROR_NO_MEMORY);
>+      g_assert_false (have_memory);
>+      goto out;
>+    }
>+
>+  /* The modified message has the same fields, except possibly the
>+   * interface. */
>+  g_assert_cmpint (dbus_message_get_type (modified), ==,
>+                   dbus_message_get_type (original));
>+  g_assert_cmpstr (dbus_message_get_path (modified), ==,
>+                   dbus_message_get_path (original));
>+  g_assert_cmpstr (dbus_message_get_member (modified), ==,
>+                   dbus_message_get_member (original));
>+  g_assert_cmpstr (dbus_message_get_error_name (modified), ==,
>+                   dbus_message_get_error_name (original));
>+  g_assert_cmpstr (dbus_message_get_destination (modified), ==,
>+                   dbus_message_get_destination (original));
>+  g_assert_cmpstr (dbus_message_get_sender (modified), ==,
>+                   dbus_message_get_sender (original));
>+  g_assert_cmpstr (dbus_message_get_signature (modified), ==,
>+                   dbus_message_get_signature (original));
>+  g_assert_cmpint (dbus_message_get_no_reply (modified), ==,
>+                   dbus_message_get_no_reply (original));
>+  g_assert_cmpint (dbus_message_get_serial (modified), ==,
>+                   dbus_message_get_serial (original));
>+  g_assert_cmpint (dbus_message_get_reply_serial (modified), ==,
>+                   dbus_message_get_reply_serial (original));
>+  g_assert_cmpint (dbus_message_get_auto_start (modified), ==,
>+                   dbus_message_get_auto_start (original));
>+  g_assert_cmpint (dbus_message_get_allow_interactive_authorization (modified),
>+                   ==,
>+                   dbus_message_get_allow_interactive_authorization (original));
>+
>+  if (dbus_message_get_args (modified, &error,
>+                             DBUS_TYPE_STRING, &new_body,
>+                             DBUS_TYPE_INVALID))
>+    {
>+      g_assert_cmpstr (new_body, ==, body);
>+    }
>+  else
>+    {
>+      g_assert_cmpstr (error.name, ==, DBUS_ERROR_NO_MEMORY);
>+      g_assert_false (have_memory);
>+      goto out;
>+    }
>+
>+  if (f->mode != NULL && g_str_equal (f->mode, "change"))
>+    {
>+      /* We edited the interface field in-place to turn it into the
>+       * unknown field, so it doesn't have an interface any more. */
>+      g_assert_cmpstr (dbus_message_get_interface (modified), ==, NULL);
>+    }
>+  else
>+    {
>+      /* We didn't change the interface. */
>+      g_assert_cmpstr (dbus_message_get_interface (modified), ==,
>+                       dbus_message_get_interface (original));
>+    }
>+
>+  /* Copy the modified message so we can filter it. */
>+  filtered = dbus_message_demarshal (_dbus_string_get_const_data (&modified_blob),
>+                                     _dbus_string_get_length (&modified_blob),
>+                                     &error);
>+
>+  if (filtered == NULL)
>+    {
>+      g_assert_cmpstr (error.name, ==, DBUS_ERROR_NO_MEMORY);
>+      g_assert_false (have_memory);
>+      goto out;
>+    }
>+
>+  /* TODO: Actually filter its header */
>+
>+  /* All known headers are the same as in the modified message that was
>+   * deserialized from the same blob */
>+  g_assert_cmpint (dbus_message_get_type (filtered), ==,
>+                   dbus_message_get_type (modified));
>+  g_assert_cmpstr (dbus_message_get_path (filtered), ==,
>+                   dbus_message_get_path (modified));
>+  g_assert_cmpstr (dbus_message_get_member (filtered), ==,
>+                   dbus_message_get_member (modified));
>+  g_assert_cmpstr (dbus_message_get_error_name (filtered), ==,
>+                   dbus_message_get_error_name (modified));
>+  g_assert_cmpstr (dbus_message_get_destination (filtered), ==,
>+                   dbus_message_get_destination (modified));
>+  g_assert_cmpstr (dbus_message_get_sender (filtered), ==,
>+                   dbus_message_get_sender (modified));
>+  g_assert_cmpstr (dbus_message_get_signature (filtered), ==,
>+                   dbus_message_get_signature (modified));
>+  g_assert_cmpint (dbus_message_get_no_reply (filtered), ==,
>+                   dbus_message_get_no_reply (modified));
>+  g_assert_cmpint (dbus_message_get_serial (filtered), ==,
>+                   dbus_message_get_serial (modified));
>+  g_assert_cmpint (dbus_message_get_reply_serial (filtered), ==,
>+                   dbus_message_get_reply_serial (modified));
>+  g_assert_cmpint (dbus_message_get_auto_start (filtered), ==,
>+                   dbus_message_get_auto_start (modified));
>+  g_assert_cmpint (dbus_message_get_allow_interactive_authorization (modified),
>+                   ==,
>+                   dbus_message_get_allow_interactive_authorization (original));
>+
>+  /* The body is also the same */
>+  if (dbus_message_get_args (filtered, &error,
>+                             DBUS_TYPE_STRING, &new_body,
>+                             DBUS_TYPE_INVALID))
>+    {
>+      g_assert_cmpstr (new_body, ==, body);
>+    }
>+  else
>+    {
>+      g_assert_cmpstr (error.name, ==, DBUS_ERROR_NO_MEMORY);
>+      g_assert_false (have_memory);
>+      goto out;
>+    }
>+
>+  /* We can't use _dbus_header_get_field_raw() because it asserts that
>+   * the field in question is in range. */
>+  _dbus_type_reader_init (&reader, DBUS_COMPILER_BYTE_ORDER,
>+                          &_dbus_header_signature_str,
>+                          FIELDS_ARRAY_SIGNATURE_OFFSET,
>+                          &filtered->header.data,
>+                          FIELDS_ARRAY_LENGTH_OFFSET);
>+  _dbus_type_reader_recurse (&reader, &array);
>+
>+  while (_dbus_type_reader_get_current_type (&array) != DBUS_TYPE_INVALID)
>+    {
>+      DBusTypeReader sub;
>+      unsigned char field_code;
>+
>+      _dbus_type_reader_recurse (&array, &sub);
>+
>+      g_assert_cmpint (_dbus_type_reader_get_current_type (&sub),
>+                       ==, DBUS_TYPE_BYTE);
>+      _dbus_type_reader_read_basic (&sub, &field_code);
>+
>+      if (field_code == NOT_A_HEADER_FIELD)
>+        {
>+          g_test_message ("TODO: header field was not filtered out");
>+        }
>+
>+      _dbus_type_reader_next (&array);
>+    }
>+
>+  /* Sending a message through the dbus-daemon currently preserves
>+   * 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))
>+    {
>+      g_assert_false (have_memory);
>+      goto out;
>+    }
>+
>+  while (g_queue_get_length (&f->held_messages) < 1)
>+    test_main_context_iterate (f->ctx, TRUE);
>+
>+  relayed = g_queue_pop_head (&f->held_messages);
>+  g_assert_cmpint (g_queue_get_length (&f->held_messages), ==, 0);
>+
>+  /* The message relayed through the dbus-daemon has the same known
>+   * fields and content as the one we sent, except that it has a sender
>+   * and a serial number. */
>+  g_assert_cmpint (dbus_message_get_type (relayed), ==,
>+                   dbus_message_get_type (modified));
>+  g_assert_cmpstr (dbus_message_get_path (relayed), ==,
>+                   dbus_message_get_path (modified));
>+  g_assert_cmpstr (dbus_message_get_member (relayed), ==,
>+                   dbus_message_get_member (modified));
>+  g_assert_cmpstr (dbus_message_get_error_name (relayed), ==,
>+                   dbus_message_get_error_name (modified));
>+  g_assert_cmpstr (dbus_message_get_destination (relayed), ==,
>+                   dbus_message_get_destination (modified));
>+  g_assert_cmpstr (dbus_message_get_sender (relayed), ==,
>+                   dbus_bus_get_unique_name (f->left_conn));
>+  g_assert_cmpstr (dbus_message_get_signature (relayed), ==,
>+                   dbus_message_get_signature (modified));
>+  g_assert_cmpint (dbus_message_get_no_reply (relayed), ==,
>+                   dbus_message_get_no_reply (modified));
>+  g_assert_cmpint (dbus_message_get_serial (relayed), !=, 0);
>+  g_assert_cmpint (dbus_message_get_reply_serial (relayed), ==,
>+                   dbus_message_get_reply_serial (modified));
>+  g_assert_cmpint (dbus_message_get_auto_start (relayed), ==,
>+                   dbus_message_get_auto_start (modified));
>+  g_assert_cmpint (dbus_message_get_allow_interactive_authorization (modified),
>+                   ==,
>+                   dbus_message_get_allow_interactive_authorization (original));
>+
>+  if (dbus_message_get_args (relayed, &error,
>+                             DBUS_TYPE_STRING, &new_body,
>+                             DBUS_TYPE_INVALID))
>+    {
>+      g_assert_cmpstr (new_body, ==, body);
>+    }
>+  else
>+    {
>+      g_assert_cmpstr (error.name, ==, DBUS_ERROR_NO_MEMORY);
>+      g_assert_false (have_memory);
>+      goto out;
>+    }
>+
>+  _dbus_type_reader_init (&reader, DBUS_COMPILER_BYTE_ORDER,
>+                          &_dbus_header_signature_str,
>+                          FIELDS_ARRAY_SIGNATURE_OFFSET,
>+                          &relayed->header.data,
>+                          FIELDS_ARRAY_LENGTH_OFFSET);
>+  _dbus_type_reader_recurse (&reader, &array);
>+
>+  while (_dbus_type_reader_get_current_type (&array) != DBUS_TYPE_INVALID)
>+    {
>+      DBusTypeReader sub;
>+      unsigned char field_code;
>+
>+      _dbus_type_reader_recurse (&array, &sub);
>+
>+      g_assert_cmpint (_dbus_type_reader_get_current_type (&sub),
>+                       ==, DBUS_TYPE_BYTE);
>+      _dbus_type_reader_read_basic (&sub, &field_code);
>+
>+      if (field_code == NOT_A_HEADER_FIELD)
>+        {
>+          g_test_message ("TODO: header field was passed through");
>+        }
>+
>+      _dbus_type_reader_next (&array);
>+    }
>+
>+out:
>+  g_clear_error (&gerror);
>+  _dbus_string_free (&modified_blob);
>+  dbus_free (blob);
>+  dbus_clear_message (&relayed);
>+  dbus_clear_message (&filtered);
>+  dbus_clear_message (&modified);
>+  dbus_clear_message (&original);
>+  dbus_error_free (&error);
>+
>+  /* We'd normally do this in teardown_dbus_daemon(), but for this test
>+   * we want to do it here so we can dbus_shutdown(). */
>+  if (f->left_conn != NULL)
>+    {
>+      dbus_connection_close (f->left_conn);
>+      test_connection_shutdown (f->ctx, f->left_conn);
>+    }
>+
>+  if (f->right_conn != NULL)
>+    {
>+      if (added_hold_filter)
>+        dbus_connection_remove_filter (f->right_conn, hold_filter, f);
>+
>+      g_queue_foreach (&f->held_messages, (GFunc) dbus_message_unref, NULL);
>+      g_queue_clear (&f->held_messages);
>+
>+      dbus_connection_close (f->right_conn);
>+      test_connection_shutdown (f->ctx, f->right_conn);
>+    }
>+
>+  dbus_clear_connection (&f->left_conn);
>+  dbus_clear_connection (&f->right_conn);
>+  g_clear_pointer (&f->ctx, test_main_context_unref);
>+
>+  dbus_shutdown ();
>+  g_assert_cmpint (_dbus_get_malloc_blocks_outstanding (), ==, 0);
>+
>+  return !g_test_failed ();
>+}
>+
>+typedef struct
>+{
>+  const gchar *name;
>+  DBusTestMemoryFunction function;
>+  const gchar *mode;
>+} OOMTestCase;
>+
>+static void
>+test_oom_wrapper (Fixture *f,
>+                  gconstpointer data)
>+{
>+  const OOMTestCase *test = data;
>+
>+  f->mode = test->mode;
>+
>+  if (_dbus_getenv ("DBUS_TEST_SLOW") != NULL &&
>+      atoi (_dbus_getenv ("DBUS_TEST_SLOW")) >= 2)
>+    {
>+      /* 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);
>+    }
>+}
>+
>+static GQueue *test_cases_to_free = NULL;
>+
>+static void
>+add_oom_test (const gchar *name,
>+              DBusTestMemoryFunction function,
>+              const gchar *mode)
>+{
>+  /* By using GLib memory allocation here, we avoid being affected by
>+   * dbus_shutdown() or contributing to
>+   * _dbus_get_malloc_blocks_outstanding() */
>+  OOMTestCase *test_case = g_new0 (OOMTestCase, 1);
>+
>+  test_case->name = name;
>+  test_case->function = function;
>+  test_case->mode = mode;
>+  g_test_add (name, Fixture, test_case, setup_dbus_daemon,
>+              test_oom_wrapper, teardown_dbus_daemon);
>+  g_queue_push_tail (test_cases_to_free, test_case);
>+}
>+
>+int
>+main (int argc,
>+      char **argv)
>+{
>+  int ret;
>+
>+  /* 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);
>+
>+  test_cases_to_free = g_queue_new ();
>+  add_oom_test ("/message/weird-header-field/none", test_weird_header_field,
>+                NULL);
>+  add_oom_test ("/message/weird-header-field/append", test_weird_header_field,
>+                "append");
>+  add_oom_test ("/message/weird-header-field/change", test_weird_header_field,
>+                "change");
>+  add_oom_test ("/message/weird-header-field/prepend", test_weird_header_field,
>+                "prepend");
>+
>+  ret = g_test_run ();
>+
>+  g_queue_free_full (test_cases_to_free, g_free);
>+  return ret;
>+}
>-- 
>2.15.1
>

Actions: View | Splinter Review

Attachments on bug 100317: 135758 | 135759 | 135760 | 135761 | 135762 | 135763 | 135764 | 135765 | 135766 | 135863 | 135864 | 135865 | 136098 | 136099 | 136100 | 136101 | 136102 | 136103 | 136104 | 136106 | 136107 | 136108 | 136287 | 136288 | 136289 | 136290 | 136296 | 136297

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.