From 79d32c2fc18dcd62e9e12ca871676d35697c9d41 Mon Sep 17 00:00:00 2001 From: David Zeuthen Date: Thu, 17 Jun 2010 17:58:25 -0400 Subject: [PATCH] GDBusMessage: Fix bug when deserializing a message See https://bugzilla.gnome.org/show_bug.cgi?id=621838 for the whole story. The problem was that we ended up reading data from arrays of arrays when we were just supposed to be aligning the buffers. Also add a host of debug infrastructure that was needed to find the root cause. For now it can be turned on only via defining DEBUG_SERIALIZER. In the future we might want to make it work via G_DBUS_DEBUG. In a nutshell, the added debug info looks like this Parsing blob (blob_len = 0x0084 bytes) 0000: 6c 01 00 01 3c 00 00 00 41 00 00 00 37 00 00 00 l...<...A...7... 0010: 08 01 67 00 08 61 61 79 61 7b 73 76 7d 00 00 00 ..g..aaya{sv}... 0020: 01 01 6f 00 08 00 00 00 2f 66 6f 6f 2f 62 61 72 ..o...../foo/bar 0030: 00 00 00 00 00 00 00 00 03 01 73 00 06 00 00 00 ..........s..... 0040: 4d 65 6d 62 65 72 00 00 00 00 00 00 34 00 00 00 Member......4... 0050: 03 00 00 00 63 77 64 00 01 73 00 00 23 00 00 00 ....cwd..s..#... 0060: 2f 68 6f 6d 65 2f 64 61 76 69 64 7a 2f 48 61 63 /home/davidz/Hac 0070: 6b 69 6e 67 2f 67 6c 69 62 2f 67 69 6f 2f 74 65 king/glib/gio/te 0080: 73 74 73 00 sts. Parsing headers (blob_len = 0x0084 bytes) Reading type a{yv} from offset 0x000c: array spans 0x0037 bytes Reading type {yv} from offset 0x0010 Reading type y from offset 0x0010: 0x08 ' Reading type v from offset 0x0011 Reading type g from offset 0x0014: 'aaya{sv}' Reading type {yv} from offset 0x001e Reading type y from offset 0x0020: 0x01 '' Reading type v from offset 0x0021 Reading type o from offset 0x0024: '/foo/bar' Reading type {yv} from offset 0x0031 Reading type y from offset 0x0038: 0x03 '' Reading type v from offset 0x0039 Reading type s from offset 0x003c: 'Member' Parsing body (blob_len = 0x0084 bytes) Reading type (aaya{sv}) from offset 0x0047 Reading type aay from offset 0x0048: array spans 0x0000 bytes Reading type a{sv} from offset 0x004c: array spans 0x0034 bytes Reading type {sv} from offset 0x0050 Reading type s from offset 0x0050: 'cwd' Reading type v from offset 0x0058 Reading type s from offset 0x005b: '/home/davidz/Hacking/glib/gio/tests' OK Signed-off-by: David Zeuthen --- gio/gdbusmessage.c | 132 +++++++++++++++++++++++++++++++++++----- gio/gdbusprivate.c | 10 +-- gio/gdbusprivate.h | 2 + gio/tests/gdbus-serialization.c | 9 +++ 4 files changed, 134 insertions(+), 19 deletions(-) diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c index 18833c8..337ac75 100644 --- a/gio/gdbusmessage.c +++ b/gio/gdbusmessage.c @@ -20,6 +20,9 @@ * Author: David Zeuthen */ +/* Uncomment to debug serializer code */ +/* #define DEBUG_SERIALIZER */ + #include "config.h" #include @@ -42,6 +45,7 @@ #include "gmemoryoutputstream.h" #include "gseekable.h" #include "gioerror.h" +#include "gdbusprivate.h" #ifdef G_OS_UNIX #include "gunixfdlist.h" @@ -682,9 +686,14 @@ ensure_input_padding (GMemoryInputStream *mis, offset = g_seekable_tell (G_SEEKABLE (mis)); wanted_offset = ((offset + padding_size - 1) / padding_size) * padding_size; - /*g_debug ("ensure_input_padding(%d) pushed offset 0x%04x to 0x%04x", (gint) padding_size, (gint) offset, (gint) wanted_offset);*/ - - return g_seekable_seek (G_SEEKABLE (mis), wanted_offset, G_SEEK_SET, NULL, error); + if (offset != wanted_offset) + { + return g_seekable_seek (G_SEEKABLE (mis), wanted_offset, G_SEEK_SET, NULL, error); + } + else + { + return TRUE; + } } static gchar * @@ -759,15 +768,29 @@ parse_value_from_blob (GMemoryInputStream *mis, GDataInputStream *dis, const GVariantType *type, gboolean just_align, + guint indent, GError **error) { GVariant *ret; GError *local_error; + gboolean is_leaf; - /*g_debug ("Reading type %s from offset 0x%04x", g_variant_type_dup_string (type), (gint) g_seekable_tell (G_SEEKABLE (mis)));*/ +#ifdef DEBUG_SERIALIZER + if (!just_align) + { + gchar *s; + s = g_variant_type_dup_string (type); + g_print ("%*sReading type %s from offset 0x%04x", + indent, "", + s, + (gint) g_seekable_tell (G_SEEKABLE (mis))); + g_free (s); + } +#endif /* DEBUG_SERIALIZER */ ret = NULL; + is_leaf = TRUE; local_error = NULL; if (g_variant_type_equal (type, G_VARIANT_TYPE_BOOLEAN)) { @@ -969,18 +992,31 @@ parse_value_from_blob (GMemoryInputStream *mis, if (!ensure_input_padding (mis, 4, &local_error)) goto fail; - array_len = g_data_input_stream_read_uint32 (dis, NULL, &local_error); - if (local_error != NULL) - goto fail; - if (array_len > (2<<26)) + if (just_align) { - g_set_error (&local_error, - G_IO_ERROR, - G_IO_ERROR_INVALID_ARGUMENT, - _("Encountered array of length %" G_GUINT32_FORMAT " bytes. Maximum length is 2<<26 bytes."), - array_len); - goto fail; + array_len = 0; + } + else + { + array_len = g_data_input_stream_read_uint32 (dis, NULL, &local_error); + if (local_error != NULL) + goto fail; + + is_leaf = FALSE; +#ifdef DEBUG_SERIALIZER + g_print (": array spans 0x%04x bytes\n", array_len); +#endif /* DEBUG_SERIALIZER */ + + if (array_len > (2<<26)) + { + g_set_error (&local_error, + G_IO_ERROR, + G_IO_ERROR_INVALID_ARGUMENT, + _("Encountered array of length %" G_GUINT32_FORMAT " bytes. Maximum length is 2<<26 bytes."), + array_len); + goto fail; + } } g_variant_builder_init (&builder, type); @@ -993,6 +1029,7 @@ parse_value_from_blob (GMemoryInputStream *mis, dis, element_type, TRUE, + indent + 2, &local_error); g_assert (item == NULL); } @@ -1008,6 +1045,7 @@ parse_value_from_blob (GMemoryInputStream *mis, dis, element_type, FALSE, + indent + 2, &local_error); if (item == NULL) { @@ -1038,6 +1076,11 @@ parse_value_from_blob (GMemoryInputStream *mis, if (!ensure_input_padding (mis, 8, &local_error)) goto fail; + is_leaf = FALSE; +#ifdef DEBUG_SERIALIZER + g_print ("\n"); +#endif /* DEBUG_SERIALIZER */ + if (!just_align) { key_type = g_variant_type_key (type); @@ -1045,6 +1088,7 @@ parse_value_from_blob (GMemoryInputStream *mis, dis, key_type, FALSE, + indent + 2, &local_error); if (key == NULL) goto fail; @@ -1054,6 +1098,7 @@ parse_value_from_blob (GMemoryInputStream *mis, dis, value_type, FALSE, + indent + 2, &local_error); if (value == NULL) { @@ -1068,6 +1113,11 @@ parse_value_from_blob (GMemoryInputStream *mis, if (!ensure_input_padding (mis, 8, &local_error)) goto fail; + is_leaf = FALSE; +#ifdef DEBUG_SERIALIZER + g_print ("\n"); +#endif /* DEBUG_SERIALIZER */ + if (!just_align) { const GVariantType *element_type; @@ -1082,6 +1132,7 @@ parse_value_from_blob (GMemoryInputStream *mis, dis, element_type, FALSE, + indent + 2, &local_error); if (item == NULL) { @@ -1097,6 +1148,11 @@ parse_value_from_blob (GMemoryInputStream *mis, } else if (g_variant_type_is_variant (type)) { + is_leaf = FALSE; +#ifdef DEBUG_SERIALIZER + g_print ("\n"); +#endif /* DEBUG_SERIALIZER */ + if (!just_align) { guchar siglen; @@ -1126,6 +1182,7 @@ parse_value_from_blob (GMemoryInputStream *mis, dis, variant_type, FALSE, + indent + 2, &local_error); g_variant_type_free (variant_type); if (value == NULL) @@ -1147,9 +1204,38 @@ parse_value_from_blob (GMemoryInputStream *mis, } g_assert ((just_align && ret == NULL) || (!just_align && ret != NULL)); + +#ifdef DEBUG_SERIALIZER + if (ret != NULL) + { + if (is_leaf) + { + gchar *s; + if (g_variant_type_equal (type, G_VARIANT_TYPE_BYTE)) + { + s = g_strdup_printf ("0x%02x '%c'", g_variant_get_byte (ret), g_variant_get_byte (ret)); + } + else + { + s = g_variant_print (ret, FALSE); + } + g_print (": %s\n", s); + g_free (s); + } + } +#endif /* DEBUG_SERIALIZER */ + return ret; fail: +#ifdef DEBUG_SERIALIZER + g_print ("\n" + "%*sFAILURE: %s (%s, %d)\n", + indent, "", + local_error->message, + g_quark_to_string (local_error->domain), + local_error->code); +#endif /* DEBUG_SERIALIZER */ g_propagate_error (error, local_error); return NULL; } @@ -1306,10 +1392,24 @@ g_dbus_message_new_from_blob (guchar *blob, message_body_len = g_data_input_stream_read_uint32 (dis, NULL, NULL); message->priv->serial = g_data_input_stream_read_uint32 (dis, NULL, NULL); +#ifdef DEBUG_SERIALIZER + g_print ("Parsing blob (blob_len = 0x%04x bytes)\n", (gint) blob_len); + { + gchar *s; + s = _g_dbus_hexdump ((const gchar *) blob, blob_len, 2); + g_print ("%s\n", s); + g_free (s); + } +#endif /* DEBUG_SERIALIZER */ + +#ifdef DEBUG_SERIALIZER + g_print ("Parsing headers (blob_len = 0x%04x bytes)\n", (gint) blob_len); +#endif /* DEBUG_SERIALIZER */ headers = parse_value_from_blob (mis, dis, G_VARIANT_TYPE ("a{yv}"), FALSE, + 2, error); if (headers == NULL) goto out; @@ -1362,10 +1462,14 @@ g_dbus_message_new_from_blob (guchar *blob, tupled_signature_str = g_strdup_printf ("(%s)", signature_str); variant_type = g_variant_type_new (tupled_signature_str); g_free (tupled_signature_str); +#ifdef DEBUG_SERIALIZER + g_print ("Parsing body (blob_len = 0x%04x bytes)\n", (gint) blob_len); +#endif /* DEBUG_SERIALIZER */ message->priv->body = parse_value_from_blob (mis, dis, variant_type, FALSE, + 2, error); if (message->priv->body == NULL) { diff --git a/gio/gdbusprivate.c b/gio/gdbusprivate.c index ed1ece9..76809ee 100644 --- a/gio/gdbusprivate.c +++ b/gio/gdbusprivate.c @@ -56,8 +56,8 @@ /* ---------------------------------------------------------------------------------------------------- */ -static gchar * -hexdump (const gchar *data, gsize len, guint indent) +gchar * +_g_dbus_hexdump (const gchar *data, gsize len, guint indent) { guint n, m; GString *ret; @@ -589,7 +589,7 @@ _g_dbus_worker_do_read_cb (GInputStream *input_stream, if (message == NULL) { gchar *s; - s = hexdump (worker->read_buffer, worker->read_buffer_cur_size, 2); + s = _g_dbus_hexdump (worker->read_buffer, worker->read_buffer_cur_size, 2); g_warning ("Error decoding D-Bus message of %" G_GSIZE_FORMAT " bytes\n" "The error is: %s\n" "The payload is as follows:\n" @@ -621,7 +621,7 @@ _g_dbus_worker_do_read_cb (GInputStream *input_stream, s = g_dbus_message_print (message, 2); g_print ("%s", s); g_free (s); - s = hexdump (worker->read_buffer, worker->read_buffer_cur_size, 2); + s = _g_dbus_hexdump (worker->read_buffer, worker->read_buffer_cur_size, 2); g_print ("%s\n", s); g_free (s); } @@ -830,7 +830,7 @@ write_message (GDBusWorker *worker, s = g_dbus_message_print (data->message, 2); g_print ("%s", s); g_free (s); - s = hexdump (data->blob, data->blob_size, 2); + s = _g_dbus_hexdump (data->blob, data->blob_size, 2); g_print ("%s\n", s); g_free (s); } diff --git a/gio/gdbusprivate.h b/gio/gdbusprivate.h index ba2c8f4..337c278 100644 --- a/gio/gdbusprivate.h +++ b/gio/gdbusprivate.h @@ -75,6 +75,8 @@ gboolean _g_dbus_address_parse_entry (const gchar *address_entry, GVariantType * _g_dbus_compute_complete_signature (GDBusArgInfo **args); +gchar *_g_dbus_hexdump (const gchar *data, gsize len, guint indent); + /* ---------------------------------------------------------------------------------------------------- */ #ifdef G_OS_WIN32 diff --git a/gio/tests/gdbus-serialization.c b/gio/tests/gdbus-serialization.c index 3a49f07..edd7a76 100644 --- a/gio/tests/gdbus-serialization.c +++ b/gio/tests/gdbus-serialization.c @@ -647,6 +647,15 @@ message_serialize_complex (void) "value 1: array:\n" "value 2: array:\n" " string: `Something'\n"); + + /* https://bugzilla.gnome.org/show_bug.cgi?id=621838 */ + check_serialization (g_variant_new_parsed ("(@aay [], {'cwd': <'/home/davidz/Hacking/glib/gio/tests'>})"), + "value 0: array:\n" + "value 1: array:\n" + " dict_entry:\n" + " string: `cwd'\n" + " variant:\n" + " string: `/home/davidz/Hacking/glib/gio/tests'\n"); } -- 2.7.4