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.
This commit is contained in:
Thomas Haller 2020-03-26 12:41:13 +01:00
parent 8280c85fac
commit 64da830b07
2 changed files with 14 additions and 8 deletions

View file

@ -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),

View file

@ -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");