From aa4f823781185fb18187714798795d7e4b0c9b65 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Fri, 11 Feb 2005 03:37:03 +0000 Subject: 2005-02-10 Havoc Pennington * test/glib/test-dbus-glib.c (main): fix so this test doesn't fail (call dbus_g_proxy_add_signal) * dbus/dbus-server-unix.c (_dbus_server_new_for_tcp_socket): escape the hostname (_dbus_server_new_for_domain_socket): escape the path * dbus/dbus-address.c (dbus_address_escape_value): new (dbus_address_unescape_value): new (dbus_parse_address): unescape values * dbus/dbus-string.c (_dbus_string_append_byte_as_hex): new function * doc/dbus-specification.xml: explain how to escape values in addresses --- ChangeLog | 18 +++ dbus/dbus-address.c | 324 +++++++++++++++++++++++++++++++++++++++++- dbus/dbus-address.h | 5 +- dbus/dbus-internals.h | 3 + dbus/dbus-server-debug-pipe.c | 4 +- dbus/dbus-server-unix.c | 10 +- dbus/dbus-string.c | 43 ++++-- dbus/dbus-string.h | 2 + doc/dbus-specification.xml | 29 +++- test/glib/test-dbus-glib.c | 30 +--- 10 files changed, 417 insertions(+), 51 deletions(-) diff --git a/ChangeLog b/ChangeLog index a60a7ff3..085e542d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,21 @@ +2005-02-10 Havoc Pennington + + * test/glib/test-dbus-glib.c (main): fix so this test doesn't fail + (call dbus_g_proxy_add_signal) + + * dbus/dbus-server-unix.c (_dbus_server_new_for_tcp_socket): + escape the hostname + (_dbus_server_new_for_domain_socket): escape the path + + * dbus/dbus-address.c (dbus_address_escape_value): new + (dbus_address_unescape_value): new + (dbus_parse_address): unescape values + + * dbus/dbus-string.c (_dbus_string_append_byte_as_hex): new function + + * doc/dbus-specification.xml: explain how to escape values in + addresses + 2005-02-10 Havoc Pennington * dbus/dbus-message-factory.c (generate_special): modify test to diff --git a/dbus/dbus-address.c b/dbus/dbus-address.c index 44675197..d5c38a84 100644 --- a/dbus/dbus-address.c +++ b/dbus/dbus-address.c @@ -2,7 +2,7 @@ /* dbus-address.c Server address parser. * * Copyright (C) 2003 CodeFactory AB - * Copyright (C) 2004 Red Hat, Inc. + * Copyright (C) 2004,2005 Red Hat, Inc. * * Licensed under the Academic Free License version 2.1 * @@ -140,7 +140,7 @@ dbus_address_entry_get_method (DBusAddressEntry *entry) * * @param entry the entry. * @param key the key. - * @returns the key value. This string must not be fred. + * @returns the key value. This string must not be freed. */ const char * dbus_address_entry_get_value (DBusAddressEntry *entry, @@ -165,15 +165,150 @@ dbus_address_entry_get_value (DBusAddressEntry *entry, return NULL; } +#define _DBUS_ADDRESS_OPTIONALLY_ESCAPED_BYTE(b) \ + (((b) >= 'a' && (b) <= 'z') || \ + ((b) >= 'A' && (b) <= 'Z') || \ + ((b) >= '0' && (b) <= '9') || \ + (b) == '-' || \ + (b) == '_' || \ + (b) == '/' || \ + (b) == '\\' || \ + (b) == '.') + +/** + * Appends an escaped version of one string to another string, + * using the D-BUS address escaping mechanism + * + * @param escaped the string to append to + * @param unescaped the string to escape + * @returns #FALSE if no memory + */ +dbus_bool_t +_dbus_address_append_escaped (DBusString *escaped, + const DBusString *unescaped) +{ + const char *p; + const char *end; + dbus_bool_t ret; + int orig_len; + + ret = FALSE; + + orig_len = _dbus_string_get_length (escaped); + p = _dbus_string_get_const_data (unescaped); + end = p + _dbus_string_get_length (unescaped); + while (p != end) + { + if (_DBUS_ADDRESS_OPTIONALLY_ESCAPED_BYTE (*p)) + { + if (!_dbus_string_append_byte (escaped, *p)) + goto out; + } + else + { + if (!_dbus_string_append_byte (escaped, '%')) + goto out; + if (!_dbus_string_append_byte_as_hex (escaped, *p)) + goto out; + } + + ++p; + } + + ret = TRUE; + + out: + if (!ret) + _dbus_string_set_length (escaped, orig_len); + return ret; +} + +static dbus_bool_t +append_unescaped_value (DBusString *unescaped, + const DBusString *escaped, + int escaped_start, + int escaped_len, + DBusError *error) +{ + const char *p; + const char *end; + dbus_bool_t ret; + + ret = FALSE; + + p = _dbus_string_get_const_data (escaped) + escaped_start; + end = p + escaped_len; + while (p != end) + { + if (_DBUS_ADDRESS_OPTIONALLY_ESCAPED_BYTE (*p)) + { + if (!_dbus_string_append_byte (unescaped, *p)) + goto out; + } + else if (*p == '%') + { + /* Efficiency is king */ + char buf[3]; + DBusString hex; + int hex_end; + + ++p; + + if ((p + 2) > end) + { + dbus_set_error (error, DBUS_ERROR_BAD_ADDRESS, + "In D-BUS address, percent character was not followed by two hex digits"); + goto out; + } + + buf[0] = *p; + ++p; + buf[1] = *p; + buf[2] = '\0'; + + _dbus_string_init_const (&hex, buf); + + if (!_dbus_string_hex_decode (&hex, 0, &hex_end, + unescaped, + _dbus_string_get_length (unescaped))) + goto out; + + if (hex_end != 2) + { + dbus_set_error (error, DBUS_ERROR_BAD_ADDRESS, + "In D-BUS address, percent character was followed by characters other than hex digits"); + goto out; + } + } + else + { + /* Error, should have been escaped */ + dbus_set_error (error, DBUS_ERROR_BAD_ADDRESS, + "In D-BUS address, character '%c' should have been escaped\n", + *p); + goto out; + } + + ++p; + } + + ret = TRUE; + + out: + if (!ret && error && !dbus_error_is_set (error)) + _DBUS_SET_OOM (error); + + _dbus_assert (ret || error == NULL || dbus_error_is_set (error)); + + return ret; +} + /** * Parses an address string of the form: * * method:key=value,key=value;method:key=value * * @todo document address format in the specification - * - * @todo need to be able to escape ';' and ',' in the - * key values, and the parsing needs to handle that. * * @param address the address. * @param entry return location to an array of entries. @@ -306,9 +441,10 @@ dbus_parse_address (const char *address, goto error; } - if (!_dbus_string_copy_len (&str, equals_pos + 1, comma_pos - equals_pos - 1, value, 0)) + if (!append_unescaped_value (value, &str, equals_pos + 1, + comma_pos - equals_pos - 1, error)) { - dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); + _dbus_assert (error == NULL || dbus_error_is_set (error)); _dbus_string_free (key); _dbus_string_free (value); @@ -386,11 +522,124 @@ dbus_parse_address (const char *address, } +/** + * Escapes the given string as a value in a key=value pair + * for a D-BUS address. + * + * @param value the unescaped value + * @returns newly-allocated escaped value or #NULL if no memory + */ +char* +dbus_address_escape_value (const char *value) +{ + DBusString escaped; + DBusString unescaped; + char *ret; + + ret = NULL; + + _dbus_string_init_const (&unescaped, value); + + if (!_dbus_string_init (&escaped)) + return NULL; + + if (!_dbus_address_append_escaped (&escaped, &unescaped)) + goto out; + + if (!_dbus_string_steal_data (&escaped, &ret)) + goto out; + + out: + _dbus_string_free (&escaped); + return ret; +} + +/** + * Unescapes the given string as a value in a key=value pair + * for a D-BUS address. + * + * @param value the escaped value + * @param error error to set if the unescaping fails + * @returns newly-allocated unescaped value or #NULL if no memory + */ +char* +dbus_address_unescape_value (const char *value, + DBusError *error) +{ + DBusString unescaped; + DBusString escaped; + char *ret; + + ret = NULL; + + _dbus_string_init_const (&escaped, value); + + if (!_dbus_string_init (&unescaped)) + return NULL; + + if (!append_unescaped_value (&unescaped, &escaped, + 0, _dbus_string_get_length (&escaped), + error)) + goto out; + + if (!_dbus_string_steal_data (&unescaped, &ret)) + goto out; + + out: + if (ret == NULL && error && !dbus_error_is_set (error)) + _DBUS_SET_OOM (error); + + _dbus_assert (ret != NULL || error == NULL || dbus_error_is_set (error)); + + _dbus_string_free (&unescaped); + return ret; +} /** @} */ /* End of public API */ #ifdef DBUS_BUILD_TESTS #include "dbus-test.h" +#include + +typedef struct +{ + const char *escaped; + const char *unescaped; +} EscapeTest; + +static const EscapeTest escape_tests[] = { + { "abcde", "abcde" }, + { "", "" }, + { "%20%20", " " }, + { "%24", "$" }, + { "%25", "%" }, + { "abc%24", "abc$" }, + { "%24abc", "$abc" }, + { "abc%24abc", "abc$abc" }, + { "/", "/" }, + { "-", "-" }, + { "_", "_" }, + { "A", "A" }, + { "I", "I" }, + { "Z", "Z" }, + { "a", "a" }, + { "i", "i" }, + { "z", "z" } +}; + +static const char* invalid_escaped_values[] = { + "%a", + "%q", + "%az", + "%%", + "%$$", + "abc%a", + "%axyz", + "%", + "$", + " ", + "*" +}; dbus_bool_t _dbus_address_test (void) @@ -398,8 +647,69 @@ _dbus_address_test (void) DBusAddressEntry **entries; int len; DBusError error; + int i; dbus_error_init (&error); + + i = 0; + while (i < _DBUS_N_ELEMENTS (escape_tests)) + { + const EscapeTest *test = &escape_tests[i]; + char *escaped; + char *unescaped; + + escaped = dbus_address_escape_value (test->unescaped); + if (escaped == NULL) + _dbus_assert_not_reached ("oom"); + + if (strcmp (escaped, test->escaped) != 0) + { + _dbus_warn ("Escaped '%s' as '%s' should have been '%s'\n", + test->unescaped, escaped, test->escaped); + exit (1); + } + dbus_free (escaped); + + unescaped = dbus_address_unescape_value (test->escaped, &error); + if (unescaped == NULL) + { + _dbus_warn ("Failed to unescape '%s': %s\n", + test->escaped, error.message); + dbus_error_free (&error); + exit (1); + } + + if (strcmp (unescaped, test->unescaped) != 0) + { + _dbus_warn ("Unescaped '%s' as '%s' should have been '%s'\n", + test->escaped, unescaped, test->unescaped); + exit (1); + } + dbus_free (unescaped); + + ++i; + } + + i = 0; + while (i < _DBUS_N_ELEMENTS (invalid_escaped_values)) + { + char *unescaped; + + unescaped = dbus_address_unescape_value (invalid_escaped_values[i], + &error); + if (unescaped != NULL) + { + _dbus_warn ("Should not have successfully unescaped '%s' to '%s'\n", + invalid_escaped_values[i], unescaped); + dbus_free (unescaped); + exit (1); + } + + _dbus_assert (dbus_error_is_set (&error)); + dbus_error_free (&error); + + ++i; + } if (!dbus_parse_address ("unix:path=/tmp/foo;debug:name=test,sliff=sloff;", &entries, &len, &error)) diff --git a/dbus/dbus-address.h b/dbus/dbus-address.h index a98682e6..b82495d0 100644 --- a/dbus/dbus-address.h +++ b/dbus/dbus-address.h @@ -41,8 +41,9 @@ const char *dbus_address_entry_get_value (DBusAddressEntry *entry, const char *dbus_address_entry_get_method (DBusAddressEntry *entry); void dbus_address_entries_free (DBusAddressEntry **entries); - - +char* dbus_address_escape_value (const char *value); +char* dbus_address_unescape_value (const char *value, + DBusError *error); #endif /* DBUS_ADDRESS_H */ diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index 3f0a34fe..be19e1bc 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -284,6 +284,9 @@ _DBUS_DECLARE_GLOBAL_LOCK (message_cache); dbus_bool_t _dbus_threads_init_debug (void); +dbus_bool_t _dbus_address_append_escaped (DBusString *escaped, + const DBusString *unescaped); + DBUS_END_DECLS #endif /* DBUS_INTERNALS_H */ diff --git a/dbus/dbus-server-debug-pipe.c b/dbus/dbus-server-debug-pipe.c index 0f8f1eda..090274aa 100644 --- a/dbus/dbus-server-debug-pipe.c +++ b/dbus/dbus-server-debug-pipe.c @@ -134,6 +134,7 @@ _dbus_server_debug_pipe_new (const char *server_name, { DBusServerDebugPipe *debug_server; DBusString address; + DBusString name_str; _DBUS_ASSERT_ERROR_IS_CLEAR (error); @@ -154,8 +155,9 @@ _dbus_server_debug_pipe_new (const char *server_name, if (!_dbus_string_init (&address)) goto nomem_1; + _dbus_string_init_const (&name_str, server_name); if (!_dbus_string_append (&address, "debug-pipe:name=") || - !_dbus_string_append (&address, server_name)) + !_dbus_address_append_escaped (&address, &name_str)) goto nomem_2; debug_server->name = _dbus_strdup (server_name); diff --git a/dbus/dbus-server-unix.c b/dbus/dbus-server-unix.c index 12f4ca6a..f576b427 100644 --- a/dbus/dbus-server-unix.c +++ b/dbus/dbus-server-unix.c @@ -279,6 +279,7 @@ _dbus_server_new_for_domain_socket (const char *path, int listen_fd; DBusString address; char *path_copy; + DBusString path_str; _DBUS_ASSERT_ERROR_IS_CLEAR (error); @@ -288,11 +289,12 @@ _dbus_server_new_for_domain_socket (const char *path, return NULL; } + _dbus_string_init_const (&path_str, path); if ((abstract && !_dbus_string_append (&address, "unix:abstract=")) || (!abstract && !_dbus_string_append (&address, "unix:path=")) || - !_dbus_string_append (&address, path)) + !_dbus_address_append_escaped (&address, &path_str)) { dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); goto failed_0; @@ -355,6 +357,7 @@ _dbus_server_new_for_tcp_socket (const char *host, DBusServer *server; int listen_fd; DBusString address; + DBusString host_str; _DBUS_ASSERT_ERROR_IS_CLEAR (error); @@ -366,9 +369,10 @@ _dbus_server_new_for_tcp_socket (const char *host, if (host == NULL) host = "localhost"; - + + _dbus_string_init_const (&host_str, host); if (!_dbus_string_append (&address, "tcp:host=") || - !_dbus_string_append (&address, host) || + !_dbus_address_append_escaped (&address, &host_str) || !_dbus_string_append (&address, ",port=") || !_dbus_string_append_int (&address, port)) { diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c index f64d4746..5088bca9 100644 --- a/dbus/dbus-string.c +++ b/dbus/dbus-string.c @@ -2236,6 +2236,38 @@ _dbus_string_starts_with_c_str (const DBusString *a, } #endif /* DBUS_BUILD_TESTS */ +/** + * Appends a two-character hex digit to a string, where the hex digit + * has the value of the given byte. + * + * @param str the string + * @param byte the byte + * @returns #FALSE if no memory + */ +dbus_bool_t +_dbus_string_append_byte_as_hex (DBusString *str, + int byte) +{ + const char hexdigits[16] = { + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', + 'a', 'b', 'c', 'd', 'e', 'f' + }; + + if (!_dbus_string_append_byte (str, + hexdigits[(byte >> 4)])) + return FALSE; + + if (!_dbus_string_append_byte (str, + hexdigits[(byte & 0x0f)])) + { + _dbus_string_set_length (str, + _dbus_string_get_length (str) - 1); + return FALSE; + } + + return TRUE; +} + /** * Encodes a string in hex, the way MD5 and SHA-1 are usually * encoded. (Each byte is two hex digits.) @@ -2253,10 +2285,6 @@ _dbus_string_hex_encode (const DBusString *source, int insert_at) { DBusString result; - const char hexdigits[16] = { - '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', - 'a', 'b', 'c', 'd', 'e', 'f' - }; const unsigned char *p; const unsigned char *end; dbus_bool_t retval; @@ -2274,14 +2302,9 @@ _dbus_string_hex_encode (const DBusString *source, while (p != end) { - if (!_dbus_string_append_byte (&result, - hexdigits[(*p >> 4)])) + if (!_dbus_string_append_byte_as_hex (&result, *p)) goto out; - if (!_dbus_string_append_byte (&result, - hexdigits[(*p & 0x0f)])) - goto out; - ++p; } diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h index 84098226..522a99b7 100644 --- a/dbus/dbus-string.h +++ b/dbus/dbus-string.h @@ -253,6 +253,8 @@ dbus_bool_t _dbus_string_pop_line (DBusString *source, DBusString *dest); void _dbus_string_delete_first_word (DBusString *str); void _dbus_string_delete_leading_blanks (DBusString *str); +dbus_bool_t _dbus_string_append_byte_as_hex (DBusString *str, + int byte); dbus_bool_t _dbus_string_hex_encode (const DBusString *source, int start, DBusString *dest, diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index 75ffeea7..b56e0fc5 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -2131,13 +2131,40 @@ Server addresses consist of a transport name followed by a colon, and then an optional, comma-separated list of keys and values in the form key=value. - [FIXME how do you escape colon, comma, and semicolon in the values of the key=value pairs?] + Each value is escaped. For example: unix:path=/tmp/dbus-test Which is the address to a unix socket with the path /tmp/dbus-test. + + Value escaping is similar to URI escaping but simpler. + + + + The set of optionally-escaped bytes is: + [0-9A-Za-z_-/.\]. To escape, each + byte (note, not character) which is not in the + set of optionally-escaped bytes must be replaced with an ASCII + percent (%) and the value of the byte in hex. + The hex value must always be two digits, even if the first digit is + zero. The optionally-escaped bytes may be escaped if desired. + + + + + To unescape, append each byte in the value; if a byte is an ASCII + percent (%) character then append the following + hex value instead. It is an error if a % byte + does not have two hex digits following. It is an error if a + non-optionally-escaped byte is seen unescaped. + + + + The set of optionally-escaped bytes is intended to preserve address + readability and convenience. + [FIXME clarify if attempting to connect to each is a requirement or just a suggestion] diff --git a/test/glib/test-dbus-glib.c b/test/glib/test-dbus-glib.c index 5f565a0a..cee9316b 100644 --- a/test/glib/test-dbus-glib.c +++ b/test/glib/test-dbus-glib.c @@ -16,35 +16,9 @@ timed_exit (gpointer loop) static void foo_signal_handler (DBusGProxy *proxy, + double d, void *user_data) { -#if 0 - double d; - - /* FIXME - need to fix up dbus_gproxy_signal_connect() to be able to - * get signal args - */ - - 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); - } -#endif - n_times_foo_received += 1; g_main_loop_quit (loop); @@ -225,6 +199,8 @@ main (int argc, char **argv) /* Test oneway call and signal handling */ + dbus_g_proxy_add_signal (proxy, "Foo", DBUS_TYPE_DOUBLE_AS_STRING); + dbus_g_proxy_connect_signal (proxy, "Foo", G_CALLBACK (foo_signal_handler), NULL, NULL); -- cgit