colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libnm: refactor "ipv6" argument of _nm_utils_dns_option_validate()

Browse Source 
_nm_utils_dns_option_validate() allows specifying the address family,
and filters based on that. Note that all options are valid for IPv6,
but some are not valid for IPv4.

It's not obvious, that such filtering is only performed if
"option_descs" argument is provied. Otherwise, the "ipv6" argument is
ignored.

Regardless, it's also confusing to have a boolean "ipv6". When most
callers don't want a filtering based on the address family. They
actually don't want any filtering at all, as they don't pass an
"option_descs". At the same time passing a TRUE/FALSE "ipv6" is
redundant and ignored. It should be possible, to explicitly not select
an address family (as it's ignored anyway).

Instead, make the "gboolean ipv6" argument an "int addr_family".
Selecting AF_UNSPEC means clearly to accept any address family.
This commit is contained in:
Thomas Haller
2023-11-15 19:53:44 +01:00
parent 405a2fa166
commit 3f8431f069
5 changed files with 68 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/core/nm-config-data.c
View File

@@ -1219,7 +1219,7 @@ load_global_dns(GKeyFile *keyfile, gboolean internal)
if (strv) { if (strv) {
nm_strv_cleanup(strv, TRUE, TRUE, TRUE); nm_strv_cleanup(strv, TRUE, TRUE, TRUE);
for (i = 0, j = 0; strv[i]; i++) { for (i = 0, j = 0; strv[i]; i++) {
if (_nm_utils_dns_option_validate(strv[i], NULL, NULL, TRUE, NULL)) if (_nm_utils_dns_option_validate(strv[i], NULL, NULL, AF_UNSPEC, NULL))
strv[j++] = strv[i]; strv[j++] = strv[i];
else else
g_free(strv[i]); g_free(strv[i]);
@@ -1453,7 +1453,7 @@ nm_global_dns_config_from_dbus(const GValue *value, GError **error)
nm_strv_cleanup(strv, TRUE, TRUE, TRUE); nm_strv_cleanup(strv, TRUE, TRUE, TRUE);
for (i = 0, j = 0; strv && strv[i]; i++) { for (i = 0, j = 0; strv && strv[i]; i++) {
if (_nm_utils_dns_option_validate(strv[i], NULL, NULL, TRUE, NULL)) if (_nm_utils_dns_option_validate(strv[i], NULL, NULL, AF_UNSPEC, NULL))
strv[j++] = strv[i]; strv[j++] = strv[i];
else else
g_free(strv[i]); g_free(strv[i]);

6
src/libnm-core-impl/nm-setting-ip-config.c
View File

@@ -4434,7 +4434,7 @@ nm_setting_ip_config_next_valid_dns_option(NMSettingIPConfig *setting, guint idx
if (_nm_utils_dns_option_validate(priv->dns_options->pdata[idx], if (_nm_utils_dns_option_validate(priv->dns_options->pdata[idx],
NULL, NULL,
NULL, NULL,
NM_IS_SETTING_IP6_CONFIG(setting), NM_SETTING_IP_CONFIG_GET_ADDR_FAMILY(setting),
_nm_utils_dns_option_descs)) _nm_utils_dns_option_descs))
return idx; return idx;
} }
@@ -4462,7 +4462,7 @@ nm_setting_ip_config_add_dns_option(NMSettingIPConfig *setting, const char *dns_
g_return_val_if_fail(dns_option != NULL, FALSE); g_return_val_if_fail(dns_option != NULL, FALSE);
g_return_val_if_fail(dns_option[0] != '\0', FALSE); g_return_val_if_fail(dns_option[0] != '\0', FALSE);
if (!_nm_utils_dns_option_validate(dns_option, NULL, NULL, FALSE, NULL)) if (!_nm_utils_dns_option_validate(dns_option, NULL, NULL, AF_UNSPEC, NULL))
return FALSE; return FALSE;
priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting);
@@ -6208,7 +6208,7 @@ set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *ps
else else
priv->dns_options = g_ptr_array_new_with_free_func(g_free); priv->dns_options = g_ptr_array_new_with_free_func(g_free);
for (i = 0; strv[i]; i++) { for (i = 0; strv[i]; i++) {
if (_nm_utils_dns_option_validate(strv[i], NULL, NULL, FALSE, NULL) if (_nm_utils_dns_option_validate(strv[i], NULL, NULL, AF_UNSPEC, NULL)
&& _nm_utils_dns_option_find_idx(priv->dns_options, strv[i]) < 0) && _nm_utils_dns_option_find_idx(priv->dns_options, strv[i]) < 0)
g_ptr_array_add(priv->dns_options, g_strdup(strv[i])); g_ptr_array_add(priv->dns_options, g_strdup(strv[i]));
} }

29
src/libnm-core-impl/nm-utils.c
View File

@@ -4694,7 +4694,7 @@ _nm_utils_strstrdictkey_create(const char *v1, const char *v2)
static gboolean static gboolean
validate_dns_option(const char *name, validate_dns_option(const char *name,
gboolean numeric, gboolean numeric,
gboolean ipv6, int addr_family,
const NMUtilsDNSOptionDesc *option_descs) const NMUtilsDNSOptionDesc *option_descs)
{ {
const NMUtilsDNSOptionDesc *desc; const NMUtilsDNSOptionDesc *desc;
@@ -4703,8 +4703,15 @@ validate_dns_option(const char *name,
return !!*name; return !!*name;
for (desc = option_descs; desc->name; desc++) { for (desc = option_descs; desc->name; desc++) {
if (nm_streq(name, desc->name) && numeric == desc->numeric && (!desc->ipv6_only || ipv6)) if (!nm_streq(name, desc->name))
return TRUE; continue;
if ((!!numeric) != (!!desc->numeric))
continue;
if (addr_family != AF_UNSPEC) {
if (desc->ipv6_only && addr_family != AF_INET6)
continue;
}
return TRUE;
} }
return FALSE; return FALSE;
@@ -4715,7 +4722,9 @@ validate_dns_option(const char *name,
* @option: option string * @option: option string
* @out_name: (out) (optional) (nullable): the option name * @out_name: (out) (optional) (nullable): the option name
* @out_value: (out) (optional): the option value * @out_value: (out) (optional): the option value
* @ipv6: whether the option refers to a IPv6 configuration * @addr_family: AF_INET/AF_INET6 to only allow options for the specified address
* family. AF_UNSPEC to allow either. This argument is ignored, if @option_descs
* is NULL.
* @option_descs: (nullable): an array of NMUtilsDNSOptionDesc which describes the * @option_descs: (nullable): an array of NMUtilsDNSOptionDesc which describes the
* valid options * valid options
* *
@@ -4731,7 +4740,7 @@ gboolean
_nm_utils_dns_option_validate(const char *option, _nm_utils_dns_option_validate(const char *option,
char **out_name, char **out_name,
long *out_value, long *out_value,
gboolean ipv6, int addr_family,
const NMUtilsDNSOptionDesc *option_descs) const NMUtilsDNSOptionDesc *option_descs)
{ {
gs_free char *option0_free = NULL; gs_free char *option0_free = NULL;
@@ -4742,6 +4751,8 @@ _nm_utils_dns_option_validate(const char *option,
g_return_val_if_fail(option != NULL, FALSE); g_return_val_if_fail(option != NULL, FALSE);
nm_assert_addr_family_or_unspec(addr_family);
NM_SET_OUT(out_name, NULL); NM_SET_OUT(out_name, NULL);
NM_SET_OUT(out_value, -1); NM_SET_OUT(out_value, -1);
@@ -4750,7 +4761,7 @@ _nm_utils_dns_option_validate(const char *option,
delim = strchr(option, ':'); delim = strchr(option, ':');
if (!delim) { if (!delim) {
if (!validate_dns_option(option, FALSE, ipv6, option_descs)) if (!validate_dns_option(option, FALSE, addr_family, option_descs))
return FALSE; return FALSE;
NM_SET_OUT(out_name, g_strdup(option)); NM_SET_OUT(out_name, g_strdup(option));
return TRUE; return TRUE;
@@ -4765,7 +4776,7 @@ _nm_utils_dns_option_validate(const char *option,
option0 = nm_strndup_a(300, option, delim - option, &option0_free); option0 = nm_strndup_a(300, option, delim - option, &option0_free);
if (!validate_dns_option(option0, TRUE, ipv6, option_descs)) if (!validate_dns_option(option0, TRUE, addr_family, option_descs))
return FALSE; return FALSE;
option1_num = _nm_utils_ascii_str_to_int64(option1, 10, 0, G_MAXINT32, -1); option1_num = _nm_utils_ascii_str_to_int64(option1, 10, 0, G_MAXINT32, -1);
@@ -4794,13 +4805,13 @@ _nm_utils_dns_option_find_idx(GPtrArray *array, const char *option)
gs_free char *option_name = NULL; gs_free char *option_name = NULL;
guint i; guint i;
if (!_nm_utils_dns_option_validate(option, &option_name, NULL, FALSE, NULL)) if (!_nm_utils_dns_option_validate(option, &option_name, NULL, AF_UNSPEC, NULL))
return -1; return -1;
for (i = 0; i < array->len; i++) { for (i = 0; i < array->len; i++) {
gs_free char *tmp_name = NULL; gs_free char *tmp_name = NULL;
if (_nm_utils_dns_option_validate(array->pdata[i], &tmp_name, NULL, FALSE, NULL)) { if (_nm_utils_dns_option_validate(array->pdata[i], &tmp_name, NULL, AF_UNSPEC, NULL)) {
if (nm_streq(tmp_name, option_name)) if (nm_streq(tmp_name, option_name))
return i; return i;
} }

72
src/libnm-core-impl/tests/test-general.c
View File

@@ -8803,23 +8803,35 @@ test_nm_ptrarray_len(void)
static void static void
test_nm_utils_dns_option_validate_do(char *option, test_nm_utils_dns_option_validate_do(char *option,
gboolean ipv6, int addr_family,
const NMUtilsDNSOptionDesc *descs, const NMUtilsDNSOptionDesc *descs,
gboolean exp_result, gboolean exp_result,
char *exp_name, char *exp_name,
gboolean exp_value) gboolean exp_value)
{ {
char *name; gs_free char *name = NULL;
long value = 0; long value = 0;
gboolean result; gboolean result;
result = _nm_utils_dns_option_validate(option, &name, &value, ipv6, descs); if (!descs) {
g_assert(addr_family == AF_UNSPEC);
addr_family = nmtst_rand_select(AF_UNSPEC, AF_INET, AF_INET6);
}
result = _nm_utils_dns_option_validate(option, &name, &value, addr_family, descs);
g_assert(result == exp_result); g_assert(result == exp_result);
g_assert_cmpstr(name, ==, exp_name); g_assert_cmpstr(name, ==, exp_name);
g_assert(value == exp_value); g_assert(value == exp_value);
g_free(name); nm_clear_g_free(&name);
if (result && descs) {
result = _nm_utils_dns_option_validate(option, &name, &value, AF_UNSPEC, descs);
g_assert(result == exp_result);
g_assert_cmpstr(name, ==, exp_name);
g_assert(value == exp_value);
}
} }
static const NMUtilsDNSOptionDesc opt_descs[] = { static const NMUtilsDNSOptionDesc opt_descs[] = {
@@ -8833,34 +8845,34 @@ static const NMUtilsDNSOptionDesc opt_descs[] = {
static void static void
test_nm_utils_dns_option_validate(void) test_nm_utils_dns_option_validate(void)
{ {
/* opt ipv6 descs result name value */ /* (opt, addr_family, descs, result, name, value) */
test_nm_utils_dns_option_validate_do("", FALSE, NULL, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do("", AF_UNSPEC, NULL, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do(":", FALSE, NULL, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do(":", AF_UNSPEC, NULL, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do(":1", FALSE, NULL, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do(":1", AF_UNSPEC, NULL, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do(":val", FALSE, NULL, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do(":val", AF_UNSPEC, NULL, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do("opt", FALSE, NULL, TRUE, "opt", -1); test_nm_utils_dns_option_validate_do("opt", AF_UNSPEC, NULL, TRUE, "opt", -1);
test_nm_utils_dns_option_validate_do("opt:", FALSE, NULL, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do("opt:", AF_UNSPEC, NULL, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do("opt:12", FALSE, NULL, TRUE, "opt", 12); test_nm_utils_dns_option_validate_do("opt:12", AF_UNSPEC, NULL, TRUE, "opt", 12);
test_nm_utils_dns_option_validate_do("opt:12 ", FALSE, NULL, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do("opt:12 ", AF_UNSPEC, NULL, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do("opt:val", FALSE, NULL, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do("opt:val", AF_UNSPEC, NULL, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do("opt:2val", FALSE, NULL, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do("opt:2val", AF_UNSPEC, NULL, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do("opt:2:3", FALSE, NULL, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do("opt:2:3", AF_UNSPEC, NULL, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do("opt-6", FALSE, NULL, TRUE, "opt-6", -1); test_nm_utils_dns_option_validate_do("opt-6", AF_UNSPEC, NULL, TRUE, "opt-6", -1);
test_nm_utils_dns_option_validate_do("opt1", FALSE, opt_descs, TRUE, "opt1", -1); test_nm_utils_dns_option_validate_do("opt1", AF_INET, opt_descs, TRUE, "opt1", -1);
test_nm_utils_dns_option_validate_do("opt1", TRUE, opt_descs, TRUE, "opt1", -1); test_nm_utils_dns_option_validate_do("opt1", AF_INET6, opt_descs, TRUE, "opt1", -1);
test_nm_utils_dns_option_validate_do("opt1:3", FALSE, opt_descs, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do("opt1:3", AF_INET, opt_descs, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do("opt2", FALSE, opt_descs, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do("opt2", AF_INET, opt_descs, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do("opt2:5", FALSE, opt_descs, TRUE, "opt2", 5); test_nm_utils_dns_option_validate_do("opt2:5", AF_INET, opt_descs, TRUE, "opt2", 5);
test_nm_utils_dns_option_validate_do("opt3", FALSE, opt_descs, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do("opt3", AF_INET, opt_descs, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do("opt3", TRUE, opt_descs, TRUE, "opt3", -1); test_nm_utils_dns_option_validate_do("opt3", AF_INET6, opt_descs, TRUE, "opt3", -1);
test_nm_utils_dns_option_validate_do("opt4", FALSE, opt_descs, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do("opt4", AF_INET, opt_descs, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do("opt4", TRUE, opt_descs, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do("opt4", AF_INET6, opt_descs, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do("opt4:40", FALSE, opt_descs, FALSE, NULL, -1); test_nm_utils_dns_option_validate_do("opt4:40", AF_INET, opt_descs, FALSE, NULL, -1);
test_nm_utils_dns_option_validate_do("opt4:40", TRUE, opt_descs, TRUE, "opt4", 40); test_nm_utils_dns_option_validate_do("opt4:40", AF_INET6, opt_descs, TRUE, "opt4", 40);
} }
static void static void

2
src/libnm-core-intern/nm-core-internal.h
View File

@@ -411,7 +411,7 @@ extern const NMUtilsDNSOptionDesc _nm_utils_dns_option_descs[];
gboolean _nm_utils_dns_option_validate(const char *option, gboolean _nm_utils_dns_option_validate(const char *option,
char **out_name, char **out_name,
long *out_value, long *out_value,
gboolean ipv6, int addr_family,
const NMUtilsDNSOptionDesc *option_descs); const NMUtilsDNSOptionDesc *option_descs);
gssize _nm_utils_dns_option_find_idx(GPtrArray *array, const char *option); gssize _nm_utils_dns_option_find_idx(GPtrArray *array, const char *option);