From ba7b035f5bcdd2ccb6ac463ccf163b60d26af6c0 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Wed, 13 Jun 2018 10:50:35 +0100 Subject: [PATCH] g_dbus_is_supported_address(): set error if returning FALSE Previously, calling: g_dbus_is_supported_address ("some-imaginary-transport:", NULL) correctly returned FALSE; but calling: g_dbus_is_supported_address ("some-imaginary-transport:", &error) crashed with: GLib-GIO:ERROR:../gio/gdbusaddress.c:434:g_dbus_is_supported_address: assertion failed: (ret || (!ret && (error == NULL || *error != NULL))) This was because, if the address component did not start with a known transport, no error was set. Fix this, reusing an error string used by the corresponding else branch in g_dbus_address_connect(), and adjust the test to pass both NULL and non-NULL GError **s to this function in every test case. This case: g_assert (!g_dbus_is_supported_address ("some-imaginary-transport:foo=bar;unix:path=/this/is/valid", NULL)); would have caught this bug with a non-NULL GError **. --- gio/gdbusaddress.c | 4 ++ gio/tests/gdbus-addresses.c | 77 +++++++++++++++++++++++-------------- 2 files changed, 53 insertions(+), 28 deletions(-) diff --git a/gio/gdbusaddress.c b/gio/gdbusaddress.c index 2191c115a..f377378ec 100644 --- a/gio/gdbusaddress.c +++ b/gio/gdbusaddress.c @@ -418,6 +418,10 @@ g_dbus_is_supported_address (const gchar *string, supported = is_valid_nonce_tcp (a[n], key_value_pairs, error); else if (g_strcmp0 (a[n], "autolaunch:") == 0) supported = TRUE; + else + g_set_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT, + _("Unknown or unsupported transport “%s” for address “%s”"), + transport_name, a[n]); g_free (transport_name); g_hash_table_unref (key_value_pairs); diff --git a/gio/tests/gdbus-addresses.c b/gio/tests/gdbus-addresses.c index 2e662d000..0ab05661a 100644 --- a/gio/tests/gdbus-addresses.c +++ b/gio/tests/gdbus-addresses.c @@ -39,65 +39,86 @@ test_empty_address (void) g_error_free (error); } +static void +assert_is_supported_address (const gchar *address) +{ + GError *error = NULL; + + g_assert_true (g_dbus_is_supported_address (address, NULL)); + g_assert_true (g_dbus_is_supported_address (address, &error)); + g_assert_no_error (error); +} + +static void +assert_not_supported_address (const gchar *address) +{ + GError *error = NULL; + + g_assert_false (g_dbus_is_supported_address (address, NULL)); + g_assert_false (g_dbus_is_supported_address (address, &error)); + g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); + g_clear_error (&error); +} + #ifdef G_OS_UNIX static void test_unix_address (void) { - g_assert (!g_dbus_is_supported_address ("some-imaginary-transport:foo=bar", NULL)); - g_assert (g_dbus_is_supported_address ("unix:path=/tmp/dbus-test", NULL)); - g_assert (g_dbus_is_supported_address ("unix:abstract=/tmp/dbus-another-test", NULL)); + assert_not_supported_address ("some-imaginary-transport:foo=bar"); + assert_is_supported_address ("unix:path=/tmp/dbus-test"); + assert_is_supported_address ("unix:abstract=/tmp/dbus-another-test"); g_assert (g_dbus_is_address ("unix:foo=bar")); - g_assert (!g_dbus_is_supported_address ("unix:foo=bar", NULL)); + assert_not_supported_address ("unix:foo=bar"); g_assert (!g_dbus_is_address ("unix:path=/foo;abstract=/bar")); - g_assert (!g_dbus_is_supported_address ("unix:path=/foo;abstract=/bar", NULL)); - g_assert (g_dbus_is_supported_address ("unix:path=/tmp/concrete;unix:abstract=/tmp/abstract", NULL)); + assert_not_supported_address ("unix:path=/foo;abstract=/bar"); + assert_is_supported_address ("unix:path=/tmp/concrete;unix:abstract=/tmp/abstract"); g_assert (g_dbus_is_address ("some-imaginary-transport:foo=bar")); g_assert (g_dbus_is_address ("some-imaginary-transport:foo=bar;unix:path=/this/is/valid")); - g_assert (!g_dbus_is_supported_address ("some-imaginary-transport:foo=bar;unix:path=/this/is/valid", NULL)); + assert_not_supported_address ("some-imaginary-transport:foo=bar;unix:path=/this/is/valid"); } #endif static void test_nonce_tcp_address (void) { - g_assert (g_dbus_is_supported_address ("nonce-tcp:host=localhost,port=42,noncefile=/foo/bar", NULL)); - g_assert (g_dbus_is_supported_address ("nonce-tcp:host=localhost,port=42,noncefile=/foo/bar,family=ipv6", NULL)); - g_assert (g_dbus_is_supported_address ("nonce-tcp:host=localhost,port=42,noncefile=/foo/bar,family=ipv4", NULL)); + assert_is_supported_address ("nonce-tcp:host=localhost,port=42,noncefile=/foo/bar"); + assert_is_supported_address ("nonce-tcp:host=localhost,port=42,noncefile=/foo/bar,family=ipv6"); + assert_is_supported_address ("nonce-tcp:host=localhost,port=42,noncefile=/foo/bar,family=ipv4"); - g_assert (!g_dbus_is_supported_address ("nonce-tcp:host=localhost,port=42,noncefile=/foo/bar,family=blah", NULL)); - g_assert (!g_dbus_is_supported_address ("nonce-tcp:host=localhost,port=420000,noncefile=/foo/bar,family=ipv4", NULL)); - g_assert (!g_dbus_is_supported_address ("nonce-tcp:host=,port=x42,noncefile=/foo/bar,family=ipv4", NULL)); - g_assert (!g_dbus_is_supported_address ("nonce-tcp:host=,port=42x,noncefile=/foo/bar,family=ipv4", NULL)); - g_assert (!g_dbus_is_supported_address ("nonce-tcp:host=,port=420000,noncefile=/foo/bar,family=ipv4", NULL)); + assert_not_supported_address ("nonce-tcp:host=localhost,port=42,noncefile=/foo/bar,family=blah"); + assert_not_supported_address ("nonce-tcp:host=localhost,port=420000,noncefile=/foo/bar,family=ipv4"); + assert_not_supported_address ("nonce-tcp:host=,port=x42,noncefile=/foo/bar,family=ipv4"); + assert_not_supported_address ("nonce-tcp:host=,port=42x,noncefile=/foo/bar,family=ipv4"); + assert_not_supported_address ("nonce-tcp:host=,port=420000,noncefile=/foo/bar,family=ipv4"); } static void test_tcp_address (void) { - g_assert (g_dbus_is_supported_address ("tcp:host=localhost", NULL)); - g_assert (!g_dbus_is_supported_address ("tcp:host=localhost,noncefile=/tmp/foo", NULL)); - g_assert (g_dbus_is_supported_address ("tcp:host=localhost,port=42", NULL)); - g_assert (!g_dbus_is_supported_address ("tcp:host=localhost,port=-1", NULL)); - g_assert (!g_dbus_is_supported_address ("tcp:host=localhost,port=420000", NULL)); - g_assert (!g_dbus_is_supported_address ("tcp:host=localhost,port=42x", NULL)); - g_assert (g_dbus_is_supported_address ("tcp:host=localhost,port=42,family=ipv4", NULL)); - g_assert (g_dbus_is_supported_address ("tcp:host=localhost,port=42,family=ipv6", NULL)); - g_assert (!g_dbus_is_supported_address ("tcp:host=localhost,port=42,family=sopranos", NULL)); + assert_is_supported_address ("tcp:host=localhost"); + assert_not_supported_address ("tcp:host=localhost,noncefile=/tmp/foo"); + assert_is_supported_address ("tcp:host=localhost,port=42"); + assert_not_supported_address ("tcp:host=localhost,port=-1"); + assert_not_supported_address ("tcp:host=localhost,port=420000"); + assert_not_supported_address ("tcp:host=localhost,port=42x"); + assert_is_supported_address ("tcp:host=localhost,port=42,family=ipv4"); + assert_is_supported_address ("tcp:host=localhost,port=42,family=ipv6"); + assert_not_supported_address ("tcp:host=localhost,port=42,family=sopranos"); } static void test_autolaunch_address (void) { - g_assert (g_dbus_is_supported_address ("autolaunch:", NULL)); + assert_is_supported_address ("autolaunch:"); } static void test_mixed_address (void) { - g_assert (g_dbus_is_supported_address ("unix:path=/tmp/dbus1;unix:path=/tmp/dbus2", NULL)); - g_assert (g_dbus_is_supported_address ("tcp:host=localhost,port=42;autolaunch:", NULL)); - g_assert (!g_dbus_is_supported_address ("tcp:host=localhost,port=42;tcp:family=bla", NULL)); + assert_is_supported_address ("unix:path=/tmp/dbus1;unix:path=/tmp/dbus2"); + assert_is_supported_address ("tcp:host=localhost,port=42;autolaunch:"); + assert_not_supported_address ("tcp:host=localhost,port=42;tcp:family=bla"); } static const struct { const char *before; const char *after; } escaping[] = {