shaba/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd.git synced 2025-02-27 01:57:35 +03:00
Code

Rework unit_name_mangle_with_suffix() to (very slightly) simplify the path

Browse Source 
'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.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2023-09-16 20:22:29 +02:00
parent 660087dc9c
commit 5342eb4633
4 changed files with 81 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
man/systemd-escape.xml
View File

@ -90,10 +90,14 @@
<term><option>--path</option></term>
<term><option>-p</option></term>
<listitem><para>When escaping or unescaping a string, assume it refers to a file system path. This eliminates
leading, trailing or duplicate <literal>/</literal> characters and rejects <literal>.</literal> and
<literal>..</literal> path components. This is particularly useful for generating strings suitable for
unescaping with the <literal>%f</literal> specifier in unit files, see
<listitem><para>When escaping or unescaping a string, assume it refers to a file system path. This
simplifies the path (leading, trailing, and duplicate <literal>/</literal> characters are removed,
no-op path <literal>.</literal> components are removed, and for absolute paths, leading
<literal>..</literal> components are removed). After the simplification, the path must not contain
<literal>..</literal>.</para>
<para>This is particularly useful for generating strings suitable for unescaping with the
<literal>%f</literal> specifier in unit files, see
<citerefentry><refentrytitle>systemd.unit</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
</para>

30
src/basic/unit-name.c
View File

@ -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?)" : "");

48
src/test/test-unit-name.c
View File

@ -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,

10
test/units/testsuite-74.escape.sh
View File

@ -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})")