From 50c25505f62786756519ef1e194883360eda82e0 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Sun, 26 Jan 2003 07:48:16 +0000 Subject: 2003-01-26 Havoc Pennington The unit tests pass, but otherwise untested. If it breaks, the tests should have been better. ;-) * bus/driver.c (bus_driver_handle_hello): return if we disconnect the connection. * dbus/dbus-message.c: redo everything so we maintain message->header as the only copy of the various fields. This avoids the possibility of out-of-memory in some cases, for example dbus_message_lock() can't run out of memory anymore, and avoids extra copying. Figured I may as well go ahead and do this since it was busted for dbus_message_lock to not return failure on OOM, and dbus_message_write_header was totally unchecked for OOM. Also fixed some random other bugs. * dbus/dbus-marshal.c (_dbus_marshal_get_field_end_pos): verify that strings are nul-terminated. Also, end_pos can be equal to string length just not greater than, I think. (_dbus_marshal_set_int32): new function (_dbus_marshal_set_uint32): new function (_dbus_marshal_set_string): new function * dbus/dbus-connection.c (_dbus_connection_new_for_transport): fix a warning, init timeout_list to NULL (dbus_connection_send_message): don't use uninitialized variable "serial" * dbus/dbus-string.c (_dbus_string_replace_len): new function --- ChangeLog | 31 ++ bus/driver.c | 1 + dbus/dbus-connection.c | 3 +- dbus/dbus-marshal.c | 117 +++++++- dbus/dbus-marshal.h | 16 +- dbus/dbus-message-internal.h | 2 + dbus/dbus-message.c | 659 ++++++++++++++++++++++++++++++++----------- dbus/dbus-message.h | 2 +- dbus/dbus-string.c | 128 ++++++++- dbus/dbus-string.h | 11 + dbus/dbus-sysdeps.c | 1 + 11 files changed, 790 insertions(+), 181 deletions(-) diff --git a/ChangeLog b/ChangeLog index e82ae6f6..42ccbc52 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,34 @@ +2003-01-26 Havoc Pennington + + The unit tests pass, but otherwise untested. If it breaks, the + tests should have been better. ;-) + + * bus/driver.c (bus_driver_handle_hello): return if we disconnect + the connection. + + * dbus/dbus-message.c: redo everything so we maintain + message->header as the only copy of the various fields. + This avoids the possibility of out-of-memory in some cases, + for example dbus_message_lock() can't run out of memory anymore, + and avoids extra copying. Figured I may as well go ahead and do + this since it was busted for dbus_message_lock to not return + failure on OOM, and dbus_message_write_header was totally + unchecked for OOM. Also fixed some random other bugs. + + * dbus/dbus-marshal.c (_dbus_marshal_get_field_end_pos): verify + that strings are nul-terminated. Also, end_pos can be equal + to string length just not greater than, I think. + (_dbus_marshal_set_int32): new function + (_dbus_marshal_set_uint32): new function + (_dbus_marshal_set_string): new function + + * dbus/dbus-connection.c (_dbus_connection_new_for_transport): fix + a warning, init timeout_list to NULL + (dbus_connection_send_message): don't use uninitialized variable + "serial" + + * dbus/dbus-string.c (_dbus_string_replace_len): new function + 2003-01-26 Anders Carlsson * bus/driver.c: (bus_driver_handle_hello), diff --git a/bus/driver.c b/bus/driver.c index 1fd26a26..9f159a22 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -147,6 +147,7 @@ bus_driver_handle_hello (DBusConnection *connection, { dbus_free (name); dbus_connection_disconnect (connection); + return; } _DBUS_HANDLE_OOM (_dbus_string_init (&unique_name, _DBUS_INT_MAX)); diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 5d69203f..be7384b2 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -302,6 +302,7 @@ _dbus_connection_new_for_transport (DBusTransport *transport) watch_list = NULL; connection = NULL; handler_table = NULL; + timeout_list = NULL; watch_list = _dbus_watch_list_new (); if (watch_list == NULL) @@ -657,7 +658,7 @@ dbus_connection_send_message (DBusConnection *connection, } if (client_serial) - *client_serial = serial; + *client_serial = _dbus_message_get_client_serial (message); _dbus_message_lock (message); diff --git a/dbus/dbus-marshal.c b/dbus/dbus-marshal.c index 231e4b3b..aa6e2c6b 100644 --- a/dbus/dbus-marshal.c +++ b/dbus/dbus-marshal.c @@ -134,6 +134,98 @@ _dbus_pack_int32 (dbus_int32_t value, *((dbus_int32_t*)(data)) = DBUS_INT32_TO_BE (value); } +/** + * Sets the 4 bytes at the given offset to a marshaled signed integer, + * replacing anything found there previously. + * + * @param str the string to write the marshalled int to + * @param offset the byte offset where int should be written + * @param byte_order the byte order to use + * @param value the value + * + */ +void +_dbus_marshal_set_int32 (DBusString *str, + int byte_order, + int offset, + dbus_int32_t value) +{ + char *data; + + _dbus_assert (byte_order == DBUS_LITTLE_ENDIAN || + byte_order == DBUS_BIG_ENDIAN); + + _dbus_string_get_data_len (str, &data, offset, 4); + + _dbus_pack_int32 (value, byte_order, data); +} + +/** + * Sets the 4 bytes at the given offset to a marshaled unsigned + * integer, replacing anything found there previously. + * + * @param str the string to write the marshalled int to + * @param offset the byte offset where int should be written + * @param byte_order the byte order to use + * @param value the value + * + */ +void +_dbus_marshal_set_uint32 (DBusString *str, + int byte_order, + int offset, + dbus_uint32_t value) +{ + char *data; + + _dbus_assert (byte_order == DBUS_LITTLE_ENDIAN || + byte_order == DBUS_BIG_ENDIAN); + + _dbus_string_get_data_len (str, &data, offset, 4); + + _dbus_pack_uint32 (value, byte_order, data); +} + +/** + * Sets the existing marshaled string at the given offset with + * a new marshaled string. The given offset must point to + * an existing string or the wrong length will be deleted + * and replaced with the new string. + * + * @param str the string to write the marshalled string to + * @param offset the byte offset where string should be written + * @param byte_order the byte order to use + * @param value the value + * @returns #TRUE on success + * + */ +dbus_bool_t +_dbus_marshal_set_string (DBusString *str, + int byte_order, + int offset, + const DBusString *value) +{ + int old_len; + int new_len; + + _dbus_assert (byte_order == DBUS_LITTLE_ENDIAN || + byte_order == DBUS_BIG_ENDIAN); + + old_len = _dbus_demarshal_uint32 (str, byte_order, + offset, NULL); + + new_len = _dbus_string_get_length (value); + + if (!_dbus_string_replace_len (value, 0, new_len, + str, offset + 4, old_len)) + return FALSE; + + _dbus_marshal_set_uint32 (str, byte_order, + offset, new_len); + + return TRUE; +} + /** * Marshals a double value. * @@ -175,7 +267,7 @@ _dbus_marshal_int32 (DBusString *str, return FALSE; if (byte_order != DBUS_COMPILER_BYTE_ORDER) - swap_bytes ((unsigned char *)&value, sizeof (dbus_int32_t)); + value = DBUS_INT32_SWAP_LE_BE (value); return _dbus_string_append_len (str, (const char *)&value, sizeof (dbus_int32_t)); } @@ -197,7 +289,7 @@ _dbus_marshal_uint32 (DBusString *str, return FALSE; if (byte_order != DBUS_COMPILER_BYTE_ORDER) - swap_bytes ((unsigned char *)&value, sizeof (dbus_uint32_t)); + value = DBUS_UINT32_SWAP_LE_BE (value); return _dbus_string_append_len (str, (const char *)&value, sizeof (dbus_uint32_t)); } @@ -752,8 +844,13 @@ _dbus_demarshal_string_array (DBusString *str, /** * Returns the position right after the end position - * end position of a field + * end position of a field. Validates the field + * contents as required (e.g. ensures that + * string fields have a valid length and + * are nul-terminated). * + * @todo security: audit the field validation code. + * * @todo warns on invalid type in a message, but * probably the whole message needs to be dumped, * or we might even drop the connection due @@ -810,6 +907,18 @@ _dbus_marshal_get_field_end_pos (DBusString *str, *end_pos = pos + len + 1; + if (*end_pos > _dbus_string_get_length (str)) + { + _dbus_verbose ("string length outside length of the message\n"); + return FALSE; + } + + if (_dbus_string_get_byte (str, pos+len) != '\0') + { + _dbus_verbose ("string field not nul-terminated\n"); + return FALSE; + } + break; } @@ -888,7 +997,7 @@ _dbus_marshal_get_field_end_pos (DBusString *str, return FALSE; } - if (*end_pos >= _dbus_string_get_length (str)) + if (*end_pos > _dbus_string_get_length (str)) return FALSE; return TRUE; diff --git a/dbus/dbus-marshal.h b/dbus/dbus-marshal.h index 5184318e..bdeeccb9 100644 --- a/dbus/dbus-marshal.h +++ b/dbus/dbus-marshal.h @@ -46,11 +46,12 @@ (((dbus_uint32_t) (val) & (dbus_uint32_t) 0xff000000U) >> 24))) #define DBUS_UINT32_SWAP_LE_BE(val) (DBUS_UINT32_SWAP_LE_BE_CONSTANT (val)) +#define DBUS_INT32_SWAP_LE_BE(val) ((dbus_int32_t)DBUS_UINT32_SWAP_LE_BE_CONSTANT (val)) #ifdef WORDS_BIGENDIAN #define DBUS_INT32_TO_BE(val) ((dbus_int32_t) (val)) #define DBUS_UINT32_TO_BE(val) ((dbus_uint32_t) (val)) -#define DBUS_INT32_TO_LE(val) ((dbus_int32_t) DBUS_UINT32_SWAP_LE_BE (val)) +#define DBUS_INT32_TO_LE(val) (DBUS_INT32_SWAP_LE_BE (val)) #define DBUS_UINT32_TO_LE(val) (DBUS_UINT32_SWAP_LE_BE (val)) #else #define DBUS_INT32_TO_LE(val) ((dbus_int32_t) (val)) @@ -76,6 +77,19 @@ void _dbus_pack_uint32 (dbus_uint32_t value, dbus_uint32_t _dbus_unpack_uint32 (int byte_order, const unsigned char *data); +void _dbus_marshal_set_int32 (DBusString *str, + int byte_order, + int offset, + dbus_int32_t value); +void _dbus_marshal_set_uint32 (DBusString *str, + int byte_order, + int offset, + dbus_uint32_t value); +dbus_bool_t _dbus_marshal_set_string (DBusString *str, + int byte_order, + int offset, + const DBusString *value); + dbus_bool_t _dbus_marshal_int32 (DBusString *str, int byte_order, diff --git a/dbus/dbus-message-internal.h b/dbus/dbus-message-internal.h index 00adb40b..44ce62a3 100644 --- a/dbus/dbus-message-internal.h +++ b/dbus/dbus-message-internal.h @@ -39,6 +39,8 @@ void _dbus_message_unlock (DBusMessage *message); void _dbus_message_set_client_serial (DBusMessage *message, dbus_int32_t client_serial); dbus_int32_t _dbus_message_get_client_serial (DBusMessage *message); +dbus_bool_t _dbus_message_set_reply_serial (DBusMessage *message, + dbus_int32_t reply_serial); dbus_int32_t _dbus_message_get_reply_serial (DBusMessage *message); void _dbus_message_add_size_counter (DBusMessage *message, DBusCounter *counter); diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 962a2215..bf3c1b18 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -40,6 +40,26 @@ * @{ */ +enum +{ + FIELD_HEADER_LENGTH, + FIELD_BODY_LENGTH, + FIELD_CLIENT_SERIAL, + FIELD_NAME, + FIELD_SERVICE, + FIELD_SENDER, + FIELD_REPLY_SERIAL, + + FIELD_LAST +}; + +typedef struct +{ + int offset; /**< Offset to start of field (location of name of field + * for named fields) + */ +} HeaderField; + /** * @brief Internals of DBusMessage * @@ -56,16 +76,13 @@ struct DBusMessage * independently realloc it. */ + HeaderField header_fields[FIELD_LAST]; /**< Track the location + * of each field in "header" + */ + DBusString body; /**< Body network data. */ char byte_order; /**< Message byte order. */ - - char *name; /**< Message name. */ - char *service; /**< Message destination service. */ - char *sender; /**< Message sender service. */ - - dbus_int32_t client_serial; /**< Client serial or -1 if not set */ - dbus_int32_t reply_serial; /**< Reply serial or -1 if not set */ DBusCounter *size_counter; /**< Counter for the size of the message, or #NULL */ long size_counter_delta; /**< Size we incremented the size counter by. */ @@ -108,6 +125,231 @@ _dbus_message_get_network_data (DBusMessage *message, *body = &message->body; } +static void +adjust_field_offsets (DBusMessage *message, + int offsets_after, + int delta) +{ + int i; + + if (delta == 0) + return; + + i = 0; + while (i < FIELD_LAST) + { + if (message->header_fields[i].offset > offsets_after) + message->header_fields[i].offset += delta; + + ++i; + } +} + +static const char* +get_string_field (DBusMessage *message, + int field, + int *len) +{ + int offset = message->header_fields[field].offset; + const char *data; + + if (offset < 0) + return NULL; + + /* offset points to string length, string data follows it */ + /* FIXME _dbus_demarshal_const_string() that returned + * a reference to the string plus its len might be nice. + */ + + if (len) + *len = _dbus_demarshal_int32 (&message->header, + message->byte_order, + offset, + NULL); + + _dbus_string_get_const_data (&message->header, + &data); + + return data + (offset + 4); +} + +static dbus_int32_t +get_int_field (DBusMessage *message, + int field) +{ + int offset = message->header_fields[field].offset; + + if (offset < 0) + return -1; /* useless if -1 is a valid value of course */ + + return _dbus_demarshal_int32 (&message->header, + message->byte_order, + offset, + NULL); +} + +static dbus_bool_t +append_int_field (DBusMessage *message, + int field, + const char *name, + int value) +{ + int orig_len; + + _dbus_assert (!message->locked); + + orig_len = _dbus_string_get_length (&message->header); + + if (!_dbus_string_align_length (&message->header, 4)) + goto failed; + + if (!_dbus_string_append_len (&message->header, name, 4)) + goto failed; + + if (!_dbus_string_append_byte (&message->header, DBUS_TYPE_INT32)) + goto failed; + + if (!_dbus_string_align_length (&message->header, 4)) + goto failed; + + message->header_fields[FIELD_REPLY_SERIAL].offset = + _dbus_string_get_length (&message->header); + + if (!_dbus_marshal_int32 (&message->header, DBUS_COMPILER_BYTE_ORDER, + value)) + goto failed; + + return TRUE; + + failed: + message->header_fields[field].offset = -1; + _dbus_string_set_length (&message->header, orig_len); + return FALSE; +} + +static dbus_bool_t +append_string_field (DBusMessage *message, + int field, + const char *name, + const char *value) +{ + int orig_len; + + _dbus_assert (!message->locked); + + orig_len = _dbus_string_get_length (&message->header); + + if (!_dbus_string_align_length (&message->header, 4)) + goto failed; + + if (!_dbus_string_append_len (&message->header, name, 4)) + goto failed; + + if (!_dbus_string_append_byte (&message->header, DBUS_TYPE_STRING)) + goto failed; + + if (!_dbus_string_align_length (&message->header, 4)) + goto failed; + + message->header_fields[field].offset = + _dbus_string_get_length (&message->header); + + if (!_dbus_marshal_string (&message->header, DBUS_COMPILER_BYTE_ORDER, + value)) + goto failed; + + return TRUE; + + failed: + message->header_fields[field].offset = -1; + _dbus_string_set_length (&message->header, orig_len); + return FALSE; +} + +static dbus_bool_t +set_int_field (DBusMessage *message, + int field, + int value) +{ + int offset = message->header_fields[field].offset; + + _dbus_assert (!message->locked); + + if (offset < 0) + { + /* need to append the field */ + + switch (field) + { + case FIELD_REPLY_SERIAL: + return append_int_field (message, field, + DBUS_HEADER_FIELD_REPLY, + value); + default: + _dbus_assert_not_reached ("appending an int field we don't support appending"); + return FALSE; + } + } + else + { + _dbus_marshal_set_int32 (&message->header, + message->byte_order, + offset, value); + + return TRUE; + } +} + +static dbus_bool_t +set_string_field (DBusMessage *message, + int field, + const char *value) +{ + int offset = message->header_fields[field].offset; + + _dbus_assert (!message->locked); + _dbus_assert (value != NULL); + + if (offset < 0) + { + /* need to append the field */ + + switch (field) + { + case FIELD_SENDER: + return append_string_field (message, field, + DBUS_HEADER_FIELD_SENDER, + value); + default: + _dbus_assert_not_reached ("appending a string field we don't support appending"); + return FALSE; + } + } + else + { + DBusString v; + int old_len; + int new_len; + + old_len = _dbus_string_get_length (&message->header); + + _dbus_string_init_const_len (&v, value, + strlen (value) + 1); /* include nul */ + if (!_dbus_marshal_set_string (&message->header, + message->byte_order, + offset, &v)) + return FALSE; + + new_len = _dbus_string_get_length (&message->header); + + adjust_field_offsets (message, + offset, + new_len - old_len); + + return TRUE; + } +} + /** * Sets the client serial of a message. * This can only be done once on a message. @@ -119,9 +361,29 @@ void _dbus_message_set_client_serial (DBusMessage *message, dbus_int32_t client_serial) { - _dbus_assert (message->client_serial == -1); - - message->client_serial = client_serial; + _dbus_assert (!message->locked); + _dbus_assert (_dbus_message_get_client_serial (message) < 0); + + set_int_field (message, FIELD_CLIENT_SERIAL, + client_serial); +} + +/** + * Sets the reply serial of a message (the client serial + * of the message this is a reply to). + * + * @param message the message + * @param reply_serial the client serial + * @returns #FALSE if not enough memory + */ +dbus_bool_t +_dbus_message_set_reply_serial (DBusMessage *message, + dbus_int32_t reply_serial) +{ + _dbus_assert (!message->locked); + + return set_int_field (message, FIELD_REPLY_SERIAL, + reply_serial); } /** @@ -134,12 +396,12 @@ _dbus_message_set_client_serial (DBusMessage *message, dbus_int32_t _dbus_message_get_client_serial (DBusMessage *message) { - return message->client_serial; + return get_int_field (message, FIELD_CLIENT_SERIAL); } /** * Returns the serial that the message is - * a reply to. + * a reply to or -1 if none. * * @param message the message * @returns the reply serial @@ -147,7 +409,7 @@ _dbus_message_get_client_serial (DBusMessage *message) dbus_int32_t _dbus_message_get_reply_serial (DBusMessage *message) { - return message->reply_serial; + return get_int_field (message, FIELD_REPLY_SERIAL); } /** @@ -183,72 +445,47 @@ _dbus_message_add_size_counter (DBusMessage *message, _dbus_counter_adjust (message->size_counter, message->size_counter_delta); } -static void -dbus_message_write_header (DBusMessage *message) +static dbus_bool_t +dbus_message_create_header (DBusMessage *message, + const char *service, + const char *name) { - char *len_data; - - _dbus_assert (message->client_serial != -1); + if (!_dbus_string_append_byte (&message->header, DBUS_COMPILER_BYTE_ORDER)) + return FALSE; - _dbus_string_append_byte (&message->header, DBUS_COMPILER_BYTE_ORDER); - _dbus_string_append_len (&message->header, "\0\0\0", 3); + if (!_dbus_string_append_len (&message->header, "\0\0\0", 3)) + return FALSE; - /* We just lengthen the string here and pack in the real length later */ - _dbus_string_lengthen (&message->header, 4); - - _dbus_marshal_int32 (&message->header, DBUS_COMPILER_BYTE_ORDER, - _dbus_string_get_length (&message->body)); + message->header_fields[FIELD_HEADER_LENGTH].offset = 4; + if (!_dbus_marshal_int32 (&message->header, message->byte_order, 0)) + return FALSE; - /* Marshal client serial */ - _dbus_assert (message->client_serial >= 0); - _dbus_marshal_int32 (&message->header, DBUS_COMPILER_BYTE_ORDER, - message->client_serial); + message->header_fields[FIELD_BODY_LENGTH].offset = 8; + if (!_dbus_marshal_int32 (&message->header, message->byte_order, 0)) + return FALSE; + message->header_fields[FIELD_CLIENT_SERIAL].offset = 12; + if (!_dbus_marshal_int32 (&message->header, message->byte_order, -1)) + return FALSE; + /* Marshal message service */ - if (message->service) - { - _dbus_string_align_length (&message->header, 4); - _dbus_string_append_len (&message->header, DBUS_HEADER_FIELD_SERVICE, 4); - _dbus_string_append_byte (&message->header, DBUS_TYPE_STRING); - - _dbus_marshal_string (&message->header, DBUS_COMPILER_BYTE_ORDER, - message->service); - } - - /* Marshal message name */ - _dbus_assert (message->name != NULL); - _dbus_string_align_length (&message->header, 4); - _dbus_string_append_len (&message->header, DBUS_HEADER_FIELD_NAME, 4); - _dbus_string_append_byte (&message->header, DBUS_TYPE_STRING); - - _dbus_marshal_string (&message->header, DBUS_COMPILER_BYTE_ORDER, message->name); - - /* Marshal reply serial */ - if (message->reply_serial != -1) + if (service != NULL) { - _dbus_string_align_length (&message->header, 4); - _dbus_string_append_len (&message->header, DBUS_HEADER_FIELD_REPLY, 4); - - _dbus_string_append_byte (&message->header, DBUS_TYPE_INT32); - _dbus_marshal_int32 (&message->header, DBUS_COMPILER_BYTE_ORDER, - message->reply_serial); + if (!append_string_field (message, + FIELD_SERVICE, + DBUS_HEADER_FIELD_SERVICE, + service)) + return FALSE; } - /* Marshal sender */ - if (message->sender) - { - _dbus_string_align_length (&message->header, 4); - _dbus_string_append_len (&message->header, DBUS_HEADER_FIELD_SENDER, 4); - _dbus_string_append_byte (&message->header, DBUS_TYPE_STRING); + _dbus_assert (name != NULL); + if (!append_string_field (message, + FIELD_NAME, + DBUS_HEADER_FIELD_NAME, + name)) + return FALSE; - _dbus_marshal_string (&message->header, DBUS_COMPILER_BYTE_ORDER, - message->sender); - } - - /* Fill in the length */ - _dbus_string_get_data_len (&message->header, &len_data, 4, 4); - _dbus_pack_int32 (_dbus_string_get_length (&message->header), - DBUS_COMPILER_BYTE_ORDER, len_data); + return TRUE; } /** @@ -264,9 +501,18 @@ void _dbus_message_lock (DBusMessage *message) { if (!message->locked) - dbus_message_write_header (message); - - message->locked = TRUE; + { + /* Fill in our lengths */ + set_int_field (message, + FIELD_HEADER_LENGTH, + _dbus_string_get_length (&message->header)); + + set_int_field (message, + FIELD_BODY_LENGTH, + _dbus_string_get_length (&message->body)); + + message->locked = TRUE; + } } /** @} */ @@ -291,6 +537,43 @@ _dbus_message_lock (DBusMessage *message) * sent to another application. */ +static DBusMessage* +dbus_message_new_empty_header (void) +{ + DBusMessage *message; + int i; + + message = dbus_new0 (DBusMessage, 1); + if (message == NULL) + return NULL; + + message->refcount = 1; + message->byte_order = DBUS_COMPILER_BYTE_ORDER; + + i = 0; + while (i < FIELD_LAST) + { + message->header_fields[i].offset = -1; + ++i; + } + + if (!_dbus_string_init (&message->header, _DBUS_INT_MAX)) + { + dbus_free (message); + return NULL; + } + + if (!_dbus_string_init (&message->body, _DBUS_INT_MAX)) + { + _dbus_string_free (&message->header); + dbus_free (message); + return NULL; + } + + return message; +} + + /** * Constructs a new message. Returns #NULL if memory * can't be allocated for the message. @@ -308,33 +591,13 @@ dbus_message_new (const char *service, { DBusMessage *message; - message = dbus_new0 (DBusMessage, 1); + message = dbus_message_new_empty_header (); if (message == NULL) return NULL; - message->refcount = 1; - message->byte_order = DBUS_COMPILER_BYTE_ORDER; - - message->service = _dbus_strdup (service); - message->name = _dbus_strdup (name); - - message->client_serial = -1; - message->reply_serial = -1; - - if (!_dbus_string_init (&message->header, _DBUS_INT_MAX)) + if (!dbus_message_create_header (message, service, name)) { - dbus_free (message->service); - dbus_free (message->name); - dbus_free (message); - return NULL; - } - - if (!_dbus_string_init (&message->body, _DBUS_INT_MAX)) - { - dbus_free (message->service); - dbus_free (message->name); - _dbus_string_free (&message->header); - dbus_free (message); + dbus_message_unref (message); return NULL; } @@ -357,15 +620,24 @@ dbus_message_new_reply (const char *name, DBusMessage *original_message) { DBusMessage *message; + const char *sender; + + sender = get_string_field (original_message, + FIELD_SENDER, NULL); - _dbus_assert (original_message->sender != NULL); + _dbus_assert (sender != NULL); - message = dbus_message_new (original_message->sender, name); + message = dbus_message_new (sender, name); if (message == NULL) return NULL; - message->reply_serial = original_message->client_serial; + if (!_dbus_message_set_reply_serial (message, + _dbus_message_get_client_serial (original_message))) + { + dbus_message_unref (message); + return NULL; + } return message; } @@ -408,10 +680,7 @@ dbus_message_unref (DBusMessage *message) _dbus_string_free (&message->header); _dbus_string_free (&message->body); - - dbus_free (message->sender); - dbus_free (message->service); - dbus_free (message->name); + dbus_free (message); } } @@ -425,7 +694,7 @@ dbus_message_unref (DBusMessage *message) const char* dbus_message_get_name (DBusMessage *message) { - return message->name; + return get_string_field (message, FIELD_NAME, NULL); } /** @@ -437,7 +706,7 @@ dbus_message_get_name (DBusMessage *message) const char* dbus_message_get_service (DBusMessage *message) { - return message->service; + return get_string_field (message, FIELD_SERVICE, NULL); } /** @@ -695,6 +964,8 @@ dbus_message_append_string_array (DBusMessage *message, * field followed by a pointer to where the value should be * stored. The list is terminated with 0. * + * @todo rename get_args to avoid confusion with header fields + * * @param message the message * @param first_field_type the first field type * @param ... location for first field value, then list of type-location pairs @@ -727,6 +998,8 @@ dbus_message_get_fields (DBusMessage *message, * * @todo We need to free the field data when an error occurs. * + * @todo rename get_args_valist to avoid confusion with header fields + * * @see dbus_message_get_fields * @param message the message * @param first_field_type type of the first field @@ -846,7 +1119,7 @@ dbus_message_get_fields_valist (DBusMessage *message, spec_type = va_arg (var_args, int); if (spec_type != 0 && !dbus_message_iter_next (iter)) { - _dbus_verbose ("More fields than exist in the message were specified\n"); + _dbus_verbose ("More fields than exist in the message were specified or field is corrupt\n"); dbus_message_iter_unref (iter); return DBUS_RESULT_INVALID_FIELDS; @@ -868,6 +1141,8 @@ dbus_message_get_fields_valist (DBusMessage *message, * ref/unref is kind of annoying to deal with, and slower too. * This implies not ref'ing the message from the iter. * + * @todo rename get_args_iter to avoid confusion with header fields + * * @param message the message * @returns a new iter. */ @@ -1101,24 +1376,37 @@ dbus_message_iter_get_string_array (DBusMessageIter *iter, } /** - * Sets the message sender. + * Sets the message sender. + * + * @todo implement #NULL sender to unset * * @param message the message * @param sender the sender + * @returns #FALSE if not enough memory */ -void +dbus_bool_t dbus_message_set_sender (DBusMessage *message, - const char *sender) + const char *sender) { - _dbus_assert (!message->locked); + _dbus_assert (!message->locked); - message->sender = _dbus_strdup (sender); + if (sender == NULL) + { + _dbus_warn ("need to implement unsetting sender field\n"); + return TRUE; + } + else + { + return set_string_field (message, + FIELD_SENDER, + sender); + } } const char* dbus_message_get_sender (DBusMessage *message) { - return message->sender; + return get_string_field (message, FIELD_SENDER, NULL); } /** @} */ @@ -1306,24 +1594,28 @@ static dbus_bool_t decode_header_data (DBusString *data, int header_len, int byte_order, - dbus_int32_t *client_serial, - dbus_int32_t *reply_serial, - char **service, - char **name, - char **sender) + HeaderField fields[FIELD_LAST]) { const char *field; int pos, new_pos; + int i; - /* First demarshal the client serial */ - *client_serial = _dbus_demarshal_int32 (data, byte_order, 12, &pos); - - *reply_serial = -1; - *service = NULL; - *name = NULL; - *sender = NULL; + if (header_len < 16) + return FALSE; + i = 0; + while (i < FIELD_LAST) + { + fields[i].offset = -1; + ++i; + } + + fields[FIELD_HEADER_LENGTH].offset = 4; + fields[FIELD_BODY_LENGTH].offset = 8; + fields[FIELD_CLIENT_SERIAL].offset = 12; + /* Now handle the fields */ + pos = 16; while (pos < header_len) { pos = _DBUS_ALIGN_VALUE (pos, 4); @@ -1342,50 +1634,68 @@ decode_header_data (DBusString *data, switch (DBUS_UINT32_FROM_BE (*(int*)field)) { case DBUS_HEADER_FIELD_SERVICE_AS_UINT32: - if (*service != NULL) + if (fields[FIELD_SERVICE].offset >= 0) { _dbus_verbose ("%s field provided twice\n", DBUS_HEADER_FIELD_SERVICE); - goto failed; + return FALSE; } - *service = _dbus_demarshal_string (data, byte_order, pos + 1, &new_pos); - /* FIXME check for demarshal failure SECURITY */ + fields[FIELD_SERVICE].offset = _DBUS_ALIGN_VALUE (pos + 1, 4); + _dbus_verbose ("Found service name at offset %d\n", + fields[FIELD_SERVICE].offset); break; + case DBUS_HEADER_FIELD_NAME_AS_UINT32: - if (*name != NULL) + if (fields[FIELD_NAME].offset >= 0) { _dbus_verbose ("%s field provided twice\n", DBUS_HEADER_FIELD_NAME); - goto failed; + return FALSE; } - *name = _dbus_demarshal_string (data, byte_order, pos + 1, &new_pos); - /* FIXME check for demarshal failure SECURITY */ + fields[FIELD_NAME].offset = _DBUS_ALIGN_VALUE (pos + 1, 4); + + _dbus_verbose ("Found message name at offset %d\n", + fields[FIELD_NAME].offset); break; case DBUS_HEADER_FIELD_SENDER_AS_UINT32: - if (*sender != NULL) - { - _dbus_verbose ("%s field provided twice\n", - DBUS_HEADER_FIELD_NAME); - goto failed; - } + if (fields[FIELD_SENDER].offset >= 0) + { + _dbus_verbose ("%s field provided twice\n", + DBUS_HEADER_FIELD_SENDER); + return FALSE; + } + + fields[FIELD_SENDER].offset = _DBUS_ALIGN_VALUE (pos + 1, 4); - *sender = _dbus_demarshal_string (data, byte_order, pos + 1, &new_pos); - /* FIXME check for demarshal failure SECURITY */ + _dbus_verbose ("Found sender name at offset %d\n", + fields[FIELD_NAME].offset); break; + case DBUS_HEADER_FIELD_REPLY_AS_UINT32: - *reply_serial = _dbus_demarshal_int32 (data, byte_order, pos + 1, &new_pos); + if (fields[FIELD_REPLY_SERIAL].offset >= 0) + { + _dbus_verbose ("%s field provided twice\n", + DBUS_HEADER_FIELD_REPLY); + return FALSE; + } + + fields[FIELD_REPLY_SERIAL].offset = _DBUS_ALIGN_VALUE (pos + 1, 4); + _dbus_verbose ("Found reply serial at offset %d\n", + fields[FIELD_REPLY_SERIAL].offset); break; + default: _dbus_verbose ("Ignoring an unknown header field: %c%c%c%c\n", field[0], field[1], field[2], field[3]); - - if (!_dbus_marshal_get_field_end_pos (data, byte_order, pos, &new_pos)) - goto failed; } + /* This function has to validate the fields */ + if (!_dbus_marshal_get_field_end_pos (data, byte_order, pos, &new_pos)) + return FALSE; + if (new_pos > header_len) return FALSE; @@ -1393,11 +1703,6 @@ decode_header_data (DBusString *data, } return TRUE; - - failed: - dbus_free (*service); - dbus_free (*name); - return FALSE; } /** @@ -1457,32 +1762,32 @@ _dbus_message_loader_return_buffer (DBusMessageLoader *loader, if (_dbus_string_get_length (&loader->data) >= header_len + body_len) { - dbus_int32_t client_serial, reply_serial; - char *service, *name, *sender; - + HeaderField fields[FIELD_LAST]; + int i; + /* FIXME right now if this doesn't have enough memory, the * loader becomes corrupted. Instead we should just not * parse this message for now. */ if (!decode_header_data (&loader->data, header_len, byte_order, - &client_serial, &reply_serial, &service, &name, &sender)) + fields)) { loader->corrupted = TRUE; return; } - message = dbus_message_new (service, name); - message->reply_serial = reply_serial; - message->client_serial = client_serial; - dbus_message_set_sender (message, sender); - - dbus_free (service); - dbus_free (name); - dbus_free (sender); - + message = dbus_message_new_empty_header (); if (message == NULL) break; /* ugh, postpone this I guess. */ - + + /* Copy in the offsets we found */ + i = 0; + while (i < FIELD_LAST) + { + message->header_fields[i] = fields[i]; + ++i; + } + if (!_dbus_list_append (&loader->messages, message)) { dbus_message_unref (message); @@ -1492,9 +1797,16 @@ _dbus_message_loader_return_buffer (DBusMessageLoader *loader, _dbus_assert (_dbus_string_get_length (&message->header) == 0); _dbus_assert (_dbus_string_get_length (&message->body) == 0); - /* Delete header part */ - _dbus_string_delete (&loader->data, 0, header_len); - + _dbus_assert (_dbus_string_get_length (&loader->data) >= + (header_len + body_len)); + + if (!_dbus_string_move_len (&loader->data, 0, header_len, &message->header, 0)) + { + _dbus_list_remove_last (&loader->messages, message); + dbus_message_unref (message); + break; + } + if (!_dbus_string_move_len (&loader->data, 0, body_len, &message->body, 0)) { dbus_bool_t result; @@ -1509,9 +1821,10 @@ _dbus_message_loader_return_buffer (DBusMessageLoader *loader, break; } + _dbus_assert (_dbus_string_get_length (&message->header) == header_len); _dbus_assert (_dbus_string_get_length (&message->body) == body_len); - _dbus_verbose ("Loaded message %p\n", message); + _dbus_verbose ("Loaded message %p\n", message); } else break; @@ -1650,13 +1963,17 @@ _dbus_message_test (void) /* Test the vararg functions */ message = dbus_message_new ("org.freedesktop.DBus.Test", "testMessage"); - message->client_serial = 1; + _dbus_message_set_client_serial (message, 1); dbus_message_append_fields (message, DBUS_TYPE_INT32, -0x12345678, DBUS_TYPE_STRING, "Test string", DBUS_TYPE_DOUBLE, 3.14159, 0); - + _dbus_verbose_bytes_of_string (&message->header, 0, + _dbus_string_get_length (&message->header)); + _dbus_verbose_bytes_of_string (&message->body, 0, + _dbus_string_get_length (&message->body)); + if (dbus_message_get_fields (message, DBUS_TYPE_INT32, &our_int, DBUS_TYPE_STRING, &our_str, @@ -1677,8 +1994,8 @@ _dbus_message_test (void) dbus_message_unref (message); message = dbus_message_new ("org.freedesktop.DBus.Test", "testMessage"); - message->client_serial = 1; - message->reply_serial = 0x12345678; + _dbus_message_set_client_serial (message, 1); + _dbus_message_set_reply_serial (message, 0x12345678); dbus_message_append_string (message, "Test string"); dbus_message_append_int32 (message, -0x12345678); @@ -1723,7 +2040,7 @@ _dbus_message_test (void) if (!message) _dbus_assert_not_reached ("received a NULL message"); - if (message->reply_serial != 0x12345678) + if (_dbus_message_get_reply_serial (message) != 0x12345678) _dbus_assert_not_reached ("reply serial fields differ"); message_iter_test (message); diff --git a/dbus/dbus-message.h b/dbus/dbus-message.h index 879645b6..ccc32075 100644 --- a/dbus/dbus-message.h +++ b/dbus/dbus-message.h @@ -46,7 +46,7 @@ void dbus_message_unref (DBusMessage *message); const char* dbus_message_get_name (DBusMessage *message); const char* dbus_message_get_service (DBusMessage *message); -void dbus_message_set_sender (DBusMessage *message, +dbus_bool_t dbus_message_set_sender (DBusMessage *message, const char *sender); const char* dbus_message_get_sender (DBusMessage *message); diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c index 257935eb..e1707bcc 100644 --- a/dbus/dbus-string.c +++ b/dbus/dbus-string.c @@ -194,6 +194,25 @@ _dbus_string_init (DBusString *str, void _dbus_string_init_const (DBusString *str, const char *value) +{ + _dbus_string_init_const_len (str, value, + strlen (value)); +} + +/** + * Initializes a constant string with a length. The value parameter is + * not copied (should be static), and the string may never be + * modified. It is safe but not necessary to call _dbus_string_free() + * on a const string. + * + * @param str memory to use for the string + * @param value a string to be stored in str (not copied!!!) + * @param len the length to use + */ +void +_dbus_string_init_const_len (DBusString *str, + const char *value, + int len) { DBusRealString *real; @@ -203,7 +222,7 @@ _dbus_string_init_const (DBusString *str, real = (DBusRealString*) str; real->str = (char*) value; - real->len = strlen (real->str); + real->len = len; real->allocated = real->len; real->max_length = real->len; real->constant = TRUE; @@ -357,6 +376,23 @@ _dbus_string_get_const_data_len (const DBusString *str, *data_return = real->str + start; } +/** + * Gets the byte at the given position. + * + * @param str the string + * @param start the position + * @returns the byte at that position + */ +char +_dbus_string_get_byte (const DBusString *str, + int start) +{ + DBUS_CONST_STRING_PREAMBLE (str); + _dbus_assert (start < real->len); + + return real->str[start]; +} + /** * Like _dbus_string_get_data(), but removes the * gotten data from the original string. The caller @@ -760,7 +796,7 @@ _dbus_string_delete (DBusString *str, _dbus_assert (start >= 0); _dbus_assert (len >= 0); _dbus_assert ((start + len) <= real->len); - + delete (real, start, len); } @@ -789,9 +825,12 @@ copy (DBusRealString *source, DBusRealString *dest, int insert_at) { + if (len == 0) + return TRUE; + if (!open_gap (len, dest, insert_at)) return FALSE; - + memcpy (dest->str + insert_at, source->str + start, len); @@ -938,6 +977,44 @@ _dbus_string_copy_len (const DBusString *source, insert_at); } +/** + * Replaces a segment of dest string with a segment of source string. + * + * @todo optimize the case where the two lengths are the same, and + * avoid memmoving the data in the trailing part of the string twice. + * + * @param source the source string + * @param start where to start copying the source string + * @param len length of segment to copy + * @param dest the destination string + * @param replace_at start of segment of dest string to replace + * @param replace_len length of segment of dest string to replace + * @returns #FALSE if not enough memory + * + */ +dbus_bool_t +_dbus_string_replace_len (const DBusString *source, + int start, + int len, + DBusString *dest, + int replace_at, + int replace_len) +{ + DBUS_STRING_COPY_PREAMBLE (source, start, dest, replace_at); + _dbus_assert (len >= 0); + _dbus_assert ((start + len) <= real_source->len); + _dbus_assert (replace_at >= 0); + _dbus_assert ((replace_at + replace_len) <= real_dest->len); + + if (!copy (real_source, start, len, + real_dest, replace_at)) + return FALSE; + + delete (real_dest, replace_at + len, replace_len); + + return TRUE; +} + /* Unicode macros from GLib */ /** computes length and mask of a unicode character @@ -1931,6 +2008,51 @@ _dbus_string_test (void) _dbus_string_free (&str); _dbus_string_free (&other); + /* Check replace */ + + if (!_dbus_string_init (&str, _DBUS_INT_MAX)) + _dbus_assert_not_reached ("failed to init string"); + + if (!_dbus_string_append (&str, "Hello World")) + _dbus_assert_not_reached ("could not append to string"); + + i = _dbus_string_get_length (&str); + + if (!_dbus_string_init (&other, _DBUS_INT_MAX)) + _dbus_assert_not_reached ("could not init string"); + + if (!_dbus_string_replace_len (&str, 0, _dbus_string_get_length (&str), + &other, 0, _dbus_string_get_length (&other))) + _dbus_assert_not_reached ("could not replace"); + + _dbus_assert (_dbus_string_get_length (&str) == i); + _dbus_assert (_dbus_string_get_length (&other) == i); + _dbus_assert (_dbus_string_equal_c_str (&other, "Hello World")); + + if (!_dbus_string_replace_len (&str, 0, _dbus_string_get_length (&str), + &other, 5, 1)) + _dbus_assert_not_reached ("could not replace center space"); + + _dbus_assert (_dbus_string_get_length (&str) == i); + _dbus_assert (_dbus_string_get_length (&other) == i * 2 - 1); + _dbus_assert (_dbus_string_equal_c_str (&other, + "HelloHello WorldWorld")); + + + if (!_dbus_string_replace_len (&str, 1, 1, + &other, + _dbus_string_get_length (&other) - 1, + 1)) + _dbus_assert_not_reached ("could not replace end character"); + + _dbus_assert (_dbus_string_get_length (&str) == i); + _dbus_assert (_dbus_string_get_length (&other) == i * 2 - 1); + _dbus_assert (_dbus_string_equal_c_str (&other, + "HelloHello WorldWorle")); + + _dbus_string_free (&str); + _dbus_string_free (&other); + /* Check append/get unichar */ if (!_dbus_string_init (&str, _DBUS_INT_MAX)) diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h index b2335086..c5564e9c 100644 --- a/dbus/dbus-string.h +++ b/dbus/dbus-string.h @@ -48,6 +48,9 @@ dbus_bool_t _dbus_string_init (DBusString *str, int max_length); void _dbus_string_init_const (DBusString *str, const char *value); +void _dbus_string_init_const_len (DBusString *str, + const char *value, + int len); void _dbus_string_free (DBusString *str); void _dbus_string_lock (DBusString *str); @@ -63,6 +66,8 @@ void _dbus_string_get_const_data_len (const DBusString *str, const char **data_return, int start, int len); +char _dbus_string_get_byte (const DBusString *str, + int start); dbus_bool_t _dbus_string_steal_data (DBusString *str, char **data_return); dbus_bool_t _dbus_string_steal_data_len (DBusString *str, @@ -118,6 +123,12 @@ dbus_bool_t _dbus_string_copy_len (const DBusString *source, DBusString *dest, int insert_at); +dbus_bool_t _dbus_string_replace_len (const DBusString *source, + int start, + int len, + DBusString *dest, + int replace_at, + int replace_len); void _dbus_string_get_unichar (const DBusString *str, int start, diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c index c91a06ff..45405cc2 100644 --- a/dbus/dbus-sysdeps.c +++ b/dbus/dbus-sysdeps.c @@ -35,6 +35,7 @@ #include #include #include +#include #ifdef HAVE_WRITEV #include #endif -- cgit