diff --git a/gio/gdbusaddress.c b/gio/gdbusaddress.c index 1f7d19396..e88695139 100644 --- a/gio/gdbusaddress.c +++ b/gio/gdbusaddress.c @@ -127,12 +127,14 @@ is_valid_unix (const gchar *address_entry, GList *keys; GList *l; const gchar *path; + const gchar *dir; const gchar *tmpdir; const gchar *abstract; ret = FALSE; keys = NULL; path = NULL; + dir = NULL; tmpdir = NULL; abstract = NULL; @@ -142,6 +144,8 @@ is_valid_unix (const gchar *address_entry, const gchar *key = l->data; if (g_strcmp0 (key, "path") == 0) path = g_hash_table_lookup (key_value_pairs, key); + else if (g_strcmp0 (key, "dir") == 0) + dir = g_hash_table_lookup (key_value_pairs, key); else if (g_strcmp0 (key, "tmpdir") == 0) tmpdir = g_hash_table_lookup (key_value_pairs, key); else if (g_strcmp0 (key, "abstract") == 0) @@ -158,9 +162,8 @@ is_valid_unix (const gchar *address_entry, } } - if ((path != NULL && tmpdir != NULL) || - (tmpdir != NULL && abstract != NULL) || - (abstract != NULL && path != NULL)) + /* Exactly one key must be set */ + if ((path != NULL) + (dir != NULL) + (tmpdir != NULL) + (abstract != NULL) > 1) { g_set_error (error, G_IO_ERROR, @@ -169,12 +172,12 @@ is_valid_unix (const gchar *address_entry, address_entry); goto out; } - else if (path == NULL && tmpdir == NULL && abstract == NULL) + else if (path == NULL && dir == NULL && tmpdir == NULL && abstract == NULL) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT, - _("Address “%s” is invalid (need exactly one of path, tmpdir or abstract keys)"), + _("Address “%s” is invalid (need exactly one of path, dir, tmpdir, or abstract keys)"), address_entry); goto out; } diff --git a/gio/gdbusserver.c b/gio/gdbusserver.c index b3459ce47..26f75d4b3 100644 --- a/gio/gdbusserver.c +++ b/gio/gdbusserver.c @@ -101,6 +101,7 @@ struct _GDBusServer gchar *client_address; + gchar *unix_socket_path; GSocketListener *listener; gboolean is_using_listener; gulong run_signal_handler_id; @@ -161,6 +162,17 @@ static void initable_iface_init (GInitableIface *initable_iface); G_DEFINE_TYPE_WITH_CODE (GDBusServer, g_dbus_server, G_TYPE_OBJECT, G_IMPLEMENT_INTERFACE (G_TYPE_INITABLE, initable_iface_init)) +static void +g_dbus_server_dispose (GObject *object) +{ + GDBusServer *server = G_DBUS_SERVER (object); + + if (server->active) + g_dbus_server_stop (server); + + G_OBJECT_CLASS (g_dbus_server_parent_class)->dispose (object); +} + static void g_dbus_server_finalize (GObject *object) { @@ -183,10 +195,20 @@ g_dbus_server_finalize (GObject *object) memset (server->nonce, '\0', 16); g_free (server->nonce); } - /* we could unlink the nonce file but I don't - * think it's really worth the effort/risk - */ - g_free (server->nonce_file); + + if (server->unix_socket_path) + { + if (g_unlink (server->unix_socket_path) != 0) + g_warning ("Failed to delete %s: %s", server->unix_socket_path, g_strerror (errno)); + g_free (server->unix_socket_path); + } + + if (server->nonce_file) + { + if (g_unlink (server->nonce_file) != 0) + g_warning ("Failed to delete %s: %s", server->nonce_file, g_strerror (errno)); + g_free (server->nonce_file); + } g_main_context_unref (server->main_context_at_construction); @@ -270,6 +292,7 @@ g_dbus_server_class_init (GDBusServerClass *klass) { GObjectClass *gobject_class = G_OBJECT_CLASS (klass); + gobject_class->dispose = g_dbus_server_dispose; gobject_class->finalize = g_dbus_server_finalize; gobject_class->set_property = g_dbus_server_set_property; gobject_class->get_property = g_dbus_server_get_property; @@ -641,7 +664,7 @@ random_ascii (void) return ret; } -/* note that address_entry has already been validated => exactly one of path, tmpdir or abstract keys are set */ +/* note that address_entry has already been validated => exactly one of path, dir, tmpdir, or abstract keys are set */ static gboolean try_unix (GDBusServer *server, const gchar *address_entry, @@ -650,6 +673,7 @@ try_unix (GDBusServer *server, { gboolean ret; const gchar *path; + const gchar *dir; const gchar *tmpdir; const gchar *abstract; GSocketAddress *address; @@ -658,6 +682,7 @@ try_unix (GDBusServer *server, address = NULL; path = g_hash_table_lookup (key_value_pairs, "path"); + dir = g_hash_table_lookup (key_value_pairs, "dir"); tmpdir = g_hash_table_lookup (key_value_pairs, "tmpdir"); abstract = g_hash_table_lookup (key_value_pairs, "abstract"); @@ -665,20 +690,21 @@ try_unix (GDBusServer *server, { address = g_unix_socket_address_new (path); } - else if (tmpdir != NULL) + else if (dir != NULL || tmpdir != NULL) { gint n; GString *s; GError *local_error; retry: - s = g_string_new (tmpdir); + s = g_string_new (tmpdir != NULL ? tmpdir : dir); g_string_append (s, "/dbus-"); for (n = 0; n < 8; n++) g_string_append_c (s, random_ascii ()); - /* prefer abstract namespace if available */ - if (g_unix_socket_address_abstract_names_supported ()) + /* prefer abstract namespace if available for tmpdir: addresses + * abstract namespace is disallowed for dir: addresses */ + if (tmpdir != NULL && g_unix_socket_address_abstract_names_supported ()) address = g_unix_socket_address_new_with_type (s->str, -1, G_UNIX_SOCKET_ADDRESS_ABSTRACT); @@ -713,7 +739,7 @@ try_unix (GDBusServer *server, g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, - _("Abstract name space not supported")); + _("Abstract namespace not supported")); goto out; } address = g_unix_socket_address_new_with_type (abstract, @@ -743,24 +769,30 @@ try_unix (GDBusServer *server, /* Fill out client_address if the connection attempt worked */ if (ret) { + const char *address_path; + char *escaped_path; + server->is_using_listener = TRUE; + address_path = g_unix_socket_address_get_path (G_UNIX_SOCKET_ADDRESS (address)); + escaped_path = g_dbus_address_escape_value (address_path); switch (g_unix_socket_address_get_address_type (G_UNIX_SOCKET_ADDRESS (address))) { case G_UNIX_SOCKET_ADDRESS_ABSTRACT: - server->client_address = g_strdup_printf ("unix:abstract=%s", - g_unix_socket_address_get_path (G_UNIX_SOCKET_ADDRESS (address))); + server->client_address = g_strdup_printf ("unix:abstract=%s", escaped_path); break; case G_UNIX_SOCKET_ADDRESS_PATH: - server->client_address = g_strdup_printf ("unix:path=%s", - g_unix_socket_address_get_path (G_UNIX_SOCKET_ADDRESS (address))); + server->client_address = g_strdup_printf ("unix:path=%s", escaped_path); + server->unix_socket_path = g_strdup (address_path); break; default: g_assert_not_reached (); break; } + + g_free (escaped_path); } g_object_unref (address); } @@ -852,6 +884,7 @@ try_tcp (GDBusServer *server, gsize bytes_written; gsize bytes_remaining; char *file_escaped; + char *host_escaped; server->nonce = g_new0 (guchar, 16); for (n = 0; n < 16; n++) @@ -891,11 +924,13 @@ try_tcp (GDBusServer *server, } if (!g_close (fd, error)) goto out; - file_escaped = g_uri_escape_string (server->nonce_file, "/\\", FALSE); + host_escaped = g_dbus_address_escape_value (host); + file_escaped = g_dbus_address_escape_value (server->nonce_file); server->client_address = g_strdup_printf ("nonce-tcp:host=%s,port=%d,noncefile=%s", - host, + host_escaped, port_num, file_escaped); + g_free (host_escaped); g_free (file_escaped); } else diff --git a/gio/tests/gdbus-addresses.c b/gio/tests/gdbus-addresses.c index d6a5c2360..b7c81fc06 100644 --- a/gio/tests/gdbus-addresses.c +++ b/gio/tests/gdbus-addresses.c @@ -113,9 +113,13 @@ test_unix_address (void) g_assert_false (g_dbus_is_address ("unix:path=/foo;abstract=/bar")); assert_is_supported_address ("unix:path=/tmp/concrete;unix:abstract=/tmp/abstract"); assert_is_supported_address ("unix:tmpdir=/tmp"); + assert_is_supported_address ("unix:dir=/tmp"); assert_not_supported_address ("unix:tmpdir=/tmp,path=/tmp"); assert_not_supported_address ("unix:tmpdir=/tmp,abstract=/tmp/foo"); + assert_not_supported_address ("unix:tmpdir=/tmp,dir=/tmp"); assert_not_supported_address ("unix:path=/tmp,abstract=/tmp/foo"); + assert_not_supported_address ("unix:path=/tmp,dir=/tmp"); + assert_not_supported_address ("unix:abstract=/tmp/foo,dir=/tmp"); assert_not_supported_address ("unix:"); #endif } diff --git a/gio/tests/gdbus-peer.c b/gio/tests/gdbus-peer.c index c21b9e9f2..c46340386 100644 --- a/gio/tests/gdbus-peer.c +++ b/gio/tests/gdbus-peer.c @@ -53,6 +53,7 @@ static gboolean is_unix = TRUE; static gboolean is_unix = FALSE; #endif +static gchar *tmpdir = NULL; static gchar *tmp_address = NULL; static gchar *test_guid = NULL; static GMutex service_loop_lock; @@ -267,6 +268,58 @@ on_proxy_signal_received_with_name_set (GDBusProxy *proxy, /* ---------------------------------------------------------------------------------------------------- */ +static void +setup_test_address (void) +{ + if (is_unix) + { + g_test_message ("Testing with unix:tmpdir address"); + if (g_unix_socket_address_abstract_names_supported ()) + tmp_address = g_strdup ("unix:tmpdir=/tmp/gdbus-test-"); + else + { + tmpdir = g_dir_make_tmp ("gdbus-test-XXXXXX", NULL); + tmp_address = g_strdup_printf ("unix:tmpdir=%s", tmpdir); + } + } + else + tmp_address = g_strdup ("nonce-tcp:"); +} + +#ifdef G_OS_UNIX +static void +setup_dir_test_address (void) +{ + g_test_message ("Testing with unix:dir address"); + tmpdir = g_dir_make_tmp ("gdbus-test-XXXXXX", NULL); + tmp_address = g_strdup_printf ("unix:dir=%s", tmpdir); +} + +static void +setup_path_test_address (void) +{ + g_test_message ("Testing with unix:path address"); + tmpdir = g_dir_make_tmp ("gdbus-test-XXXXXX", NULL); + tmp_address = g_strdup_printf ("unix:path=%s/gdbus-peer-socket", tmpdir); +} +#endif + +static void +teardown_test_address (void) +{ + g_free (tmp_address); + if (tmpdir) + { + /* Ensuring the rmdir succeeds also ensures any sockets created on the + * filesystem are also deleted. + */ + g_assert_cmpint (g_rmdir (tmpdir), ==, 0); + g_clear_pointer (&tmpdir, g_free); + } +} + +/* ---------------------------------------------------------------------------------------------------- */ + static gboolean on_authorize_authenticated_peer (GDBusAuthObserver *observer, GIOStream *stream, @@ -656,7 +709,7 @@ read_all_from_fd (gint fd, gsize *out_len, GError **error) #endif static void -test_peer (void) +do_test_peer (void) { GDBusConnection *c; GDBusConnection *c2; @@ -977,6 +1030,27 @@ test_peer (void) g_thread_join (service_thread); } +static void +test_peer (void) +{ + /* Run this test multiple times using different address formats to ensure + * they all work. + */ + setup_test_address (); + do_test_peer (); + teardown_test_address (); + +#ifdef G_OS_UNIX + setup_dir_test_address (); + do_test_peer (); + teardown_test_address (); + + setup_path_test_address (); + do_test_peer (); + teardown_test_address (); +#endif +} + /* ---------------------------------------------------------------------------------------------------- */ typedef struct @@ -1116,6 +1190,8 @@ delayed_message_processing (void) GThread *service_thread; guint n; + setup_test_address (); + data = g_new0 (DmpData, 1); service_thread = g_thread_new ("dmp", @@ -1160,6 +1236,7 @@ delayed_message_processing (void) g_main_loop_quit (data->loop); g_thread_join (service_thread); dmp_data_free (data); + teardown_test_address (); } /* ---------------------------------------------------------------------------------------------------- */ @@ -1351,12 +1428,16 @@ test_nonce_tcp (void) g_error_free (error); g_assert (c == NULL); - g_free (nonce_file); + /* Recreate the nonce-file so we can ensure the server deletes it when stopped. */ + g_assert_cmpint (g_creat (nonce_file, 0600), !=, -1); g_dbus_server_stop (server); g_object_unref (server); server = NULL; + g_assert_false (g_file_test (nonce_file, G_FILE_TEST_EXISTS)); + g_free (nonce_file); + g_main_loop_quit (service_loop); g_thread_join (service_thread); @@ -1616,6 +1697,8 @@ codegen_test_peer (void) GVariant *value; const gchar *s; + setup_test_address (); + /* bring up a server - we run the server in a different thread to avoid deadlocks */ service_thread = g_thread_new ("codegen_test_peer", codegen_service_thread_func, @@ -1719,6 +1802,8 @@ codegen_test_peer (void) g_object_unref (animal1); g_object_unref (animal2); g_thread_join (service_thread); + + teardown_test_address (); } /* ---------------------------------------------------------------------------------------------------- */ @@ -1730,7 +1815,6 @@ main (int argc, { gint ret; GDBusNodeInfo *introspection_data = NULL; - gchar *tmpdir = NULL; g_test_init (&argc, &argv, NULL); @@ -1740,19 +1824,6 @@ main (int argc, test_guid = g_dbus_generate_guid (); - if (is_unix) - { - if (g_unix_socket_address_abstract_names_supported ()) - tmp_address = g_strdup ("unix:tmpdir=/tmp/gdbus-test-"); - else - { - tmpdir = g_dir_make_tmp ("gdbus-test-XXXXXX", NULL); - tmp_address = g_strdup_printf ("unix:tmpdir=%s", tmpdir); - } - } - else - tmp_address = g_strdup ("nonce-tcp:"); - /* all the tests rely on a shared main loop */ loop = g_main_loop_new (NULL, FALSE); @@ -1769,13 +1840,6 @@ main (int argc, g_main_loop_unref (loop); g_free (test_guid); g_dbus_node_info_unref (introspection_data); - if (is_unix) - g_free (tmp_address); - if (tmpdir) - { - g_rmdir (tmpdir); - g_free (tmpdir); - } return ret; }