libnm/tests: add tests for libnm handling invalid connections

Add test showing how libnm/libnm-glib handles invalid connections,
i.e. connections that fail nm_connection_verify(). libnm implements
settings a static types (via different NMSetting types). This makes
it unavoidable that eventually a newer server version will
expose connections that fail verification in the client.

For example, master added a new base type NMSettingTun. This setting type
was not backported to legacy libnm-glib, thus such connection will not verify.
Also, we want that newer server versions are backward compatible with older
library versions. Thus also a pre-NMSettingTun libnm version has the same
problem.

The test shows that libnm is agnostic to whether the connection verifies.
That is consistent behavior, but possibly problematic because most
accessors to connections assert against a valid connection. Thus using
the common nm_connection*() functions on an invalid connection can lead
to problems.
Also, due to the static nature of our NMSetting types, some properties
can be silently dropped and thus mangling the connection without the
library user noticing.

libnm-glib prints a g_warning() whenever parsing an invalid connection.
When an invalid connection is added initially, it is exposed to the library
user. When a connection gets later invalidated due to an update, the
connection disappears and it stays missing even if a subsequent update
makes the connection valid again.

libnm-glib's behavior indicates that we might wanted to hide invalid
connections from the user. But it's very broken there.

diff --git a/libnm-glib/tests/test-nm-client.c b/libnm-glib/tests/test-nm-client.c
index b9b36bf..f528bc2 100644 (file)
--- a/libnm-glib/tests/test-nm-client.c
+++ b/libnm-glib/tests/test-nm-client.c
@@ -32,6 +32,8 @@
 #include "nm-device-wifi.h"
 #include "nm-device-ethernet.h"
 #include "nm-device-wimax.h"
+#include "nm-connection.h"
+#include "nm-setting.h"
 
 #include "nm-test-libnm-utils.h"
 
@@ -874,6 +876,263 @@ test_client_manager_running (void)
 
 /*******************************************************************/
 
+static GPtrArray *
+_slist_to_array (GPtrArray **connections, GSList *list)
+{
+       GPtrArray *array;
+       const GSList *iter;
+
+       if (!*connections)
+               *connections = array = g_ptr_array_new ();
+       else {
+               array = *connections;
+               g_ptr_array_set_size (array, 0);
+       }
+       for (iter = list; iter; iter = iter->next)
+               g_ptr_array_add (array, iter->data);
+       g_slist_free (list);
+       return array;
+}
+
+static gboolean
+_test_connection_invalid_find_connections (gpointer element, gpointer needle, gpointer user_data)
+{
+       NMRemoteConnection *con = NM_REMOTE_CONNECTION (element);
+       const char *path = needle;
+
+       g_assert (NM_IS_REMOTE_CONNECTION (con));
+       g_assert (path && *path);
+
+       return strcmp (path, nm_connection_get_path ((NMConnection *) con)) == 0;
+}
+
+#define ASSERT_IDX(i) \
+       g_assert_cmpint (idx[i], >=, 0); \
+       g_assert (path##i && *path##i); \
+       g_assert (NM_IS_REMOTE_CONNECTION (connections->pdata[idx[i]])); \
+       g_assert_cmpstr (nm_connection_get_path (connections->pdata[idx[i]]), ==, path##i);
+
+static void
+test_connection_invalid (void)
+{
+       NMTSTC_SERVICE_INFO_SETUP (my_sinfo)
+       gs_unref_object NMConnection *connection = NULL;
+       NMSettingConnection *s_con;
+       gs_unref_object NMRemoteSettings *settings = NULL;
+       gs_unref_ptrarray GPtrArray *connections = NULL;
+       gs_free char *path0 = NULL;
+       gs_free char *path1 = NULL;
+       gs_free char *path2 = NULL;
+       gs_free char *uuid2 = NULL;
+       gsize n_found;
+       gssize idx[3];
+
+       /**************************************************************************
+        * Add two connection before starting libnm. One valid, one invalid.
+        *************************************************************************/
+
+       connection = nmtst_create_minimal_connection ("test-connection-invalid-0", NULL, NM_SETTING_WIRED_SETTING_NAME, &s_con);
+       nmtst_connection_normalize (connection);
+       g_object_set (s_con,
+                     NM_SETTING_CONNECTION_UUID, nmtst_uuid_generate (),
+                     NULL);
+       nmtstc_service_add_connection (my_sinfo,
+                                      connection,
+                                      TRUE,
+                                      &path0);
+
+       nm_connection_remove_setting (connection, NM_TYPE_SETTING_WIRED);
+       g_object_set (s_con,
+                     NM_SETTING_CONNECTION_ID, "test-connection-invalid-1",
+                     NM_SETTING_CONNECTION_TYPE, "invalid-type-1",
+                     NM_SETTING_CONNECTION_UUID, nmtst_uuid_generate (),
+                     NULL);
+       nmtstc_service_add_connection (my_sinfo,
+                                      connection,
+                                      FALSE,
+                                      &path1);
+
+       nmtst_main_loop_run (loop, 100);
+
+       settings = nmtstc_nm_remote_settings_new ();
+
+       g_test_expect_message ("libnm-glib", G_LOG_LEVEL_WARNING, "*replace_settings: error updating connection*");
+
+       nmtst_main_loop_run (loop, 100);
+
+       g_test_assert_expected_messages ();
+
+       _slist_to_array (&connections, nm_remote_settings_list_connections (settings));
+
+       g_assert_cmpint (connections->len, ==, 2);
+       n_found = nmtst_find_all_indexes (connections->pdata,
+                                         connections->len,
+                                         (gpointer *) ((const char *[]) { path0, path1 }),
+                                         2,
+                                         _test_connection_invalid_find_connections,
+                                         NULL,
+                                         idx);
+       g_assert_cmpint (n_found, ==, 2);
+       ASSERT_IDX (0);
+       ASSERT_IDX (1);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[0]]);
+       nmtst_assert_connection_unnormalizable (connections->pdata[idx[1]], 0, 0);
+
+       /**************************************************************************
+        * After having the client up and running, add another invalid connection
+        *************************************************************************/
+
+       g_object_set (s_con,
+                     NM_SETTING_CONNECTION_ID, "test-connection-invalid-2",
+                     NM_SETTING_CONNECTION_TYPE, "invalid-type-2",
+                     NM_SETTING_CONNECTION_UUID, (uuid2 = g_strdup (nmtst_uuid_generate ())),
+                     NULL);
+       nmtstc_service_add_connection (my_sinfo,
+                                      connection,
+                                      FALSE,
+                                      &path2);
+
+       g_test_expect_message ("libnm-glib", G_LOG_LEVEL_WARNING, "*replace_settings: error updating connection*");
+
+       nmtst_main_loop_run (loop, 100);
+
+       g_test_assert_expected_messages ();
+
+       _slist_to_array (&connections, nm_remote_settings_list_connections (settings));
+
+       g_assert_cmpint (connections->len, ==, 3);
+       n_found = nmtst_find_all_indexes (connections->pdata,
+                                         connections->len,
+                                         (gpointer *) ((const char *[]) { path0, path1, path2 }),
+                                         3,
+                                         _test_connection_invalid_find_connections,
+                                         NULL,
+                                         idx);
+       g_assert_cmpint (n_found, ==, 3);
+       ASSERT_IDX (0);
+       ASSERT_IDX (1);
+       ASSERT_IDX (2);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[0]]);
+       nmtst_assert_connection_unnormalizable (connections->pdata[idx[1]], 0, 0);
+       nmtst_assert_connection_unnormalizable (connections->pdata[idx[2]], 0, 0);
+
+       /**************************************************************************
+        * Modify the invalid connection. Connection disappears
+        *************************************************************************/
+
+       g_object_set (s_con,
+                     NM_SETTING_CONNECTION_ID, "test-connection-invalid-2x",
+                     NULL);
+       nmtstc_service_update_connection (my_sinfo,
+                                         path2,
+                                         connection,
+                                         FALSE);
+
+       g_test_expect_message ("libnm-glib", G_LOG_LEVEL_WARNING, "*replace_settings: error updating connection*");
+
+       nmtst_main_loop_run (loop, 100);
+
+       g_test_assert_expected_messages ();
+
+       _slist_to_array (&connections, nm_remote_settings_list_connections (settings));
+
+       g_assert_cmpint (connections->len, ==, 2);
+       n_found = nmtst_find_all_indexes (connections->pdata,
+                                         connections->len,
+                                         (gpointer *) ((const char *[]) { path0, path1, path2 }),
+                                         3,
+                                         _test_connection_invalid_find_connections,
+                                         NULL,
+                                         idx);
+       g_assert_cmpint (n_found, ==, 2);
+       ASSERT_IDX (0);
+       ASSERT_IDX (1);
+       g_assert_cmpint (idx[2], ==, -1);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[0]]);
+       nmtst_assert_connection_unnormalizable (connections->pdata[idx[1]], 0, 0);
+
+       /**************************************************************************
+        * Modify the invalid connection again. Note that the connection stays
+        * invisible (although it exists, and is valid).
+        *************************************************************************/
+
+       g_clear_object (&connection);
+       connection = nmtst_create_minimal_connection ("test-connection-invalid-2", NULL, NM_SETTING_WIRED_SETTING_NAME, &s_con);
+       nmtst_connection_normalize (connection);
+       g_object_set (s_con,
+                     NM_SETTING_CONNECTION_ID, "test-connection-invalid-2z",
+                     NM_SETTING_CONNECTION_TYPE, "802-3-ethernet",
+                     NM_SETTING_CONNECTION_UUID, uuid2,
+                     NULL);
+
+       nmtstc_service_update_connection (my_sinfo,
+                                         path2,
+                                         connection,
+                                         FALSE);
+
+       nmtst_main_loop_run (loop, 100);
+
+       _slist_to_array (&connections, nm_remote_settings_list_connections (settings));
+
+       g_assert_cmpint (connections->len, ==, 2);
+       n_found = nmtst_find_all_indexes (connections->pdata,
+                                         connections->len,
+                                         (gpointer *) ((const char *[]) { path0, path1, path2 }),
+                                         3,
+                                         _test_connection_invalid_find_connections,
+                                         NULL,
+                                         idx);
+       g_assert_cmpint (n_found, ==, 2);
+       ASSERT_IDX (0);
+       ASSERT_IDX (1);
+       g_assert_cmpint (idx[2], ==, -1);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[0]]);
+       nmtst_assert_connection_unnormalizable (connections->pdata[idx[1]], 0, 0);
+
+
+       /**************************************************************************
+        * Modify the invalid connection and make it valid
+        *************************************************************************/
+
+       g_clear_object (&connection);
+       connection = nmtst_create_minimal_connection ("test-connection-invalid-1", NULL, NM_SETTING_WIRED_SETTING_NAME, &s_con);
+       nmtst_connection_normalize (connection);
+       g_object_set (s_con,
+                     NM_SETTING_CONNECTION_ID, "test-connection-invalid-1x",
+                     NM_SETTING_CONNECTION_TYPE, "802-3-ethernet",
+                     NM_SETTING_CONNECTION_UUID, nm_connection_get_uuid (connections->pdata[idx[1]]),
+                     NULL);
+
+       nmtstc_service_update_connection (my_sinfo,
+                                         path1,
+                                         connection,
+                                         FALSE);
+
+       nmtst_main_loop_run (loop, 100);
+
+       _slist_to_array (&connections, nm_remote_settings_list_connections (settings));
+
+       g_assert_cmpint (connections->len, ==, 2);
+       n_found = nmtst_find_all_indexes (connections->pdata,
+                                         connections->len,
+                                         (gpointer *) ((const char *[]) { path0, path1, path2 }),
+                                         3,
+                                         _test_connection_invalid_find_connections,
+                                         NULL,
+                                         idx);
+       g_assert_cmpint (n_found, ==, 2);
+       ASSERT_IDX (0);
+       ASSERT_IDX (1);
+       g_assert_cmpint (idx[2], ==, -1);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[0]]);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[1]]);
+       g_assert_cmpstr ("test-connection-invalid-1x", ==, nm_connection_get_id (connections->pdata[idx[1]]));
+
+#undef ASSERT_IDX
+}
+
+/*******************************************************************/
+
 NMTST_DEFINE ();
 
 int
@@ -888,6 +1147,7 @@ main (int argc, char **argv)
        g_test_add_func ("/libnm-glib/wimax-nsp-added-removed", test_wimax_nsp_added_removed);
        g_test_add_func ("/libnm-glib/devices-array", test_devices_array);
        g_test_add_func ("/libnm-glib/client-manager-running", test_client_manager_running);
+       g_test_add_func ("/libnm/connection/invalid", test_connection_invalid);
 
        return g_test_run ();
 }

diff --git a/libnm/tests/test-nm-client.c b/libnm/tests/test-nm-client.c
index f2701ab..5be6914 100644 (file)
--- a/libnm/tests/test-nm-client.c
+++ b/libnm/tests/test-nm-client.c
@@ -1228,6 +1228,243 @@ test_device_connection_compatibility (void)
 
 /*******************************************************************/
 
+static gboolean
+_test_connection_invalid_find_connections (gpointer element, gpointer needle, gpointer user_data)
+{
+       NMRemoteConnection *con = NM_REMOTE_CONNECTION (element);
+       const char *path = needle;
+
+       g_assert (NM_IS_REMOTE_CONNECTION (con));
+       g_assert (path && *path);
+
+       return strcmp (path, nm_connection_get_path ((NMConnection *) con)) == 0;
+}
+
+#define ASSERT_IDX(i) \
+       g_assert_cmpint (idx[i], >=, 0); \
+       g_assert (path##i && *path##i); \
+       g_assert (NM_IS_REMOTE_CONNECTION (connections->pdata[idx[i]])); \
+       g_assert_cmpstr (nm_connection_get_path (connections->pdata[idx[i]]), ==, path##i);
+
+static void
+test_connection_invalid (void)
+{
+       NMTSTC_SERVICE_INFO_SETUP (my_sinfo)
+       gs_unref_object NMConnection *connection = NULL;
+       NMSettingConnection *s_con;
+       gs_unref_object NMClient *client = NULL;
+       const GPtrArray *connections;
+       gs_free_error GError *error = NULL;
+       gs_free char *path0 = NULL;
+       gs_free char *path1 = NULL;
+       gs_free char *path2 = NULL;
+       gs_free char *uuid2 = NULL;
+       gsize n_found;
+       gssize idx[3];
+
+       /**************************************************************************
+        * Add two connection before starting libnm. One valid, one invalid.
+        *************************************************************************/
+
+       connection = nmtst_create_minimal_connection ("test-connection-invalid-0", NULL, NM_SETTING_WIRED_SETTING_NAME, &s_con);
+       nmtst_connection_normalize (connection);
+       g_object_set (s_con,
+                     NM_SETTING_CONNECTION_UUID, nmtst_uuid_generate (),
+                     NULL);
+       nmtstc_service_add_connection (my_sinfo,
+                                      connection,
+                                      TRUE,
+                                      &path0);
+
+       nm_connection_remove_setting (connection, NM_TYPE_SETTING_WIRED);
+       g_object_set (s_con,
+                     NM_SETTING_CONNECTION_ID, "test-connection-invalid-1",
+                     NM_SETTING_CONNECTION_TYPE, "invalid-type-1",
+                     NM_SETTING_CONNECTION_UUID, nmtst_uuid_generate (),
+                     NULL);
+       nmtstc_service_add_connection (my_sinfo,
+                                      connection,
+                                      FALSE,
+                                      &path1);
+
+
+       client = nm_client_new (NULL, &error);
+       g_assert_no_error (error);
+
+       connections = nm_client_get_connections (client);
+       g_assert (connections);
+
+       g_assert_cmpint (connections->len, ==, 2);
+       n_found = nmtst_find_all_indexes (connections->pdata,
+                                         connections->len,
+                                         (gpointer *) ((const char *[]) { path0, path1 }),
+                                         2,
+                                         _test_connection_invalid_find_connections,
+                                         NULL,
+                                         idx);
+       g_assert_cmpint (n_found, ==, 2);
+       ASSERT_IDX (0);
+       ASSERT_IDX (1);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[0]]);
+       nmtst_assert_connection_unnormalizable (connections->pdata[idx[1]], 0, 0);
+
+       /**************************************************************************
+        * After having the client up and running, add another invalid connection
+        *************************************************************************/
+
+       g_object_set (s_con,
+                     NM_SETTING_CONNECTION_ID, "test-connection-invalid-2",
+                     NM_SETTING_CONNECTION_TYPE, "invalid-type-2",
+                     NM_SETTING_CONNECTION_UUID, (uuid2 = g_strdup (nmtst_uuid_generate ())),
+                     NULL);
+       nmtstc_service_add_connection (my_sinfo,
+                                      connection,
+                                      FALSE,
+                                      &path2);
+
+
+       nmtst_main_loop_run (loop, 100);
+
+
+       connections = nm_client_get_connections (client);
+       g_assert (connections);
+
+       g_assert_cmpint (connections->len, ==, 3);
+       n_found = nmtst_find_all_indexes (connections->pdata,
+                                         connections->len,
+                                         (gpointer *) ((const char *[]) { path0, path1, path2 }),
+                                         3,
+                                         _test_connection_invalid_find_connections,
+                                         NULL,
+                                         idx);
+       g_assert_cmpint (n_found, ==, 3);
+       ASSERT_IDX (0);
+       ASSERT_IDX (1);
+       ASSERT_IDX (2);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[0]]);
+       nmtst_assert_connection_unnormalizable (connections->pdata[idx[1]], 0, 0);
+       nmtst_assert_connection_unnormalizable (connections->pdata[idx[2]], 0, 0);
+
+       /**************************************************************************
+        * Modify the invalid connection (still invalid)
+        *************************************************************************/
+
+       g_object_set (s_con,
+                     NM_SETTING_CONNECTION_ID, "test-connection-invalid-2x",
+                     NULL);
+       nmtstc_service_update_connection (my_sinfo,
+                                         path2,
+                                         connection,
+                                         FALSE);
+
+       nmtst_main_loop_run (loop, 100);
+
+       connections = nm_client_get_connections (client);
+       g_assert (connections);
+
+       g_assert_cmpint (connections->len, ==, 3);
+       n_found = nmtst_find_all_indexes (connections->pdata,
+                                         connections->len,
+                                         (gpointer *) ((const char *[]) { path0, path1, path2 }),
+                                         3,
+                                         _test_connection_invalid_find_connections,
+                                         NULL,
+                                         idx);
+       g_assert_cmpint (n_found, ==, 3);
+       ASSERT_IDX (0);
+       ASSERT_IDX (1);
+       ASSERT_IDX (2);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[0]]);
+       nmtst_assert_connection_unnormalizable (connections->pdata[idx[1]], 0, 0);
+       nmtst_assert_connection_unnormalizable (connections->pdata[idx[2]], 0, 0);
+       g_assert_cmpstr ("test-connection-invalid-2x", ==, nm_connection_get_id (connections->pdata[idx[2]]));
+
+       /**************************************************************************
+        * Modify the invalid connection (now becomes valid)
+        *************************************************************************/
+
+       g_clear_object (&connection);
+       connection = nmtst_create_minimal_connection ("test-connection-invalid-2", NULL, NM_SETTING_WIRED_SETTING_NAME, &s_con);
+       nmtst_connection_normalize (connection);
+       g_object_set (s_con,
+                     NM_SETTING_CONNECTION_ID, "test-connection-invalid-2z",
+                     NM_SETTING_CONNECTION_TYPE, "802-3-ethernet",
+                     NM_SETTING_CONNECTION_UUID, uuid2,
+                     NULL);
+
+       nmtstc_service_update_connection (my_sinfo,
+                                         path2,
+                                         connection,
+                                         FALSE);
+
+       nmtst_main_loop_run (loop, 100);
+
+       connections = nm_client_get_connections (client);
+       g_assert (connections);
+
+       g_assert_cmpint (connections->len, ==, 3);
+       n_found = nmtst_find_all_indexes (connections->pdata,
+                                         connections->len,
+                                         (gpointer *) ((const char *[]) { path0, path1, path2 }),
+                                         3,
+                                         _test_connection_invalid_find_connections,
+                                         NULL,
+                                         idx);
+       g_assert_cmpint (n_found, ==, 3);
+       ASSERT_IDX (0);
+       ASSERT_IDX (1);
+       ASSERT_IDX (2);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[0]]);
+       nmtst_assert_connection_unnormalizable (connections->pdata[idx[1]], 0, 0);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[2]]);
+       g_assert_cmpstr ("test-connection-invalid-2z", ==, nm_connection_get_id (connections->pdata[idx[2]]));
+
+       /**************************************************************************
+        * Modify the invalid connection and make it valid
+        *************************************************************************/
+
+       g_clear_object (&connection);
+       connection = nmtst_create_minimal_connection ("test-connection-invalid-1", NULL, NM_SETTING_WIRED_SETTING_NAME, &s_con);
+       nmtst_connection_normalize (connection);
+       g_object_set (s_con,
+                     NM_SETTING_CONNECTION_ID, "test-connection-invalid-1x",
+                     NM_SETTING_CONNECTION_TYPE, "802-3-ethernet",
+                     NM_SETTING_CONNECTION_UUID, nm_connection_get_uuid (connections->pdata[idx[1]]),
+                     NULL);
+
+       nmtstc_service_update_connection (my_sinfo,
+                                         path1,
+                                         connection,
+                                         FALSE);
+
+       nmtst_main_loop_run (loop, 100);
+
+       connections = nm_client_get_connections (client);
+       g_assert (connections);
+
+       g_assert_cmpint (connections->len, ==, 3);
+       n_found = nmtst_find_all_indexes (connections->pdata,
+                                         connections->len,
+                                         (gpointer *) ((const char *[]) { path0, path1, path2 }),
+                                         3,
+                                         _test_connection_invalid_find_connections,
+                                         NULL,
+                                         idx);
+       g_assert_cmpint (n_found, ==, 3);
+       ASSERT_IDX (0);
+       ASSERT_IDX (1);
+       ASSERT_IDX (2);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[0]]);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[1]]);
+       nmtst_assert_connection_verifies_without_normalization (connections->pdata[idx[2]]);
+       g_assert_cmpstr ("test-connection-invalid-1x", ==, nm_connection_get_id (connections->pdata[idx[1]]));
+       g_assert_cmpstr ("test-connection-invalid-2z", ==, nm_connection_get_id (connections->pdata[idx[2]]));
+
+#undef ASSERT_IDX
+}
+
+/*******************************************************************/
+
 NMTST_DEFINE ();
 
 int
@@ -1249,6 +1486,7 @@ main (int argc, char **argv)
        g_test_add_func ("/libnm/activate-virtual", test_activate_virtual);
        g_test_add_func ("/libnm/activate-failed", test_activate_failed);
        g_test_add_func ("/libnm/device-connection-compatibility", test_device_connection_compatibility);
+       g_test_add_func ("/libnm/connection/invalid", test_connection_invalid);
 
        return g_test_run ();
 }