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 --- ChangeLog | 54 +++ bus/bus.c | 1 + bus/dispatch.c | 6 +- bus/driver.c | 2 +- configure.in | 1 + 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 +- doc/TODO | 26 +- doc/dbus-specification.xml | 127 ++--- glib/dbus-gobject.c | 4 +- glib/dbus-gproxy.c | 12 +- test/break-loader.c | 2 +- test/data/valid-messages/lots-of-arguments.message | 2 +- .../data/valid-service-files/debug-glib.service.in | 3 + test/glib/Makefile.am | 7 +- test/glib/run-test.sh | 3 +- test/glib/test-dbus-glib.c | 56 ++- test/test-service.c | 88 +++- 27 files changed, 874 insertions(+), 254 deletions(-) create mode 100644 test/data/valid-service-files/debug-glib.service.in diff --git a/ChangeLog b/ChangeLog index 3a1f06a6..029fa716 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,57 @@ +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 + 2003-10-16 Havoc Pennington * glib/dbus-gtest-main.c: bracket with #ifdef DBUS_BUILD_TESTS diff --git a/bus/bus.c b/bus/bus.c index 9db8c411..43882c59 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -919,6 +919,7 @@ bus_context_check_security_policy (BusContext *context, type = dbus_message_get_type (message); /* dispatch.c was supposed to ensure these invariants */ + /* FIXME this assertion is failing in make check */ _dbus_assert (dbus_message_get_destination (message) != NULL || type == DBUS_MESSAGE_TYPE_SIGNAL); _dbus_assert (type == DBUS_MESSAGE_TYPE_SIGNAL || diff --git a/bus/dispatch.c b/bus/dispatch.c index 6c5eadf1..26dd4fc8 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -173,10 +173,8 @@ bus_dispatch (DBusConnection *connection, #endif /* DBUS_ENABLE_VERBOSE_MODE */ /* If service_name is NULL, if it's a signal we send it to all - * connections with a match rule. If it's not a signal, it goes to - * the bus daemon but doesn't go "on the bus"; e.g. a peer-to-peer - * ping. Handle these immediately, especially disconnection - * messages. There are no security policy checks on these. + * connections with a match rule. If it's not a signal, there + * are some special cases here but mostly we just bail out. */ if (service_name == NULL) { diff --git a/bus/driver.c b/bus/driver.c index 4d345698..9681d84f 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -245,7 +245,7 @@ create_unique_client_name (BusRegistry *registry, if (!_dbus_string_append_int (str, next_major_number)) return FALSE; - if (!_dbus_string_append (str, "-")) + if (!_dbus_string_append (str, ".")) return FALSE; if (!_dbus_string_append_int (str, next_minor_number)) diff --git a/configure.in b/configure.in index e128fddf..a2f0e121 100644 --- a/configure.in +++ b/configure.in @@ -857,6 +857,7 @@ AC_SUBST(TEST_$1) TEST_PATH(SERVICE_DIR, data/valid-service-files) TEST_PATH(SERVICE_BINARY, test-service) +TEST_PATH(GLIB_SERVICE_BINARY, test-service-glib) TEST_PATH(EXIT_BINARY, test-exit) TEST_PATH(SEGFAULT_BINARY, test-segfault) TEST_PATH(SLEEP_FOREVER_BINARY, test-sleep-forever) 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 * diff --git a/doc/TODO b/doc/TODO index 98d70b0c..f9cb9db8 100644 --- a/doc/TODO +++ b/doc/TODO @@ -26,19 +26,10 @@ which of these functions to include, in light of the fact that GLib/Qt native stubs will probably also exist. - - The message handler interface needs rethinking, perhaps handlers should be able - to return an error that automatically gets turned into a message; most likely - some basic spec'ing out of the GLib/Qt level stubs/skels stuff will be - needed to understand the right approach. - - assorted _-prefixed symbols in libdbus aren't actually used by libdbus, only by the message bus. These bloat up the library size. Not sure how to fix, really. - - if you send a message to a service then block for reply, and the service exits/crashes - after the message bus has processed your message but before the service has replied, - it would be nice if the message bus sent you an error reply. - - build and install the Doxygen manual in Makefile when --enable-docs - if you send the same message to multiple connections, the serial number @@ -89,8 +80,6 @@ - add dbus_message_has_path(), maybe has_member/interface - - The OBJECT_PATH type is not documented in the spec. - - re_align_field_recurse() in dbus-message.c is broken because it crashes on some types of header field values. security problem. @@ -99,18 +88,21 @@ be coded to handle it restarting - modify the wire protocol to keep the args signature separate - from the args themselves. Make the name of TYPE_NAMED part + from the args themselves. Make the name of TYPE_CUSTOM part of the type signature, rather than part of the value. Then you have the full typecheck in a single string. - - rename TYPE_NAMED to TYPE_CUSTOM, probably a clearer name. - - dbus_message_iter_init_array_iterator has "iter" and "iterator" in the same function name - the GLib bindings varargs take DBUS_TYPE_WHATEVER and return stuff allocated with dbus_malloc(); should this be made more "G" at some expense in code duplication? + You also still have to use some D-BUS functions such as + dbus_message_get_args() which takes a DBusError. + Probably we need to either fully encapsulate and hide + dbus/dbus.h, or encapsulate it slightly less e.g. no + GError. - need to define bus behavior if you send a message to yourself; is it an error, or allowed? If allowed, @@ -124,3 +116,9 @@ - the varargs dbus_message_get_args() needs to support OBJECT_PATH and OBJECT_PATH_ARRAY + + - recursive dispatch, see dbus_connection_dispatch() + + - the auth protocol may as well use hex encoding instead of + base64, then we can dump the base64 implementation and + save some bloat. diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index 807769b7..42bd5138 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -300,7 +300,7 @@ PATH 1 - STRING + OBJECT_PATH The object to send the message to; objects are identified by a path, "/foo/bar" @@ -388,56 +388,60 @@ INVALID - 0 + 0 (ASCII NUL) Not a valid type code (error if it appears in a message) NIL - 1 - Marks an "unset" or "nonexistent" argument + 118 (ASCII 'v') + Marks a "void"/"unset"/"nonexistent"/"null" argument BYTE - 2 + 121 (ASCII 'y') 8-bit unsigned integer BOOLEAN - 3 + 98 (ASCII 'b') Boolean value, 0 is FALSE and 1 is TRUE. Everything else is invalid. INT32 - 4 + 105 (ASCII 'i') 32-bit signed integer UINT32 - 5 + 117 (ASCII 'u') 32-bit unsigned integer INT64 - 6 + 120 (ASCII 'x') 64-bit signed integer UINT64 - 7 + 116 (ASCII 't') 64-bit unsigned integer DOUBLE - 8 + 100 (ASCII 'd') IEEE 754 double STRING - 9 + 115 (ASCII 's') UTF-8 string (must be valid UTF-8). Must be zero terminated. - NAMED - 10 + CUSTOM + 99 (ASCII 'c') A named byte array, used for custom types ARRAY - 11 + 97 (ASCII 'a') Array DICT - 12 + 109 (ASCII 'm') A dictionary of key/value pairs + + OBJECT_PATH + 111 (ASCII 'o') + Name of an object @@ -490,10 +494,12 @@ byte. - NAMED + CUSTOM A string (encoded as the STRING type above) giving the name of the type followed by an UINT32 aligned to 4-byte boundary indicating the data length in bytes, followed by the data. + The string has some restrictions on its content, see + . ARRAY @@ -513,6 +519,10 @@ as a byte with typecode and how that type normally would be encoded alone. + + OBJECT_PATH + Encoded as if it were a STRING. + @@ -523,41 +533,46 @@ Valid names - The various header fields of type STRING have some restrictions - on the string's format. + The various names in D-BUS messages have some restrictions. - - Service names + + Interface names - Services have names with type STRING, meaning that + Interfaces have names with type STRING, meaning that they must be valid UTF-8. However, there are also some - additional restrictions that apply to service names + additional restrictions that apply to interface names specifically: - They must contain at least one '.' (period) character - They must not begin with a '.' (period) character - They must not exceed 256 bytes in length - They must be at least 1 byte in length + They are composed of 1 or more elements separated by + a period ('.') character. All elements must contain at least + one character. + + + Each element must only contain the ASCII characters + "[A-Z][a-z][0-9]_" and must not begin with a digit. + + + + They must contain at least one '.' (period) + character (and thus at least two elements). + + + They must not begin with a '.' (period) character. + They must not exceed 256 bytes in length. + They must be at least 1 byte in length. - - As a special exception, base service names (those beginning with a colon - (':') character) need not contain a period. - - - FIXME really, shouldn't we ban basically everything non-alphanumeric - so the name will work in all programming languages? - - Interface names - - Interface names have the same restrictions as service names, - but do not have the special exception for names beginning with - a colon. - + + Service names - FIXME really, shouldn't we ban basically everything non-alphanumeric - so the name will work in all programming languages? + Service names have the same restrictions as interface names, with a + special exception for base services. A base service name's first + element must start with a colon (':') character. After the colon, any + characters in the range "[A-Z][a-z][0-9]_" may appear. Elements after + the first must follow the usual rules, except that they may start with + a digit. Service names not starting with a colon have none of these + exceptions and follow the same rules as interface names. @@ -565,24 +580,23 @@ Method names: - May not contain the '.' (period) character + Must only contain the ASCII characters + "[A-Z][a-z][0-9]_" and may not begin with a + digit. + Must not contain the '.' (period) character Must not exceed 256 bytes in length Must be at least 1 byte in length - - FIXME really, shouldn't we ban basically everything non-alphanumeric - so the name will work in all programming languages? - Path names - A path must begin with an ASCII '/' (slash) character. Paths may not - end with a slash character unless the path is the one-byte string - "/". Two slash characters may not appear adjacent to one another (the - empty string is not a valid "subdirectory"). Paths may not exceed - 256 bytes in length. + A path (type OBJECT_PATH) must begin with an ASCII '/' (slash) + character. Paths may not end with a slash character unless the path is + the one-byte string "/". Two slash characters may not appear adjacent + to one another (the empty string is not a valid "subdirectory"). Paths + may not exceed 256 bytes in length. @@ -590,9 +604,12 @@ Error names have the same restrictions as interface names. + + + Custom types - FIXME really, shouldn't we ban basically everything non-alphanumeric - so the name will work in all programming languages? + Custom type names for values of type CUSTOM follow the same + restrictions as interface names. diff --git a/glib/dbus-gobject.c b/glib/dbus-gobject.c index 6e65770f..1e13ed46 100644 --- a/glib/dbus-gobject.c +++ b/glib/dbus-gobject.c @@ -167,8 +167,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: diff --git a/glib/dbus-gproxy.c b/glib/dbus-gproxy.c index 90f00b25..02c53c56 100644 --- a/glib/dbus-gproxy.c +++ b/glib/dbus-gproxy.c @@ -377,7 +377,7 @@ gproxy_list_free (DBusGProxyList *list) static char* gproxy_get_match_rule (DBusGProxy *proxy) { - /* FIXME Some sort of escaping is required here I think */ + /* FIXME Escaping is required here */ if (proxy->service) return g_strdup_printf ("type='signal',sender='%s',path='%s',interface='%s'", @@ -604,6 +604,14 @@ dbus_gproxy_manager_filter (DBusConnection *connection, else list = NULL; +#if 0 + g_print ("proxy got %s,%s,%s = list %p\n", + tri, + tri + strlen (tri) + 1, + tri + strlen (tri) + 1 + strlen (tri + strlen (tri) + 1) + 1, + list); +#endif + g_free (tri); /* Emit the signal */ @@ -1140,6 +1148,8 @@ dbus_gproxy_call_no_reply (DBusGProxy *proxy, NULL)) goto oom; + return; + oom: g_error ("Out of memory"); } diff --git a/test/break-loader.c b/test/break-loader.c index 5a0c61f2..e514d793 100644 --- a/test/break-loader.c +++ b/test/break-loader.c @@ -406,7 +406,7 @@ random_type (void) DBUS_TYPE_UINT64, DBUS_TYPE_DOUBLE, DBUS_TYPE_STRING, - DBUS_TYPE_NAMED, + DBUS_TYPE_CUSTOM, DBUS_TYPE_ARRAY, DBUS_TYPE_DICT, DBUS_TYPE_OBJECT_PATH diff --git a/test/data/valid-messages/lots-of-arguments.message b/test/data/valid-messages/lots-of-arguments.message index d3f6a4ee..bdaae0f8 100644 --- a/test/data/valid-messages/lots-of-arguments.message +++ b/test/data/valid-messages/lots-of-arguments.message @@ -31,7 +31,7 @@ DOUBLE_ARRAY { 0.1, 0.2, 3.1415926, 2.7183, 10.0, 9.99 } TYPE ARRAY TYPE STRING STRING_ARRAY { 'Hello', 'This', 'Is', 'A', 'String', 'Array!' } -TYPE NAMED +TYPE CUSTOM STRING 'named type' BYTE_ARRAY { 'b', 'i', 'n', 'a', 'r', 'y', 'd', 'a', 't', 'a' } diff --git a/test/data/valid-service-files/debug-glib.service.in b/test/data/valid-service-files/debug-glib.service.in new file mode 100644 index 00000000..199fd808 --- /dev/null +++ b/test/data/valid-service-files/debug-glib.service.in @@ -0,0 +1,3 @@ +[D-BUS Service] +Name=org.freedesktop.DBus.TestSuiteGLibService +Exec=@TEST_GLIB_SERVICE_BINARY@ diff --git a/test/glib/Makefile.am b/test/glib/Makefile.am index 608e7d01..ba1f181d 100644 --- a/test/glib/Makefile.am +++ b/test/glib/Makefile.am @@ -32,13 +32,18 @@ endif ## we use noinst_PROGRAMS not check_PROGRAMS for TESTS so that we ## build even when not doing "make check" -noinst_PROGRAMS= test-dbus-glib $(THREAD_APPS) +noinst_PROGRAMS= test-dbus-glib test-service-glib $(THREAD_APPS) test_dbus_glib_SOURCES= \ test-dbus-glib.c test_dbus_glib_LDADD= $(top_builddir)/glib/libdbus-glib-1.la +test_service_glib_SOURCES= \ + test-service-glib.c + +test_service_glib_LDADD= $(top_builddir)/glib/libdbus-glib-1.la + else ### not building tests diff --git a/test/glib/run-test.sh b/test/glib/run-test.sh index a51396bb..ce2f469b 100755 --- a/test/glib/run-test.sh +++ b/test/glib/run-test.sh @@ -5,6 +5,7 @@ SCRIPTNAME=$0 function die() { if ! test -z "$DBUS_SESSION_BUS_PID" ; then + echo "killing message bus" kill -9 $DBUS_SESSION_BUS_PID fi echo $SCRIPTNAME: $* >&2 @@ -45,7 +46,7 @@ fi echo "Started test bus pid $DBUS_SESSION_BUS_PID at $DBUS_SESSION_BUS_ADDRESS" -$DBUS_TOP_BUILDDIR/test/glib/test-dbus-glib || die "test-dbus-glib failed" +$DEBUG $DBUS_TOP_BUILDDIR/test/glib/test-dbus-glib || die "test-dbus-glib failed" ## we kill -TERM so gcov data can be written out diff --git a/test/glib/test-dbus-glib.c b/test/glib/test-dbus-glib.c index fb8be4cc..5cb31235 100644 --- a/test/glib/test-dbus-glib.c +++ b/test/glib/test-dbus-glib.c @@ -4,11 +4,43 @@ #include #include +static GMainLoop *loop = NULL; +static int n_times_foo_received = 0; + +static void +foo_signal_handler (DBusGProxy *proxy, + DBusMessage *signal, + void *user_data) +{ + double d; + DBusError derror; + + if (!dbus_message_is_signal (signal, + "org.freedesktop.TestSuite", + "Foo")) + { + g_printerr ("Signal handler received the wrong message\n"); + exit (1); + } + + dbus_error_init (&derror); + if (!dbus_message_get_args (signal, &derror, DBUS_TYPE_DOUBLE, + &d, DBUS_TYPE_INVALID)) + { + g_printerr ("failed to get signal args: %s\n", derror.message); + dbus_error_free (&derror); + exit (1); + } + + n_times_foo_received += 1; + + g_main_loop_quit (loop); +} + int main (int argc, char **argv) { DBusConnection *connection; - GMainLoop *loop; GError *error; DBusGProxy *driver; DBusGProxy *proxy; @@ -135,7 +167,7 @@ main (int argc, char **argv) proxy = dbus_gproxy_new_for_service (connection, "org.freedesktop.DBus.TestSuiteEchoService", - "/fixme/the/test/service/ignores/this", /* FIXME */ + "/org/freedesktop/TestSuite", "org.freedesktop.TestSuite"); call = dbus_gproxy_begin_call (proxy, "Echo", @@ -156,6 +188,26 @@ main (int argc, char **argv) g_print ("String echoed = \"%s\"\n", str); dbus_free (str); + + /* Test oneway call and signal handling */ + + dbus_gproxy_connect_signal (proxy, "Foo", + foo_signal_handler, + NULL, NULL); + + dbus_gproxy_call_no_reply (proxy, "EmitFoo", + DBUS_TYPE_INVALID); + + dbus_connection_flush (connection); + + g_main_loop_run (loop); + + if (n_times_foo_received != 1) + { + g_printerr ("Foo signal received %d times, should have been 1\n", + n_times_foo_received); + exit (1); + } g_object_unref (G_OBJECT (driver)); g_object_unref (G_OBJECT (proxy)); diff --git a/test/test-service.c b/test/test-service.c index c72a43af..6f77c3da 100644 --- a/test/test-service.c +++ b/test/test-service.c @@ -28,6 +28,8 @@ handle_echo (DBusConnection *connection, DBusError error; DBusMessage *reply; char *s; + + _dbus_verbose ("sending reply to Echo method\n"); dbus_error_init (&error); @@ -72,21 +74,77 @@ handle_echo (DBusConnection *connection, return DBUS_HANDLER_RESULT_HANDLED; } +static void +path_unregistered_func (DBusConnection *connection, + void *user_data) +{ + /* connection was finalized */ +} + static DBusHandlerResult -filter_func (DBusConnection *connection, - DBusMessage *message, - void *user_data) -{ +path_message_func (DBusConnection *connection, + DBusMessage *message, + void *user_data) +{ if (dbus_message_is_method_call (message, "org.freedesktop.TestSuite", "Echo")) return handle_echo (connection, message); else if (dbus_message_is_method_call (message, "org.freedesktop.TestSuite", - "Exit") || - dbus_message_is_signal (message, - DBUS_INTERFACE_ORG_FREEDESKTOP_LOCAL, - "Disconnected")) + "Exit")) + { + dbus_connection_disconnect (connection); + quit (); + return DBUS_HANDLER_RESULT_HANDLED; + } + else if (dbus_message_is_method_call (message, + "org.freedesktop.TestSuite", + "EmitFoo")) + { + /* Emit the Foo signal */ + DBusMessage *signal; + + _dbus_verbose ("emitting signal Foo\n"); + + signal = dbus_message_new_signal ("/org/freedesktop/TestSuite", + "org.freedesktop.TestSuite", + "Foo"); + if (signal == NULL) + die ("No memory\n"); + + if (!dbus_message_append_args (signal, + DBUS_TYPE_DOUBLE, 42.6, + DBUS_TYPE_INVALID)) + die ("No memory"); + + if (!dbus_connection_send (connection, signal, NULL)) + die ("No memory\n"); + + return DBUS_HANDLER_RESULT_HANDLED; + } + else + return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; +} + +static DBusObjectPathVTable +echo_vtable = { + path_unregistered_func, + path_message_func, + NULL, +}; + +/* Pre-exploded path, "/org/freedesktop/TestSuite" */ +static const char* echo_path[] = { "org", "freedesktop", "TestSuite", NULL }; + +static DBusHandlerResult +filter_func (DBusConnection *connection, + DBusMessage *message, + void *user_data) +{ + if (dbus_message_is_signal (message, + DBUS_INTERFACE_ORG_FREEDESKTOP_LOCAL, + "Disconnected")) { dbus_connection_disconnect (connection); quit (); @@ -107,11 +165,11 @@ main (int argc, int result; dbus_error_init (&error); - connection = dbus_bus_get (DBUS_BUS_SESSION, &error); + connection = dbus_bus_get (DBUS_BUS_ACTIVATION, &error); if (connection == NULL) { - _dbus_verbose ("*** Failed to open connection to activating message bus: %s\n", - error.message); + fprintf (stderr, "*** Failed to open connection to activating message bus: %s\n", + error.message); dbus_error_free (&error); return 1; } @@ -127,13 +185,19 @@ main (int argc, filter_func, NULL, NULL)) die ("No memory"); + if (!dbus_connection_register_object_path (connection, + echo_path, + &echo_vtable, + NULL)) + die ("No memory"); + printf ("Acquiring service\n"); result = dbus_bus_acquire_service (connection, "org.freedesktop.DBus.TestSuiteEchoService", 0, &error); if (dbus_error_is_set (&error)) { - printf ("Error %s", error.message); + fprintf (stderr, "Error %s", error.message); _dbus_verbose ("*** Failed to acquire service: %s\n", error.message); dbus_error_free (&error); -- cgit