From: Lennart Poettering Date: Tue, 14 Jul 2026 12:40:14 +0000 (+0200) Subject: string-util: replace version_is_valid()/version_is_valid_version_spec() by a common... X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=093cac3fe8b4561b2a4662df9151156dac745457;p=thirdparty%2Fsystemd.git string-util: replace version_is_valid()/version_is_valid_version_spec() by a common call Let's take inspiration from string_is_safe() and take a flags field that allows fine tuning the validation. Then port over all current users of either function to the new logic. Note that this *does* change behaviour in various cases: 1. Generally: we'll now always accept the full UAPI.10 alphabet, including the "~" and "^" characters. As far as I can see there's no downside to this liberalization as none of the current consumers of the two functions uses these characters for anything else. 2. systemd-analyze compare-version will now accept version strings with "_" and "+" without complaining. I see no downside here, it just normalizes these debugging tools, to make them accept what most our other tools accept. 3. "bootctl link" will not accept empty version strings anymore Which is a bugfix I guess. 4. vpick will now refuse "_" and "+" in version strings. It kinda already did, because when parsing versions from filenames it uses "_" and "+" as name, architecture and attempt counter separators. We now systematically refuse it everywhere else in vpick too. This is hence a clean-up. Fixes: #28906 Replaces: #42815 #41937 --- diff --git a/src/analyze/analyze-compare-versions.c b/src/analyze/analyze-compare-versions.c index 5c15fd044d6..a3293d73b72 100644 --- a/src/analyze/analyze-compare-versions.c +++ b/src/analyze/analyze-compare-versions.c @@ -17,9 +17,9 @@ int verb_compare_versions(int argc, char *argv[], uintptr_t _data, void *userdat /* We only output a warning on invalid version strings (instead of failing), since the comparison * functions try to handle invalid strings gracefully and it's still interesting to see what the * comparison result will be. */ - if (!version_is_valid_versionspec(v1)) + if (!version_is_valid(v1, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS|VERSION_ALLOW_EMPTY)) log_warning("Version string 1 contains disallowed characters, they will be treated as separators: %s", v1); - if (!version_is_valid_versionspec(v2)) + if (!version_is_valid(v2, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS|VERSION_ALLOW_EMPTY)) log_warning("Version string 2 contains disallowed characters, they will be treated as separators: %s", v2); if (argc == 3) { diff --git a/src/basic/string-util.c b/src/basic/string-util.c index a806d2ddfa3..8de801a74b4 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -1462,29 +1462,47 @@ char* find_line_after_internal(const char *haystack, const char *needle) { return NULL; } -bool version_is_valid(const char *s) { - if (isempty(s)) - return false; +bool version_is_valid(const char *s, VersionFlags flags) { - if (!filename_part_is_valid(s)) - return false; + /* Validates a version string superficially. This does not proces the version string in any + * semantical way, it mostly just validates that its charset is reasonable. */ - /* This is a superset of the characters used by semver. We additionally allow "_". */ - if (!in_charset(s, ALPHANUMERICAL ".-+_")) + if (FLAGS_SET(flags, VERSION_ALLOW_EMPTY) ? !s : isempty(s)) return false; - return true; -} - -bool version_is_valid_versionspec(const char *s) { - if (!filename_part_is_valid(s)) return false; - if (!in_charset(s, ALPHANUMERICAL ".-~^")) - return false; + /* We always allow all characters specified by the UAPI.10 Version Specification, i.e. 0-9, a-z, A-Z, + * ".", "-", "~", "^". + * + * If the relevant flags are set we'll also allow "+" and "_" separators. + * + * Note that with SemVer allows 0-9, a-z, A-Z, "+", "-", ".", hence with VERSION_ALLOW_PLUS we + * implement a superset of it. + * + * If you wonder when to use which flags: when validating foreign versions (e.g. distribution + * versions in /etc/os-release or so) validate liberally, i.e. add + * VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS. When validating our own versioned objects (e.g. vpick + * or so) validate more strictly, and in particular refuse characters such as "_" and "+" that may be + * used for separating component names or boot attempt counters. Also: first – if appropriate – split + * the string into individual components. For example, if the string consists of a name and a + * version, separated by some character, only pass the version part to this function. The name part + * may pass verification, but it's cleaner to not rely on that. + * + * For details about UAPI.10 see: + * + * → https://uapi-group.org/specifications/specs/version_format_specification/ */ - return true; + char charset[] = ALPHANUMERICAL ".-~^" /* plus room for the two chars below: */ "\0\0"; + size_t l = strlen(charset); + + if (FLAGS_SET(flags, VERSION_ALLOW_UNDERSCORE)) + charset[l++] = '_'; + if (FLAGS_SET(flags, VERSION_ALLOW_PLUS)) + charset[l++] = '+'; + + return in_charset(s, charset); } ssize_t strlevenshtein(const char *x, const char *y) { diff --git a/src/basic/string-util.h b/src/basic/string-util.h index 8f4cc69a54b..d0b614f775a 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -315,8 +315,13 @@ char* find_line_after_internal(const char *haystack, const char *needle); #define find_line_after(haystack, needle) \ const_generic(haystack, find_line_after_internal(haystack, needle)) -bool version_is_valid(const char *s) _pure_; -bool version_is_valid_versionspec(const char *s) _pure_; +typedef enum VersionFlags { + VERSION_ALLOW_EMPTY = 1 << 0, + VERSION_ALLOW_UNDERSCORE = 1 << 1, /* Allow "_" as separator (recommended separator) */ + VERSION_ALLOW_PLUS = 1 << 2, /* Allow "+" as separator (sometimes used as separator for boot attempt counters) */ +} VersionFlags; + +bool version_is_valid(const char *s, VersionFlags flags) _pure_; ssize_t strlevenshtein(const char *x, const char *y); diff --git a/src/bootctl/bootctl-link.c b/src/bootctl/bootctl-link.c index b4706670857..ed1753af99e 100644 --- a/src/bootctl/bootctl-link.c +++ b/src/bootctl/bootctl-link.c @@ -1492,7 +1492,7 @@ static int vl_link_prepare(sd_varlink *link, LinkParameters *p) { if (p->context.entry_title && !efi_loader_entry_title_valid(p->context.entry_title)) return sd_varlink_error_invalid_parameter_name(link, "entryTitle"); - if (p->context.entry_version && !version_is_valid_versionspec(p->context.entry_version)) + if (p->context.entry_version && !version_is_valid(p->context.entry_version, /* flags= */ 0)) return sd_varlink_error_invalid_parameter_name(link, "entryVersion"); if (p->context.entry_commit != 0 && !entry_commit_valid(p->context.entry_commit)) diff --git a/src/bootctl/bootctl.c b/src/bootctl/bootctl.c index 72d96bc1b0e..238102095ed 100644 --- a/src/bootctl/bootctl.c +++ b/src/bootctl/bootctl.c @@ -695,7 +695,7 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { break; } - if (!version_is_valid_versionspec(opts.arg)) + if (!version_is_valid(opts.arg, /* flags= */ 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Not a valid boot menu entry version: %s", opts.arg); r = free_and_strdup_warn(&arg_entry_version, opts.arg); diff --git a/src/libsystemd/sd-json/json-util.c b/src/libsystemd/sd-json/json-util.c index d69e37a3ce6..6cbb61cb5e3 100644 --- a/src/libsystemd/sd-json/json-util.c +++ b/src/libsystemd/sd-json/json-util.c @@ -449,7 +449,7 @@ int json_dispatch_const_version(const char *name, sd_json_variant *variant, sd_j return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a string.", strna(name)); const char *version = sd_json_variant_string(variant); - if (!version_is_valid(version)) + if (!version_is_valid(version, FLAGS_SET(flags, SD_JSON_STRICT) ? 0 : VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a valid version string.", strna(name)); *n = version; diff --git a/src/shared/vpick.c b/src/shared/vpick.c index c661f92fafc..4f709c9e19a 100644 --- a/src/shared/vpick.c +++ b/src/shared/vpick.c @@ -447,7 +447,7 @@ static int make_choice( *underscore = 0; } - if (!version_is_valid(e)) { + if (!version_is_valid(e, /* flags= */ 0)) { log_debug("Version string '%s' of entry '%s' is invalid, ignoring entry.", e, (*entry)->d_name); continue; } diff --git a/src/sysupdate/sysupdate-pattern.c b/src/sysupdate/sysupdate-pattern.c index 459c8fda29d..294499f4af8 100644 --- a/src/sysupdate/sysupdate-pattern.c +++ b/src/sysupdate/sysupdate-pattern.c @@ -268,7 +268,7 @@ int pattern_match(const char *pattern, const char *s, InstanceMetadata *ret) { switch (e->type) { case PATTERN_VERSION: - if (!version_is_valid(t)) { + if (!version_is_valid(t, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) { log_debug("Version string is not valid, refusing: %s", t); goto nope; } diff --git a/src/sysupdate/sysupdate-transfer.c b/src/sysupdate/sysupdate-transfer.c index 30e879574f0..e80dcaf39a0 100644 --- a/src/sysupdate/sysupdate-transfer.c +++ b/src/sysupdate/sysupdate-transfer.c @@ -141,7 +141,7 @@ static int config_parse_protect_version( return 0; } - if (!version_is_valid(resolved)) { + if (!version_is_valid(resolved, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) { log_syntax(unit, LOG_WARNING, filename, line, 0, "ProtectVersion= string is not valid, ignoring: %s", resolved); return 0; @@ -180,7 +180,7 @@ static int config_parse_min_version( return 0; } - if (!version_is_valid(rvalue)) { + if (!version_is_valid(resolved, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) { log_syntax(unit, LOG_WARNING, filename, line, 0, "MinVersion= string is not valid, ignoring: %s", resolved); return 0; diff --git a/src/sysupdate/sysupdated.c b/src/sysupdate/sysupdated.c index bc5a2439b5a..9bbcc870bc9 100644 --- a/src/sysupdate/sysupdated.c +++ b/src/sysupdate/sysupdated.c @@ -952,7 +952,7 @@ static int target_method_describe(sd_bus_message *msg, void *userdata, sd_bus_er if (r < 0) return r; - if (!version_is_valid(version)) + if (!version_is_valid(version, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid version"); if ((flags & ~SD_SYSUPDATE_FLAGS_ALL) != 0) @@ -1102,7 +1102,7 @@ static int target_method_acquire(sd_bus_message *msg, void *userdata, sd_bus_err if (isempty(version)) action = "org.freedesktop.sysupdate1.update"; else { - if (!version_is_valid(version)) + if (!version_is_valid(version, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid version"); action = "org.freedesktop.sysupdate1.update-to-version"; @@ -1190,7 +1190,7 @@ static int target_method_install(sd_bus_message *msg, void *userdata, sd_bus_err if (isempty(version)) action = "org.freedesktop.sysupdate1.update"; else { - if (!version_is_valid(version)) + if (!version_is_valid(version, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid version"); action = "org.freedesktop.sysupdate1.update-to-version"; diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 970421c7abb..a18999cf8a5 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -1408,13 +1408,13 @@ TEST(strstrafter) { } TEST(version_is_valid) { - assert_se(!version_is_valid(NULL)); - assert_se(!version_is_valid("")); - assert_se(version_is_valid("0")); - assert_se(version_is_valid("5")); - assert_se(version_is_valid("999999")); - assert_se(version_is_valid("999999.5")); - assert_se(version_is_valid("6.2.12-300.fc38.x86_64")); + assert_se(!version_is_valid(NULL, /* flags= */ 0)); + assert_se(!version_is_valid("", /* flags= */ 0)); + assert_se(version_is_valid("0", /* flags= */ 0)); + assert_se(version_is_valid("5", /* flags= */ 0)); + assert_se(version_is_valid("999999", /* flags= */ 0)); + assert_se(version_is_valid("999999.5", /* flags= */ 0)); + assert_se(version_is_valid("6.2.12-300.fc38.x86_64", /* flags= */ VERSION_ALLOW_UNDERSCORE)); } TEST(strextendn) { diff --git a/src/vpick/vpick-tool.c b/src/vpick/vpick-tool.c index 85f2b5c1cbb..8dd22e0ea28 100644 --- a/src/vpick/vpick-tool.c +++ b/src/vpick/vpick-tool.c @@ -115,7 +115,7 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { break; OPTION_SHORT('V', "VERSION", "Look for specified version"): - if (!version_is_valid(opts.arg)) + if (!version_is_valid(opts.arg, /* flags= */ 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid version string: %s", opts.arg); r = free_and_strdup_warn(&arg_filter_version, opts.arg);