libnm: allow setting empty vpn.data item

Until now, nm_setting_vpn_add_data_item() would reject empty data values.
This leads for example to an assertion failure, if you write a keyfile
that assigns an empty value to a key. Keyfile reader would not check that
the value is non-empty before calling nm_setting_vpn_add_data_item().

Anyway, I think we should not require having non-empty data elements. It's
an unnecessary and sometimes harmful restriction. NetworkManager doesn't understand
not care about the content of the vpn data. That is up the VPN plugins. Sometimes
and empty value may be desirable.

Also, the NM_SETTING_VPN_DATA property setter wouldn't filter out empty
values either. So it was always possible to use some libnm API to set data
with empty values. The restriction in nm_setting_vpn_add_data_item() was
inconsistent.

2 files changed, 14 insertions, 8 deletions

diff --git a/libnm-core/nm-setting-vpn.c b/libnm-core/nm-setting-vpn.c
index 30d67b1ea4..cf9cbd1d91 100644
--- a/libnm-core/nm-setting-vpn.c
+++ b/libnm-core/nm-setting-vpn.c
@@ -163,20 +163,28 @@ nm_setting_vpn_get_num_data_items (NMSettingVpn *setting)
  * nm_setting_vpn_add_data_item:
  * @setting: the #NMSettingVpn
  * @key: a name that uniquely identifies the given value @item
- * @item: the value to be referenced by @key
+ * @item: (allow-none): the value to be referenced by @key
  *
  * Establishes a relationship between @key and @item internally in the
  * setting which may be retrieved later.  Should not be used to store passwords
  * or other secrets, which is what nm_setting_vpn_add_secret() is for.
+ *
+ * Before 1.24, @item must not be %NULL and not an empty string. Since 1.24,
+ * @item can be set to an empty string. It can also be set to %NULL to unset
+ * the key. In that case, the behavior is as if calling nm_setting_vpn_remove_data_item().
  **/
 void
 nm_setting_vpn_add_data_item (NMSettingVpn *setting,
                               const char *key,
                               const char *item)
 {
+	if (!item) {
+		nm_setting_vpn_remove_data_item (setting, key);
+		return;
+	}
+
 	g_return_if_fail (NM_IS_SETTING_VPN (setting));
 	g_return_if_fail (key && key[0]);
-	g_return_if_fail (item && item[0]);
 
 	g_hash_table_insert (_ensure_strdict (&NM_SETTING_VPN_GET_PRIVATE (setting)->data, FALSE),
 	                     g_strdup (key),
diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c
index e0d0d55fa2..5d3494d869 100644
--- a/libnm-core/tests/test-general.c
+++ b/libnm-core/tests/test-general.c
@@ -1172,13 +1172,11 @@ test_setting_vpn_items (void)
 	nm_setting_vpn_add_data_item (s_vpn, "", "");
 	g_test_assert_expected_messages ();
 
-	NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (item && item[0]));
-	nm_setting_vpn_add_data_item (s_vpn, "foobar1", NULL);
-	g_test_assert_expected_messages ();
-
-	NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (item && item[0]));
 	nm_setting_vpn_add_data_item (s_vpn, "foobar1", "");
-	g_test_assert_expected_messages ();
+	g_assert_cmpstr (nm_setting_vpn_get_data_item (s_vpn, "foobar1"), ==, "");
+
+	nm_setting_vpn_add_data_item (s_vpn, "foobar1", NULL);
+	g_assert_cmpstr (nm_setting_vpn_get_data_item (s_vpn, "foobar1"), ==, NULL);
 
 	NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key && key[0]));
 	nm_setting_vpn_add_data_item (s_vpn, NULL, "blahblah1");