From 4a9436ec831613139a9291956901813f3baf25d2 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 5 Nov 2021 12:28:29 +0100 Subject: [PATCH] GVariant: fix dbus_message_copy() dbus_message_copy() did not take into account differences between locked and unlocked GVariant messages. This commit adds support for converting from locked to unlocked GVariant message when a copy is made. Additionally: - it fixes initialization of the read iterator for unlocked messages; - locking or warning on such initialization is no longer needed. Change-Id: I4d316e1b1ae4e9af194ddc329833147c8c6a8055 --- dbus/dbus-marshal-gvariant.c | 132 ++++++++++++++++++++++++++++++++++++++++--- dbus/dbus-marshal-gvariant.h | 2 + dbus/dbus-message.c | 34 +++++++++++ 3 files changed, 160 insertions(+), 8 deletions(-) diff --git a/dbus/dbus-marshal-gvariant.c b/dbus/dbus-marshal-gvariant.c index 7a456c0..a3ecb07 100644 --- a/dbus/dbus-marshal-gvariant.c +++ b/dbus/dbus-marshal-gvariant.c @@ -860,12 +860,26 @@ _dbus_message_finalize_gvariant (DBusMessage *message, dbus_bool_t remove_signat static size_t _dbus_message_gvariant_get_body_length (DBusMessage *message) { - size_t body_len = _dbus_string_get_length (&message->body); - size_t message_len = body_len + _dbus_string_get_length (&message->header.data); - size_t offset_size = bus_gvariant_determine_word_size (message_len, 0); + size_t body_len; + size_t message_len; + size_t offset_size; + + body_len = _dbus_string_get_length (&message->body); + + /* Unlocked messages don't have any body offset or signature appended yet. */ + if (!message->locked) + return body_len; + + /* We need to find where the root container actually ends. + * We know there are a zero-byte, a signature and a body offset appended. + * We check offset's size, skip it, and go back through the signature until + * the zero byte, which is one byte past the end of the root container. + */ + message_len = body_len + _dbus_string_get_length (&message->header.data); + offset_size = bus_gvariant_determine_word_size (message_len, 0); if (body_len <= offset_size) - return 0; + return 0; body_len -= offset_size; @@ -1472,10 +1486,6 @@ void _dbus_type_reader_gvariant_init (DBusTypeReader *reader, DBusMessage *message) { -#ifndef DBUS_DISABLE_CHECKS - if (!message->locked) - _dbus_warn ("Warning: do not use dbus_message_iter_init() on an unlocked message; lock it first with dbus_message_lock()\n"); -#endif reader->gvariant = TRUE; /* GVariant wraps contents into struct, but in this place type is already * stripped off the parentheses (see get_const_signature()). @@ -1484,3 +1494,109 @@ _dbus_type_reader_gvariant_init (DBusTypeReader *reader, reader->value_end = _dbus_message_gvariant_get_body_length (message); reader->n_offsets = _dbus_reader_count_offsets (reader); } + +dbus_bool_t +_dbus_message_gvariant_convert_to_unlocked (DBusMessage *message) +{ + const char *signature; + DBusTypeReader reader; + DBusString signature_str; + int count_variable_sized = 0; + + _dbus_assert (_dbus_message_is_gvariant (message)); + _dbus_assert (message->locked); + + /* Differences between locked and unlocked GVariant messages: + * Locked: + * - body contains footer (zero byte, signature in parentheses, body offset); + * - there is no signature field in header; + * - 'signature' member is set if the signature was needed; + * - 'gvariant_body_last_'* members are unused. + * Unlocked: + * - body ends with root container offsets, if any; + * - header contains signature field; + * - 'signature' member is NULL; + * - 'gvariant_body_last_'* members are set to proper values. + * + * So, to convert to unlocked we need to: + * - check if the last type is a variable-size type; if so, we need + * to remember its end position for adding it as an offset to the body + * on first append (set 'gvariant_body_last_offset'); + * - set 'gvariant_body_last_pos' to correct writing position (just before the offsets); + * - extract signature from the body and move it to the header field; + * - ensure the 'signature' member is NULL; + * - shrink body length to end just after the offsets. + */ + + /* Get the signature C-string, we'll need it to move it from the body + * to the header field, and for iterating over types. + * We need to clear message->signature later, as the function sets it. + */ + signature = dbus_message_get_signature (message); + if (!signature) + { + _dbus_string_free (message->signature); + message->signature = NULL; + return FALSE; + } + + /* Initialize 'reader' for iterating over types from the signature. + */ + _dbus_string_init_const (&signature_str, signature); + reader.type_str = &signature_str; + reader.type_pos = 0; + + _dbus_type_reader_gvariant_init (&reader, message); + + /* If the last value is variable-sized, then the last offset is equal to the writing position + * The writing position is just before offsets. */ + message->gvariant_body_last_pos = reader.value_end - + (reader.n_offsets * bus_gvariant_determine_word_size (reader.value_end, 0)); + + /* Count variable-sized types on the root container level. */ + while (_dbus_string_get_byte (reader.type_str, reader.type_pos) != 0) + { + if (_dbus_reader_get_type_fixed_size (&reader, NULL) == 0) + count_variable_sized++; + _dbus_type_signature_next (_dbus_string_get_const_data(reader.type_str), &reader.type_pos); + } + + /* Check if all the offsets are present in the body. + * If not, then the last type is of variable size, and will be a subject to add to the body + * on the first append to this message. + */ + if (count_variable_sized == reader.n_offsets) + message->gvariant_body_last_offset = GVARIANT_LAST_OFFSET_NOT_SET; + else + message->gvariant_body_last_offset = message->gvariant_body_last_pos; + + /* Convert to unlocked. We'll modify our message now. */ + message->locked = FALSE; + + /* Copy signature to the header */ + if (!_dbus_header_set_field_basic (&message->header, + DBUS_HEADER_FIELD_SIGNATURE, + DBUS_TYPE_SIGNATURE, + &signature)) + { + _dbus_string_free (message->signature); + message->signature = NULL; + return FALSE; + } + + /* Unlocked DBusMessage needs to have NULL 'signature' member, since it has the above header field + * for signatures. Our 'message' has the member set, because dbus_message_get_signature() added it. + * Let's get rid of it. + */ + _dbus_assert (message->signature); + _dbus_string_free (message->signature); + message->signature = NULL; + + /* Unlocked messages have no signature, variant's zero byte, and final offset in the body. + * Cut them off. Luckily, _dbus_type_reader_gvariant_init() has already computed the correct + * length for us. + */ + _dbus_string_set_length (&message->body, reader.value_end); + + return TRUE; +} diff --git a/dbus/dbus-marshal-gvariant.h b/dbus/dbus-marshal-gvariant.h index 07da4da..ee27515 100644 --- a/dbus/dbus-marshal-gvariant.h +++ b/dbus/dbus-marshal-gvariant.h @@ -130,4 +130,6 @@ dbus_bool_t _dbus_writer_unrecurse_gvariant_write (DBusTypeWriter void _dbus_type_reader_gvariant_init (DBusTypeReader *reader, DBusMessage *message); +dbus_bool_t _dbus_message_gvariant_convert_to_unlocked (DBusMessage *message); + #endif /* DBUS_MARSHAL_GVARIANT_H */ diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 7d44a2e..fe43ef1 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -1841,9 +1841,43 @@ dbus_message_copy (const DBusMessage *message) #endif + if (_dbus_message_is_gvariant(message)) + { + if (message->locked) + { + /* This is a copy of locked GVariant message, so temporarily set it + * to locked to correctly extract data. Locked messages contain + * signature in the message body, while in unlocked messages it is + * in the struct DBusMessage in the header. + */ + retval->locked = TRUE; + if (!_dbus_message_gvariant_convert_to_unlocked (retval)) + goto failed_copy; + } + + if (message->unique_sender) + { + retval->unique_sender = dbus_new (DBusString, 1); + + if (!retval->unique_sender) + goto failed_copy; + + if (!_dbus_string_init_preallocated (retval->unique_sender, + _dbus_string_get_length (message->unique_sender))) + goto failed_copy_unique_sender; + + if (!_dbus_string_copy (message->unique_sender, 0, retval->unique_sender, 0)) + goto failed_copy_unique_sender; + } + } + _dbus_message_trace_ref (retval, 0, 1, "copy"); return retval; + failed_copy_unique_sender: + _dbus_string_free (retval->unique_sender); + dbus_free (retval->unique_sender); + failed_copy: _dbus_header_free (&retval->header); _dbus_string_free (&retval->body); -- 2.7.4