mirror of
https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git
synced 2025-12-26 17:00:08 +01:00
290e515311
For historic reasons is NMSettingBond implemented differently from other settings. It uses a strdict, and adds some validation on top of that. The idea was probably to be able to treat bond options more generically. But in practice we cannot treat them as opaque values, but need to know, validate and understand all the options. Thus, this implementation with a strdict is not nice. The user can set the GObject property NM_SETTING_BOND_OPTIONS to any strdict, and the setter performs no validation or normalization. That is probably good, because g_object_set() cannot return an error to signalize invalid settings. As often, we have corresponding C API like nm_setting_bond_add_option() and nm_setting_bond_remove_option(). It should be possible to get the same result both with the C API and with the GObject property setting. Since there is already a way to set certain invalid values, it does not help if the C API tries to prevent that. That implies, that also add-option does not perform additional validation and sets whatever the user asks. Remove all validation from nm_setting_bond_add_option() and nm_setting_bond_remove_option(). This validation was anyway only very basic. It was calling nm_setting_bond_validate_option(), which can check whether the string is (for example) and integer, but it cannot do validation beyond one option. In most cases, the validation needs to take into account the bond mode or other options, so validating one option in isolation is not very useful. Proper validation should instead be done via nm_connection_verify(). However, due to another historic oddity, that verification is very forgiving too and doesn't reject many invalid settings when it should. That is hard to fix, because making validation more strict can break existing (and working) configurations. However, verify() already contains basic validation via nm_setting_bond_validate_option(). So in the previous behavior nm_setting_bond_add_option() would silently do nothing (only returning %FALSE) for invalid options, while now it would add the invalid options to the dictionary -- only to have it later fail validation during nm_connection_verify(). That is a slight change in behavior, however it seems preferable. It seems preferable and acceptable because most users that call nm_setting_bond_add_option() already understand the meaning and valid values. Keyfile and ifcfg-rh readers are the few exceptions, which really just parse a string dictionary, without need to understand them. But nmtui or nmstate already know the option they want to set. They don't expect a failure there, nor do they need the validation. Note that this change in behavior could be dangerous for example for the keyfile/ifcfg-rh readers, which silently ignored errors before. We don't want them to start failing if they read invalid options from a file, so instead let those callers explicitly pre-validate the value and log an warning. https://bugzilla.redhat.com/show_bug.cgi?id=1887523 |
||
|---|---|---|
| .. | ||
| tests | ||
| meson.build | ||
| nm-client-utils.c | ||
| nm-client-utils.h | ||
| nm-meta-setting-access.c | ||
| nm-meta-setting-access.h | ||
| nm-meta-setting-desc.c | ||
| nm-meta-setting-desc.h | ||
| nm-polkit-listener.c | ||
| nm-polkit-listener.h | ||
| nm-secret-agent-simple.c | ||
| nm-secret-agent-simple.h | ||
| nm-vpn-helpers.c | ||
| nm-vpn-helpers.h | ||
| qrcodegen.c | ||
| qrcodegen.h | ||
| settings-docs.h.in | ||
| settings-docs.xsl | ||