team: merge branch 'bg/team-config-validation'

Restore the validation on team and team port configuration.

11 files changed, 124 insertions, 20 deletions

diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index f39d41c58e..57f964095e 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -28,7 +28,6 @@
 #include "nm-connection.h"
 #include "nm-connection-private.h"
 #include "nm-utils.h"
-#include "nm-utils-private.h"
 #include "nm-setting-private.h"
 #include "nm-core-internal.h"
 
@@ -916,7 +915,7 @@ _normalize_team_config (NMConnection *self, GHashTable *parameters)
 	if (s_team) {
 		const char *config = nm_setting_team_get_config (s_team);
 
-		if (config && !_nm_utils_check_valid_json (config, NULL)) {
+		if (config && !*config) {
 			g_object_set (s_team, NM_SETTING_TEAM_CONFIG, NULL, NULL);
 			return TRUE;
 		}
@@ -932,7 +931,7 @@ _normalize_team_port_config (NMConnection *self, GHashTable *parameters)
 	if (s_team_port) {
 		const char *config = nm_setting_team_port_get_config (s_team_port);
 
-		if (config && !_nm_utils_check_valid_json (config, NULL)) {
+		if (config && !*config) {
 			g_object_set (s_team_port, NM_SETTING_TEAM_PORT_CONFIG, NULL, NULL);
 			return TRUE;
 		}
diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h
index 94e473d3c6..e491bce6da 100644
--- a/libnm-core/nm-core-internal.h
+++ b/libnm-core/nm-core-internal.h
@@ -326,4 +326,9 @@ gboolean _nm_setting_bond_option_supported (const char *option, NMBondMode mode)
 
 gboolean _nm_utils_inet6_is_token (const struct in6_addr *in6addr);
 
+/***********************************************************/
+
+gboolean    _nm_utils_check_valid_json  (const char *json, GError **error);
+gboolean    _nm_utils_team_config_equal (const char *conf1, const char *conf2, gboolean port);
+
 #endif
diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c
index 54a694eff7..16f0c225b9 100644
--- a/libnm-core/nm-keyfile-reader.c
+++ b/libnm-core/nm-keyfile-reader.c
@@ -1167,6 +1167,24 @@ parity_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key)
 	g_object_set (setting, key, parity, NULL);
 }
 
+static void
+team_config_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key)
+{
+	const char *setting_name = nm_setting_get_name (setting);
+	gs_free char *conf = NULL;
+	gs_free_error GError *error = NULL;
+
+	conf = nm_keyfile_plugin_kf_get_string (info->keyfile, setting_name, key, NULL);
+	if (conf && conf[0] && !_nm_utils_check_valid_json (conf, &error)) {
+		handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
+		             _("ignoring invalid team configuration: %s"),
+		             error->message);
+		g_clear_pointer (&conf, g_free);
+	}
+
+	g_object_set (G_OBJECT (setting), key, conf, NULL);
+}
+
 typedef struct {
 	const char *setting_name;
 	const char *key;
@@ -1285,6 +1303,14 @@ static KeyParser key_parsers[] = {
 	  NM_SETTING_SERIAL_PARITY,
 	  TRUE,
 	  parity_parser },
+	{ NM_SETTING_TEAM_SETTING_NAME,
+	  NM_SETTING_TEAM_CONFIG,
+	  TRUE,
+	  team_config_parser },
+	{ NM_SETTING_TEAM_PORT_SETTING_NAME,
+	  NM_SETTING_TEAM_CONFIG,
+	  TRUE,
+	  team_config_parser },
 	{ NULL, NULL, FALSE }
 };
 
diff --git a/libnm-core/nm-setting-team-port.c b/libnm-core/nm-setting-team-port.c
index 0d175d5ebd..e3ddbdad7c 100644
--- a/libnm-core/nm-setting-team-port.c
+++ b/libnm-core/nm-setting-team-port.c
@@ -122,13 +122,8 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 			                "%s.%s: ",
 			                NM_SETTING_TEAM_PORT_SETTING_NAME,
 			                NM_SETTING_TEAM_PORT_CONFIG);
-			/* for backward compatibility, we accept invalid json and normalize it */
-			if (!priv->config[0]) {
-				/* be more forgiving to "" and let it verify() as valid because
-				 * at least anaconda used to write such configs */
-				return NM_SETTING_VERIFY_NORMALIZABLE;
-			}
-			return NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
+			/* We treat an empty string as no config for compatibility. */
+			return *priv->config ? FALSE : NM_SETTING_VERIFY_NORMALIZABLE;
 		}
 	}
 
diff --git a/libnm-core/nm-setting-team.c b/libnm-core/nm-setting-team.c
index a559e0db78..5bf11ed921 100644
--- a/libnm-core/nm-setting-team.c
+++ b/libnm-core/nm-setting-team.c
@@ -94,13 +94,8 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 			                "%s.%s: ",
 			                NM_SETTING_TEAM_SETTING_NAME,
 			                NM_SETTING_TEAM_CONFIG);
-			/* for backward compatibility, we accept invalid json and normalize it */
-			if (!priv->config[0]) {
-				/* be more forgiving to "" and let it verify() as valid because
-				 * at least anaconda used to write such configs */
-				return NM_SETTING_VERIFY_NORMALIZABLE;
-			}
-			return NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
+			/* We treat an empty string as no config for compatibility. */
+			return *priv->config ? FALSE : NM_SETTING_VERIFY_NORMALIZABLE;
 		}
 	}
 
diff --git a/libnm-core/nm-utils-private.h b/libnm-core/nm-utils-private.h
index bd54756ac3..fab3803808 100644
--- a/libnm-core/nm-utils-private.h
+++ b/libnm-core/nm-utils-private.h
@@ -31,9 +31,6 @@
 gboolean    _nm_utils_string_slist_validate (GSList *list,
                                              const char **valid_values);
 
-gboolean    _nm_utils_check_valid_json  (const char *json, GError **error);
-gboolean    _nm_utils_team_config_equal (const char *conf1, const char *conf2, gboolean port);
-
 /* D-Bus transform funcs */
 
 GVariant   *_nm_utils_hwaddr_cloned_get (NMSetting     *setting,
diff --git a/libnm-core/tests/test-keyfile.c b/libnm-core/tests/test-keyfile.c
index ecb7cd5a67..4ec6b806cd 100644
--- a/libnm-core/tests/test-keyfile.c
+++ b/libnm-core/tests/test-keyfile.c
@@ -27,6 +27,7 @@
 #include "nm-setting-connection.h"
 #include "nm-setting-wired.h"
 #include "nm-setting-8021x.h"
+#include "nm-setting-team.h"
 
 #include "nm-utils/nm-test-utils.h"
 
@@ -518,6 +519,52 @@ test_8021x_cert_read (void)
 	CLEAR (&con, &keyfile);
 }
 
+static void
+test_team_conf_read_valid (void)
+{
+	GKeyFile *keyfile = NULL;
+	gs_unref_object NMConnection *con = NULL;
+	NMSettingTeam *s_team;
+
+	con = nmtst_create_connection_from_keyfile (
+	      "[connection]\n"
+	      "type=team\n"
+	      "interface-name=nm-team1\n"
+	      "[team]\n"
+	      "config={\"foo\":\"bar\"}",
+	      "/test_team_conf_read/valid", NULL);
+
+	g_assert (con);
+	s_team = nm_connection_get_setting_team (con);
+	g_assert (s_team);
+	g_assert_cmpstr (nm_setting_team_get_config (s_team), ==, "{\"foo\":\"bar\"}");
+
+	CLEAR (&con, &keyfile);
+}
+
+static void
+test_team_conf_read_invalid (void)
+{
+	GKeyFile *keyfile = NULL;
+	gs_unref_object NMConnection *con = NULL;
+	NMSettingTeam *s_team;
+
+	con = nmtst_create_connection_from_keyfile (
+	      "[connection]\n"
+	      "type=team\n"
+	      "interface-name=nm-team1\n"
+	      "[team]\n"
+	      "config={foobar}",
+	      "/test_team_conf_read/invalid", NULL);
+
+	g_assert (con);
+	s_team = nm_connection_get_setting_team (con);
+	g_assert (s_team);
+	g_assert (nm_setting_team_get_config (s_team) == NULL);
+
+	CLEAR (&con, &keyfile);
+}
+
 /******************************************************************************/
 
 NMTST_DEFINE ();
@@ -528,6 +575,8 @@ int main (int argc, char **argv)
 
 	g_test_add_func ("/core/keyfile/test_8021x_cert", test_8021x_cert);
 	g_test_add_func ("/core/keyfile/test_8021x_cert_read", test_8021x_cert_read);
+	g_test_add_func ("/core/keyfile/test_team_conf_read/valid", test_team_conf_read_valid);
+	g_test_add_func ("/core/keyfile/test_team_conf_read/invalid", test_team_conf_read_invalid);
 
 	return g_test_run ();
 }
diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
index d1ea597a68..afc9238dc1 100644
--- a/src/settings/plugins/ifcfg-rh/reader.c
+++ b/src/settings/plugins/ifcfg-rh/reader.c
@@ -4238,6 +4238,7 @@ bond_connection_from_ifcfg (const char *file,
 static char *
 read_team_config (shvarFile *ifcfg, const char *key, GError **error)
 {
+	gs_free_error GError *local_error = NULL;
 	char *value;
 	size_t l;
 
@@ -4258,6 +4259,12 @@ read_team_config (shvarFile *ifcfg, const char *key, GError **error)
 		return NULL;
 	}
 	svUnescape (value);
+
+	if (value && value[0] && !_nm_utils_check_valid_json (value, &local_error)) {
+		PARSE_WARNING ("ignoring invalid team configuration: %s", local_error->message);
+		g_clear_pointer (&value, g_free);
+	}
+
 	return value;
 }
 
diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
index 7531ec0388..de9c1ed73f 100644
--- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
@@ -134,6 +134,7 @@ EXTRA_DIST = \
 	ifcfg-test-fcoe-vn2vn \
 	ifcfg-test-team-master-1 \
 	ifcfg-test-team-master-2 \
+	ifcfg-test-team-master-invalid \
 	ifcfg-test-team-port-1 \
 	ifcfg-test-team-port-2 \
 	ifcfg-test-team-port-empty-config \
diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-invalid b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-invalid
new file mode 100644
index 0000000000..4534882eea
--- /dev/null
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-invalid
@@ -0,0 +1,4 @@
+DEVICE=team0
+ONBOOT=no
+BOOTPROTO=dhcp
+TEAM_CONFIG="{ foobar }"
diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
index 3e3f7ca4f5..9ed48bebe0 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -8454,6 +8454,31 @@ test_read_team_master (gconstpointer user_data)
 }
 
 static void
+test_read_team_master_invalid (gconstpointer user_data)
+{
+	const char *const PATH_NAME = user_data;
+	NMConnection *connection;
+	NMSettingConnection *s_con;
+	NMSettingTeam *s_team;
+
+	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_MESSAGE, "*ignoring invalid team configuration*");
+	connection = _connection_from_file (PATH_NAME, NULL, TYPE_ETHERNET, NULL);
+	g_test_assert_expected_messages ();
+
+	g_assert_cmpstr (nm_connection_get_interface_name (connection), ==, "team0");
+
+	s_con = nm_connection_get_setting_connection (connection);
+	g_assert (s_con);
+	g_assert_cmpstr (nm_setting_connection_get_connection_type (s_con), ==, NM_SETTING_TEAM_SETTING_NAME);
+
+	s_team = nm_connection_get_setting_team (connection);
+	g_assert (s_team);
+	g_assert (nm_setting_team_get_config (s_team) == NULL);
+
+	g_object_unref (connection);
+}
+
+static void
 test_write_team_master (void)
 {
 	NMConnection *connection, *reread;
@@ -9050,6 +9075,7 @@ int main (int argc, char **argv)
 
 	g_test_add_data_func (TPATH "team/read-master-1", TEST_IFCFG_DIR"/network-scripts/ifcfg-test-team-master-1", test_read_team_master);
 	g_test_add_data_func (TPATH "team/read-master-2", TEST_IFCFG_DIR"/network-scripts/ifcfg-test-team-master-2", test_read_team_master);
+	g_test_add_data_func (TPATH "team/read-master-invalid", TEST_IFCFG_DIR"/network-scripts/ifcfg-test-team-master-invalid", test_read_team_master_invalid);
 	g_test_add_func (TPATH "team/write-master", test_write_team_master);
 	g_test_add_data_func (TPATH "team/read-port-1", TEST_IFCFG_DIR"/network-scripts/ifcfg-test-team-port-1", test_read_team_port);
 	g_test_add_data_func (TPATH "team/read-port-2", TEST_IFCFG_DIR"/network-scripts/ifcfg-test-team-port-2", test_read_team_port);