From 91ed0449e96051cde51625c8cacecfd5760abd31 Mon Sep 17 00:00:00 2001

From: Beniamino Galvani <bgalvani@redhat.com>

Date: Tue, 30 Aug 2016 15:22:04 +0200

Subject: [PATCH 1/3] Revert "libnm-core/team: normalize invalid config to

 NULL"

It's better to fail the validation of any invalid configuration

instead of silently ignoring it.

This reverts commit 476810c29016d569ac3885542a6c91e7af8a7f6d.

(cherry picked from commit 39ad134b0ca1918084b2b1fc5856cc0b7a6becfc)

(cherry picked from commit 4f5059306be98748d2bede9cb5f011e889c81805)

---

 libnm-core/nm-connection.c        |  5 ++---

 libnm-core/nm-setting-team-port.c | 12 ++----------

 libnm-core/nm-setting-team.c      | 12 ++----------

 3 files changed, 6 insertions(+), 23 deletions(-)

diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c

index f39d41c..57f9640 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-setting-team-port.c b/libnm-core/nm-setting-team-port.c

index 0d175d5..123304f 100644

--- a/libnm-core/nm-setting-team-port.c

+++ b/libnm-core/nm-setting-team-port.c

@@ -122,19 +122,11 @@ 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;

 		}

 	}

-	/* NOTE: normalizable/normalizable-errors must appear at the end with decreasing severity.

-	 * Take care to properly order statements with priv->config above. */

-

 	return TRUE;

 }

diff --git a/libnm-core/nm-setting-team.c b/libnm-core/nm-setting-team.c

index a559e0d..df89694 100644

--- a/libnm-core/nm-setting-team.c

+++ b/libnm-core/nm-setting-team.c

@@ -94,19 +94,11 @@ 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;

 		}

 	}

-	/* NOTE: normalizable/normalizable-errors must appear at the end with decreasing severity.

-	 * Take care to properly order statements with priv->config above. */

-

 	return TRUE;

 }

-- 

2.5.5

From f1e9f50ebe20b32a3f5903b6b07c437c23bd5ac1 Mon Sep 17 00:00:00 2001

From: Beniamino Galvani <bgalvani@redhat.com>

Date: Tue, 30 Aug 2016 15:21:16 +0200

Subject: [PATCH 2/3] team: normalize invalid configuration during load

Now that we validate the JSON syntax of a team/team-port

configuration, any existing connection with invalid JSON configuration

would fail to load and disappear upon upgrade. Instead, modify the

setting plugins to emit a warning but still load the connection with

empty configuration.

(cherry picked from commit d6ec009afd7dadf925d5bb8e5dd855c493dd0104)

(cherry picked from commit 67f064f11ba3858ea9429f0989b81feeaaa9d8bb)

---

 libnm-core/nm-core-internal.h                      |  5 +++

 libnm-core/nm-keyfile-reader.c                     | 26 ++++++++++++

 libnm-core/nm-utils-private.h                      |  3 --

 libnm-core/tests/test-keyfile.c                    | 49 ++++++++++++++++++++++

 src/settings/plugins/ifcfg-rh/reader.c             |  7 ++++

 .../ifcfg-rh/tests/network-scripts/Makefile.am     |  1 +

 .../network-scripts/ifcfg-test-team-master-invalid |  4 ++

 .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c         | 26 ++++++++++++

 8 files changed, 118 insertions(+), 3 deletions(-)

 create mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-invalid

diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h

index 94e473d..e491bce 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 54a694e..16f0c22 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-utils-private.h b/libnm-core/nm-utils-private.h

index bd54756..fab3803 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 ecb7cd5..4ec6b80 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 d1ea597..afc9238 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 7531ec0..de9c1ed 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 0000000..4534882

--- /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 3e3f7ca..9ed48be 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);

-- 

2.5.5

From 875d750a9329e4478f3dec107d77041d51a12742 Mon Sep 17 00:00:00 2001

From: Beniamino Galvani <bgalvani@redhat.com>

Date: Tue, 30 Aug 2016 16:37:54 +0200

Subject: [PATCH 3/3] libnm: restore verify() comments in team/team-port

 settings

Restore the comments removed in commit

a524091966afb884cdb8db48067d5599a685a8eb.

(cherry picked from commit d06279f3db794f5c7177d85b4dd5e732d6a90364)

(cherry picked from commit bf422e972abe9c14b385401ad72b01f3fcef8d40)

---

 libnm-core/nm-setting-team-port.c | 3 +++

 libnm-core/nm-setting-team.c      | 3 +++

 2 files changed, 6 insertions(+)

diff --git a/libnm-core/nm-setting-team-port.c b/libnm-core/nm-setting-team-port.c

index 123304f..e3ddbda 100644

--- a/libnm-core/nm-setting-team-port.c

+++ b/libnm-core/nm-setting-team-port.c

@@ -127,6 +127,9 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)

 		}

 	}

+	/* NOTE: normalizable/normalizable-errors must appear at the end with decreasing severity.

+	 * Take care to properly order statements with priv->config above. */

+

 	return TRUE;

 }

diff --git a/libnm-core/nm-setting-team.c b/libnm-core/nm-setting-team.c

index df89694..5bf11ed 100644

--- a/libnm-core/nm-setting-team.c

+++ b/libnm-core/nm-setting-team.c

@@ -99,6 +99,9 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)

 		}

 	}

+	/* NOTE: normalizable/normalizable-errors must appear at the end with decreasing severity.

+	 * Take care to properly order statements with priv->config above. */

+

 	return TRUE;

 }

-- 

2.5.5