From bf9a49a5534316353b9fdda1c40026781bc6bda8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 17:33:31 +0200 Subject: [PATCH 1/9] test-path-util: add simple test for is_path() and is_device_path() I think that those functions should be adjusted, but let's first add a test to establish current behaviour. --- src/test/test-path-util.c | 98 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 94 insertions(+), 4 deletions(-) diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index 8c3a27f228..c6a746b441 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -26,10 +26,6 @@ TEST(path) { assert_se(path_is_absolute("/")); assert_se(!path_is_absolute("./")); - assert_se(is_path("/dir")); - assert_se(is_path("a/b")); - assert_se(!is_path(".")); - assert_se(streq(basename("./aa/bb/../file.da."), "file.da.")); assert_se(streq(basename("/aa///.file"), ".file")); assert_se(streq(basename("/aa///file..."), "file...")); @@ -48,6 +44,100 @@ TEST(path) { assert_se(!path_equal_ptr(NULL, "/a")); } +TEST(is_path) { + assert_se(!is_path("foo")); + assert_se(!is_path("dos.ext")); + assert_se( is_path("/dir")); + assert_se( is_path("a/b")); + assert_se( is_path("a/b.ext")); + + assert_se(!is_path(".")); + assert_se(!is_path("")); + assert_se(!is_path("..")); + + assert_se( is_path("/dev")); + assert_se( is_path("/./dev")); + assert_se( is_path("/./dev/.")); + assert_se( is_path("/./dev.")); + assert_se( is_path("//dev")); + assert_se( is_path("///dev")); + assert_se( is_path("/dev/")); + assert_se( is_path("///dev/")); + assert_se( is_path("/./dev/")); + assert_se( is_path("/../dev/")); + assert_se( is_path("/dev/sda")); + assert_se( is_path("/dev/sda5")); + assert_se( is_path("/dev/sda5b3")); + assert_se( is_path("/dev/sda5b3/idontexit")); + assert_se( is_path("/../dev/sda")); + assert_se( is_path("/../../dev/sda5")); + assert_se( is_path("/../../../dev/sda5b3")); + assert_se( is_path("/.././.././dev/sda5b3/idontexit")); + assert_se( is_path("/sys")); + assert_se( is_path("/sys/")); + assert_se( is_path("/./sys")); + assert_se( is_path("/./sys/.")); + assert_se( is_path("/./sys.")); + assert_se( is_path("/sys/what")); + assert_se( is_path("/sys/something/..")); + assert_se( is_path("/sys/something/../")); + assert_se( is_path("/sys////")); + assert_se( is_path("/sys////.")); + assert_se( is_path("/sys/..")); + assert_se( is_path("/sys/../")); + assert_se( is_path("/usr/../dev/sda")); +} + +TEST(is_device_path) { + assert_se(!is_device_path("foo")); + assert_se(!is_device_path("dos.ext")); + assert_se(!is_device_path("/dir")); + assert_se(!is_device_path("a/b")); + assert_se(!is_device_path("a/b.ext")); + + assert_se(!is_device_path(".")); + assert_se(!is_device_path("")); + assert_se(!is_device_path("..")); + + assert_se( is_device_path("/dev")); + assert_se( is_device_path("/./dev")); + assert_se( is_device_path("/./dev/.")); + assert_se(!is_device_path("/./dev.")); + assert_se( is_device_path("/./dev/foo")); + assert_se( is_device_path("/./dev/./foo")); + assert_se(!is_device_path("/./dev./foo")); + assert_se( is_device_path("//dev")); + assert_se( is_device_path("///dev")); + assert_se( is_device_path("/dev/")); + assert_se( is_device_path("///dev/")); + assert_se( is_device_path("/./dev/")); + assert_se(!is_device_path("/../dev/")); + assert_se( is_device_path("/dev/sda")); + assert_se( is_device_path("/dev/sda5")); + assert_se( is_device_path("/dev/sda5b3")); + assert_se( is_device_path("/dev/sda5b3/idontexit")); + assert_se(!is_device_path("/../dev/sda")); + assert_se(!is_device_path("/../../dev/sda5")); + assert_se(!is_device_path("/../../../dev/sda5b3")); + assert_se(!is_device_path("/.././.././dev/sda5b3/idontexit")); + assert_se( is_device_path("/sys")); + assert_se( is_device_path("/sys/")); + assert_se( is_device_path("/./sys")); + assert_se( is_device_path("/./sys/.")); + assert_se(!is_device_path("/./sys.")); + assert_se( is_device_path("/./sys/foo")); + assert_se( is_device_path("/./sys/./foo")); + assert_se(!is_device_path("/./sys./foo")); + assert_se( is_device_path("/sys/what")); + assert_se( is_device_path("/sys/something/..")); + assert_se( is_device_path("/sys/something/../")); + assert_se( is_device_path("/sys////")); + assert_se( is_device_path("/sys////.")); + assert_se( is_device_path("/sys/..")); + assert_se( is_device_path("/sys/../")); + assert_se(!is_device_path("/usr/../dev/sda")); +} + static void test_path_simplify_one(const char *in, const char *out, PathSimplifyFlags flags) { char *p; From 8f1998b8d3a5bfe61ee4d6d6aa6bb2efb94074c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 19:15:05 +0200 Subject: [PATCH 2/9] basic/path-util: do not say that /dev and /sys are device paths "/dev" or "/dev/" is the mount point, not a device path. In particular, 'systemctl status /dev' clearly does not refer to a device, so let's tweak the code a bit to say that those are not device paths. (Treating "/../dev" same as "/dev" would be also be reasonable, but that requires chase(), which requires disk access, which we don't want to do from this lightweight function.) --- src/basic/path-util.c | 11 +++++++++-- src/test/test-path-util.c | 28 ++++++++++++++-------------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 7204c80e79..beec70ad77 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -1272,9 +1272,16 @@ bool hidden_or_backup_file(const char *filename) { bool is_device_path(const char *path) { /* Returns true for paths that likely refer to a device, either by path in sysfs or to something in - * /dev. */ + * /dev. This accepts any path that starts with /dev/ or /sys/ and has something after that prefix. + * It does not actually resolve the path. + * + * Examples: + * /dev/sda, /dev/sda/foo, /sys/class, /dev/.., /sys/.., /./dev/foo → yes. + * /../dev/sda, /dev, /sys, /usr/path, /usr/../dev/sda → no. + */ - return PATH_STARTSWITH_SET(path, "/dev/", "/sys/"); + const char *p = PATH_STARTSWITH_SET(ASSERT_PTR(path), "/dev/", "/sys/"); + return !isempty(p); } bool valid_device_node_path(const char *path) { diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index c6a746b441..e8d6ff39e8 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -99,18 +99,18 @@ TEST(is_device_path) { assert_se(!is_device_path("")); assert_se(!is_device_path("..")); - assert_se( is_device_path("/dev")); - assert_se( is_device_path("/./dev")); - assert_se( is_device_path("/./dev/.")); + assert_se(!is_device_path("/dev")); + assert_se(!is_device_path("/./dev")); + assert_se(!is_device_path("/./dev/.")); assert_se(!is_device_path("/./dev.")); assert_se( is_device_path("/./dev/foo")); assert_se( is_device_path("/./dev/./foo")); assert_se(!is_device_path("/./dev./foo")); - assert_se( is_device_path("//dev")); - assert_se( is_device_path("///dev")); - assert_se( is_device_path("/dev/")); - assert_se( is_device_path("///dev/")); - assert_se( is_device_path("/./dev/")); + assert_se(!is_device_path("//dev")); + assert_se(!is_device_path("///dev")); + assert_se(!is_device_path("/dev/")); + assert_se(!is_device_path("///dev/")); + assert_se(!is_device_path("/./dev/")); assert_se(!is_device_path("/../dev/")); assert_se( is_device_path("/dev/sda")); assert_se( is_device_path("/dev/sda5")); @@ -120,10 +120,10 @@ TEST(is_device_path) { assert_se(!is_device_path("/../../dev/sda5")); assert_se(!is_device_path("/../../../dev/sda5b3")); assert_se(!is_device_path("/.././.././dev/sda5b3/idontexit")); - assert_se( is_device_path("/sys")); - assert_se( is_device_path("/sys/")); - assert_se( is_device_path("/./sys")); - assert_se( is_device_path("/./sys/.")); + assert_se(!is_device_path("/sys")); + assert_se(!is_device_path("/sys/")); + assert_se(!is_device_path("/./sys")); + assert_se(!is_device_path("/./sys/.")); assert_se(!is_device_path("/./sys.")); assert_se( is_device_path("/./sys/foo")); assert_se( is_device_path("/./sys/./foo")); @@ -131,8 +131,8 @@ TEST(is_device_path) { assert_se( is_device_path("/sys/what")); assert_se( is_device_path("/sys/something/..")); assert_se( is_device_path("/sys/something/../")); - assert_se( is_device_path("/sys////")); - assert_se( is_device_path("/sys////.")); + assert_se(!is_device_path("/sys////")); + assert_se(!is_device_path("/sys////.")); assert_se( is_device_path("/sys/..")); assert_se( is_device_path("/sys/../")); assert_se(!is_device_path("/usr/../dev/sda")); From 00d811a5482fda1a6c2b9362d047da2dcd1d7418 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 19:16:14 +0200 Subject: [PATCH 3/9] basic/path-util: change 'char *func' to 'char* func' Both styles were mixed in the file, but I find the latter much nicer, because it's not the func that is the pointer, but the return type. --- src/basic/path-util.c | 16 ++++++++-------- src/basic/path-util.h | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index beec70ad77..47fb33eba7 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -40,7 +40,7 @@ int path_split_and_make_absolute(const char *p, char ***ret) { return r; } -char *path_make_absolute(const char *p, const char *prefix) { +char* path_make_absolute(const char *p, const char *prefix) { assert(p); /* Makes every item in the list an absolute path by prepending @@ -252,7 +252,7 @@ int path_strv_make_absolute_cwd(char **l) { return 0; } -char **path_strv_resolve(char **l, const char *root) { +char** path_strv_resolve(char **l, const char *root) { unsigned k = 0; bool enomem = false; int r; @@ -333,7 +333,7 @@ char **path_strv_resolve(char **l, const char *root) { return l; } -char **path_strv_resolve_uniq(char **l, const char *root) { +char** path_strv_resolve_uniq(char **l, const char *root) { if (strv_isempty(l)) return l; @@ -344,7 +344,7 @@ char **path_strv_resolve_uniq(char **l, const char *root) { return strv_uniq(l); } -char *path_simplify_full(char *path, PathSimplifyFlags flags) { +char* path_simplify_full(char *path, PathSimplifyFlags flags) { bool add_slash = false, keep_trailing_slash; char *f = ASSERT_PTR(path); int r; @@ -397,7 +397,7 @@ char *path_simplify_full(char *path, PathSimplifyFlags flags) { return path; } -char *path_startswith_full(const char *path, const char *prefix, bool accept_dot_dot) { +char* path_startswith_full(const char *path, const char *prefix, bool accept_dot_dot) { assert(path); assert(prefix); @@ -807,7 +807,7 @@ int fsck_exists_for_fstype(const char *fstype) { return executable_is_good(checker); } -static const char *skip_slash_or_dot(const char *p) { +static const char* skip_slash_or_dot(const char *p) { for (; !isempty(p); p++) { if (*p == '/') continue; @@ -891,7 +891,7 @@ int path_find_first_component(const char **p, bool accept_dot_dot, const char ** return len; } -static const char *skip_slash_or_dot_backward(const char *path, const char *q) { +static const char* skip_slash_or_dot_backward(const char *path, const char *q) { assert(path); assert(!q || q >= path); @@ -1000,7 +1000,7 @@ int path_find_last_component(const char *path, bool accept_dot_dot, const char * return len; } -const char *last_path_component(const char *path) { +const char* last_path_component(const char *path) { /* Finds the last component of the path, preserving the optional trailing slash that signifies a directory. * diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 507aec27c5..3f60f4d133 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -53,7 +53,7 @@ int safe_getcwd(char **ret); int path_make_absolute_cwd(const char *p, char **ret); int path_make_relative(const char *from, const char *to, char **ret); int path_make_relative_parent(const char *from_child, const char *to, char **ret); -char *path_startswith_full(const char *path, const char *prefix, bool accept_dot_dot) _pure_; +char* path_startswith_full(const char *path, const char *prefix, bool accept_dot_dot) _pure_; static inline char* path_startswith(const char *path, const char *prefix) { return path_startswith_full(path, prefix, true); } @@ -80,7 +80,7 @@ typedef enum PathSimplifyFlags { PATH_SIMPLIFY_KEEP_TRAILING_SLASH = 1 << 0, } PathSimplifyFlags; -char *path_simplify_full(char *path, PathSimplifyFlags flags); +char* path_simplify_full(char *path, PathSimplifyFlags flags); static inline char* path_simplify(char *path) { return path_simplify_full(path, 0); } @@ -159,7 +159,7 @@ int fsck_exists_for_fstype(const char *fstype); int path_find_first_component(const char **p, bool accept_dot_dot, const char **ret); int path_find_last_component(const char *path, bool accept_dot_dot, const char **next, const char **ret); -const char *last_path_component(const char *path); +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); @@ -196,7 +196,7 @@ static inline const char *skip_dev_prefix(const char *p) { } bool empty_or_root(const char *path); -static inline const char *empty_to_root(const char *path) { +static inline const char* empty_to_root(const char *path) { return isempty(path) ? "/" : path; } From a410b3a3d41f327d0c7265748ae27109f1b8239a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 18 Sep 2023 11:34:52 +0200 Subject: [PATCH 4/9] basic: indent prototypes of two functions with many arguments --- src/basic/path-util.c | 9 ++++++++- src/basic/unit-name.c | 8 +++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 47fb33eba7..187fe2886b 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -657,7 +657,14 @@ static int find_executable_impl(const char *name, const char *root, char **ret_f return 0; } -int find_executable_full(const char *name, const char *root, char **exec_search_path, bool use_path_envvar, char **ret_filename, int *ret_fd) { +int find_executable_full( + const char *name, + const char *root, + char **exec_search_path, + bool use_path_envvar, + char **ret_filename, + int *ret_fd) { + int last_error = -ENOENT, r = 0; const char *p = NULL; diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index 3b739c758a..a6ef0c4321 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -701,7 +701,13 @@ static bool do_escape_mangle(const char *f, bool allow_globs, char *t) { * * If @allow_globs, globs characters are preserved. Otherwise, they are escaped. */ -int unit_name_mangle_with_suffix(const char *name, const char *operation, UnitNameMangle flags, const char *suffix, char **ret) { +int unit_name_mangle_with_suffix( + const char *name, + const char *operation, + UnitNameMangle flags, + const char *suffix, + char **ret) { + _cleanup_free_ char *s = NULL; bool mangled, suggest_escape = true; int r; From 003ccba65072a74c18e3e45d4c1f8ea7346b2f01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Sep 2023 16:52:54 +0200 Subject: [PATCH 5/9] basic/path-util: make path_simplify() skip leading '/..' I.e., /.. becomes /, /../foo becomes /foo, /../../bar becomes /bar, etc. We can do this unconditionally, without access to the file system, because the parent of the root directory always resolves to. /.. in other places is handled as before, because resolving it properly would require access to the file system which we don't want to do in path_simplify(). --- src/basic/path-util.c | 16 +++++++++++---- src/test/test-path-util.c | 42 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 187fe2886b..b036191144 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -345,7 +345,7 @@ char** path_strv_resolve_uniq(char **l, const char *root) { } char* path_simplify_full(char *path, PathSimplifyFlags flags) { - bool add_slash = false, keep_trailing_slash; + bool add_slash = false, keep_trailing_slash, absolute, beginning = true; char *f = ASSERT_PTR(path); int r; @@ -354,6 +354,8 @@ char* path_simplify_full(char *path, PathSimplifyFlags flags) { * * ///foo//./bar/. becomes /foo/bar * .//./foo//./bar/. becomes foo/bar + * /../foo/bar becomes /foo/bar + * /../foo/bar/.. becomes /foo/bar/.. */ if (isempty(path)) @@ -361,8 +363,8 @@ char* path_simplify_full(char *path, PathSimplifyFlags flags) { keep_trailing_slash = FLAGS_SET(flags, PATH_SIMPLIFY_KEEP_TRAILING_SLASH) && endswith(path, "/"); - if (path_is_absolute(path)) - f++; + absolute = path_is_absolute(path); + f += absolute; /* Keep leading /, if present. */ for (const char *p = f;;) { const char *e; @@ -371,11 +373,17 @@ char* path_simplify_full(char *path, PathSimplifyFlags flags) { if (r == 0) break; + if (r > 0 && absolute && beginning && path_startswith(e, "..")) + /* If we're at the beginning of an absolute path, we can safely skip ".." */ + continue; + + beginning = false; + if (add_slash) *f++ = '/'; if (r < 0) { - /* if path is invalid, then refuse to simplify remaining part. */ + /* if path is invalid, then refuse to simplify the remaining part. */ memmove(f, p, strlen(p) + 1); return path; } diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index e8d6ff39e8..96a4fce630 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -158,7 +158,7 @@ TEST(path_simplify) { test_path_simplify_one("///", "/", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); test_path_simplify_one("///.//", "/", 0); test_path_simplify_one("///.//.///", "/", 0); - test_path_simplify_one("////.././///../.", "/../..", 0); + test_path_simplify_one("////.././///../.", "/", 0); test_path_simplify_one(".", ".", 0); test_path_simplify_one("./", ".", 0); test_path_simplify_one("./", "./", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); @@ -174,6 +174,46 @@ TEST(path_simplify) { "../aaa/.bbb/../c./d.dd/..eeee/..", 0); test_path_simplify_one("abc///", "abc/", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/../abc", "/abc", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/../abc///", "/abc/", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/../abc///", "/abc", 0); + test_path_simplify_one("/../abc", "/abc", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/../abc///..", "/abc/..", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/../abc///../", "/abc/../", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/../abc///../", "/abc/..", 0); + + test_path_simplify_one("/../../abc", "/abc", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/../../abc///", "/abc/", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/../../abc///", "/abc", 0); + test_path_simplify_one("/../../abc", "/abc", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/../../abc///../..", "/abc/../..", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/../../abc///../../", "/abc/../../", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/../../abc///../../", "/abc/../..", 0); + + test_path_simplify_one("/.././../abc", "/abc", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/.././../abc///", "/abc/", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/.././../abc///", "/abc", 0); + test_path_simplify_one("/.././../abc", "/abc", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/.././../abc///../..", "/abc/../..", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/.././../abc///../../", "/abc/../../", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/.././../abc///../../", "/abc/../..", 0); + + test_path_simplify_one("/./.././../abc", "/abc", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/./.././../abc///", "/abc/", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/./.././../abc///", "/abc", 0); + test_path_simplify_one("/./.././../abc", "/abc", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/./.././../abc///../..", "/abc/../..", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/./.././../abc///../../", "/abc/../../", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/./.././../abc///../../", "/abc/../..", 0); + + test_path_simplify_one("/.../abc", "/.../abc", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/.../abc///", "/.../abc/", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/.../abc///", "/.../abc", 0); + test_path_simplify_one("/.../abc", "/.../abc", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/.../abc///...", "/.../abc/...", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/.../abc///.../", "/.../abc/.../", PATH_SIMPLIFY_KEEP_TRAILING_SLASH); + test_path_simplify_one("/.../abc///.../", "/.../abc/...", 0); + memset(foo, 'a', sizeof(foo) -1); char_array_0(foo); From d01481cae5812e3dc1d4c3be45e1af991177706e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Sep 2023 17:42:53 +0200 Subject: [PATCH 6/9] delta: inline iterator variable --- src/delta/delta.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/delta/delta.c b/src/delta/delta.c index beca7ea975..d830c5e162 100644 --- a/src/delta/delta.c +++ b/src/delta/delta.c @@ -611,9 +611,7 @@ static int run(int argc, char *argv[]) { pager_open(arg_pager_flags); if (optind < argc) { - int i; - - for (i = optind; i < argc; i++) { + for (int i = optind; i < argc; i++) { path_simplify(argv[i]); k = process_suffix_chop(argv[i]); From 7902df12638943be7d3ee935e9837ced3c282eab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Sep 2023 17:44:38 +0200 Subject: [PATCH 7/9] core/dbus-path: use structured initialization in one more place --- src/core/dbus-path.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/dbus-path.c b/src/core/dbus-path.c index 22de551c54..ced40e70d0 100644 --- a/src/core/dbus-path.c +++ b/src/core/dbus-path.c @@ -97,7 +97,6 @@ static int bus_path_set_transient_property( if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *k = NULL; - PathSpec *s; k = strdup(path); if (!k) @@ -105,14 +104,16 @@ static int bus_path_set_transient_property( path_simplify(k); - s = new0(PathSpec, 1); + PathSpec *s = new(PathSpec, 1); if (!s) return -ENOMEM; - s->unit = u; - s->path = TAKE_PTR(k); - s->type = t; - s->inotify_fd = -EBADF; + *s = (PathSpec) { + .unit = u, + .path = TAKE_PTR(k), + .type = t, + .inotify_fd = -EBADF, + }; LIST_PREPEND(spec, p->specs, s); From 660087dc9c4a5c610da99e7b6b1772e371eb0a80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Sep 2023 17:38:17 +0200 Subject: [PATCH 8/9] tree-wide: add path_simplify_alloc() and use it path_simplify_full()/path_simplify() are changed to allow a NULL path, for which a NULL is returned. Generally, callers have already asserted before that the argument is nonnull. This way path_simplify_full()/path_simplify() and path_simplify_alloc() behave consistently. In sd-device.c, logging in device_set_syspath() is intentionally dropped: other branches don't log. In mount-tool.c, logging in parse_argv() is changed to log the user-specified value, not the simplified string. In an error message, we should show the actual argument we got, not some transformed version. --- src/basic/cgroup-util.c | 21 ++++---------- src/basic/fileio.c | 22 ++++++--------- src/basic/path-util.c | 10 +++---- src/basic/path-util.h | 16 +++++++++++ src/basic/unit-name.c | 9 +++--- src/basic/user-util.c | 16 ++--------- src/core/dbus-path.c | 8 ++---- src/core/dbus-socket.c | 8 ++---- src/libsystemd/sd-device/sd-device.c | 8 ++---- src/mount/mount-tool.c | 41 ++++++++++++---------------- src/portable/portable.c | 19 +++++-------- src/shared/install.c | 21 +++++--------- src/udev/udev-node.c | 9 +++--- 13 files changed, 84 insertions(+), 124 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index b2b0ddd190..c088a84532 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -947,6 +947,7 @@ int cg_is_empty_recursive(const char *controller, const char *path) { int cg_split_spec(const char *spec, char **ret_controller, char **ret_path) { _cleanup_free_ char *controller = NULL, *path = NULL; + int r; assert(spec); @@ -955,11 +956,9 @@ int cg_split_spec(const char *spec, char **ret_controller, char **ret_path) { return -EINVAL; if (ret_path) { - path = strdup(spec); - if (!path) - return -ENOMEM; - - path_simplify(path); + r = path_simplify_alloc(spec, &path); + if (r < 0) + return r; } } else { @@ -1006,22 +1005,14 @@ int cg_split_spec(const char *spec, char **ret_controller, char **ret_path) { int cg_mangle_path(const char *path, char **result) { _cleanup_free_ char *c = NULL, *p = NULL; - char *t; int r; assert(path); assert(result); /* First, check if it already is a filesystem path */ - if (path_startswith(path, "/sys/fs/cgroup")) { - - t = strdup(path); - if (!t) - return -ENOMEM; - - *result = path_simplify(t); - return 0; - } + if (path_startswith(path, "/sys/fs/cgroup")) + return path_simplify_alloc(path, result); /* Otherwise, treat it as cg spec */ r = cg_split_spec(path, &c, &p); diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 48ffb4e5e6..ee1621822c 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1110,6 +1110,7 @@ int search_and_fopen( char **ret_path) { _cleanup_strv_free_ char **copy = NULL; + int r; assert(filename); assert(mode); @@ -1123,13 +1124,9 @@ int search_and_fopen( return -errno; if (ret_path) { - char *p; - - p = strdup(filename); - if (!p) - return -ENOMEM; - - *ret_path = path_simplify(p); + r = path_simplify_alloc(filename, ret_path); + if (r < 0) + return r; } *ret = TAKE_PTR(f); @@ -1152,6 +1149,7 @@ int search_and_fopen_nulstr( char **ret_path) { _cleanup_strv_free_ char **s = NULL; + int r; if (path_is_absolute(filename)) { _cleanup_fclose_ FILE *f = NULL; @@ -1161,13 +1159,9 @@ int search_and_fopen_nulstr( return -errno; if (ret_path) { - char *p; - - p = strdup(filename); - if (!p) - return -ENOMEM; - - *ret_path = path_simplify(p); + r = path_simplify_alloc(filename, ret_path); + if (r < 0) + return r; } *ret = TAKE_PTR(f); diff --git a/src/basic/path-util.c b/src/basic/path-util.c index b036191144..6810bf66aa 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -132,11 +132,9 @@ int path_make_relative(const char *from, const char *to, char **ret) { return -ENOMEM; } else { /* 'to' is inside of 'from'. */ - result = strdup(t); - if (!result) - return -ENOMEM; - - path_simplify(result); + r = path_simplify_alloc(t, &result); + if (r < 0) + return r; if (!path_is_valid(result)) return -EINVAL; @@ -346,7 +344,7 @@ char** path_strv_resolve_uniq(char **l, const char *root) { char* path_simplify_full(char *path, PathSimplifyFlags flags) { bool add_slash = false, keep_trailing_slash, absolute, beginning = true; - char *f = ASSERT_PTR(path); + char *f = path; int r; /* Removes redundant inner and trailing slashes. Also removes unnecessary dots. diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 3f60f4d133..6d943e967f 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -85,6 +85,22 @@ static inline char* path_simplify(char *path) { return path_simplify_full(path, 0); } +static inline int path_simplify_alloc(const char *path, char **ret) { + assert(ret); + + if (!path) { + *ret = NULL; + return 0; + } + + char *t = strdup(path); + if (!t) + return -ENOMEM; + + *ret = path_simplify(t); + return 0; +} + static inline bool path_equal_ptr(const char *a, const char *b) { return !!a == !!b && (!a || path_equal(a, b)); } diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index a6ef0c4321..06877227e4 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -389,15 +389,14 @@ int unit_name_unescape(const char *f, char **ret) { int unit_name_path_escape(const char *f, char **ret) { _cleanup_free_ char *p = NULL; char *s; + int r; assert(f); assert(ret); - p = strdup(f); - if (!p) - return -ENOMEM; - - path_simplify(p); + r = path_simplify_alloc(f, &p); + if (r < 0) + return r; if (empty_or_root(p)) s = strdup("-"); diff --git a/src/basic/user-util.c b/src/basic/user-util.c index 5c39847733..9ae8577238 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -589,7 +589,6 @@ int getgroups_alloc(gid_t** gids) { int get_home_dir(char **ret) { struct passwd *p; const char *e; - char *h; uid_t u; assert(ret); @@ -622,18 +621,12 @@ int get_home_dir(char **ret) { return -EINVAL; found: - h = strdup(e); - if (!h) - return -ENOMEM; - - *ret = path_simplify(h); - return 0; + return path_simplify_alloc(e, ret); } int get_shell(char **ret) { struct passwd *p; const char *e; - char *s; uid_t u; assert(ret); @@ -665,12 +658,7 @@ int get_shell(char **ret) { return -EINVAL; found: - s = strdup(e); - if (!s) - return -ENOMEM; - - *ret = path_simplify(s); - return 0; + return path_simplify_alloc(e, ret); } int reset_uid_gid(void) { diff --git a/src/core/dbus-path.c b/src/core/dbus-path.c index ced40e70d0..8cb6a265fa 100644 --- a/src/core/dbus-path.c +++ b/src/core/dbus-path.c @@ -98,11 +98,9 @@ static int bus_path_set_transient_property( if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *k = NULL; - k = strdup(path); - if (!k) - return -ENOMEM; - - path_simplify(k); + r = path_simplify_alloc(path, &k); + if (r < 0) + return r; PathSpec *s = new(PathSpec, 1); if (!s) diff --git a/src/core/dbus-socket.c b/src/core/dbus-socket.c index 09a3a9502b..f88ecbeae4 100644 --- a/src/core/dbus-socket.c +++ b/src/core/dbus-socket.c @@ -383,11 +383,9 @@ static int bus_socket_set_transient_property( if (!path_is_absolute(a) || !path_is_valid(a)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid socket path: %s", a); - p->path = strdup(a); - if (!p->path) - return log_oom(); - - path_simplify(p->path); + r = path_simplify_alloc(a, &p->path); + if (r < 0) + return r; } else if (streq(t, "Netlink")) { r = socket_address_parse_netlink(&p->address, a); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 97e2e727c0..82c2c6df1c 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -232,11 +232,9 @@ int device_set_syspath(sd_device *device, const char *_syspath, bool verify) { "sd-device: Syspath '%s' is not a subdirectory of /sys", _syspath); - syspath = strdup(_syspath); - if (!syspath) - return log_oom_debug(); - - path_simplify(syspath); + r = path_simplify_alloc(_syspath, &syspath); + if (r < 0) + return r; } assert_se(devpath = startswith(syspath, "/sys")); diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index a0dcdf53b6..90feb2f197 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -83,30 +83,26 @@ STATIC_DESTRUCTOR_REGISTER(arg_property, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_automount_property, strv_freep); static int parse_where(const char *input, char **ret_where) { - _cleanup_free_ char *where = NULL; int r; assert(input); assert(ret_where); if (arg_transport == BUS_TRANSPORT_LOCAL) { - r = chase(input, NULL, CHASE_NONEXISTENT, &where, NULL); + r = chase(input, NULL, CHASE_NONEXISTENT, ret_where, NULL); if (r < 0) return log_error_errno(r, "Failed to make path %s absolute: %m", input); } else { - where = strdup(input); - if (!where) - return log_oom(); - - path_simplify(where); - - if (!path_is_absolute(where)) + if (!path_is_absolute(input)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Path must be absolute when operating remotely: %s", - where); + input); + + r = path_simplify_alloc(input, ret_where); + if (r < 0) + return log_error_errno(r, "Failed to simplify path %s: %m", input); } - *ret_where = TAKE_PTR(where); return 0; } @@ -441,17 +437,16 @@ static int parse_argv(int argc, char *argv[]) { r = chase(u, NULL, 0, &arg_mount_what, NULL); if (r < 0) return log_error_errno(r, "Failed to make path %s absolute: %m", u); + } else { - arg_mount_what = strdup(argv[optind]); - if (!arg_mount_what) - return log_oom(); - - path_simplify(arg_mount_what); - - if (!path_is_absolute(arg_mount_what)) + if (!path_is_absolute(argv[optind])) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Path must be absolute when operating remotely: %s", - arg_mount_what); + argv[optind]); + + r = path_simplify_alloc(argv[optind], &arg_mount_what); + if (r < 0) + return log_error_errno(r, "Failed to simplify path: %m"); } } @@ -1054,11 +1049,9 @@ static int action_umount( for (int i = optind; i < argc; i++) { _cleanup_free_ char *p = NULL; - p = strdup(argv[i]); - if (!p) - return log_oom(); - - path_simplify(p); + r = path_simplify_alloc(argv[i], &p); + if (r < 0) + return r; r = stop_mounts(bus, p); if (r < 0) diff --git a/src/portable/portable.c b/src/portable/portable.c index 545a3ed5d2..52f38dc83e 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -838,6 +838,7 @@ static int portable_changes_add( _cleanup_free_ char *p = NULL, *s = NULL; PortableChange *c; + int r; assert(path); assert(!changes == !n_changes); @@ -855,19 +856,13 @@ static int portable_changes_add( return -ENOMEM; *changes = c; - p = strdup(path); - if (!p) - return -ENOMEM; + r = path_simplify_alloc(path, &p); + if (r < 0) + return r; - path_simplify(p); - - if (source) { - s = strdup(source); - if (!s) - return -ENOMEM; - - path_simplify(s); - } + r = path_simplify_alloc(source, &s); + if (r < 0) + return r; c[(*n_changes)++] = (PortableChange) { .type_or_errno = type_or_errno, diff --git a/src/shared/install.c b/src/shared/install.c index c171a2330e..3186444d2f 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -285,6 +285,7 @@ InstallChangeType install_changes_add( _cleanup_free_ char *p = NULL, *s = NULL; InstallChange *c; + int r; assert(!changes == !n_changes); assert(INSTALL_CHANGE_TYPE_VALID(type)); @@ -303,21 +304,13 @@ InstallChangeType install_changes_add( return -ENOMEM; *changes = c; - if (path) { - p = strdup(path); - if (!p) - return -ENOMEM; + r = path_simplify_alloc(path, &p); + if (r < 0) + return r; - path_simplify(p); - } - - if (source) { - s = strdup(source); - if (!s) - return -ENOMEM; - - path_simplify(s); - } + r = path_simplify_alloc(source, &s); + if (r < 0) + return r; c[(*n_changes)++] = (InstallChange) { .type = type, diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 75845413cf..6ba38087dc 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -331,15 +331,14 @@ static int stack_directory_get_name(const char *slink, char **ret) { _cleanup_free_ char *s = NULL, *dirname = NULL; char name_enc[NAME_MAX+1]; const char *name; + int r; assert(slink); assert(ret); - s = strdup(slink); - if (!s) - return -ENOMEM; - - path_simplify(s); + r = path_simplify_alloc(slink, &s); + if (r < 0) + return r; if (!path_is_normalized(s)) return -EINVAL; From 5342eb4633e4a913e0049946ea31cc665894cc10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 20:22:29 +0200 Subject: [PATCH 9/9] Rework unit_name_mangle_with_suffix() to (very slightly) simplify the path 'systemctl status /../dev' now looks for 'dev.mount', not '-..-dev.service', and 'systemctl status /../foo' looks for 'foo.mount', not '-..-foo.service'. I think this much more useful. I think the escaping is not very useful, so I plan to submit a later series which changes that behaviour. But I think this first step here is already useful on its own. Note that the patch is smaller than it seems: before, is_device_path() would return true only for absolute paths, so moving of is_device_path() under the path_is_absolute() conditional doesn't influence the logic. --- man/systemd-escape.xml | 12 +++++--- src/basic/unit-name.c | 30 +++++++++++-------- src/test/test-unit-name.c | 48 ++++++++++++++++++++++++++++++- test/units/testsuite-74.escape.sh | 10 +++++-- 4 files changed, 81 insertions(+), 19 deletions(-) diff --git a/man/systemd-escape.xml b/man/systemd-escape.xml index 6a5f31a2e3..1d351c7f9b 100644 --- a/man/systemd-escape.xml +++ b/man/systemd-escape.xml @@ -90,10 +90,14 @@ - When escaping or unescaping a string, assume it refers to a file system path. This eliminates - leading, trailing or duplicate / characters and rejects . and - .. path components. This is particularly useful for generating strings suitable for - unescaping with the %f specifier in unit files, see + When escaping or unescaping a string, assume it refers to a file system path. This + simplifies the path (leading, trailing, and duplicate / characters are removed, + no-op path . components are removed, and for absolute paths, leading + .. components are removed). After the simplification, the path must not contain + ... + + This is particularly useful for generating strings suitable for unescaping with the + %f specifier in unit files, see systemd.unit5. diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index 06877227e4..8bf28ba749 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -708,7 +708,7 @@ int unit_name_mangle_with_suffix( char **ret) { _cleanup_free_ char *s = NULL; - bool mangled, suggest_escape = true; + bool mangled, suggest_escape = true, warn = flags & UNIT_NAME_MANGLE_WARN; int r; assert(name); @@ -729,22 +729,28 @@ int unit_name_mangle_with_suffix( if (string_is_glob(name) && in_charset(name, VALID_CHARS_GLOB)) { if (flags & UNIT_NAME_MANGLE_GLOB) goto good; - log_full(flags & UNIT_NAME_MANGLE_WARN ? LOG_NOTICE : LOG_DEBUG, + log_full(warn ? LOG_NOTICE : LOG_DEBUG, "Glob pattern passed%s%s, but globs are not supported for this.", operation ? " " : "", strempty(operation)); suggest_escape = false; } - if (is_device_path(name)) { - r = unit_name_from_path(name, ".device", ret); - if (r >= 0) - return 1; - if (r != -EINVAL) - return r; - } - if (path_is_absolute(name)) { - r = unit_name_from_path(name, ".mount", ret); + _cleanup_free_ char *n = NULL; + + r = path_simplify_alloc(name, &n); + if (r < 0) + return r; + + if (is_device_path(n)) { + r = unit_name_from_path(n, ".device", ret); + if (r >= 0) + return 1; + if (r != -EINVAL) + return r; + } + + r = unit_name_from_path(n, ".mount", ret); if (r >= 0) return 1; if (r != -EINVAL) @@ -757,7 +763,7 @@ int unit_name_mangle_with_suffix( mangled = do_escape_mangle(name, flags & UNIT_NAME_MANGLE_GLOB, s); if (mangled) - log_full(flags & UNIT_NAME_MANGLE_WARN ? LOG_NOTICE : LOG_DEBUG, + log_full(warn ? LOG_NOTICE : LOG_DEBUG, "Invalid unit name \"%s\" escaped as \"%s\"%s.", name, s, suggest_escape ? " (maybe you should use systemd-escape?)" : ""); diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c index ada8330408..8e9332c33e 100644 --- a/src/test/test-unit-name.c +++ b/src/test/test-unit-name.c @@ -201,8 +201,12 @@ TEST(unit_name_to_path) { static void test_unit_name_mangle_one(bool allow_globs, const char *pattern, const char *expect, int ret) { _cleanup_free_ char *t = NULL; + int r; - assert_se(unit_name_mangle(pattern, (allow_globs * UNIT_NAME_MANGLE_GLOB) | UNIT_NAME_MANGLE_WARN, &t) == ret); + r = unit_name_mangle(pattern, (allow_globs * UNIT_NAME_MANGLE_GLOB) | UNIT_NAME_MANGLE_WARN, &t); + log_debug("%s: %s -> %d, %s", __func__, pattern, r, strnull(t)); + + assert_se(r == ret); puts(strna(t)); assert_se(streq_ptr(t, expect)); @@ -234,6 +238,48 @@ TEST(unit_name_mangle) { test_unit_name_mangle_one(true, "ü*", "\\xc3\\xbc*", 1); } +static void test_unit_name_mangle_with_suffix_one(const char *arg, int expected, const char *expected_name) { + _cleanup_free_ char *s = NULL; + int r; + + r = unit_name_mangle_with_suffix(arg, NULL, 0, ".service", &s); + log_debug("%s: %s -> %d, %s", __func__, arg, r, strnull(s)); + + assert_se(r == expected); + assert_se(streq_ptr(s, expected_name)); +} + +TEST(unit_name_mangle_with_suffix) { + test_unit_name_mangle_with_suffix_one("", -EINVAL, NULL); + + test_unit_name_mangle_with_suffix_one("/dev", 1, "dev.mount"); + test_unit_name_mangle_with_suffix_one("/../dev", 1, "dev.mount"); + test_unit_name_mangle_with_suffix_one("/../dev/.", 1, "dev.mount"); + /* We don't skip the last '..', and it makes this an invalid device or mount name */ + test_unit_name_mangle_with_suffix_one("/.././dev/..", 1, "-..-.-dev-...service"); + test_unit_name_mangle_with_suffix_one("/.././dev", 1, "dev.mount"); + test_unit_name_mangle_with_suffix_one("/./.././../dev/", 1, "dev.mount"); + + test_unit_name_mangle_with_suffix_one("/dev/sda", 1, "dev-sda.device"); + test_unit_name_mangle_with_suffix_one("/dev/sda5", 1, "dev-sda5.device"); + + test_unit_name_mangle_with_suffix_one("/sys", 1, "sys.mount"); + test_unit_name_mangle_with_suffix_one("/../sys", 1, "sys.mount"); + test_unit_name_mangle_with_suffix_one("/../sys/.", 1, "sys.mount"); + /* We don't skip the last '..', and it makes this an invalid device or mount name */ + test_unit_name_mangle_with_suffix_one("/.././sys/..", 1, "-..-.-sys-...service"); + test_unit_name_mangle_with_suffix_one("/.././sys", 1, "sys.mount"); + test_unit_name_mangle_with_suffix_one("/./.././../sys/", 1, "sys.mount"); + + test_unit_name_mangle_with_suffix_one("/proc", 1, "proc.mount"); + test_unit_name_mangle_with_suffix_one("/../proc", 1, "proc.mount"); + test_unit_name_mangle_with_suffix_one("/../proc/.", 1, "proc.mount"); + /* We don't skip the last '..', and it makes this an invalid device or mount name */ + test_unit_name_mangle_with_suffix_one("/.././proc/..", 1, "-..-.-proc-...service"); + test_unit_name_mangle_with_suffix_one("/.././proc", 1, "proc.mount"); + test_unit_name_mangle_with_suffix_one("/./.././../proc/", 1, "proc.mount"); +} + TEST_RET(unit_printf, .sd_booted = true) { _cleanup_free_ char *architecture, *os_image_version, *boot_id = NULL, *os_build_id, diff --git a/test/units/testsuite-74.escape.sh b/test/units/testsuite-74.escape.sh index 584c7340b5..e398d4010b 100755 --- a/test/units/testsuite-74.escape.sh +++ b/test/units/testsuite-74.escape.sh @@ -65,13 +65,19 @@ assert_eq "$(systemd-escape --unescape --instance 'mount-my-stuff@-this-is-where assert_eq "$(systemd-escape --unescape --instance --path 'mount-my-stuff@this-is-where-my-stuff-is-\x20with\x20spaces\x20though\x20.service')" \ '/this/is/where/my/stuff/is/ with spaces though ' -# --path +# --path, reversible cases check_escape / '-' --path check_escape '/hello/world' 'hello-world' --path check_escape '/mnt/smb/おにぎり' \ 'mnt-smb-\xe3\x81\x8a\xe3\x81\xab\xe3\x81\x8e\xe3\x82\x8a' \ --path +# --path, non-reversible cases +assert_eq "$(systemd-escape --path ///////////////)" '-' +assert_eq "$(systemd-escape --path /..)" '-' +assert_eq "$(systemd-escape --path /../.././../.././)" '-' +assert_eq "$(systemd-escape --path /../.././../.././foo)" 'foo' + # --mangle assert_eq "$(systemd-escape --mangle 'hello-world')" 'hello-world.service' assert_eq "$(systemd-escape --mangle '/mount/this')" 'mount-this.mount' @@ -94,7 +100,7 @@ assert_eq "$(systemd-escape --mangle 'trailing-whitespace.mount ')" 'trailing-wh (! systemd-escape --instance 'hello@hello.service') (! systemd-escape --instance --template=hello@.service 'hello@hello.service') (! systemd-escape --unescape --instance --path 'mount-my-stuff@-this-is-where-my-stuff-is-\x20with\x20spaces\x20though\x20.service') -(! systemd-escape --path '/../hello') +(! systemd-escape --path '/../hello/..') (! systemd-escape --path '.') (! systemd-escape --path '..') (! systemd-escape --path "$(set +x; printf '%0.sa' {0..256})")