diff options
| -rw-r--r-- | ChangeLog | 62 | ||||
| -rw-r--r-- | dbus/dbus-auth.c | 2 | ||||
| -rw-r--r-- | dbus/dbus-marshal.c | 4 | ||||
| -rw-r--r-- | dbus/dbus-message-builder.c | 11 | ||||
| -rw-r--r-- | dbus/dbus-message.c | 501 | ||||
| -rw-r--r-- | dbus/dbus-string.c | 35 | ||||
| -rw-r--r-- | dbus/dbus-string.h | 3 | ||||
| -rw-r--r-- | doc/TODO | 4 | ||||
| -rw-r--r-- | doc/dbus-specification.sgml | 16 | ||||
| -rw-r--r-- | test/break-loader.c | 4 | ||||
| -rw-r--r-- | test/data/invalid-messages/bad-header-field-alignment.message | 34 | ||||
| -rw-r--r-- | test/data/valid-messages/unknown-header-field.message | 9 |
12 files changed, 441 insertions, 244 deletions
@@ -1,3 +1,65 @@ +2003-09-25 Mark McLoughlin <mark@skynet.ie> + + * doc/dbus-specification.sgml: don't require header fields + to be 4-byte aligned and specify that fields should be + distinguished from padding by the fact that zero is not + a valid field name. + + * doc/TODO: remove re-alignment item and add item to doc + the OBJECT_PATH type. + + * dbus/dbus-message.c: + (HeaderField): rename the original member to value_offset + and introduce a name_offset member to keep track of where + the field actually begins. + (adjust_field_offsets): remove. + (append_int_field), (append_uint_field), + (append_string_field): don't align the start of the header + field to a 4-byte boundary. + (get_next_field): impl finding the next marhsalled field + after a given field. + (re_align_field_recurse): impl re-aligning a number of + already marshalled fields. + (delete_field): impl deleting a field of any type and + re-aligning any following fields. + (delete_int_or_uint_field), (delete_string_field): remove. + (set_int_field), (set_uint_field): no need to re-check + that we have the correct type for the field. + (set_string_field): ditto and impl re-aligning any + following fields. + (decode_header_data): update to take into account that + the fields aren't 4-byte aligned any more and the new + way to distinguish padding from header fields. Also, + don't exit when there is too much header padding. + (process_test_subdir): print the directory. + (_dbus_message_test): add test to make sure a following + field is re-aligned correctly after field deletion. + + * dbus/dbus-string.[ch]: + (_dbus_string_insert_bytes): rename from insert_byte and + allow the insert of multiple bytes. + (_dbus_string_test): test inserting multiple bytes. + + * dbus/dbus-marshal.c: (_dbus_marshal_set_string): add + warning note to docs about having to re-align any + marshalled values following the string. + + * dbus/dbus-message-builder.c: + (append_string_field), (_dbus_message_data_load): + don't align the header field. + + * dbus/dbus-auth.c: (process_test_subdir): print the + directory. + + * test/break-loader.c: (randomly_add_one_byte): upd. for + insert_byte change. + + * test/data/invalid-messages/bad-header-field-alignment.message: + new test case. + + * test/data/valid-messages/unknown-header-field.message: shove + a dict in the unknown field. + 2003-09-25 Seth Nickell <seth@gnome.org> * python/dbus.py: diff --git a/dbus/dbus-auth.c b/dbus/dbus-auth.c index 90c72fd5..cdfd3bb2 100644 --- a/dbus/dbus-auth.c +++ b/dbus/dbus-auth.c @@ -2380,7 +2380,7 @@ process_test_subdir (const DBusString *test_base_dir, goto failed; } - printf ("Testing:\n"); + printf ("Testing %s:\n", subdir); next: while (_dbus_directory_get_next_file (dir, &filename, &error)) diff --git a/dbus/dbus-marshal.c b/dbus/dbus-marshal.c index 3d6184e9..cb989891 100644 --- a/dbus/dbus-marshal.c +++ b/dbus/dbus-marshal.c @@ -395,6 +395,10 @@ _dbus_marshal_set_uint64 (DBusString *str, * an existing string or the wrong length will be deleted * and replaced with the new string. * + * Note: no attempt is made by this function to re-align + * any data which has been already marshalled after this + * string. Use with caution. + * * @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 diff --git a/dbus/dbus-message-builder.c b/dbus/dbus-message-builder.c index 7e2dff0d..c9dc8ca5 100644 --- a/dbus/dbus-message-builder.c +++ b/dbus/dbus-message-builder.c @@ -300,12 +300,6 @@ append_string_field (DBusString *dest, { int len; - if (!_dbus_string_align_length (dest, 4)) - { - _dbus_warn ("could not align field name\n"); - return FALSE; - } - if (!_dbus_string_append_byte (dest, field)) { _dbus_warn ("couldn't append field name byte\n"); @@ -710,11 +704,6 @@ _dbus_message_data_load (DBusString *dest, goto parse_failed; } - if (unalign) - unalign = FALSE; - else - _dbus_string_align_length (dest, 4); - if (!_dbus_string_append_byte (dest, field)) { _dbus_warn ("could not append header field name byte\n"); diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 824d85bf..fe56e011 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -47,9 +47,8 @@ */ typedef struct { - int offset; /**< Offset to start of field (location of name of field - * for named fields) - */ + int name_offset; /**< Offset to name of field */ + int value_offset; /**< Offset to value of field */ } HeaderField; /** Offset to byte order from start of header */ @@ -183,26 +182,6 @@ append_header_padding (DBusMessage *message) return TRUE; } -static void -adjust_field_offsets (DBusMessage *message, - int offsets_after, - int delta) -{ - int i; - - if (delta == 0) - return; - - i = 0; - while (i <= DBUS_HEADER_FIELD_LAST) - { - if (message->header_fields[i].offset > offsets_after) - message->header_fields[i].offset += delta; - - ++i; - } -} - #ifdef DBUS_BUILD_TESTS /* tests-only until it's actually used */ static dbus_int32_t @@ -213,7 +192,7 @@ get_int_field (DBusMessage *message, _dbus_assert (field <= DBUS_HEADER_FIELD_LAST); - offset = message->header_fields[field].offset; + offset = message->header_fields[field].value_offset; if (offset < 0) return -1; /* useless if -1 is a valid value of course */ @@ -233,7 +212,7 @@ get_uint_field (DBusMessage *message, _dbus_assert (field <= DBUS_HEADER_FIELD_LAST); - offset = message->header_fields[field].offset; + offset = message->header_fields[field].value_offset; if (offset < 0) return -1; /* useless if -1 is a valid value of course */ @@ -252,7 +231,7 @@ get_string_field (DBusMessage *message, int offset; const char *data; - offset = message->header_fields[field].offset; + offset = message->header_fields[field].value_offset; _dbus_assert (field <= DBUS_HEADER_FIELD_LAST); @@ -283,7 +262,7 @@ get_path_field_decomposed (DBusMessage *message, { int offset; - offset = message->header_fields[field].offset; + offset = message->header_fields[field].value_offset; _dbus_assert (field <= DBUS_HEADER_FIELD_LAST); @@ -306,16 +285,12 @@ append_int_field (DBusMessage *message, int field, int value) { - int orig_len; - _dbus_assert (!message->locked); clear_header_padding (message); - orig_len = _dbus_string_get_length (&message->header); - - if (!_dbus_string_align_length (&message->header, 4)) - goto failed; + message->header_fields[field].name_offset = + _dbus_string_get_length (&message->header); if (!_dbus_string_append_byte (&message->header, field)) goto failed; @@ -326,7 +301,7 @@ append_int_field (DBusMessage *message, if (!_dbus_string_align_length (&message->header, 4)) goto failed; - message->header_fields[field].offset = + message->header_fields[field].value_offset = _dbus_string_get_length (&message->header); if (!_dbus_marshal_int32 (&message->header, message->byte_order, @@ -339,8 +314,10 @@ append_int_field (DBusMessage *message, return TRUE; failed: - message->header_fields[field].offset = -1; - _dbus_string_set_length (&message->header, orig_len); + _dbus_string_set_length (&message->header, + message->header_fields[field].name_offset); + message->header_fields[field].name_offset = -1; + message->header_fields[field].value_offset = -1; /* this must succeed because it was allocated on function entry and * DBusString doesn't ever realloc smaller @@ -356,16 +333,12 @@ append_uint_field (DBusMessage *message, int field, int value) { - int orig_len; - _dbus_assert (!message->locked); clear_header_padding (message); - orig_len = _dbus_string_get_length (&message->header); - - if (!_dbus_string_align_length (&message->header, 4)) - goto failed; + message->header_fields[field].name_offset = + _dbus_string_get_length (&message->header); if (!_dbus_string_append_byte (&message->header, field)) goto failed; @@ -376,7 +349,7 @@ append_uint_field (DBusMessage *message, if (!_dbus_string_align_length (&message->header, 4)) goto failed; - message->header_fields[field].offset = + message->header_fields[field].value_offset = _dbus_string_get_length (&message->header); if (!_dbus_marshal_uint32 (&message->header, message->byte_order, @@ -389,8 +362,10 @@ append_uint_field (DBusMessage *message, return TRUE; failed: - message->header_fields[field].offset = -1; - _dbus_string_set_length (&message->header, orig_len); + _dbus_string_set_length (&message->header, + message->header_fields[field].name_offset); + message->header_fields[field].name_offset = -1; + message->header_fields[field].value_offset = -1; /* this must succeed because it was allocated on function entry and * DBusString doesn't ever realloc smaller @@ -406,16 +381,12 @@ append_string_field (DBusMessage *message, int type, const char *value) { - int orig_len; - _dbus_assert (!message->locked); clear_header_padding (message); - orig_len = _dbus_string_get_length (&message->header); - - if (!_dbus_string_align_length (&message->header, 4)) - goto failed; + message->header_fields[field].name_offset = + _dbus_string_get_length (&message->header); if (!_dbus_string_append_byte (&message->header, field)) goto failed; @@ -426,7 +397,7 @@ append_string_field (DBusMessage *message, if (!_dbus_string_align_length (&message->header, 4)) goto failed; - message->header_fields[field].offset = + message->header_fields[field].value_offset = _dbus_string_get_length (&message->header); if (!_dbus_marshal_string (&message->header, message->byte_order, @@ -439,8 +410,10 @@ append_string_field (DBusMessage *message, return TRUE; failed: - message->header_fields[field].offset = -1; - _dbus_string_set_length (&message->header, orig_len); + _dbus_string_set_length (&message->header, + message->header_fields[field].name_offset); + message->header_fields[field].name_offset = -1; + message->header_fields[field].value_offset = -1; /* this must succeed because it was allocated on function entry and * DBusString doesn't ever realloc smaller @@ -451,71 +424,205 @@ append_string_field (DBusMessage *message, return FALSE; } -#ifdef DBUS_BUILD_TESTS -/* This isn't used, but building it when tests are enabled just to - * keep it compiling if we need it in future - */ -static void -delete_int_or_uint_field (DBusMessage *message, - int field) +static int +get_next_field (DBusMessage *message, + int field) { - int offset = message->header_fields[field].offset; + int offset = message->header_fields[field].name_offset; + int closest; + int i; + int retval = DBUS_HEADER_FIELD_INVALID; - _dbus_assert (!message->locked); - - if (offset < 0) - return; + i = 0; + closest = _DBUS_INT_MAX; + while (i < DBUS_HEADER_FIELD_LAST) + { + if (message->header_fields[i].name_offset > offset && + message->header_fields[i].name_offset < closest) + { + closest = message->header_fields[i].name_offset; + retval = i; + } + ++i; + } - clear_header_padding (message); - - /* The field typecode and name take up 4 bytes */ - _dbus_string_delete (&message->header, - offset - 4, - 8); + return retval; +} - message->header_fields[field].offset = -1; - - adjust_field_offsets (message, - offset - 4, - - 8); +static dbus_bool_t +re_align_field_recurse (DBusMessage *message, + int field, + int offset) +{ + int old_name_offset = message->header_fields[field].name_offset; + int old_value_offset = message->header_fields[field].value_offset; + int prev_padding, padding, delta; + int type; + int next_field; + int pos = offset; + + /* padding between the typecode byte and the value itself */ + prev_padding = old_value_offset - old_name_offset + 2; + + pos++; + type = _dbus_string_get_byte (&message->header, pos); + + pos++; + switch (type) + { + case DBUS_TYPE_NIL: + case DBUS_TYPE_BYTE: + case DBUS_TYPE_BOOLEAN: + padding = 0; + break; + case DBUS_TYPE_INT32: + case DBUS_TYPE_UINT32: + case DBUS_TYPE_STRING: + case DBUS_TYPE_OBJECT_PATH: + padding = _DBUS_ALIGN_VALUE (pos, 4) - pos; + break; + case DBUS_TYPE_INT64: + case DBUS_TYPE_UINT64: + case DBUS_TYPE_DOUBLE: + padding = _DBUS_ALIGN_VALUE (pos, 8) - pos; + break; + case DBUS_TYPE_NAMED: + case DBUS_TYPE_ARRAY: + case DBUS_TYPE_DICT: + _dbus_assert_not_reached ("no defined header fields may contain a named, array or dict value"); + break; + case DBUS_TYPE_INVALID: + default: + _dbus_assert_not_reached ("invalid type in marshalled header"); + break; + } + + delta = padding - prev_padding; + if (delta > 0) + { + if (!_dbus_string_insert_bytes (&message->header, pos, delta, 0)) + return FALSE; + } + else if (delta < 0) + { + _dbus_string_delete (&message->header, pos, -delta); + } + + next_field = get_next_field (message, field); + if (next_field != DBUS_HEADER_FIELD_INVALID) + { + int next_offset = message->header_fields[next_field].name_offset; + + _dbus_assert (next_offset > 0); + + if (!re_align_field_recurse (message, field, + pos + padding + (next_offset - old_value_offset))) + goto failed; + } + else + { + if (!append_header_padding (message)) + goto failed; + } + + message->header_fields[field].name_offset = offset; + message->header_fields[field].value_offset = pos + padding; + + return TRUE; + + failed: + if (delta > 0) + { + _dbus_string_delete (&message->header, pos, delta); + } + else if (delta < 0) + { + /* this must succeed because it was allocated on function entry and + * DBusString doesn't ever realloc smaller + */ + _dbus_string_insert_bytes (&message->header, pos, -delta, 0); + } - append_header_padding (message); + return FALSE; } -#endif -static void -delete_string_field (DBusMessage *message, - int field) +static dbus_bool_t +delete_field (DBusMessage *message, + int field) { - int offset = message->header_fields[field].offset; - int len; - int delete_len; - + int offset = message->header_fields[field].name_offset; + int next_field; + _dbus_assert (!message->locked); if (offset < 0) - return; + return FALSE; clear_header_padding (message); - - get_string_field (message, field, &len); - - /* The field typecode and name take up 4 bytes, and the nul - * termination is 1 bytes, string length integer is 4 bytes - */ - delete_len = 4 + 4 + 1 + len; - - _dbus_string_delete (&message->header, - offset - 4, - delete_len); - message->header_fields[field].offset = -1; - - adjust_field_offsets (message, - offset - 4, - - delete_len); + next_field = get_next_field (message, field); + if (next_field == DBUS_HEADER_FIELD_INVALID) + { + _dbus_string_set_length (&message->header, offset); - append_header_padding (message); + message->header_fields[field].name_offset = -1; + message->header_fields[field].value_offset = -1; + + /* this must succeed because it was allocated on function entry and + * DBusString doesn't ever realloc smaller + */ + if (!append_header_padding (message)) + _dbus_assert_not_reached ("failed to reappend header padding"); + + return TRUE; + } + else + { + DBusString deleted; + int next_offset = message->header_fields[next_field].name_offset; + + _dbus_assert (next_offset > 0); + + if (!_dbus_string_init (&deleted)) + goto failed; + + if (!_dbus_string_move_len (&message->header, + offset, next_offset - offset, + &deleted, 0)) + { + _dbus_string_free (&deleted); + goto failed; + } + + /* appends the header padding */ + if (!re_align_field_recurse (message, next_field, offset)) + { + /* this must succeed because it was allocated on function entry and + * DBusString doesn't ever realloc smaller + */ + if (!_dbus_string_copy (&deleted, 0, &message->header, offset)) + _dbus_assert_not_reached ("failed to revert to original field"); + + _dbus_string_free (&deleted); + goto failed; + } + + _dbus_string_free (&deleted); + + message->header_fields[field].name_offset = -1; + message->header_fields[field].value_offset = -1; + + return TRUE; + + failed: + /* this must succeed because it was allocated on function entry and + * DBusString doesn't ever realloc smaller + */ + if (!append_header_padding (message)) + _dbus_assert_not_reached ("failed to reappend header padding"); + + return FALSE; + } } #ifdef DBUS_BUILD_TESTS @@ -524,20 +631,14 @@ set_int_field (DBusMessage *message, int field, int value) { - int offset = message->header_fields[field].offset; + int offset = message->header_fields[field].value_offset; _dbus_assert (!message->locked); if (offset < 0) { /* need to append the field */ - - switch (field) - { - default: - _dbus_assert_not_reached ("appending an int field we don't support appending"); - return FALSE; - } + return append_int_field (message, field, value); } else { @@ -555,23 +656,14 @@ set_uint_field (DBusMessage *message, int field, dbus_uint32_t value) { - int offset = message->header_fields[field].offset; + int offset = message->header_fields[field].value_offset; _dbus_assert (!message->locked); if (offset < 0) { /* need to append the field */ - - switch (field) - { - case DBUS_HEADER_FIELD_REPLY_SERIAL: - return append_uint_field (message, field, value); - - default: - _dbus_assert_not_reached ("appending a uint field we don't support appending"); - return FALSE; - } + return append_uint_field (message, field, value); } else { @@ -589,7 +681,7 @@ set_string_field (DBusMessage *message, int type, const char *value) { - int offset = message->header_fields[field].offset; + int offset = message->header_fields[field].value_offset; _dbus_assert (!message->locked); _dbus_assert (value != NULL); @@ -597,52 +689,59 @@ set_string_field (DBusMessage *message, if (offset < 0) { /* need to append the field */ - - switch (field) - { - case DBUS_HEADER_FIELD_PATH: - case DBUS_HEADER_FIELD_SENDER_SERVICE: - case DBUS_HEADER_FIELD_INTERFACE: - case DBUS_HEADER_FIELD_MEMBER: - case DBUS_HEADER_FIELD_ERROR_NAME: - case DBUS_HEADER_FIELD_SERVICE: - return append_string_field (message, field, type, value); - - default: - _dbus_assert_not_reached ("appending a string field we don't support appending"); - return FALSE; - } + return append_string_field (message, field, type, value); } else { DBusString v; - int old_len; - int new_len; + char *old_value; + int next_field; + int next_offset; int len; clear_header_padding (message); - - old_len = _dbus_string_get_length (&message->header); + + old_value = _dbus_demarshal_string (&message->header, + message->byte_order, + offset, + &next_offset); + if (!old_value) + goto failed; len = strlen (value); - + _dbus_string_init_const_len (&v, value, len + 1); /* include nul */ if (!_dbus_marshal_set_string (&message->header, message->byte_order, - offset, &v, - len)) - goto failed; - - new_len = _dbus_string_get_length (&message->header); + offset, &v, len)) + { + dbus_free (old_value); + goto failed; + } - adjust_field_offsets (message, - offset, - new_len - old_len); + next_field = get_next_field (message, field); + if (next_field != DBUS_HEADER_FIELD_INVALID) + { + /* re-appends the header padding */ + if (!re_align_field_recurse (message, next_field, next_offset)) + { + len = strlen (old_value); + + _dbus_string_init_const_len (&v, old_value, + len + 1); /* include nul */ + if (!_dbus_marshal_set_string (&message->header, + message->byte_order, + offset, &v, len)) + _dbus_assert_not_reached ("failed to revert to original string"); + + dbus_free (old_value); + goto failed; + } + } + + dbus_free (old_value); - if (!append_header_padding (message)) - goto failed; - return TRUE; failed: @@ -983,7 +1082,8 @@ dbus_message_new_empty_header (void) i = 0; while (i <= DBUS_HEADER_FIELD_LAST) { - message->header_fields[i].offset = -1; + message->header_fields[i].name_offset = -1; + message->header_fields[i].value_offset = -1; ++i; } @@ -1281,7 +1381,7 @@ dbus_message_copy (const DBusMessage *message) for (i = 0; i <= DBUS_HEADER_FIELD_LAST; i++) { - retval->header_fields[i].offset = message->header_fields[i].offset; + retval->header_fields[i] = message->header_fields[i]; } return retval; @@ -1390,7 +1490,7 @@ dbus_message_set_path (DBusMessage *message, if (object_path == NULL) { - delete_string_field (message, DBUS_HEADER_FIELD_PATH); + delete_field (message, DBUS_HEADER_FIELD_PATH); return TRUE; } else @@ -1464,7 +1564,7 @@ dbus_message_set_interface (DBusMessage *message, if (interface == NULL) { - delete_string_field (message, DBUS_HEADER_FIELD_INTERFACE); + delete_field (message, DBUS_HEADER_FIELD_INTERFACE); return TRUE; } else @@ -1512,7 +1612,7 @@ dbus_message_set_member (DBusMessage *message, if (member == NULL) { - delete_string_field (message, DBUS_HEADER_FIELD_MEMBER); + delete_field (message, DBUS_HEADER_FIELD_MEMBER); return TRUE; } else @@ -1559,8 +1659,7 @@ dbus_message_set_error_name (DBusMessage *message, if (error_name == NULL) { - delete_string_field (message, - DBUS_HEADER_FIELD_ERROR_NAME); + delete_field (message, DBUS_HEADER_FIELD_ERROR_NAME); return TRUE; } else @@ -1604,7 +1703,7 @@ dbus_message_set_destination (DBusMessage *message, if (destination == NULL) { - delete_string_field (message, DBUS_HEADER_FIELD_SERVICE); + delete_field (message, DBUS_HEADER_FIELD_SERVICE); return TRUE; } else @@ -4081,8 +4180,7 @@ dbus_message_set_sender (DBusMessage *message, if (sender == NULL) { - delete_string_field (message, - DBUS_HEADER_FIELD_SENDER_SERVICE); + delete_field (message, DBUS_HEADER_FIELD_SENDER_SERVICE); return TRUE; } else @@ -4550,7 +4648,7 @@ decode_string_field (const DBusString *data, _dbus_assert (header_field != NULL); _dbus_assert (field_data != NULL); - if (header_field->offset >= 0) + if (header_field->name_offset >= 0) { _dbus_verbose ("%s field provided twice\n", _dbus_header_field_to_string (field)); @@ -4575,12 +4673,13 @@ decode_string_field (const DBusString *data, _dbus_string_init_const (field_data, _dbus_string_get_const_data (data) + string_data_pos); - header_field->offset = _DBUS_ALIGN_VALUE (pos, 4); + header_field->name_offset = pos; + header_field->value_offset = _DBUS_ALIGN_VALUE (pos, 4); #if 0 _dbus_verbose ("Found field %s at offset %d\n", _dbus_header_field_to_string (field), - header_field->offset); + header_field->value_offset); #endif return TRUE; @@ -4609,29 +4708,18 @@ decode_header_data (const DBusString *data, i = 0; while (i <= DBUS_HEADER_FIELD_LAST) { - fields[i].offset = -1; + fields[i].name_offset = -1; + fields[i].value_offset = -1; ++i; } - /* Now handle the named fields. A real named field is at least 1 - * byte for the name, plus a type code (1 byte) plus padding, plus - * the field value. So if we have less than 8 bytes left, it must - * be alignment padding, not a field. While >= 8 bytes can't be - * entirely alignment padding. - */ pos = 16; - while ((pos + 7) < header_len) + while (pos < header_len) { - pos = _DBUS_ALIGN_VALUE (pos, 4); - - if ((pos + 1) > header_len) - { - _dbus_verbose ("not enough space remains in header for header field value\n"); - return FALSE; - } - field = _dbus_string_get_byte (data, pos); - pos += 1; + if (field == DBUS_HEADER_FIELD_INVALID) + break; /* Must be padding */ + pos++; if (!_dbus_marshal_validate_type (data, pos, &type, &pos)) { @@ -4737,7 +4825,7 @@ decode_header_data (const DBusString *data, * type. */ - if (fields[field].offset >= 0) + if (fields[field].name_offset >= 0) { _dbus_verbose ("path field provided twice\n"); return FALSE; @@ -4748,15 +4836,16 @@ decode_header_data (const DBusString *data, return FALSE; } - fields[field].offset = _DBUS_ALIGN_VALUE (pos, 4); + fields[field].name_offset = pos; + fields[field].value_offset = _DBUS_ALIGN_VALUE (pos, 4); /* No forging signals from the local path */ { const char *s; s = _dbus_string_get_const_data_len (data, - fields[field].offset, + fields[field].value_offset, _dbus_string_get_length (data) - - fields[field].offset); + fields[field].value_offset); if (strcmp (s, DBUS_PATH_ORG_FREEDESKTOP_LOCAL) == 0) { _dbus_verbose ("Message is on the local path\n"); @@ -4765,11 +4854,11 @@ decode_header_data (const DBusString *data, } _dbus_verbose ("Found path at offset %d\n", - fields[field].offset); + fields[field].value_offset); break; case DBUS_HEADER_FIELD_REPLY_SERIAL: - if (fields[field].offset >= 0) + if (fields[field].name_offset >= 0) { _dbus_verbose ("reply field provided twice\n"); return FALSE; @@ -4781,10 +4870,11 @@ decode_header_data (const DBusString *data, return FALSE; } - fields[field].offset = _DBUS_ALIGN_VALUE (pos, 4); + fields[field].name_offset = pos; + fields[field].value_offset = _DBUS_ALIGN_VALUE (pos, 4); _dbus_verbose ("Found reply serial at offset %d\n", - fields[field].offset); + fields[field].value_offset); break; default: @@ -4798,7 +4888,11 @@ decode_header_data (const DBusString *data, if (pos < header_len) { /* Alignment padding, verify that it's nul */ - _dbus_assert ((header_len - pos) < 8); + if ((header_len - pos) >= 8) + { + _dbus_verbose ("too much header alignment padding\n"); + return FALSE; + } if (!_dbus_string_validate_nul (data, pos, (header_len - pos))) @@ -4813,25 +4907,25 @@ decode_header_data (const DBusString *data, { case DBUS_MESSAGE_TYPE_SIGNAL: case DBUS_MESSAGE_TYPE_METHOD_CALL: - if (fields[DBUS_HEADER_FIELD_PATH].offset < 0) + if (fields[DBUS_HEADER_FIELD_PATH].value_offset < 0) { _dbus_verbose ("No path field provided\n"); return FALSE; } /* FIXME make this optional, only for method calls */ - if (fields[DBUS_HEADER_FIELD_INTERFACE].offset < 0) + if (fields[DBUS_HEADER_FIELD_INTERFACE].value_offset < 0) { _dbus_verbose ("No interface field provided\n"); return FALSE; } - if (fields[DBUS_HEADER_FIELD_MEMBER].offset < 0) + if (fields[DBUS_HEADER_FIELD_MEMBER].value_offset < 0) { _dbus_verbose ("No member field provided\n"); return FALSE; } break; case DBUS_MESSAGE_TYPE_ERROR: - if (fields[DBUS_HEADER_FIELD_ERROR_NAME].offset < 0) + if (fields[DBUS_HEADER_FIELD_ERROR_NAME].value_offset < 0) { _dbus_verbose ("No error-name field provided\n"); return FALSE; @@ -6069,7 +6163,7 @@ process_test_subdir (const DBusString *test_base_dir, goto failed; } - printf ("Testing:\n"); + printf ("Testing %s:\n", subdir); next: while (_dbus_directory_get_next_file (dir, &filename, &error)) @@ -6432,11 +6526,14 @@ _dbus_message_test (const char *test_data_dir) _dbus_assert (dbus_message_is_method_call (message, "Foo.TestInterface", "TestMethod")); _dbus_message_set_serial (message, 1234); - dbus_message_set_sender (message, "org.foo.bar"); - _dbus_assert (dbus_message_has_sender (message, "org.foo.bar")); + /* string length including nul byte not a multiple of 4 */ + dbus_message_set_sender (message, "org.foo.bar1"); + _dbus_assert (dbus_message_has_sender (message, "org.foo.bar1")); + dbus_message_set_reply_serial (message, 5678); dbus_message_set_sender (message, NULL); - _dbus_assert (!dbus_message_has_sender (message, "org.foo.bar")); + _dbus_assert (!dbus_message_has_sender (message, "org.foo.bar1")); _dbus_assert (dbus_message_get_serial (message) == 1234); + _dbus_assert (dbus_message_get_reply_serial (message) == 5678); _dbus_assert (dbus_message_has_destination (message, "org.freedesktop.DBus.TestService")); _dbus_assert (dbus_message_get_no_reply (message) == FALSE); diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c index 40363686..628cf861 100644 --- a/dbus/dbus-string.c +++ b/dbus/dbus-string.c @@ -562,26 +562,30 @@ _dbus_string_get_byte (const DBusString *str, } /** - * Inserts the given byte at the given position. + * Inserts a number of bytes of a given value at the + * given position. * * @param str the string * @param i the position + * @param n_bytes number of bytes * @param byte the value to insert * @returns #TRUE on success */ dbus_bool_t -_dbus_string_insert_byte (DBusString *str, - int i, - unsigned char byte) +_dbus_string_insert_bytes (DBusString *str, + int i, + int n_bytes, + unsigned char byte) { DBUS_STRING_PREAMBLE (str); _dbus_assert (i <= real->len); _dbus_assert (i >= 0); + _dbus_assert (n_bytes > 0); - if (!open_gap (1, real, i)) + if (!open_gap (n_bytes, real, i)) return FALSE; - real->str[i] = byte; + memset (real->str + i, byte, n_bytes); return TRUE; } @@ -3572,23 +3576,26 @@ _dbus_string_test (void) _dbus_string_set_byte (&str, 1, 'q'); _dbus_assert (_dbus_string_get_byte (&str, 1) == 'q'); - if (!_dbus_string_insert_byte (&str, 0, 255)) + if (!_dbus_string_insert_bytes (&str, 0, 1, 255)) _dbus_assert_not_reached ("can't insert byte"); - if (!_dbus_string_insert_byte (&str, 2, 'Z')) + if (!_dbus_string_insert_bytes (&str, 2, 4, 'Z')) _dbus_assert_not_reached ("can't insert byte"); - if (!_dbus_string_insert_byte (&str, _dbus_string_get_length (&str), 'W')) + if (!_dbus_string_insert_bytes (&str, _dbus_string_get_length (&str), 1, 'W')) _dbus_assert_not_reached ("can't insert byte"); _dbus_assert (_dbus_string_get_byte (&str, 0) == 255); _dbus_assert (_dbus_string_get_byte (&str, 1) == 'H'); _dbus_assert (_dbus_string_get_byte (&str, 2) == 'Z'); - _dbus_assert (_dbus_string_get_byte (&str, 3) == 'q'); - _dbus_assert (_dbus_string_get_byte (&str, 4) == 'l'); - _dbus_assert (_dbus_string_get_byte (&str, 5) == 'l'); - _dbus_assert (_dbus_string_get_byte (&str, 6) == 'o'); - _dbus_assert (_dbus_string_get_byte (&str, 7) == 'W'); + _dbus_assert (_dbus_string_get_byte (&str, 3) == 'Z'); + _dbus_assert (_dbus_string_get_byte (&str, 4) == 'Z'); + _dbus_assert (_dbus_string_get_byte (&str, 5) == 'Z'); + _dbus_assert (_dbus_string_get_byte (&str, 6) == 'q'); + _dbus_assert (_dbus_string_get_byte (&str, 7) == 'l'); + _dbus_assert (_dbus_string_get_byte (&str, 8) == 'l'); + _dbus_assert (_dbus_string_get_byte (&str, 9) == 'o'); + _dbus_assert (_dbus_string_get_byte (&str, 10) == 'W'); _dbus_string_free (&str); diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h index 70e83b33..0b7be8b0 100644 --- a/dbus/dbus-string.h +++ b/dbus/dbus-string.h @@ -72,8 +72,9 @@ void _dbus_string_set_byte (DBusString *str, unsigned char byte); unsigned char _dbus_string_get_byte (const DBusString *str, int start); -dbus_bool_t _dbus_string_insert_byte (DBusString *str, +dbus_bool_t _dbus_string_insert_bytes (DBusString *str, int i, + int n_bytes, unsigned char byte); dbus_bool_t _dbus_string_steal_data (DBusString *str, char **data_return); @@ -90,8 +90,6 @@ - document the auth protocol as a set of states and transitions, and then reimplement it in those terms - - Header fields names are required to be aligned on a 4 byte boundary - at the moment. No alignment should be neccessary. - - dbus_gproxy or dbus_g_proxy? + - The OBJECT_PATH type is not documented in the spec. diff --git a/doc/dbus-specification.sgml b/doc/dbus-specification.sgml index a2dd1b13..c772b3e5 100644 --- a/doc/dbus-specification.sgml +++ b/doc/dbus-specification.sgml @@ -265,11 +265,10 @@ </para> <para> - Header fields MUST be aligned to a 4-byte boundary. Header field - names MUST consist of a single byte, possible values of which are - defined below. Following the name, the field MUST have a type code - represented as a single unsigned byte, and then a properly-aligned - value of that type. See <xref + Header field names MUST consist of a single byte, possible values + of which are defined below. Following the name, the field MUST have + a type code represented as a single unsigned byte, and then a + properly-aligned value of that type. See <xref linkend="message-protocol-arguments"> for a description of how each type is encoded. If an implementation sees a header field name that it does not understand, it MUST ignore that field. @@ -358,10 +357,9 @@ buffer while keeping data types aligned, the total length of the header must be a multiple of 8 bytes. To achieve this, the header MUST be padded with nul bytes to align its total length on an 8-byte boundary. - The minimum number of padding bytes MUST be used. Because all possible - named fields use at least 8 bytes, implementations can distinguish - padding (which must be less than 8 bytes) from additional named fields - (which must be at least 8 bytes). + The minimum number of padding bytes MUST be used. Because zero is an + invalid field name, implementations can distinguish padding (which must be + zero initialized) from additional named fields. </para> </sect2> diff --git a/test/break-loader.c b/test/break-loader.c index ebe2606b..3771d7cc 100644 --- a/test/break-loader.c +++ b/test/break-loader.c @@ -287,8 +287,8 @@ randomly_add_one_byte (const DBusString *orig_data, i = random_int_in_range (0, _dbus_string_get_length (mutated)); - _dbus_string_insert_byte (mutated, i, - random_int_in_range (0, 256)); + _dbus_string_insert_bytes (mutated, i, 1, + random_int_in_range (0, 256)); } static void diff --git a/test/data/invalid-messages/bad-header-field-alignment.message b/test/data/invalid-messages/bad-header-field-alignment.message new file mode 100644 index 00000000..75776a37 --- /dev/null +++ b/test/data/invalid-messages/bad-header-field-alignment.message @@ -0,0 +1,34 @@ +## last field incorrectly aligned to 4 bytes + +## VALID_HEADER includes a LENGTH Header and LENGTH Body +VALID_HEADER method_call + +HEADER_FIELD INTERFACE +TYPE STRING +STRING 'org.freedesktop.Foo' + +HEADER_FIELD MEMBER +TYPE STRING +STRING 'Bar' + +HEADER_FIELD UNKNOWN +TYPE STRING +STRING 'a' + +ALIGN 4 + +HEADER_FIELD UNKNOWN +TYPE ARRAY +TYPE BYTE +ALIGN 4 +LENGTH ThisByteArray +START_LENGTH ThisByteArray +BYTE 1 +BYTE 2 +END_LENGTH ThisByteArray + + +ALIGN 8 +END_LENGTH Header +START_LENGTH Body +END_LENGTH Body diff --git a/test/data/valid-messages/unknown-header-field.message b/test/data/valid-messages/unknown-header-field.message index 973def68..ac7d624c 100644 --- a/test/data/valid-messages/unknown-header-field.message +++ b/test/data/valid-messages/unknown-header-field.message @@ -3,9 +3,16 @@ ## VALID_HEADER includes a LENGTH Header and LENGTH Body VALID_HEADER method_call REQUIRED_FIELDS + HEADER_FIELD UNKNOWN +TYPE DICT +LENGTH Dict +START_LENGTH Dict +STRING 'int32' TYPE INT32 -INT32 0xfeeb +INT32 0x12345678 +END_LENGTH Dict + ALIGN 8 END_LENGTH Header START_LENGTH Body |