Ensure return_if_fail guards for GDBusConnection and g_dbus_is_member_name()

This actually uncovered a bug in GDBusProxy.

5 files changed, 119 insertions, 32 deletions

diff --git a/docs/reference/gdbus/gdbus-standalone-sections.txt b/docs/reference/gdbus/gdbus-standalone-sections.txt
index 2d0574f..fff2306 100644
--- a/docs/reference/gdbus/gdbus-standalone-sections.txt
+++ b/docs/reference/gdbus/gdbus-standalone-sections.txt
@@ -8,12 +8,13 @@ g_dbus_address_get_stream_sync
 
 <SECTION>
 <FILE>gdbusutils</FILE>
+g_dbus_is_activated
 g_dbus_generate_guid
 g_dbus_is_guid
 g_dbus_is_name
 g_dbus_is_unique_name
+g_dbus_is_member_name
 g_dbus_is_interface_name
-g_dbus_is_activated
 </SECTION>
 
 <SECTION>
diff --git a/gdbus/gdbusconnection.c b/gdbus/gdbusconnection.c
index 8a32dc9..4d06d7a 100644
--- a/gdbus/gdbusconnection.c
+++ b/gdbus/gdbusconnection.c
@@ -694,7 +694,6 @@ gboolean
 g_dbus_connection_is_closed (GDBusConnection *connection)
 {
   g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), FALSE);
-
   return connection->priv->closed;
 }
 
@@ -709,6 +708,7 @@ g_dbus_connection_is_closed (GDBusConnection *connection)
 GDBusCapabilityFlags
 g_dbus_connection_get_capabilities (GDBusConnection *connection)
 {
+  g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), G_DBUS_CAPABILITY_FLAGS_NONE);
   return connection->priv->capabilities;
 }
 
@@ -792,7 +792,6 @@ set_closed_unlocked (GDBusConnection *connection,
 void
 g_dbus_connection_close (GDBusConnection *connection)
 {
-
   g_return_if_fail (G_IS_DBUS_CONNECTION (connection));
 
   CONNECTION_LOCK (connection);
@@ -834,11 +833,19 @@ g_dbus_connection_send_message_unlocked (GDBusConnection     *connection,
   /* TODO: check all necessary headers are present */
 
   ret = FALSE;
+  blob = NULL;
 
   if (out_serial != NULL)
     *out_serial = 0;
 
-  blob = NULL;
+  if (connection->priv->closed)
+    {
+      g_set_error_literal (error,
+                           G_IO_ERROR,
+                           G_IO_ERROR_CLOSED,
+                           _("The connection is closed"));
+      goto out;
+    }
 
   blob = g_dbus_message_to_blob (message,
                                  &blob_size,
@@ -895,6 +902,11 @@ g_dbus_connection_send_message (GDBusConnection     *connection,
                                 GError             **error)
 {
   gboolean ret;
+
+  g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), FALSE);
+  g_return_val_if_fail (G_IS_DBUS_MESSAGE (message), FALSE);
+  g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
+
   CONNECTION_LOCK (connection);
   ret = g_dbus_connection_send_message_unlocked (connection, message, out_serial, error);
   CONNECTION_UNLOCK (connection);
@@ -1082,10 +1094,6 @@ g_dbus_connection_send_message_with_reply_unlocked (GDBusConnection     *connect
   GError *error;
   volatile guint32 serial;
 
-  g_return_if_fail (G_IS_DBUS_CONNECTION (connection));
-  g_return_if_fail (G_IS_DBUS_MESSAGE (message));
-  g_return_if_fail (timeout_msec >= 0 || timeout_msec == -1);
-
   data = NULL;
 
   if (out_serial == NULL)
@@ -1178,6 +1186,10 @@ g_dbus_connection_send_message_with_reply (GDBusConnection     *connection,
                                            GAsyncReadyCallback  callback,
                                            gpointer             user_data)
 {
+  g_return_if_fail (G_IS_DBUS_CONNECTION (connection));
+  g_return_if_fail (G_IS_DBUS_MESSAGE (message));
+  g_return_if_fail (timeout_msec >= 0 || timeout_msec == -1);
+
   CONNECTION_LOCK (connection);
   g_dbus_connection_send_message_with_reply_unlocked (connection,
                                                       message,
@@ -1198,6 +1210,9 @@ g_dbus_connection_send_message_with_reply_finish (GDBusConnection     *connectio
   GDBusMessage *reply;
   GCancellable *cancellable;
 
+  g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), NULL);
+  g_return_val_if_fail (error == NULL || *error == NULL, NULL);
+
   reply = NULL;
 
   g_warn_if_fail (g_simple_async_result_get_source_tag (simple) == g_dbus_connection_send_message_with_reply);
@@ -1250,6 +1265,11 @@ g_dbus_connection_send_message_with_reply_sync (GDBusConnection     *connection,
   SendMessageSyncData *data;
   GDBusMessage *reply;
 
+  g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), NULL);
+  g_return_val_if_fail (G_IS_DBUS_MESSAGE (message), NULL);
+  g_return_val_if_fail (timeout_msec >= 0 || timeout_msec == -1, NULL);
+  g_return_val_if_fail (error == NULL || *error == NULL, NULL);
+
   data = g_new0 (SendMessageSyncData, 1);
   data->context = g_main_context_new ();
   data->loop = g_main_loop_new (data->context, FALSE);
@@ -1647,6 +1667,7 @@ g_dbus_connection_new (GIOStream              *stream,
                        GAsyncReadyCallback     callback,
                        gpointer                user_data)
 {
+  g_return_if_fail (G_IS_IO_STREAM (stream));
   g_async_initable_new_async (G_TYPE_DBUS_CONNECTION,
                               G_PRIORITY_DEFAULT,
                               cancellable,
@@ -1673,6 +1694,10 @@ g_dbus_connection_new_finish (GAsyncResult        *res,
 {
   GObject *object;
   GObject *source_object;
+
+  g_return_val_if_fail (G_IS_ASYNC_RESULT (res), NULL);
+  g_return_val_if_fail (error == NULL || *error == NULL, NULL);
+
   source_object = g_async_result_get_source_object (res);
   g_assert (source_object != NULL);
   object = g_async_initable_new_finish (G_ASYNC_INITABLE (source_object),
@@ -1708,6 +1733,8 @@ g_dbus_connection_new_sync (GIOStream              *stream,
                             GCancellable           *cancellable,
                             GError                **error)
 {
+  g_return_val_if_fail (G_IS_IO_STREAM (stream), NULL);
+  g_return_val_if_fail (error == NULL || *error == NULL, NULL);
   return g_initable_new (G_TYPE_DBUS_CONNECTION,
                          cancellable,
                          error,
@@ -1746,6 +1773,7 @@ g_dbus_connection_new_for_address (const gchar            *address,
                                    GAsyncReadyCallback     callback,
                                    gpointer                user_data)
 {
+  g_return_if_fail (address != NULL);
   g_async_initable_new_async (G_TYPE_DBUS_CONNECTION,
                               G_PRIORITY_DEFAULT,
                               cancellable,
@@ -1771,6 +1799,10 @@ g_dbus_connection_new_for_address_finish (GAsyncResult        *res,
 {
   GObject *object;
   GObject *source_object;
+
+  g_return_val_if_fail (G_IS_ASYNC_RESULT (res), NULL);
+  g_return_val_if_fail (error == NULL || *error == NULL, NULL);
+
   source_object = g_async_result_get_source_object (res);
   g_assert (source_object != NULL);
   object = g_async_initable_new_finish (G_ASYNC_INITABLE (source_object),
@@ -1805,6 +1837,8 @@ g_dbus_connection_new_for_address_sync (const gchar            *address,
                                         GCancellable           *cancellable,
                                         GError                **error)
 {
+  g_return_val_if_fail (address != NULL, NULL);
+  g_return_val_if_fail (error == NULL || *error == NULL, NULL);
   return g_initable_new (G_TYPE_DBUS_CONNECTION,
                          cancellable,
                          error,
@@ -1818,19 +1852,18 @@ g_dbus_connection_new_for_address_sync (const gchar            *address,
 /**
  * g_dbus_connection_set_exit_on_close:
  * @connection: A #GDBusConnection.
- * @exit_on_close: Whether the process is terminated when @connection
- * is closed by the remote peer.
+ * @exit_on_close: Whether the process should be terminated
+ * when @connection is closed by the remote peer.
  *
- * Sets whether the process is terminated when @connection is
- * closed by the remote peer. See
- * #GDBusConnection:exit-on-close for more details.
+ * Sets whether the process should be terminated when @connection is
+ * closed by the remote peer. See #GDBusConnection:exit-on-close for
+ * more details.
  */
 void
 g_dbus_connection_set_exit_on_close (GDBusConnection *connection,
                                      gboolean         exit_on_close)
 {
   g_return_if_fail (G_IS_DBUS_CONNECTION (connection));
-
   connection->priv->exit_on_close = exit_on_close;
 }
 
@@ -2182,8 +2215,10 @@ g_dbus_connection_signal_subscribe (GDBusConnection     *connection,
   g_return_val_if_fail (!g_dbus_connection_is_closed (connection), 0);
   g_return_val_if_fail (sender == NULL || ((strcmp (sender, "org.freedesktop.DBus") == 0 || sender[0] == ':') &&
                                            (connection->priv->flags & G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION)), 0);
+  g_return_val_if_fail (interface_name == NULL || g_dbus_is_interface_name (interface_name), 0);
+  g_return_val_if_fail (member == NULL || g_dbus_is_member_name (member), 0);
+  g_return_val_if_fail (object_path == NULL || g_variant_is_object_path (object_path), 0);
   g_return_val_if_fail (callback != NULL, 0);
-  /* TODO: check that passed in data is well-formed */
 
   CONNECTION_LOCK (connection);
 
@@ -2337,6 +2372,8 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection    *connection,
   GArray *subscribers;
   guint n;
 
+  g_return_if_fail (G_IS_DBUS_CONNECTION (connection));
+
   subscribers = g_array_new (FALSE, FALSE, sizeof (SignalSubscriber));
 
   CONNECTION_LOCK (connection);
@@ -3520,9 +3557,10 @@ g_dbus_connection_register_object (GDBusConnection            *connection,
 
   g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), 0);
   g_return_val_if_fail (!g_dbus_connection_is_closed (connection), 0);
-  g_return_val_if_fail (interface_name != NULL, 0);
   g_return_val_if_fail (object_path != NULL && g_variant_is_object_path (object_path), 0);
+  g_return_val_if_fail (interface_name == NULL || g_dbus_is_interface_name (interface_name), 0);
   g_return_val_if_fail (introspection_data != NULL, 0);
+  g_return_val_if_fail (error == NULL || *error == NULL, 0);
 
   ret = 0;
 
@@ -3664,10 +3702,10 @@ g_dbus_connection_emit_signal (GDBusConnection    *connection,
 
   g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), FALSE);
   g_return_val_if_fail (destination_bus_name == NULL || g_dbus_is_name (destination_bus_name), FALSE);
-  g_return_val_if_fail (object_path != NULL, FALSE);
-  g_return_val_if_fail (interface_name != NULL, FALSE);
-  g_return_val_if_fail (signal_name != NULL, FALSE);
-  g_return_val_if_fail ((parameters == NULL) || g_variant_is_of_type (parameters, G_VARIANT_TYPE_TUPLE), FALSE);
+  g_return_val_if_fail (object_path != NULL && g_variant_is_object_path (object_path), FALSE);
+  g_return_val_if_fail (interface_name != NULL && g_dbus_is_interface_name (interface_name), FALSE);
+  g_return_val_if_fail (signal_name != NULL && g_dbus_is_member_name (signal_name), FALSE);
+  g_return_val_if_fail (parameters == NULL || g_variant_is_of_type (parameters, G_VARIANT_TYPE_TUPLE), FALSE);
 
   message = g_dbus_message_new_signal (object_path,
                                        interface_name,
@@ -3700,7 +3738,7 @@ add_invoke_method_flags (GDBusMessage *message,
 /**
  * g_dbus_connection_invoke_method:
  * @connection: A #GDBusConnection.
- * @bus_name: A unique or well-known bus name.
+ * @bus_name: A unique or well-known bus name or %NULL if @connection is not a message bus connection.
  * @object_path: Path of remote object.
  * @interface_name: D-Bus interface to invoke method on.
  * @method_name: The name of the method to invoke.
@@ -3717,10 +3755,10 @@ add_invoke_method_flags (GDBusMessage *message,
  * @object_path owned by @bus_name.
  *
  * If @connection is closed then the operation will fail with
- * %G_IO_ERROR_CLOSED. If @cancellable is canceled, the
- * operation will fail with %G_IO_ERROR_CANCELLED. If @parameters
- * contains a value not compatible with the D-Bus protocol, the operation
- * fails with %G_IO_ERROR_INVALID_ARGUMENT.
+ * %G_IO_ERROR_CLOSED. If @cancellable is canceled, the operation will
+ * fail with %G_IO_ERROR_CANCELLED. If @parameters contains a value
+ * not compatible with the D-Bus protocol, the operation fails with
+ * %G_IO_ERROR_INVALID_ARGUMENT.
  *
  * This is an asynchronous method. When the operation is finished, @callback will be invoked
  * in the <link linkend="g-main-context-push-thread-default">thread-default main loop</link>
@@ -3745,9 +3783,11 @@ g_dbus_connection_invoke_method (GDBusConnection       *connection,
   GDBusMessage *message;
 
   g_return_if_fail (G_IS_DBUS_CONNECTION (connection));
-  g_return_if_fail (object_path != NULL);
-  g_return_if_fail (interface_name != NULL);
-  g_return_if_fail (method_name != NULL);
+  g_return_if_fail (bus_name == NULL || g_dbus_is_name (bus_name));
+  g_return_if_fail (object_path != NULL && g_variant_is_object_path (object_path));
+  g_return_if_fail (interface_name != NULL && g_dbus_is_interface_name (interface_name));
+  g_return_if_fail (method_name != NULL && g_dbus_is_member_name (method_name));
+  g_return_if_fail (timeout_msec >= 0 || timeout_msec == -1);
   g_return_if_fail ((parameters == NULL) || g_variant_is_of_type (parameters, G_VARIANT_TYPE_TUPLE));
 
   message = g_dbus_message_new_method_call (bus_name,
@@ -3821,6 +3861,10 @@ g_dbus_connection_invoke_method_finish (GDBusConnection    *connection,
   GDBusMessage *reply;
   GVariant *result;
 
+  g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), NULL);
+  g_return_val_if_fail (G_IS_ASYNC_RESULT (res), NULL);
+  g_return_val_if_fail (error == NULL || *error == NULL, NULL);
+
   result = NULL;
 
   reply = g_dbus_connection_send_message_with_reply_finish (connection, res, error);
@@ -3888,9 +3932,11 @@ g_dbus_connection_invoke_method_sync (GDBusConnection       *connection,
   result = NULL;
 
   g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), NULL);
-  g_return_val_if_fail (object_path != NULL, NULL);
-  g_return_val_if_fail (interface_name != NULL, NULL);
-  g_return_val_if_fail (method_name != NULL, NULL);
+  g_return_val_if_fail (bus_name == NULL || g_dbus_is_name (bus_name), NULL);
+  g_return_val_if_fail (object_path != NULL && g_variant_is_object_path (object_path), NULL);
+  g_return_val_if_fail (interface_name != NULL && g_dbus_is_interface_name (interface_name), NULL);
+  g_return_val_if_fail (method_name != NULL && g_dbus_is_member_name (method_name), NULL);
+  g_return_val_if_fail (timeout_msec >= 0 || timeout_msec == -1, NULL);
   g_return_val_if_fail ((parameters == NULL) || g_variant_is_of_type (parameters, G_VARIANT_TYPE_TUPLE), NULL);
 
   message = g_dbus_message_new_method_call (bus_name,
@@ -4394,7 +4440,6 @@ g_dbus_connection_register_subtree (GDBusConnection            *connection,
   ExportedSubtree *es;
 
   g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), 0);
-  g_return_val_if_fail (!g_dbus_connection_is_closed (connection), 0);
   g_return_val_if_fail (object_path != NULL && g_variant_is_object_path (object_path), 0);
   g_return_val_if_fail (vtable != NULL, 0);
   g_return_val_if_fail (error == NULL || *error == NULL, 0);
@@ -4828,6 +4873,8 @@ g_bus_get_sync (GBusType            bus_type,
 {
   GDBusConnection *connection;
 
+  g_return_val_if_fail (error == NULL || *error == NULL, NULL);
+
   connection = get_uninitialized_connection (bus_type, cancellable, error);
   if (connection == NULL)
     goto out;
@@ -4946,6 +4993,8 @@ g_bus_get_finish (GAsyncResult  *res,
   GObject *object;
   GDBusConnection *ret;
 
+  g_return_val_if_fail (error == NULL || *error == NULL, NULL);
+
   g_warn_if_fail (g_simple_async_result_get_source_tag (simple) == g_bus_get);
 
   ret = NULL;
diff --git a/gdbus/gdbusproxy.c b/gdbus/gdbusproxy.c
index e248e99..2f1ddb4 100644
--- a/gdbus/gdbusproxy.c
+++ b/gdbus/gdbusproxy.c
@@ -769,6 +769,7 @@ get_all_cb (GDBusConnection *connection,
   GVariant *result;
   GError *error;
 
+  error = NULL;
   result = g_dbus_connection_invoke_method_finish (connection,
                                                    res,
                                                    &error);
diff --git a/gdbus/gdbusutils.c b/gdbus/gdbusutils.c
index e6bdec7..b1ec0ea 100644
--- a/gdbus/gdbusutils.c
+++ b/gdbus/gdbusutils.c
@@ -192,6 +192,41 @@ g_dbus_is_unique_name (const gchar *string)
 }
 
 /**
+ * g_dbus_is_member_name:
+ * @string: The string to check.
+ *
+ * Checks if @string is a valid D-Bus member (e.g. signal or method) name.
+ *
+ * Returns: %TRUE if valid, %FALSE otherwise.
+ */
+gboolean
+g_dbus_is_member_name (const gchar *string)
+{
+  gboolean ret;
+  guint n;
+
+  ret = FALSE;
+  if (G_UNLIKELY (string == NULL))
+    goto out;
+
+  if (G_UNLIKELY (!is_valid_initial_bus_name_character (string[0], FALSE, FALSE)))
+    goto out;
+
+  for (n = 1; string[n] != '\0'; n++)
+    {
+      if (G_UNLIKELY (!is_valid_bus_name_character (string[n], FALSE)))
+        {
+          goto out;
+        }
+    }
+
+  ret = TRUE;
+
+ out:
+  return ret;
+}
+
+/**
  * g_dbus_is_interface_name:
  * @string: The string to check.
  *
diff --git a/gdbus/gdbusutils.h b/gdbus/gdbusutils.h
index 404d6f3..93d6409 100644
--- a/gdbus/gdbusutils.h
+++ b/gdbus/gdbusutils.h
@@ -36,6 +36,7 @@ gchar *g_dbus_generate_guid (void);
 
 gboolean g_dbus_is_name (const gchar *string);
 gboolean g_dbus_is_unique_name (const gchar *string);
+gboolean g_dbus_is_member_name (const gchar *string);
 gboolean g_dbus_is_interface_name (const gchar *string);
 
 gboolean g_dbus_is_activated (void);