Merge branch 'dbus-address-tests' into 'master'

Improve gdbus-address parsing tests

See merge request GNOME/glib!730

gio/gdbusaddress.c

@@ -158,22 +158,18 @@ is_valid_unix (const gchar  *address_entry,
         }
     }
 
-  if (path != NULL)
+  if ((path != NULL && tmpdir != NULL) ||
+      (tmpdir != NULL && abstract != NULL) ||
+      (abstract != NULL && path != NULL))
     {
-      if (tmpdir != NULL || abstract != NULL)
+      g_set_error (error,
-        goto meaningless;
+             G_IO_ERROR,
+             G_IO_ERROR_INVALID_ARGUMENT,
+             _("Meaningless key/value pair combination in address entry “%s”"),
+             address_entry);
+      goto out;
     }
-  else if (tmpdir != NULL)
+  else if (path == NULL && tmpdir == NULL && abstract == NULL)
-    {
-      if (path != NULL || abstract != NULL)
-        goto meaningless;
-    }
-  else if (abstract != NULL)
-    {
-      if (path != NULL || tmpdir != NULL)
-        goto meaningless;
-    }
-  else
     {
       g_set_error (error,
                    G_IO_ERROR,
@@ -183,16 +179,7 @@ is_valid_unix (const gchar  *address_entry,
       goto out;
     }
 
-
+  ret = TRUE;
-  ret= TRUE;
-  goto out;
-
- meaningless:
-  g_set_error (error,
-               G_IO_ERROR,
-               G_IO_ERROR_INVALID_ARGUMENT,
-               _("Meaningless key/value pair combination in address entry “%s”"),
-               address_entry);
 
  out:
   g_list_free (keys);
@@ -464,12 +451,21 @@ _g_dbus_address_parse_entry (const gchar  *address_entry,
                    address_entry);
       goto out;
     }
+  else if (s == address_entry)
+    {
+      g_set_error (error,
+                   G_IO_ERROR,
+                   G_IO_ERROR_INVALID_ARGUMENT,
+                   _("Transport name in address element “%s” must not be empty"),
+                   address_entry);
+      goto out;
+    }
 
   transport_name = g_strndup (address_entry, s - address_entry);
   key_value_pairs = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
 
   kv_pairs = g_strsplit (s + 1, ",", 0);
-  for (n = 0; kv_pairs != NULL && kv_pairs[n] != NULL; n++)
+  for (n = 0; kv_pairs[n] != NULL; n++)
     {
       const gchar *kv_pair = kv_pairs[n];
       gchar *key;
@@ -487,6 +483,17 @@ _g_dbus_address_parse_entry (const gchar  *address_entry,
                        address_entry);
           goto out;
         }
+      else if (s == kv_pair)
+        {
+          g_set_error (error,
+                       G_IO_ERROR,
+                       G_IO_ERROR_INVALID_ARGUMENT,
+                       _("Key/Value pair %d, “%s”, in address element “%s” must not have an empty key"),
+                       n,
+                       kv_pair,
+                       address_entry);
+          goto out;
+        }
 
       key = g_uri_unescape_segment (kv_pair, s, NULL);
       value = g_uri_unescape_segment (s + 1, kv_pair + strlen (kv_pair), NULL);
@@ -509,24 +516,18 @@ _g_dbus_address_parse_entry (const gchar  *address_entry,
   ret = TRUE;
 
 out:
-  g_strfreev (kv_pairs);
   if (ret)
     {
       if (out_transport_name != NULL)
-        *out_transport_name = transport_name;
+        *out_transport_name = g_steal_pointer (&transport_name);
-      else
-        g_free (transport_name);
       if (out_key_value_pairs != NULL)
-        *out_key_value_pairs = key_value_pairs;
+        *out_key_value_pairs = g_steal_pointer (&key_value_pairs);
-      else if (key_value_pairs != NULL)
-        g_hash_table_unref (key_value_pairs);
     }
-  else
+
-    {
+  g_clear_pointer (&key_value_pairs, g_hash_table_unref);
   g_free (transport_name);
-      if (key_value_pairs != NULL)
+  g_strfreev (kv_pairs);
-        g_hash_table_unref (key_value_pairs);
+
-    }
   return ret;
 }
 
@@ -962,7 +963,7 @@ g_dbus_address_get_stream_sync (const gchar   *address,
   last_error = NULL;
 
   addr_array = g_strsplit (address, ";", 0);
-  if (addr_array != NULL && addr_array[0] == NULL)
+  if (addr_array[0] == NULL)
     {
       last_error = g_error_new_literal (G_IO_ERROR,
                                         G_IO_ERROR_INVALID_ARGUMENT,

gio/tests/gdbus-addresses.c

@@ -39,11 +39,25 @@ test_empty_address (void)
   g_error_free (error);
 }
 
+/* Test that g_dbus_is_supported_address() returns FALSE for an unparseable
+ * address. */
+static void
+test_unsupported_address (void)
+{
+  GError *error = NULL;
+
+  g_assert_false (g_dbus_is_supported_address (";", &error));
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT);
+  g_clear_error (&error);
+}
+
 static void
 assert_is_supported_address (const gchar *address)
 {
   GError *error = NULL;
 
+  g_assert_true (g_dbus_is_address (address));
+
   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);
@@ -54,30 +68,57 @@ assert_not_supported_address (const gchar *address)
 {
   GError *error = NULL;
 
+  g_assert_true (g_dbus_is_address (address));
+
   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
+/* Test that g_dbus_is_address() returns FALSE for various differently invalid
+ * input strings. */
+static void
+test_address_parsing (void)
+{
+  assert_not_supported_address ("some-imaginary-transport:foo=bar");
+  g_assert_true (g_dbus_is_address ("some-imaginary-transport:foo=bar"));
+
+  assert_not_supported_address ("some-imaginary-transport:foo=bar;unix:path=/this/is/valid");
+
+  g_assert_false (g_dbus_is_address (""));
+  g_assert_false (g_dbus_is_address (";"));
+  g_assert_false (g_dbus_is_address (":"));
+  g_assert_false (g_dbus_is_address ("=:;"));
+  g_assert_false (g_dbus_is_address (":=;:="));
+  g_assert_false (g_dbus_is_address ("transport-name:="));
+  g_assert_false (g_dbus_is_address ("transport-name:=bar"));
+
+  g_assert_false (g_dbus_is_address ("transport-name:foo"));
+  g_assert_false (g_dbus_is_address ("transport-name:foo=%00"));
+  g_assert_false (g_dbus_is_address ("transport-name:%00=bar"));
+
+  assert_not_supported_address ("magic-tractor:");
+}
+
 static void
 test_unix_address (void)
 {
-  assert_not_supported_address ("some-imaginary-transport:foo=bar");
+#ifndef G_OS_UNIX
+  g_test_skip ("unix transport is not supported on non-Unix platforms");
+#else
   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"));
   assert_not_supported_address ("unix:foo=bar");
-  g_assert (!g_dbus_is_address ("unix:path=/foo;abstract=/bar"));
+  g_assert_false (g_dbus_is_address ("unix:path=/foo;abstract=/bar"));
-  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"));
+  assert_is_supported_address ("unix:tmpdir=/tmp");
-
+  assert_not_supported_address ("unix:tmpdir=/tmp,path=/tmp");
-  g_assert (g_dbus_is_address ("some-imaginary-transport:foo=bar;unix:path=/this/is/valid"));
+  assert_not_supported_address ("unix:tmpdir=/tmp,abstract=/tmp/foo");
-  assert_not_supported_address ("some-imaginary-transport:foo=bar;unix:path=/this/is/valid");
+  assert_not_supported_address ("unix:path=/tmp,abstract=/tmp/foo");
-}
+  assert_not_supported_address ("unix:");
 #endif
+}
 
 static void
 test_nonce_tcp_address (void)
@@ -85,12 +126,18 @@ test_nonce_tcp_address (void)
   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");
+  assert_is_supported_address ("nonce-tcp:host=localhost");
 
   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");
+  assert_not_supported_address ("nonce-tcp:meaningless-key=blah");
+  assert_not_supported_address ("nonce-tcp:host=localhost,port=-1");
+  assert_not_supported_address ("nonce-tcp:host=localhost,port=420000");
+  assert_not_supported_address ("nonce-tcp:host=localhost,port=42x");
+  assert_not_supported_address ("nonce-tcp:host=localhost,port=");
 }
 
 static void
@@ -102,6 +149,7 @@ test_tcp_address (void)
   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_not_supported_address ("tcp:host=localhost,port=");
   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");
@@ -151,9 +199,9 @@ main (int   argc,
   g_test_init (&argc, &argv, NULL);
 
   g_test_add_func ("/gdbus/empty-address", test_empty_address);
-#ifdef G_OS_UNIX
+  g_test_add_func ("/gdbus/unsupported-address", test_unsupported_address);
+  g_test_add_func ("/gdbus/address-parsing", test_address_parsing);
   g_test_add_func ("/gdbus/unix-address", test_unix_address);
-#endif
   g_test_add_func ("/gdbus/nonce-tcp-address", test_nonce_tcp_address);
   g_test_add_func ("/gdbus/tcp-address", test_tcp_address);
   g_test_add_func ("/gdbus/autolaunch-address", test_autolaunch_address);