libnm: don't assert against valid s390-option keys in nm_setting_wired_add_s390_option()

Asserting against user input is not nice, because it always requires the
caller to check the value first. Don't do that.

Also, don't even check. You can set NM_SETTING_WIRED_S390_OPTIONS
property to any values (except duplicated keys). The C add function
should not be more limited than that. This is also right because
we have verify() which checks for valid settings. And it does so beyond
only checking the keys.

So you could set NM_SETTING_WIRED_S390_OPTIONS properties to invalid
keys. And you could use nm_setting_wired_add_s390_option() to set
invalid values. No need to let nm_setting_wired_add_s390_option() check
for valid keys.

3 files changed, 17 insertions, 19 deletions

diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index 9100beb8ac..07334dbb77 100644
--- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -5118,15 +5118,15 @@ make_wired_setting(shvarFile *ifcfg, const char *file, NMSetting8021x **s_8021x,
         for (i = 0; options && options[i]; i++) {
             const char *line = options[i];
             const char *equals;
-            gboolean    valid = FALSE;
 
             equals = strchr(line, '=');
-            if (equals) {
-                ((char *) equals)[0] = '\0';
-                valid                = nm_setting_wired_add_s390_option(s_wired, line, equals + 1);
-            }
-            if (!valid)
-                PARSE_WARNING("invalid s390 OPTION '%s'", line);
+            if (!equals)
+                continue;
+
+            /* Here we don't verify the key/value further. If the file contains invalid keys,
+             * we will later reject the connection as invalid. */
+            ((char *) equals)[0] = '\0';
+            nm_setting_wired_add_s390_option(s_wired, line, equals + 1);
         }
         found = TRUE;
     }
diff --git a/src/libnm-core-impl/nm-keyfile.c b/src/libnm-core-impl/nm-keyfile.c
index 1de18180a3..bc5398e7ba 100644
--- a/src/libnm-core-impl/nm-keyfile.c
+++ b/src/libnm-core-impl/nm-keyfile.c
@@ -2286,6 +2286,8 @@ wired_s390_options_parser_full(KeyfileReaderInfo *       info,
         if (!value)
             continue;
 
+        /* Here we don't verify the key/value further. If the file contains invalid keys,
+         * we will later reject the connection as invalid. */
         nm_setting_wired_add_s390_option(s_wired,
                                          nm_keyfile_key_decode(keys[i], &key_to_free),
                                          value);
diff --git a/src/libnm-core-impl/nm-setting-wired.c b/src/libnm-core-impl/nm-setting-wired.c
index bcbf064020..ca310fee7c 100644
--- a/src/libnm-core-impl/nm-setting-wired.c
+++ b/src/libnm-core-impl/nm-setting-wired.c
@@ -524,7 +524,7 @@ nm_setting_wired_get_s390_option_by_key(NMSettingWired *setting, const char *key
     gssize                 idx;
 
     g_return_val_if_fail(NM_IS_SETTING_WIRED(setting), NULL);
-    g_return_val_if_fail(key && key[0], NULL);
+    g_return_val_if_fail(key, NULL);
 
     priv = NM_SETTING_WIRED_GET_PRIVATE(setting);
 
@@ -540,13 +540,13 @@ nm_setting_wired_get_s390_option_by_key(NMSettingWired *setting, const char *key
  * @key: key name for the option
  * @value: value for the option
  *
- * Add an option to the table.  The option is compared to an internal list
- * of allowed options.  Key names may contain only alphanumeric characters
- * (ie [a-zA-Z0-9]).  Adding a new key replaces any existing key/value pair that
- * may already exist.
+ * Add an option to the table. If the key already exists, the value gets
+ * replaced.
  *
- * Returns: %TRUE if the option was valid and was added to the internal option
- * list, %FALSE if it was not.
+ * Before 1.32, the function would assert that the key is valid. Since then,
+ * an invalid key gets silently added but renders the profile as invalid.
+ *
+ * Returns: since 1.32 this always returns %TRUE.
  **/
 gboolean
 nm_setting_wired_add_s390_option(NMSettingWired *setting, const char *key, const char *value)
@@ -555,13 +555,9 @@ nm_setting_wired_add_s390_option(NMSettingWired *setting, const char *key, const
     gssize                 idx;
 
     g_return_val_if_fail(NM_IS_SETTING_WIRED(setting), FALSE);
+    g_return_val_if_fail(key, FALSE);
     g_return_val_if_fail(value, FALSE);
 
-    if (!valid_s390_opts_check(key)) {
-        g_return_val_if_fail(key, FALSE);
-        return FALSE;
-    }
-
     priv = NM_SETTING_WIRED_GET_PRIVATE(setting);
 
     idx = nm_utils_named_value_list_find(priv->s390_options.arr, priv->s390_options.len, key, TRUE);