SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

version_is_valid() tweaks (#43025)

Browse Source 
We have two validators, and neither really makes sense. Let's unify and
clean things up.
This commit is contained in:
Lennart Poettering
2026-07-14 23:46:40 +02:00
committed by GitHub
parent 9d925ad44e e0142a76ac
commit d26de063cd
15 changed files with 143 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/analyze/analyze-compare-versions.c
+2 -2
View File
@@ -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) {
src/basic/string-util.c
+33 -14
View File
@@ -1501,28 +1501,47 @@ char* find_line_after_internal(const char *haystack, const char *needle) {
return NULL;
}
bool version_is_valid(const char *s) {
if (isempty(s))
bool version_is_valid(const char *s, VersionFlags flags) {
/* 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. */
if (FLAGS_SET(flags, VERSION_ALLOW_EMPTY) ? !s : isempty(s))
return false;
if (!filename_part_is_valid(s))
return false;
/* This is a superset of the characters used by semver. We additionally allow "," and "_". */
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);
bool version_is_valid_versionspec(const char *s) {
if (!filename_part_is_valid(s))
return false;
if (FLAGS_SET(flags, VERSION_ALLOW_UNDERSCORE))
charset[l++] = '_';
if (FLAGS_SET(flags, VERSION_ALLOW_PLUS))
charset[l++] = '+';
if (!in_charset(s, ALPHANUMERICAL "-.~^"))
return false;
return true;
return in_charset(s, charset);
}
ssize_t strlevenshtein(const char *x, const char *y) {
src/basic/string-util.h
+7 -2
View File
@@ -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);
src/bootctl/bootctl-link.c
+1 -1
View File
@@ -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))
src/bootctl/bootctl.c
+1 -1
View File
@@ -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);
src/libsystemd/sd-json/json-util.c
+1 -1
View File
@@ -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;
src/shared/bootspec.c
+5 -2
View File
@@ -492,9 +492,12 @@ static int boot_entry_load_type1(
r = free_and_strdup(&tmp.title, p);
else if (streq(field, "sort-key"))
r = free_and_strdup(&tmp.sort_key, p);
else if (streq(field, "version"))
else if (streq(field, "version")) {
if (!version_is_valid(p, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS))
log_syntax(NULL, LOG_WARNING, tmp.path, line, 0, "Version string '%s' is not a valid version, accepting anyway.", p);
r = free_and_strdup(&tmp.version, p);
else if (streq(field, "machine-id"))
} else if (streq(field, "machine-id"))
r = free_and_strdup(&tmp.machine_id, p);
else if (streq(field, "architecture"))
r = free_and_strdup(&tmp.architecture, p);
src/shared/vpick.c
+1 -1
View File
@@ -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;
}
src/sysupdate/sysupdate-pattern.c
+1 -1
View File
@@ -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;
}
src/sysupdate/sysupdate-transfer.c
+2 -2
View File
@@ -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;
src/sysupdate/sysupdate.c
+1 -1
View File
@@ -553,7 +553,7 @@ static int context_discover_update_sets_by_flag(Context *c, UpdateSetFlags flags
if (boundary && strverscmp_improved(i->metadata.version, boundary) >= 0)
continue; /* Not older than the boundary */
if (cursor && strverscmp(i->metadata.version, cursor) <= 0)
if (cursor && strverscmp_improved(i->metadata.version, cursor) <= 0)
break; /* Not newer than the cursor. The same will be true for all
* subsequent instances (due to sorting) so let's skip to the
* next transfer. */
src/sysupdate/sysupdated.c
+3 -3
View File
@@ -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";
src/test/test-string-util.c
+63 -7
View File
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: LGPL-2.1-or-later */
#include <sys/utsname.h>
#include <unistd.h>
#include "alloc-util.h"
@@ -8,6 +9,7 @@
#include "string-util.h"
#include "strv.h"
#include "tests.h"
#include "version.h"
TEST(ellipsize_mem_ansi_short) {
_cleanup_free_ char *a = ellipsize_mem("X\x1b[m", 4, 1, 50);
@@ -1408,13 +1410,67 @@ 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_FALSE(version_is_valid(NULL, /* flags= */ 0));
ASSERT_FALSE(version_is_valid("", /* flags= */ 0));
ASSERT_TRUE(version_is_valid("0", /* flags= */ 0));
ASSERT_TRUE(version_is_valid("5", /* flags= */ 0));
ASSERT_TRUE(version_is_valid("999999", /* flags= */ 0));
ASSERT_TRUE(version_is_valid("999999.5", /* flags= */ 0));
ASSERT_TRUE(version_is_valid("6.2.12-300.fc38.x86_64", VERSION_ALLOW_UNDERSCORE));
ASSERT_TRUE(version_is_valid("6.2.12-300.fc38.x86_64", VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
ASSERT_FALSE(version_is_valid("6.2.12-300.fc38.x86_64", VERSION_ALLOW_PLUS));
ASSERT_FALSE(version_is_valid("6.2.12-300.fc38.x86_64", /* flags= */ 0));
struct utsname u;
ASSERT_OK_ERRNO(uname(&u));
ASSERT_TRUE(version_is_valid(u.release, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
ASSERT_TRUE(version_is_valid(GIT_VERSION, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
ASSERT_TRUE(version_is_valid(PROJECT_VERSION_STR, /* flags= */ 0));
/* VERSION_ALLOW_EMPTY permits the empty string, but never NULL */
ASSERT_TRUE(version_is_valid("", VERSION_ALLOW_EMPTY));
ASSERT_TRUE(version_is_valid("", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
ASSERT_FALSE(version_is_valid("", VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
ASSERT_FALSE(version_is_valid(NULL, VERSION_ALLOW_EMPTY));
ASSERT_FALSE(version_is_valid(NULL, VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
/* The full UAPI.10 charset, including "~" and "^", is accepted regardless of flags */
ASSERT_TRUE(version_is_valid("1.2~rc1^5", /* flags= */ 0));
ASSERT_TRUE(version_is_valid("1.2~rc1^5", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
/* "_" and "+" require their respective flag, the other flags won't do */
ASSERT_FALSE(version_is_valid("1_2", /* flags= */ 0));
ASSERT_TRUE(version_is_valid("1_2", VERSION_ALLOW_UNDERSCORE));
ASSERT_FALSE(version_is_valid("1_2", VERSION_ALLOW_PLUS));
ASSERT_FALSE(version_is_valid("1_2", VERSION_ALLOW_EMPTY));
ASSERT_FALSE(version_is_valid("1+2", /* flags= */ 0));
ASSERT_TRUE(version_is_valid("1+2", VERSION_ALLOW_PLUS));
ASSERT_FALSE(version_is_valid("1+2", VERSION_ALLOW_UNDERSCORE));
ASSERT_FALSE(version_is_valid("1+2", VERSION_ALLOW_EMPTY));
ASSERT_FALSE(version_is_valid("1_2+3", VERSION_ALLOW_UNDERSCORE));
ASSERT_FALSE(version_is_valid("1_2+3", VERSION_ALLOW_PLUS));
ASSERT_TRUE(version_is_valid("1_2+3", VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
/* Characters outside the charset are refused, no matter which flags are set */
ASSERT_FALSE(version_is_valid("1 2", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
ASSERT_FALSE(version_is_valid("1/2", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
ASSERT_FALSE(version_is_valid("1=2", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
ASSERT_FALSE(version_is_valid("1\n2", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
ASSERT_FALSE(version_is_valid("©", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
/* "." and ".." pass the charset check and are OK as *part* of a filename, hence accepted */
ASSERT_TRUE(version_is_valid(".", /* flags= */ 0));
ASSERT_TRUE(version_is_valid("..", /* flags= */ 0));
/* Version strings must fit in a filename, i.e. no longer than NAME_MAX, no matter the flags */
_cleanup_free_ char *x = strrep("0", NAME_MAX);
ASSERT_NOT_NULL(x);
ASSERT_TRUE(version_is_valid(x, /* flags= */ 0));
_cleanup_free_ char *y = strrep("0", NAME_MAX+1);
ASSERT_NOT_NULL(y);
ASSERT_FALSE(version_is_valid(y, /* flags= */ 0));
ASSERT_FALSE(version_is_valid(y, VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS));
}
TEST(strextendn) {
src/vpick/vpick-tool.c
+1 -1
View File
@@ -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);