diff --git a/src/analyze/analyze-compare-versions.c b/src/analyze/analyze-compare-versions.c index 07b52a55f2..d210fab672 100644 --- a/src/analyze/analyze-compare-versions.c +++ b/src/analyze/analyze-compare-versions.c @@ -9,17 +9,26 @@ #include "strv.h" int verb_compare_versions(int argc, char *argv[], void *userdata) { + const char *v1 = ASSERT_PTR(argv[1]), *v2 = ASSERT_PTR(argv[argc-1]); int r; assert(IN_SET(argc, 3, 4)); assert(argv); + /* We only output a warning on invalid version strings (instead of failing), since the comparison + * functions try to handle invalid strings graceful and it's still interesting to see what the + * comparison result will be. */ + if (!version_is_valid(v1)) + log_warning("Version string 1 is not valid, comparing anyway: %s", v1); + if (!version_is_valid(v2)) + log_warning("Version string 2 is not valid, comparing anyway: %s", v2); + if (argc == 3) { - r = strverscmp_improved(ASSERT_PTR(argv[1]), ASSERT_PTR(argv[2])); + r = strverscmp_improved(v1, v2); printf("%s %s %s\n", - isempty(argv[1]) ? "''" : argv[1], + isempty(v1) ? "''" : v1, comparison_operator(r), - isempty(argv[2]) ? "''" : argv[2]); + isempty(v2) ? "''" : v2); /* This matches the exit convention used by rpmdev-vercmp. * We don't use named values because 11 and 12 don't have names. */ @@ -28,12 +37,13 @@ int verb_compare_versions(int argc, char *argv[], void *userdata) { } else { const char *op = ASSERT_PTR(argv[2]); CompareOperator operator; + assert(argc == 4); operator = parse_compare_operator(&op, COMPARE_ALLOW_TEXTUAL); if (operator < 0 || !isempty(op)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown operator \"%s\".", op); - r = version_or_fnmatch_compare(operator, ASSERT_PTR(argv[1]), ASSERT_PTR(argv[3])); + r = version_or_fnmatch_compare(operator, v1, v2); if (r < 0) return log_error_errno(r, "Failed to compare versions: %m"); diff --git a/src/basic/path-util.c b/src/basic/path-util.c index e1cc26f618..b972396249 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -1124,13 +1124,13 @@ int path_extract_directory(const char *path, char **ret) { return 0; } -bool filename_is_valid(const char *p) { +bool filename_part_is_valid(const char *p) { const char *e; - if (isempty(p)) - return false; + /* Checks f the specified string is OK to be *part* of a filename. This is different from + * filename_is_valid() as "." and ".." and "" are OK by this call, but not by filename_is_valid(). */ - if (dot_or_dot_dot(p)) /* Yes, in this context we consider "." and ".." invalid */ + if (!p) return false; e = strchrnul(p, '/'); @@ -1143,6 +1143,17 @@ bool filename_is_valid(const char *p) { return true; } +bool filename_is_valid(const char *p) { + + if (isempty(p)) + return false; + + if (dot_or_dot_dot(p)) /* Yes, in this context we consider "." and ".." invalid */ + return false; + + return filename_part_is_valid(p); +} + bool path_is_valid_full(const char *p, bool accept_dot_dot) { if (isempty(p)) return false; diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 66879d1932..af7f229908 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -163,6 +163,7 @@ const char *last_path_component(const char *path); int path_extract_filename(const char *path, char **ret); int path_extract_directory(const char *path, char **ret); +bool filename_part_is_valid(const char *p) _pure_; bool filename_is_valid(const char *p) _pure_; bool path_is_valid_full(const char *p, bool accept_dot_dot) _pure_; static inline bool path_is_valid(const char *p) { diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 41d264318a..1eedcb66f7 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -16,6 +16,7 @@ #include "macro.h" #include "memory-util.h" #include "memstream-util.h" +#include "path-util.h" #include "string-util.h" #include "strv.h" #include "terminal-util.h" @@ -1296,3 +1297,17 @@ char *startswith_strv(const char *string, char **strv) { return found; } + +bool version_is_valid(const char *s) { + if (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; + + return true; +} diff --git a/src/basic/string-util.h b/src/basic/string-util.h index b5028c9e00..0ff5b46bba 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -275,3 +275,5 @@ char *startswith_strv(const char *string, char **strv); #define STARTSWITH_SET(p, ...) \ startswith_strv(p, STRV_MAKE(__VA_ARGS__)) + +bool version_is_valid(const char *s); diff --git a/src/sysupdate/meson.build b/src/sysupdate/meson.build index 2f8c2305da..1bd6bbf9a9 100644 --- a/src/sysupdate/meson.build +++ b/src/sysupdate/meson.build @@ -15,8 +15,6 @@ systemd_sysupdate_sources = files( 'sysupdate-transfer.h', 'sysupdate-update-set.c', 'sysupdate-update-set.h', - 'sysupdate-util.c', - 'sysupdate-util.h', 'sysupdate.c', 'sysupdate.h', ) diff --git a/src/sysupdate/sysupdate-pattern.c b/src/sysupdate/sysupdate-pattern.c index 6d9c8d8f8b..c9e2067c58 100644 --- a/src/sysupdate/sysupdate-pattern.c +++ b/src/sysupdate/sysupdate-pattern.c @@ -8,7 +8,6 @@ #include "stdio-util.h" #include "string-util.h" #include "sysupdate-pattern.h" -#include "sysupdate-util.h" typedef enum PatternElementType { PATTERN_LITERAL, diff --git a/src/sysupdate/sysupdate-transfer.c b/src/sysupdate/sysupdate-transfer.c index bbc3a5bcaa..dd2b1e45eb 100644 --- a/src/sysupdate/sysupdate-transfer.c +++ b/src/sysupdate/sysupdate-transfer.c @@ -24,7 +24,6 @@ #include "sysupdate-pattern.h" #include "sysupdate-resource.h" #include "sysupdate-transfer.h" -#include "sysupdate-util.h" #include "sysupdate.h" #include "tmpfile-util.h" #include "web-util.h" diff --git a/src/sysupdate/sysupdate-util.c b/src/sysupdate/sysupdate-util.c index c7a23015ce..eacc592f62 100644 --- a/src/sysupdate/sysupdate-util.c +++ b/src/sysupdate/sysupdate-util.c @@ -2,16 +2,3 @@ #include "path-util.h" #include "sysupdate-util.h" - -bool version_is_valid(const char *s) { - if (isempty(s)) - return false; - - if (!filename_is_valid(s)) - return false; - - if (!in_charset(s, ALPHANUMERICAL ".,_-+")) - return false; - - return true; -} diff --git a/src/sysupdate/sysupdate-util.h b/src/sysupdate/sysupdate-util.h deleted file mode 100644 index afa3a9d498..0000000000 --- a/src/sysupdate/sysupdate-util.h +++ /dev/null @@ -1,6 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -#pragma once - -#include - -bool version_is_valid(const char *s); diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 2d3e40321e..e8fcc5a13c 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -1264,4 +1264,14 @@ TEST(strstrafter) { assert_se(!strstrafter(buffer, "-")); } +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")); +} + DEFINE_TEST_MAIN(LOG_DEBUG);