From 75742242000e782719bc1656f0a7da72b059e88e Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Tue, 21 Oct 2003 05:46:52 +0000 Subject: 2003-10-20 Havoc Pennington hmm, make check is currently not passing. * doc/dbus-specification.xml: add requirement that custom type names follow the same rules as interface names. * dbus/dbus-protocol.h: change some of the byte codes, to avoid duplication and allow 'c' to be 'custom'; dict is now 'm' for 'map' * doc/dbus-specification.xml: update type codes to match dbus-protocol.h, using the ASCII byte values. Rename type NAMED to CUSTOM. Add type OBJECT_PATH to the spec. 2003-10-17 Havoc Pennington * bus/driver.c (create_unique_client_name): use "." as separator in base service names instead of '-' * dbus/dbus-string.c (_dbus_string_get_byte): allow getting nul byte at the end of the string * dbus/dbus-internals.h (_DBUS_LIKELY, _DBUS_UNLIKELY): add optimization macros since string validation seems to be a slow point. * doc/dbus-specification.xml: restrict valid service/interface/member/error names. Add test suite code for the name validation. * dbus/dbus-string.c: limit service/interface/member/error names to [0-9][A-Z][a-z]_ * dbus/dbus-connection.c (dbus_connection_dispatch): add missing format arg to verbose spew * glib/dbus-gproxy.c (dbus_gproxy_call_no_reply): if not out of memory, return instead of g_error * test/test-service.c (path_message_func): support emitting a signal on request * dbus/dbus-bus.c (init_connections_unlocked): only fill in activation bus type if DBUS_BUS_ACTIVATION was set; default to assuming the activation bus was the session bus so that services started manually will still register. (init_connections_unlocked): fix so that in OOM situation we get the same semantics when retrying the function * test/test-service.c (main): change to use path registration, to test those codepaths; register with DBUS_BUS_ACTIVATION rather than DBUS_BUS_SESSION --- dbus/dbus-bus.c | 67 +++--- dbus/dbus-connection.c | 22 +- dbus/dbus-internals.c | 6 +- dbus/dbus-internals.h | 27 +++ dbus/dbus-marshal.c | 8 +- dbus/dbus-message-builder.c | 4 +- dbus/dbus-message.c | 73 ++++--- dbus/dbus-message.h | 6 +- dbus/dbus-protocol.h | 4 +- dbus/dbus-string.c | 515 +++++++++++++++++++++++++++++++++++++------- dbus/dbus-string.h | 2 +- 11 files changed, 575 insertions(+), 159 deletions(-) (limited to 'dbus') diff --git a/dbus/dbus-bus.c b/dbus/dbus-bus.c index 58df18d3..0777e746 100644 --- a/dbus/dbus-bus.c +++ b/dbus/dbus-bus.c @@ -139,6 +139,9 @@ init_connections_unlocked (void) /* Don't init these twice, we may run this code twice if * init_connections_unlocked() fails midway through. + * In practice, each block below should contain only one + * "return FALSE" or running through twice may not + * work right. */ if (bus_connection_addresses[DBUS_BUS_SYSTEM] == NULL) @@ -148,22 +151,23 @@ init_connections_unlocked (void) if (!get_from_env (&bus_connection_addresses[DBUS_BUS_SYSTEM], "DBUS_SYSTEM_BUS_ADDRESS")) return FALSE; - + } + + + if (bus_connection_addresses[DBUS_BUS_SYSTEM] == NULL) + { + /* Use default system bus address if none set in environment */ + bus_connection_addresses[DBUS_BUS_SYSTEM] = + _dbus_strdup (DBUS_SYSTEM_BUS_DEFAULT_ADDRESS); if (bus_connection_addresses[DBUS_BUS_SYSTEM] == NULL) - { - /* Use default system bus address if none set in environment */ - bus_connection_addresses[DBUS_BUS_SYSTEM] = - _dbus_strdup (DBUS_SYSTEM_BUS_DEFAULT_ADDRESS); - if (bus_connection_addresses[DBUS_BUS_SYSTEM] == NULL) - return FALSE; - - _dbus_verbose (" used default system bus \"%s\"\n", - bus_connection_addresses[DBUS_BUS_SYSTEM]); - } - else - _dbus_verbose (" used env var system bus \"%s\"\n", - bus_connection_addresses[DBUS_BUS_SYSTEM]); + return FALSE; + + _dbus_verbose (" used default system bus \"%s\"\n", + bus_connection_addresses[DBUS_BUS_SYSTEM]); } + else + _dbus_verbose (" used env var system bus \"%s\"\n", + bus_connection_addresses[DBUS_BUS_SYSTEM]); if (bus_connection_addresses[DBUS_BUS_SESSION] == NULL) { @@ -183,23 +187,38 @@ init_connections_unlocked (void) if (!get_from_env (&bus_connection_addresses[DBUS_BUS_ACTIVATION], "DBUS_ACTIVATION_ADDRESS")) return FALSE; - + _dbus_verbose (" \"%s\"\n", bus_connection_addresses[DBUS_BUS_ACTIVATION] ? bus_connection_addresses[DBUS_BUS_ACTIVATION] : "none set"); } - s = _dbus_getenv ("DBUS_ACTIVATION_BUS_TYPE"); - if (s != NULL) + if (bus_connection_addresses[DBUS_BUS_ACTIVATION] != NULL) { - _dbus_verbose ("Bus activation type was set to \"%s\"\n", s); - - if (strcmp (s, "system") == 0) - activation_bus_type = DBUS_BUS_SYSTEM; - else if (strcmp (s, "session") == 0) - activation_bus_type = DBUS_BUS_SESSION; + s = _dbus_getenv ("DBUS_ACTIVATION_BUS_TYPE"); + + if (s != NULL) + { + _dbus_verbose ("Bus activation type was set to \"%s\"\n", s); + + if (strcmp (s, "system") == 0) + activation_bus_type = DBUS_BUS_SYSTEM; + else if (strcmp (s, "session") == 0) + activation_bus_type = DBUS_BUS_SESSION; + } } - + else + { + /* Default to the session bus instead if available */ + if (bus_connection_addresses[DBUS_BUS_SESSION] != NULL) + { + bus_connection_addresses[DBUS_BUS_ACTIVATION] = + _dbus_strdup (bus_connection_addresses[DBUS_BUS_SESSION]); + if (bus_connection_addresses[DBUS_BUS_ACTIVATION] == NULL) + return FALSE; + } + } + /* If we return FALSE we have to be sure that restarting * the above code will work right */ diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index bceef2a6..61de8b1f 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -1830,6 +1830,10 @@ check_for_reply_unlocked (DBusConnection *connection, * the whole message queue for example) and has thread issues, * see comments in source * + * Does not re-enter the main loop or run filter/path-registered + * callbacks. The reply to the message will not be seen by + * filter callbacks. + * * @param connection the connection * @param client_serial the reply serial to wait for * @param timeout_milliseconds timeout in milliseconds or -1 for default @@ -2430,8 +2434,15 @@ dbus_connection_get_dispatch_status (DBusConnection *connection) * @todo some FIXME in here about handling DBUS_HANDLER_RESULT_NEED_MEMORY * * @todo right now a message filter gets run on replies to a pending - * call in here, but not in the case where we block without - * entering the main loop. + * call in here, but not in the case where we block without entering + * the main loop. Simple solution might be to just have the pending + * call stuff run before the filters. + * + * @todo FIXME what if we call out to application code to handle a + * message, holding the dispatch lock, and the application code runs + * the main loop and dispatches again? Probably deadlocks at the + * moment. Maybe we want a dispatch status of DBUS_DISPATCH_IN_PROGRESS, + * and then the GSource etc. could handle the situation? * * @param connection the connection * @returns dispatch status @@ -2581,7 +2592,8 @@ dbus_connection_dispatch (DBusConnection *connection) dbus_message_get_type (message), dbus_message_get_interface (message) ? dbus_message_get_interface (message) : - "no interface"); + "no interface", + dbus_message_get_signature (message)); result = _dbus_object_tree_dispatch_and_unlock (connection->objects, message); @@ -3009,7 +3021,9 @@ dbus_connection_set_unix_user_function (DBusConnection *connection, * * @todo we don't run filters on messages while blocking without * entering the main loop, since filters are run as part of - * dbus_connection_dispatch(). + * dbus_connection_dispatch(). This is probably a feature, as filters + * could create arbitrary reentrancy. But kind of sucks if you're + * trying to filter METHOD_RETURN for some reason. * * @param connection the connection * @param function function to handle messages diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c index 9a2aa2b1..803eac08 100644 --- a/dbus/dbus-internals.c +++ b/dbus/dbus-internals.c @@ -380,8 +380,8 @@ _dbus_type_to_string (int type) return "double"; case DBUS_TYPE_STRING: return "string"; - case DBUS_TYPE_NAMED: - return "named"; + case DBUS_TYPE_CUSTOM: + return "custom"; case DBUS_TYPE_ARRAY: return "array"; case DBUS_TYPE_DICT: @@ -448,7 +448,7 @@ _dbus_real_assert (dbus_bool_t condition, const char *file, int line) { - if (!condition) + if (_DBUS_UNLIKELY (!condition)) { _dbus_warn ("%lu: assertion failed \"%s\" file \"%s\" line %d\n", _dbus_getpid (), condition_text, file, line); diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index 0284da0e..675a3a73 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -51,6 +51,33 @@ void _dbus_verbose_reset_real (void); #define _DBUS_FUNCTION_NAME "unknown function" #endif +/* + * (code from GLib) + * + * The _DBUS_LIKELY and _DBUS_UNLIKELY macros let the programmer give hints to + * the compiler about the expected result of an expression. Some compilers + * can use this information for optimizations. + * + * The _DBUS_BOOLEAN_EXPR macro is intended to trigger a gcc warning when + * putting assignments in the macro arg + */ +#if defined(__GNUC__) && (__GNUC__ > 2) && defined(__OPTIMIZE__) +#define _DBUS_BOOLEAN_EXPR(expr) \ + __extension__ ({ \ + int _dbus_boolean_var_; \ + if (expr) \ + _dbus_boolean_var_ = 1; \ + else \ + _dbus_boolean_var_ = 0; \ + _dbus_boolean_var_; \ +}) +#define _DBUS_LIKELY(expr) (__builtin_expect (_DBUS_BOOLEAN_EXPR(expr), 1)) +#define _DBUS_UNLIKELY(expr) (__builtin_expect (_DBUS_BOOLEAN_EXPR(expr), 0)) +#else +#define _DBUS_LIKELY(expr) (expr) +#define _DBUS_UNLIKELY(expr) (expr) +#endif + #ifdef DBUS_ENABLE_VERBOSE_MODE # define _dbus_verbose _dbus_verbose_real # define _dbus_verbose_reset _dbus_verbose_reset_real diff --git a/dbus/dbus-marshal.c b/dbus/dbus-marshal.c index ecc1e1ae..da7bbd4e 100644 --- a/dbus/dbus-marshal.c +++ b/dbus/dbus-marshal.c @@ -1642,7 +1642,7 @@ _dbus_marshal_get_arg_end_pos (const DBusString *str, } break; - case DBUS_TYPE_NAMED: + case DBUS_TYPE_CUSTOM: { int len; @@ -1836,7 +1836,7 @@ validate_array_data (const DBusString *str, case DBUS_TYPE_OBJECT_PATH: case DBUS_TYPE_STRING: - case DBUS_TYPE_NAMED: + case DBUS_TYPE_CUSTOM: case DBUS_TYPE_ARRAY: case DBUS_TYPE_DICT: /* This clean recursion to validate_arg is what we @@ -2050,7 +2050,7 @@ _dbus_marshal_validate_arg (const DBusString *str, } break; - case DBUS_TYPE_NAMED: + case DBUS_TYPE_CUSTOM: { int len; @@ -2226,7 +2226,7 @@ _dbus_type_is_valid (int typecode) case DBUS_TYPE_UINT64: case DBUS_TYPE_DOUBLE: case DBUS_TYPE_STRING: - case DBUS_TYPE_NAMED: + case DBUS_TYPE_CUSTOM: case DBUS_TYPE_ARRAY: case DBUS_TYPE_DICT: case DBUS_TYPE_OBJECT_PATH: diff --git a/dbus/dbus-message-builder.c b/dbus/dbus-message-builder.c index c9dc8ca5..9f68793d 100644 --- a/dbus/dbus-message-builder.c +++ b/dbus/dbus-message-builder.c @@ -735,8 +735,8 @@ _dbus_message_data_load (DBusString *dest, code = DBUS_TYPE_STRING; else if (_dbus_string_starts_with_c_str (&line, "OBJECT_PATH")) code = DBUS_TYPE_OBJECT_PATH; - else if (_dbus_string_starts_with_c_str (&line, "NAMED")) - code = DBUS_TYPE_NAMED; + else if (_dbus_string_starts_with_c_str (&line, "CUSTOM")) + code = DBUS_TYPE_CUSTOM; else if (_dbus_string_starts_with_c_str (&line, "ARRAY")) code = DBUS_TYPE_ARRAY; else if (_dbus_string_starts_with_c_str (&line, "DICT")) diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 4c21946a..d3b1a3d0 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -488,13 +488,13 @@ re_align_field_recurse (DBusMessage *message, case DBUS_TYPE_DOUBLE: padding = _DBUS_ALIGN_VALUE (pos, 8) - pos; break; - case DBUS_TYPE_NAMED: + case DBUS_TYPE_CUSTOM: case DBUS_TYPE_ARRAY: case DBUS_TYPE_DICT: /* FIXME This is no good; we have to handle undefined header fields * also. SECURITY and spec compliance issue. */ - _dbus_assert_not_reached ("no defined header fields may contain a named, array or dict value"); + _dbus_assert_not_reached ("no defined header fields may contain a custom, array or dict value"); break; case DBUS_TYPE_INVALID: default: @@ -1832,7 +1832,7 @@ dbus_message_append_args_valist (DBusMessage *message, dbus_message_append_iter_init (message, &iter); - while (type != 0) + while (type != DBUS_TYPE_INVALID) { switch (type) { @@ -1873,7 +1873,7 @@ dbus_message_append_args_valist (DBusMessage *message, case DBUS_TYPE_OBJECT_PATH: break; - case DBUS_TYPE_NAMED: + case DBUS_TYPE_CUSTOM: { const char *name; unsigned char *data; @@ -1883,7 +1883,7 @@ dbus_message_append_args_valist (DBusMessage *message, data = va_arg (var_args, unsigned char *); len = va_arg (var_args, int); - if (!dbus_message_iter_append_named (&iter, name, data, len)) + if (!dbus_message_iter_append_custom (&iter, name, data, len)) goto errorout; break; } @@ -1934,7 +1934,7 @@ dbus_message_append_args_valist (DBusMessage *message, break; case DBUS_TYPE_NIL: case DBUS_TYPE_ARRAY: - case DBUS_TYPE_NAMED: + case DBUS_TYPE_CUSTOM: case DBUS_TYPE_DICT: _dbus_warn ("dbus_message_append_args_valist doesn't support recursive arrays\n"); goto errorout; @@ -2194,7 +2194,7 @@ dbus_message_iter_get_args_valist (DBusMessageIter *iter, break; } - case DBUS_TYPE_NAMED: + case DBUS_TYPE_CUSTOM: { char **name; unsigned char **data; @@ -2204,7 +2204,7 @@ dbus_message_iter_get_args_valist (DBusMessageIter *iter, data = va_arg (var_args, unsigned char **); len = va_arg (var_args, int *); - if (!dbus_message_iter_get_named (iter, name, data, len)) + if (!dbus_message_iter_get_custom (iter, name, data, len)) { dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); goto out; @@ -2295,7 +2295,7 @@ dbus_message_iter_get_args_valist (DBusMessageIter *iter, break; case DBUS_TYPE_NIL: case DBUS_TYPE_ARRAY: - case DBUS_TYPE_NAMED: + case DBUS_TYPE_CUSTOM: case DBUS_TYPE_DICT: _dbus_warn ("dbus_message_get_args_valist doesn't support recursive arrays\n"); dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED, NULL); @@ -2682,24 +2682,23 @@ dbus_message_iter_get_object_path (DBusMessageIter *iter) #endif /** - * Returns the name and data from a named type that an - * iterator may point to. Note that you need to check that - * the iterator points to a named type before using this - * function. + * Returns the name and data from a custom type that an iterator may + * point to. Note that you need to check that the iterator points to a + * custom type before using this function. * * @see dbus_message_iter_get_arg_type * @param iter the message iter - * @param name return location for the name + * @param name return location for the name of the custom type * @param value return location for data * @param len return location for length of data * @returns TRUE if get succeed * */ dbus_bool_t -dbus_message_iter_get_named (DBusMessageIter *iter, - char **name, - unsigned char **value, - int *len) +dbus_message_iter_get_custom (DBusMessageIter *iter, + char **name, + unsigned char **value, + int *len) { DBusMessageRealIter *real = (DBusMessageRealIter *)iter; int type, pos; @@ -2709,7 +2708,7 @@ dbus_message_iter_get_named (DBusMessageIter *iter, pos = dbus_message_iter_get_data_start (real, &type); - _dbus_assert (type == DBUS_TYPE_NAMED); + _dbus_assert (type == DBUS_TYPE_CUSTOM); _name = _dbus_demarshal_string (&real->message->body, real->message->byte_order, pos, &pos); @@ -3773,7 +3772,7 @@ dbus_message_iter_append_string (DBusMessageIter *iter, } /** - * Appends a named type data chunk to the message. A named + * Appends a custom type data chunk to the message. A custom * type is simply an arbitrary UTF-8 string used as a type * tag, plus an array of arbitrary bytes to be interpreted * according to the type tag. @@ -3785,16 +3784,16 @@ dbus_message_iter_append_string (DBusMessageIter *iter, * @returns #TRUE on success */ dbus_bool_t -dbus_message_iter_append_named (DBusMessageIter *iter, - const char *name, - const unsigned char *data, - int len) +dbus_message_iter_append_custom (DBusMessageIter *iter, + const char *name, + const unsigned char *data, + int len) { DBusMessageRealIter *real = (DBusMessageRealIter *)iter; _dbus_return_val_if_fail (dbus_message_iter_append_check (real), FALSE); - if (!dbus_message_iter_append_type (real, DBUS_TYPE_NAMED)) + if (!dbus_message_iter_append_type (real, DBUS_TYPE_CUSTOM)) return FALSE; if (!_dbus_marshal_string (&real->message->body, real->message->byte_order, name)) @@ -4865,6 +4864,11 @@ decode_string_field (const DBusString *data, return TRUE; } +/* FIXME because the service/interface/member/error names are already + * validated to be in the particular ASCII subset, UTF-8 validating + * them could be skipped as a probably-interesting optimization. + * The UTF-8 validation definitely shows up in profiles. + */ static dbus_bool_t decode_header_data (const DBusString *data, int header_len, @@ -5753,6 +5757,7 @@ dbus_message_type_from_string (const char *type_str) #ifdef DBUS_BUILD_TESTS #include "dbus-test.h" #include +#include static void message_iter_test (DBusMessage *message) @@ -5942,13 +5947,13 @@ message_iter_test (DBusMessage *message) if (!dbus_message_iter_next (&iter)) _dbus_assert_not_reached ("Reached end of arguments"); - if (dbus_message_iter_get_arg_type (&iter) != DBUS_TYPE_NAMED) + if (dbus_message_iter_get_arg_type (&iter) != DBUS_TYPE_CUSTOM) _dbus_assert_not_reached ("wrong type after dict"); - if (!dbus_message_iter_get_named (&iter, &str, &data, &len)) - _dbus_assert_not_reached ("failed to get named"); + if (!dbus_message_iter_get_custom (&iter, &str, &data, &len)) + _dbus_assert_not_reached ("failed to get custom type"); - _dbus_assert (strcmp (str, "named")==0); + _dbus_assert (strcmp (str, "MyTypeName")==0); _dbus_assert (len == 5); _dbus_assert (strcmp (data, "data")==0); dbus_free (str); @@ -6006,15 +6011,15 @@ check_message_handling_type (DBusMessageIter *iter, dbus_free (str); } break; - case DBUS_TYPE_NAMED: + case DBUS_TYPE_CUSTOM: { char *name; unsigned char *data; int len; - if (!dbus_message_iter_get_named (iter, &name, &data, &len)) + if (!dbus_message_iter_get_custom (iter, &name, &data, &len)) { - _dbus_warn ("error reading name from named type\n"); + _dbus_warn ("error reading name from custom type\n"); return FALSE; } dbus_free (data); @@ -7083,8 +7088,8 @@ _dbus_message_test (const char *test_data_dir) dbus_message_iter_append_nil (&iter); - dbus_message_iter_append_named (&iter, "named", - "data", 5); + dbus_message_iter_append_custom (&iter, "MyTypeName", + "data", 5); message_iter_test (message); diff --git a/dbus/dbus-message.h b/dbus/dbus-message.h index 24955bfa..90865789 100644 --- a/dbus/dbus-message.h +++ b/dbus/dbus-message.h @@ -1,7 +1,7 @@ /* -*- mode: C; c-file-style: "gnu" -*- */ /* dbus-message.h DBusMessage object * - * Copyright (C) 2002 Red Hat Inc. + * Copyright (C) 2002, 2003 Red Hat Inc. * * Licensed under the Academic Free License version 1.2 * @@ -164,7 +164,7 @@ double dbus_message_iter_get_double (DBusMessageIter *iter char * dbus_message_iter_get_string (DBusMessageIter *iter); char * dbus_message_iter_get_object_path (DBusMessageIter *iter); char * dbus_message_iter_get_dict_key (DBusMessageIter *iter); -dbus_bool_t dbus_message_iter_get_named (DBusMessageIter *iter, +dbus_bool_t dbus_message_iter_get_custom (DBusMessageIter *iter, char **name, unsigned char **value, int *len); @@ -226,7 +226,7 @@ dbus_bool_t dbus_message_iter_append_double (DBusMessageIter *iter, double value); dbus_bool_t dbus_message_iter_append_string (DBusMessageIter *iter, const char *value); -dbus_bool_t dbus_message_iter_append_named (DBusMessageIter *iter, +dbus_bool_t dbus_message_iter_append_custom (DBusMessageIter *iter, const char *name, const unsigned char *data, int len); diff --git a/dbus/dbus-protocol.h b/dbus/dbus-protocol.h index 94bb4dd9..6d0a83cd 100644 --- a/dbus/dbus-protocol.h +++ b/dbus/dbus-protocol.h @@ -50,9 +50,9 @@ extern "C" { #define DBUS_TYPE_UINT64 ((int) 't') #define DBUS_TYPE_DOUBLE ((int) 'd') #define DBUS_TYPE_STRING ((int) 's') -#define DBUS_TYPE_NAMED ((int) 'n') +#define DBUS_TYPE_CUSTOM ((int) 'c') #define DBUS_TYPE_ARRAY ((int) 'a') -#define DBUS_TYPE_DICT ((int) 'c') +#define DBUS_TYPE_DICT ((int) 'm') #define DBUS_TYPE_OBJECT_PATH ((int) 'o') #define DBUS_NUMBER_OF_TYPES (13) diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c index 628cf861..25868f17 100644 --- a/dbus/dbus-string.c +++ b/dbus/dbus-string.c @@ -544,7 +544,9 @@ _dbus_string_set_byte (DBusString *str, } /** - * Gets the byte at the given position. + * Gets the byte at the given position. It is + * allowed to ask for the nul byte at the end of + * the string. * * @param str the string * @param start the position @@ -555,7 +557,7 @@ _dbus_string_get_byte (const DBusString *str, int start) { DBUS_CONST_STRING_PREAMBLE (str); - _dbus_assert (start < real->len); + _dbus_assert (start <= real->len); _dbus_assert (start >= 0); return real->str[start]; @@ -2728,8 +2730,8 @@ _dbus_string_validate_ascii (const DBusString *str, end = s + len; while (s != end) { - if (*s == '\0' || - ((*s & ~0x7f) != 0)) + if (_DBUS_UNLIKELY (*s == '\0' || + ((*s & ~0x7f) != 0))) return FALSE; ++s; @@ -2765,7 +2767,15 @@ _dbus_string_validate_utf8 (const DBusString *str, _dbus_assert (start <= real->len); _dbus_assert (len >= 0); - if (len > real->len - start) + /* we are doing _DBUS_UNLIKELY() here which might be + * dubious in a generic library like GLib, but in D-BUS + * we know we're validating messages and that it would + * only be evil/broken apps that would have invalid + * UTF-8. Also, this function seems to be a performance + * bottleneck in profiles. + */ + + if (_DBUS_UNLIKELY (len > real->len - start)) return FALSE; p = real->str + start; @@ -2779,22 +2789,22 @@ _dbus_string_validate_utf8 (const DBusString *str, UTF8_COMPUTE (c, mask, char_len); - if (char_len == -1) + if (_DBUS_UNLIKELY (char_len == -1)) break; /* check that the expected number of bytes exists in the remaining length */ - if ((end - p) < char_len) + if (_DBUS_UNLIKELY ((end - p) < char_len)) break; UTF8_GET (result, p, i, mask, char_len); - if (UTF8_LENGTH (result) != char_len) /* Check for overlong UTF-8 */ + if (_DBUS_UNLIKELY (UTF8_LENGTH (result) != char_len)) /* Check for overlong UTF-8 */ break; - if (result == (dbus_unichar_t)-1) + if (_DBUS_UNLIKELY (result == (dbus_unichar_t)-1)) break; - if (!UNICODE_VALID (result)) + if (_DBUS_UNLIKELY (!UNICODE_VALID (result))) break; p += char_len; @@ -2803,7 +2813,7 @@ _dbus_string_validate_utf8 (const DBusString *str, /* See that we covered the entire length if a length was * passed in */ - if (p != end) + if (_DBUS_UNLIKELY (p != end)) return FALSE; else return TRUE; @@ -2841,7 +2851,7 @@ _dbus_string_validate_nul (const DBusString *str, end = s + len; while (s != end) { - if (*s != '\0') + if (_DBUS_UNLIKELY (*s != '\0')) return FALSE; ++s; } @@ -2917,17 +2927,24 @@ _dbus_string_validate_path (const DBusString *str, return TRUE; } +#define VALID_INITIAL_NAME_CHARACTER(c) \ + ( ((c) >= 'A' && (c) <= 'Z') || \ + ((c) >= 'a' && (c) <= 'z') || \ + ((c) == '_') ) + +#define VALID_NAME_CHARACTER(c) \ + ( ((c) >= '0' && (c) <= '9') || \ + ((c) >= 'A' && (c) <= 'Z') || \ + ((c) >= 'a' && (c) <= 'z') || \ + ((c) == '_') ) + /** * Checks that the given range of the string is a valid interface name - * in the D-BUS protocol. This includes a length restriction, etc., - * see the specification. It does not validate UTF-8, that has to be - * done separately for now. + * in the D-BUS protocol. This includes a length restriction and an + * ASCII subset, see the specification. * * @todo this is inconsistent with most of DBusString in that * it allows a start,len range that isn't in the string. - * - * @todo change spec to disallow more things, such as spaces in the - * interface name * * @param str the string * @param start first byte index to check @@ -2938,10 +2955,11 @@ dbus_bool_t _dbus_string_validate_interface (const DBusString *str, int start, int len) -{ +{ const unsigned char *s; const unsigned char *end; - dbus_bool_t saw_dot; + const unsigned char *iface; + const unsigned char *last_dot; DBUS_CONST_STRING_PREAMBLE (str); _dbus_assert (start >= 0); @@ -2957,21 +2975,41 @@ _dbus_string_validate_interface (const DBusString *str, if (len == 0) return FALSE; - saw_dot = FALSE; - s = real->str + start; - end = s + len; + last_dot = NULL; + iface = real->str + start; + end = iface + len; + s = iface; + + /* check special cases of first char so it doesn't have to be done + * in the loop. Note we know len > 0 + */ + if (_DBUS_UNLIKELY (*s == '.')) /* disallow starting with a . */ + return FALSE; + else if (_DBUS_UNLIKELY (!VALID_INITIAL_NAME_CHARACTER (*s))) + return FALSE; + else + ++s; + while (s != end) { if (*s == '.') { - saw_dot = TRUE; - break; + if (_DBUS_UNLIKELY ((s + 1) == end)) + return FALSE; + else if (_DBUS_UNLIKELY (!VALID_INITIAL_NAME_CHARACTER (*(s + 1)))) + return FALSE; + last_dot = s; + ++s; /* we just validated the next char, so skip two */ + } + else if (_DBUS_UNLIKELY (!VALID_NAME_CHARACTER (*s))) + { + return FALSE; } ++s; } - if (!saw_dot) + if (_DBUS_UNLIKELY (last_dot == NULL)) return FALSE; return TRUE; @@ -2980,15 +3018,11 @@ _dbus_string_validate_interface (const DBusString *str, /** * Checks that the given range of the string is a valid member name * in the D-BUS protocol. This includes a length restriction, etc., - * see the specification. It does not validate UTF-8, that has to be - * done separately for now. + * see the specification. * * @todo this is inconsistent with most of DBusString in that * it allows a start,len range that isn't in the string. * - * @todo change spec to disallow more things, such as spaces in the - * member name - * * @param str the string * @param start first byte index to check * @param len number of bytes to check @@ -3001,7 +3035,7 @@ _dbus_string_validate_member (const DBusString *str, { const unsigned char *s; const unsigned char *end; - dbus_bool_t saw_dot; + const unsigned char *member; DBUS_CONST_STRING_PREAMBLE (str); _dbus_assert (start >= 0); @@ -3017,23 +3051,28 @@ _dbus_string_validate_member (const DBusString *str, if (len == 0) return FALSE; - saw_dot = FALSE; - s = real->str + start; - end = s + len; + member = real->str + start; + end = member + len; + s = member; + + /* check special cases of first char so it doesn't have to be done + * in the loop. Note we know len > 0 + */ + + if (_DBUS_UNLIKELY (!VALID_INITIAL_NAME_CHARACTER (*s))) + return FALSE; + else + ++s; + while (s != end) { - if (*s == '.') + if (_DBUS_UNLIKELY (!VALID_NAME_CHARACTER (*s))) { - saw_dot = TRUE; - break; + return FALSE; } ++s; } - - /* No dot allowed in member names */ - if (saw_dot) - return FALSE; return TRUE; } @@ -3041,8 +3080,7 @@ _dbus_string_validate_member (const DBusString *str, /** * Checks that the given range of the string is a valid error name * in the D-BUS protocol. This includes a length restriction, etc., - * see the specification. It does not validate UTF-8, that has to be - * done separately for now. + * see the specification. * * @todo this is inconsistent with most of DBusString in that * it allows a start,len range that isn't in the string. @@ -3061,32 +3099,15 @@ _dbus_string_validate_error_name (const DBusString *str, return _dbus_string_validate_interface (str, start, len); } -/** - * Checks that the given range of the string is a valid service name - * in the D-BUS protocol. This includes a length restriction, etc., - * see the specification. It does not validate UTF-8, that has to be - * done separately for now. - * - * @todo this is inconsistent with most of DBusString in that - * it allows a start,len range that isn't in the string. - * - * @todo change spec to disallow more things, such as spaces in the - * service name - * - * @param str the string - * @param start first byte index to check - * @param len number of bytes to check - * @returns #TRUE if the byte range exists and is a valid name - */ -dbus_bool_t -_dbus_string_validate_service (const DBusString *str, - int start, - int len) +/* This assumes the first char exists and is ':' */ +static dbus_bool_t +_dbus_string_validate_base_service (const DBusString *str, + int start, + int len) { const unsigned char *s; const unsigned char *end; - dbus_bool_t saw_dot; - dbus_bool_t is_base_service; + const unsigned char *service; DBUS_CONST_STRING_PREAMBLE (str); _dbus_assert (start >= 0); @@ -3099,30 +3120,58 @@ _dbus_string_validate_service (const DBusString *str, if (len > DBUS_MAXIMUM_NAME_LENGTH) return FALSE; - if (len == 0) - return FALSE; + _dbus_assert (len > 0); - is_base_service = _dbus_string_get_byte (str, start) == ':'; - if (is_base_service) - return TRUE; /* can have any content */ - - /* non-base-service must have the '.' indicating a namespace */ + service = real->str + start; + end = service + len; + _dbus_assert (*service == ':'); + s = service + 1; - saw_dot = FALSE; - s = real->str + start; - end = s + len; while (s != end) { if (*s == '.') { - saw_dot = TRUE; - break; + if (_DBUS_UNLIKELY ((s + 1) == end)) + return FALSE; + if (_DBUS_UNLIKELY (!VALID_NAME_CHARACTER (*(s + 1)))) + return FALSE; + ++s; /* we just validated the next char, so skip two */ + } + else if (_DBUS_UNLIKELY (!VALID_NAME_CHARACTER (*s))) + { + return FALSE; } ++s; } - return saw_dot; + return TRUE; +} + +/** + * Checks that the given range of the string is a valid service name + * in the D-BUS protocol. This includes a length restriction, etc., + * see the specification. + * + * @todo this is inconsistent with most of DBusString in that + * it allows a start,len range that isn't in the string. + * + * @param str the string + * @param start first byte index to check + * @param len number of bytes to check + * @returns #TRUE if the byte range exists and is a valid name + */ +dbus_bool_t +_dbus_string_validate_service (const DBusString *str, + int start, + int len) +{ + if (_DBUS_UNLIKELY (len == 0)) + return FALSE; + if (_dbus_string_get_byte (str, start) == ':') + return _dbus_string_validate_base_service (str, start, len); + else + return _dbus_string_validate_interface (str, start, len); } /** @@ -3339,7 +3388,95 @@ _dbus_string_test (void) "//", "///", "foo///blah/", - "Hello World" + "Hello World", + "", + " ", + "foo bar" + }; + + const char *valid_interfaces[] = { + "org.freedesktop.Foo", + "Bar.Baz", + "Blah.Blah.Blah.Blah.Blah", + "a.b", + "a.b.c.d.e.f.g", + "a0.b1.c2.d3.e4.f5.g6", + "abc123.foo27" + }; + const char *invalid_interfaces[] = { + ".", + "", + "..", + ".Foo.Bar", + "..Foo.Bar", + "Foo.Bar.", + "Foo.Bar..", + "Foo", + "9foo.bar.baz", + "foo.bar..baz", + "foo.bar...baz", + "foo.bar.b..blah", + ":", + ":0-1", + "10", + ":11.34324", + "0.0.0", + "0..0", + "foo.Bar.%", + "foo.Bar!!", + "!Foo.bar.bz", + "foo.$.blah", + "", + " ", + "foo bar" + }; + + const char *valid_base_services[] = { + ":0", + ":a", + ":", + ":.a", + ":.1", + ":0.1", + ":000.2222", + ":.blah", + ":abce.freedesktop.blah" + }; + const char *invalid_base_services[] = { + ":-", + ":!", + ":0-10", + ":blah.", + ":blah.", + ":blah..org", + ":blah.org..", + ":..blah.org", + "", + " ", + "foo bar" + }; + + const char *valid_members[] = { + "Hello", + "Bar", + "foobar", + "_foobar", + "foo89" + }; + + const char *invalid_members[] = { + "9Hello", + "10", + "1", + "foo-bar", + "blah.org", + ".blah", + "blah.", + "Hello.", + "!foo", + "", + " ", + "foo bar" }; i = 0; @@ -3749,7 +3886,221 @@ _dbus_string_test (void) ++i; } - + + /* Interface validation */ + i = 0; + while (i < (int) _DBUS_N_ELEMENTS (valid_interfaces)) + { + _dbus_string_init_const (&str, valid_interfaces[i]); + + if (!_dbus_string_validate_interface (&str, 0, + _dbus_string_get_length (&str))) + { + _dbus_warn ("Interface \"%s\" should have been valid\n", valid_interfaces[i]); + _dbus_assert_not_reached ("invalid interface"); + } + + ++i; + } + + i = 0; + while (i < (int) _DBUS_N_ELEMENTS (invalid_interfaces)) + { + _dbus_string_init_const (&str, invalid_interfaces[i]); + + if (_dbus_string_validate_interface (&str, 0, + _dbus_string_get_length (&str))) + { + _dbus_warn ("Interface \"%s\" should have been invalid\n", invalid_interfaces[i]); + _dbus_assert_not_reached ("valid interface"); + } + + ++i; + } + + /* Service validation (check that valid interfaces are valid services, + * and invalid interfaces are invalid services except if they start with ':') + */ + i = 0; + while (i < (int) _DBUS_N_ELEMENTS (valid_interfaces)) + { + _dbus_string_init_const (&str, valid_interfaces[i]); + + if (!_dbus_string_validate_service (&str, 0, + _dbus_string_get_length (&str))) + { + _dbus_warn ("Service \"%s\" should have been valid\n", valid_interfaces[i]); + _dbus_assert_not_reached ("invalid service"); + } + + ++i; + } + + i = 0; + while (i < (int) _DBUS_N_ELEMENTS (invalid_interfaces)) + { + if (invalid_interfaces[i][0] != ':') + { + _dbus_string_init_const (&str, invalid_interfaces[i]); + + if (_dbus_string_validate_service (&str, 0, + _dbus_string_get_length (&str))) + { + _dbus_warn ("Service \"%s\" should have been invalid\n", invalid_interfaces[i]); + _dbus_assert_not_reached ("valid service"); + } + } + + ++i; + } + + /* Base service validation */ + i = 0; + while (i < (int) _DBUS_N_ELEMENTS (valid_base_services)) + { + _dbus_string_init_const (&str, valid_base_services[i]); + + if (!_dbus_string_validate_service (&str, 0, + _dbus_string_get_length (&str))) + { + _dbus_warn ("Service \"%s\" should have been valid\n", valid_base_services[i]); + _dbus_assert_not_reached ("invalid base service"); + } + + ++i; + } + + i = 0; + while (i < (int) _DBUS_N_ELEMENTS (invalid_base_services)) + { + _dbus_string_init_const (&str, invalid_base_services[i]); + + if (_dbus_string_validate_service (&str, 0, + _dbus_string_get_length (&str))) + { + _dbus_warn ("Service \"%s\" should have been invalid\n", invalid_base_services[i]); + _dbus_assert_not_reached ("valid base service"); + } + + ++i; + } + + + /* Error name validation (currently identical to interfaces) + */ + i = 0; + while (i < (int) _DBUS_N_ELEMENTS (valid_interfaces)) + { + _dbus_string_init_const (&str, valid_interfaces[i]); + + if (!_dbus_string_validate_error_name (&str, 0, + _dbus_string_get_length (&str))) + { + _dbus_warn ("Error name \"%s\" should have been valid\n", valid_interfaces[i]); + _dbus_assert_not_reached ("invalid error name"); + } + + ++i; + } + + i = 0; + while (i < (int) _DBUS_N_ELEMENTS (invalid_interfaces)) + { + if (invalid_interfaces[i][0] != ':') + { + _dbus_string_init_const (&str, invalid_interfaces[i]); + + if (_dbus_string_validate_error_name (&str, 0, + _dbus_string_get_length (&str))) + { + _dbus_warn ("Error name \"%s\" should have been invalid\n", invalid_interfaces[i]); + _dbus_assert_not_reached ("valid error name"); + } + } + + ++i; + } + + /* Member validation */ + i = 0; + while (i < (int) _DBUS_N_ELEMENTS (valid_members)) + { + _dbus_string_init_const (&str, valid_members[i]); + + if (!_dbus_string_validate_member (&str, 0, + _dbus_string_get_length (&str))) + { + _dbus_warn ("Member \"%s\" should have been valid\n", valid_members[i]); + _dbus_assert_not_reached ("invalid member"); + } + + ++i; + } + + i = 0; + while (i < (int) _DBUS_N_ELEMENTS (invalid_members)) + { + _dbus_string_init_const (&str, invalid_members[i]); + + if (_dbus_string_validate_member (&str, 0, + _dbus_string_get_length (&str))) + { + _dbus_warn ("Member \"%s\" should have been invalid\n", invalid_members[i]); + _dbus_assert_not_reached ("valid member"); + } + + ++i; + } + + /* Validate claimed length longer than real length */ + _dbus_string_init_const (&str, "abc.efg"); + if (_dbus_string_validate_service (&str, 0, 8)) + _dbus_assert_not_reached ("validated too-long string"); + if (_dbus_string_validate_interface (&str, 0, 8)) + _dbus_assert_not_reached ("validated too-long string"); + if (_dbus_string_validate_error_name (&str, 0, 8)) + _dbus_assert_not_reached ("validated too-long string"); + + _dbus_string_init_const (&str, "abc"); + if (_dbus_string_validate_member (&str, 0, 4)) + _dbus_assert_not_reached ("validated too-long string"); + + /* Validate string exceeding max name length */ + if (!_dbus_string_init (&str)) + _dbus_assert_not_reached ("no memory"); + + while (_dbus_string_get_length (&str) <= DBUS_MAXIMUM_NAME_LENGTH) + if (!_dbus_string_append (&str, "abc.def")) + _dbus_assert_not_reached ("no memory"); + + if (_dbus_string_validate_service (&str, 0, _dbus_string_get_length (&str))) + _dbus_assert_not_reached ("validated overmax string"); + if (_dbus_string_validate_interface (&str, 0, _dbus_string_get_length (&str))) + _dbus_assert_not_reached ("validated overmax string"); + if (_dbus_string_validate_error_name (&str, 0, _dbus_string_get_length (&str))) + _dbus_assert_not_reached ("validated overmax string"); + + /* overlong member */ + _dbus_string_set_length (&str, 0); + while (_dbus_string_get_length (&str) <= DBUS_MAXIMUM_NAME_LENGTH) + if (!_dbus_string_append (&str, "abc")) + _dbus_assert_not_reached ("no memory"); + + if (_dbus_string_validate_member (&str, 0, _dbus_string_get_length (&str))) + _dbus_assert_not_reached ("validated overmax string"); + + /* overlong base service */ + _dbus_string_set_length (&str, 0); + _dbus_string_append (&str, ":"); + while (_dbus_string_get_length (&str) <= DBUS_MAXIMUM_NAME_LENGTH) + if (!_dbus_string_append (&str, "abc")) + _dbus_assert_not_reached ("no memory"); + + if (_dbus_string_validate_service (&str, 0, _dbus_string_get_length (&str))) + _dbus_assert_not_reached ("validated overmax string"); + + _dbus_string_free (&str); + return TRUE; } diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h index 0b7be8b0..2441213c 100644 --- a/dbus/dbus-string.h +++ b/dbus/dbus-string.h @@ -1,7 +1,7 @@ /* -*- mode: C; c-file-style: "gnu" -*- */ /* dbus-string.h String utility class (internal to D-BUS implementation) * - * Copyright (C) 2002 Red Hat, Inc. + * Copyright (C) 2002, 2003 Red Hat, Inc. * * Licensed under the Academic Free License version 1.2 * -- cgit