When mangling names, optionally emit a warning (#8400)

The warning is not emitted for absolute paths like /dev/sda or /home, which are
converted to .device and .mount unit names without any fuss.

Most of the time it's unlikely that users use invalid unit names on purpose,
so let's warn them. Warnings are silenced when --quiet is used.

$ build/systemctl show -p Id hello@foo-bar/baz
Invalid unit name "hello@foo-bar/baz" was escaped as "hello@foo-bar-baz" (maybe you should use systemd-escape?)
Id=hello@foo-bar-baz.service

$ build/systemd-run --user --slice foo-bar/baz --unit foo-bar/foo true
Invalid unit name "foo-bar/foo" was escaped as "foo-bar-foo" (maybe you should use systemd-escape?)
Invalid unit name "foo-bar/baz" was escaped as "foo-bar-baz" (maybe you should use systemd-escape?)
Running as unit: foo-bar-foo.service

Fixes #8302.

diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c
index 0fa0472ee1312d5b17a03586e366604ddcf938cf..3937dfc5eefde4e6443bfd13b8961fd72dbd3a9a 100644 (file)
--- a/src/basic/unit-name.c
+++ b/src/basic/unit-name.c
@@ -569,28 +569,32 @@ int unit_name_to_path(const char *name, char **ret) {
         return unit_name_path_unescape(prefix, ret);
 }
 
-static char *do_escape_mangle(const char *f, UnitNameMangle allow_globs, char *t) {
+static bool do_escape_mangle(const char *f, bool allow_globs, char *t) {
         const char *valid_chars;
+        bool mangled = false;
 
         assert(f);
-        assert(IN_SET(allow_globs, UNIT_NAME_GLOB, UNIT_NAME_NOGLOB));
         assert(t);
 
-        /* We'll only escape the obvious characters here, to play
-         * safe. */
+        /* We'll only escape the obvious characters here, to play safe.
+         *
+         * Returns true if any characters were mangled, false otherwise.
+         */
 
-        valid_chars = allow_globs == UNIT_NAME_GLOB ? VALID_CHARS_GLOB : VALID_CHARS_WITH_AT;
+        valid_chars = allow_globs ? VALID_CHARS_GLOB : VALID_CHARS_WITH_AT;
 
-        for (; *f; f++) {
-                if (*f == '/')
+        for (; *f; f++)
+                if (*f == '/') {
                         *(t++) = '-';
-                else if (!strchr(valid_chars, *f))
+                        mangled = true;
+                } else if (!strchr(valid_chars, *f)) {
                         t = do_escape_char(*f, t);
-                else
+                        mangled = true;
+                } else
                         *(t++) = *f;
-        }
+        *t = 0;
 
-        return t;
+        return mangled;
 }
 
 /**
@@ -600,9 +604,10 @@ static char *do_escape_mangle(const char *f, UnitNameMangle allow_globs, char *t
  *
  *  If @allow_globs, globs characters are preserved. Otherwise, they are escaped.
  */
-int unit_name_mangle_with_suffix(const char *name, UnitNameMangle allow_globs, const char *suffix, char **ret) {
-        char *s, *t;
+int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const char *suffix, char **ret) {
+        char *s;
         int r;
+        bool mangled;
 
         assert(name);
         assert(suffix);
@@ -619,7 +624,7 @@ int unit_name_mangle_with_suffix(const char *name, UnitNameMangle allow_globs, c
                 goto good;
 
         /* Already a fully valid globbing expression? If so, no mangling is necessary either... */
-        if (allow_globs == UNIT_NAME_GLOB &&
+        if ((flags & UNIT_NAME_MANGLE_GLOB) &&
             string_is_glob(name) &&
             in_charset(name, VALID_CHARS_GLOB))
                 goto good;
@@ -644,13 +649,16 @@ int unit_name_mangle_with_suffix(const char *name, UnitNameMangle allow_globs, c
         if (!s)
                 return -ENOMEM;
 
-        t = do_escape_mangle(name, allow_globs, s);
-        *t = 0;
+        mangled = do_escape_mangle(name, flags & UNIT_NAME_MANGLE_GLOB, s);
+        if (mangled)
+                log_full(flags & UNIT_NAME_MANGLE_WARN ? LOG_NOTICE : LOG_DEBUG,
+                         "Invalid unit name \"%s\" was escaped as \"%s\" (maybe you should use systemd-escape?)",
+                         name, s);
 
         /* Append a suffix if it doesn't have any, but only if this is not a glob, so that we can allow "foo.*" as a
          * valid glob. */
-        if ((allow_globs != UNIT_NAME_GLOB || !string_is_glob(s)) && unit_name_to_type(s) < 0)
-                strcpy(t, suffix);
+        if ((!(flags & UNIT_NAME_MANGLE_GLOB) || !string_is_glob(s)) && unit_name_to_type(s) < 0)
+                strcat(s, suffix);
 
         *ret = s;
         return 1;

diff --git a/src/basic/unit-name.h b/src/basic/unit-name.h
index b47327dcaf91ce20a262fdc524b24e88a5a700ff..278d1378f7edfa7b191dc5a843286bdc550077ce 100644 (file)
--- a/src/basic/unit-name.h
+++ b/src/basic/unit-name.h
@@ -68,14 +68,14 @@ int unit_name_from_path_instance(const char *prefix, const char *path, const cha
 int unit_name_to_path(const char *name, char **ret);
 
 typedef enum UnitNameMangle {
-        UNIT_NAME_NOGLOB,
-        UNIT_NAME_GLOB,
+        UNIT_NAME_MANGLE_GLOB = 1,
+        UNIT_NAME_MANGLE_WARN = 2,
 } UnitNameMangle;
 
-int unit_name_mangle_with_suffix(const char *name, UnitNameMangle allow_globs, const char *suffix, char **ret);
+int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const char *suffix, char **ret);
 
-static inline int unit_name_mangle(const char *name, UnitNameMangle allow_globs, char **ret) {
-        return unit_name_mangle_with_suffix(name, allow_globs, ".service", ret);
+static inline int unit_name_mangle(const char *name, UnitNameMangle flags, char **ret) {
+        return unit_name_mangle_with_suffix(name, flags, ".service", ret);
 }
 
 int slice_build_parent_slice(const char *slice, char **ret);

diff --git a/src/core/device.c b/src/core/device.c
index b0dd469fd14b8eb9abc5165f7874897a95672cc5..66bfc30b5f2f7d76a3b8460916875535e1781a8a 100644 (file)
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -296,7 +296,7 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) {
                 } else {
                         /* If this is not a template, then let's mangle it so, that it becomes a valid unit name. */
 
-                        r = unit_name_mangle(word, UNIT_NAME_NOGLOB, &k);
+                        r = unit_name_mangle(word, UNIT_NAME_MANGLE_WARN, &k);
                         if (r < 0)
                                 return log_unit_error_errno(u, r, "Failed to mangle unit name \"%s\": %m", word);
                 }

diff --git a/src/debug-generator/debug-generator.c b/src/debug-generator/debug-generator.c
index 61c890d05a1a24bf23e9555c2ea99eefae230d20..cd0507ced60b27cad8d5c9fe27bb8fcbeccf6731 100644 (file)
--- a/src/debug-generator/debug-generator.c
+++ b/src/debug-generator/debug-generator.c
@@ -45,7 +45,7 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat
                 if (proc_cmdline_value_missing(key, value))
                         return 0;
 
-                r = unit_name_mangle(value, UNIT_NAME_NOGLOB, &n);
+                r = unit_name_mangle(value, UNIT_NAME_MANGLE_WARN, &n);
                 if (r < 0)
                         return log_error_errno(r, "Failed to glob unit name: %m");
 
@@ -59,7 +59,7 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat
                 if (proc_cmdline_value_missing(key, value))
                         return 0;
 
-                r = unit_name_mangle(value, UNIT_NAME_NOGLOB, &n);
+                r = unit_name_mangle(value, UNIT_NAME_MANGLE_WARN, &n);
                 if (r < 0)
                         return log_error_errno(r, "Failed to glob unit name: %m");
 

diff --git a/src/escape/escape.c b/src/escape/escape.c
index 9c35b4663e7ddee5e13f8bb13b8d6c54d279e6b1..2eb0a58673ac9cd4622d4aff8b5eeb9555bf1898 100644 (file)
--- a/src/escape/escape.c
+++ b/src/escape/escape.c
@@ -217,7 +217,7 @@ int main(int argc, char *argv[]) {
                         break;
 
                 case ACTION_MANGLE:
-                        r = unit_name_mangle(*i, UNIT_NAME_NOGLOB, &e);
+                        r = unit_name_mangle(*i, 0, &e);
                         if (r < 0) {
                                 log_error_errno(r, "Failed to mangle name: %m");
                                 goto finish;

diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c
index fc58cba64f56e442c449dc4fffe8234fb091e60e..20c457e60c127a0870939a926529c583f38bea10 100644 (file)
--- a/src/fstab-generator/fstab-generator.c
+++ b/src/fstab-generator/fstab-generator.c
@@ -237,7 +237,7 @@ static int write_dependency(FILE *f, const char *opts,
         STRV_FOREACH(s, names) {
                 char *x;
 
-                r = unit_name_mangle_with_suffix(*s, UNIT_NAME_NOGLOB, ".mount", &x);
+                r = unit_name_mangle_with_suffix(*s, 0, ".mount", &x);
                 if (r < 0)
                         return log_error_errno(r, "Failed to generate unit name: %m");
                 r = strv_consume(&units, x);

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 3c9c6df07dbf1fcffd89fd6b44a42c3daff01ae8..e8eb9ac1611098367e90a65a00971fcbbaa43f08 100644 (file)
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -1539,7 +1539,7 @@ static int add_units(sd_journal *j) {
         STRV_FOREACH(i, arg_system_units) {
                 _cleanup_free_ char *u = NULL;
 
-                r = unit_name_mangle(*i, UNIT_NAME_GLOB, &u);
+                r = unit_name_mangle(*i, UNIT_NAME_MANGLE_GLOB | (arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN), &u);
                 if (r < 0)
                         return r;
 
@@ -1584,7 +1584,7 @@ static int add_units(sd_journal *j) {
         STRV_FOREACH(i, arg_user_units) {
                 _cleanup_free_ char *u = NULL;
 
-                r = unit_name_mangle(*i, UNIT_NAME_GLOB, &u);
+                r = unit_name_mangle(*i, UNIT_NAME_MANGLE_GLOB | (arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN), &u);
                 if (r < 0)
                         return r;
 

diff --git a/src/nspawn/nspawn-register.c b/src/nspawn/nspawn-register.c
index 07d68242c6820b4d83904ab46c0362ee0a312840..838634b62422eceb9ab37fe3f2c7f556dffe6b57 100644 (file)
--- a/src/nspawn/nspawn-register.c
+++ b/src/nspawn/nspawn-register.c
@@ -293,7 +293,7 @@ int allocate_scope(
         if (r < 0)
                 return log_error_errno(r, "Could not watch job: %m");
 
-        r = unit_name_mangle_with_suffix(machine_name, UNIT_NAME_NOGLOB, ".scope", &scope);
+        r = unit_name_mangle_with_suffix(machine_name, 0, ".scope", &scope);
         if (r < 0)
                 return log_error_errno(r, "Failed to mangle scope name: %m");
 

diff --git a/src/run/run.c b/src/run/run.c
index cfab3d6f97847af7bc8262f4d88a466ab3d8a209..1da185dde9e29f4a0a053b3beaef49e05aebf7bb 100644 (file)
--- a/src/run/run.c
+++ b/src/run/run.c
@@ -520,7 +520,7 @@ static int transient_cgroup_set_properties(sd_bus_message *m) {
         if (!isempty(arg_slice)) {
                 _cleanup_free_ char *slice = NULL;
 
-                r = unit_name_mangle_with_suffix(arg_slice, UNIT_NAME_NOGLOB, ".slice", &slice);
+                r = unit_name_mangle_with_suffix(arg_slice, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".slice", &slice);
                 if (r < 0)
                         return log_error_errno(r, "Failed to mangle name '%s': %m", arg_slice);
 
@@ -984,7 +984,7 @@ static int start_transient_service(
         }
 
         if (arg_unit) {
-                r = unit_name_mangle_with_suffix(arg_unit, UNIT_NAME_NOGLOB, ".service", &service);
+                r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".service", &service);
                 if (r < 0)
                         return log_error_errno(r, "Failed to mangle unit name: %m");
         } else {
@@ -1187,7 +1187,7 @@ static int start_transient_scope(
                 return log_oom();
 
         if (arg_unit) {
-                r = unit_name_mangle_with_suffix(arg_unit, UNIT_NAME_NOGLOB, ".scope", &scope);
+                r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".scope", &scope);
                 if (r < 0)
                         return log_error_errno(r, "Failed to mangle scope name: %m");
         } else {
@@ -1358,11 +1358,11 @@ static int start_transient_trigger(
                         break;
 
                 default:
-                        r = unit_name_mangle_with_suffix(arg_unit, UNIT_NAME_NOGLOB, ".service", &service);
+                        r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".service", &service);
                         if (r < 0)
                                 return log_error_errno(r, "Failed to mangle unit name: %m");
 
-                        r = unit_name_mangle_with_suffix(arg_unit, UNIT_NAME_NOGLOB, suffix, &trigger);
+                        r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, suffix, &trigger);
                         if (r < 0)
                                 return log_error_errno(r, "Failed to mangle unit name: %m");
 

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index cae4ff023a07f05ccbbf19f612c12493cdf0bc7e..064d59313195247094cf4af1dd28e1865daeb3a7 100644 (file)
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -1810,7 +1810,7 @@ static int list_dependencies(int argc, char *argv[], void *userdata) {
         int r;
 
         if (argv[1]) {
-                r = unit_name_mangle(argv[1], UNIT_NAME_NOGLOB, &unit);
+                r = unit_name_mangle(argv[1], arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, &unit);
                 if (r < 0)
                         return log_error_errno(r, "Failed to mangle unit name: %m");
 
@@ -2116,7 +2116,7 @@ static int set_default(int argc, char *argv[], void *userdata) {
         assert(argc >= 2);
         assert(argv);
 
-        r = unit_name_mangle_with_suffix(argv[1], UNIT_NAME_NOGLOB, ".target", &unit);
+        r = unit_name_mangle_with_suffix(argv[1], arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".target", &unit);
         if (r < 0)
                 return log_error_errno(r, "Failed to mangle unit name: %m");
 
@@ -2663,7 +2663,7 @@ static int check_triggering_units(
         char **i;
         int r;
 
-        r = unit_name_mangle(name, UNIT_NAME_NOGLOB, &n);
+        r = unit_name_mangle(name, 0, &n);
         if (r < 0)
                 return log_error_errno(r, "Failed to mangle unit name: %m");
 
@@ -2949,11 +2949,12 @@ static int expand_names(sd_bus *bus, char **names, const char* suffix, char ***r
 
         STRV_FOREACH(name, names) {
                 char *t;
+                UnitNameMangle options = UNIT_NAME_MANGLE_GLOB | (arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN);
 
                 if (suffix)
-                        r = unit_name_mangle_with_suffix(*name, UNIT_NAME_GLOB, suffix, &t);
+                        r = unit_name_mangle_with_suffix(*name, options, suffix, &t);
                 else
-                        r = unit_name_mangle(*name, UNIT_NAME_GLOB, &t);
+                        r = unit_name_mangle(*name, options, &t);
                 if (r < 0)
                         return log_error_errno(r, "Failed to mangle name: %m");
 
@@ -5422,7 +5423,7 @@ static int set_property(int argc, char *argv[], void *userdata) {
         if (r < 0)
                 return bus_log_create_error(r);
 
-        r = unit_name_mangle(argv[1], UNIT_NAME_NOGLOB, &n);
+        r = unit_name_mangle(argv[1], arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, &n);
         if (r < 0)
                 return log_error_errno(r, "Failed to mangle unit name: %m");
 
@@ -6000,7 +6001,7 @@ static int mangle_names(char **original_names, char ***mangled_names) {
                                 return log_oom();
                         }
                 } else {
-                        r = unit_name_mangle(*name, UNIT_NAME_NOGLOB, i);
+                        r = unit_name_mangle(*name, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, i);
                         if (r < 0) {
                                 *i = NULL;
                                 strv_free(l);
@@ -6336,7 +6337,7 @@ static int add_dependency(int argc, char *argv[], void *userdata) {
         if (!argv[1])
                 return 0;
 
-        r = unit_name_mangle_with_suffix(argv[1], UNIT_NAME_NOGLOB, ".target", &target);
+        r = unit_name_mangle_with_suffix(argv[1], arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".target", &target);
         if (r < 0)
                 return log_error_errno(r, "Failed to mangle unit name: %m");
 

diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index 087ba085596e6fca636af509315d1c9abac3f04c..92669c6758c8288f6d701f13231d15118c00f319 100644 (file)
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -240,7 +240,7 @@ static char *sysv_translate_name(const char *name) {
         if (res)
                 *res = 0;
 
-        if (unit_name_mangle(c, UNIT_NAME_NOGLOB, &res) < 0)
+        if (unit_name_mangle(c, 0, &res) < 0)
                 return NULL;
 
         return res;

diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c
index f7598d228420ab813fbe8b5ab604045be02eb127..3c7a17b42f7afdf463ef06e361af47a07f2684b2 100644 (file)
--- a/src/test/test-unit-name.c
+++ b/src/test/test-unit-name.c
@@ -162,10 +162,10 @@ static void test_unit_name_to_path(void) {
         test_unit_name_to_path_one("home/foo", NULL, -EINVAL);
 }
 
-static void test_unit_name_mangle_one(UnitNameMangle allow_globs, const char *pattern, const char *expect, int ret) {
+static void test_unit_name_mangle_one(bool allow_globs, const char *pattern, const char *expect, int ret) {
         _cleanup_free_ char *t = NULL;
 
-        assert_se(unit_name_mangle(pattern, allow_globs, &t) == ret);
+        assert_se(unit_name_mangle(pattern, (allow_globs * UNIT_NAME_MANGLE_GLOB) | UNIT_NAME_MANGLE_WARN, &t) == ret);
         puts(strna(t));
         assert_se(streq_ptr(t, expect));
 
@@ -173,29 +173,29 @@ static void test_unit_name_mangle_one(UnitNameMangle allow_globs, const char *pa
                 _cleanup_free_ char *k = NULL;
 
                 assert_se(unit_name_is_valid(t, UNIT_NAME_ANY) ||
-                          (allow_globs == UNIT_NAME_GLOB && string_is_glob(t)));
+                          (allow_globs && string_is_glob(t)));
 
-                assert_se(unit_name_mangle(t, allow_globs, &k) == 0);
+                assert_se(unit_name_mangle(t, (allow_globs * UNIT_NAME_MANGLE_GLOB) | UNIT_NAME_MANGLE_WARN, &k) == 0);
                 assert_se(streq_ptr(t, k));
         }
 }
 
 static void test_unit_name_mangle(void) {
         puts("-------------------------------------------------");
-        test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "foo.service", "foo.service", 0);
-        test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "/home", "home.mount", 1);
-        test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "/dev/sda", "dev-sda.device", 1);
-        test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "üxknürz.service", "\\xc3\\xbcxkn\\xc3\\xbcrz.service", 1);
-        test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "foobar-meh...waldi.service", "foobar-meh...waldi.service", 0);
-        test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "_____####----.....service", "_____\\x23\\x23\\x23\\x23----.....service", 1);
-        test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "_____##@;;;,,,##----.....service", "_____\\x23\\x23@\\x3b\\x3b\\x3b\\x2c\\x2c\\x2c\\x23\\x23----.....service", 1);
-        test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "xxx@@@@/////\\\\\\\\\\yyy.service", "xxx@@@@-----\\\\\\\\\\yyy.service", 1);
-        test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "", NULL, -EINVAL);
-
-        test_unit_name_mangle_one(UNIT_NAME_GLOB, "foo.service", "foo.service", 0);
-        test_unit_name_mangle_one(UNIT_NAME_GLOB, "foo", "foo.service", 1);
-        test_unit_name_mangle_one(UNIT_NAME_GLOB, "foo*", "foo*", 0);
-        test_unit_name_mangle_one(UNIT_NAME_GLOB, "ü*", "\\xc3\\xbc*", 1);
+        test_unit_name_mangle_one(false, "foo.service", "foo.service", 0);
+        test_unit_name_mangle_one(false, "/home", "home.mount", 1);
+        test_unit_name_mangle_one(false, "/dev/sda", "dev-sda.device", 1);
+        test_unit_name_mangle_one(false, "üxknürz.service", "\\xc3\\xbcxkn\\xc3\\xbcrz.service", 1);
+        test_unit_name_mangle_one(false, "foobar-meh...waldi.service", "foobar-meh...waldi.service", 0);
+        test_unit_name_mangle_one(false, "_____####----.....service", "_____\\x23\\x23\\x23\\x23----.....service", 1);
+        test_unit_name_mangle_one(false, "_____##@;;;,,,##----.....service", "_____\\x23\\x23@\\x3b\\x3b\\x3b\\x2c\\x2c\\x2c\\x23\\x23----.....service", 1);
+        test_unit_name_mangle_one(false, "xxx@@@@/////\\\\\\\\\\yyy.service", "xxx@@@@-----\\\\\\\\\\yyy.service", 1);
+        test_unit_name_mangle_one(false, "", NULL, -EINVAL);
+
+        test_unit_name_mangle_one(true, "foo.service", "foo.service", 0);
+        test_unit_name_mangle_one(true, "foo", "foo.service", 1);
+        test_unit_name_mangle_one(true, "foo*", "foo*", 0);
+        test_unit_name_mangle_one(true, "ü*", "\\xc3\\xbc*", 1);
 }
 
 static int test_unit_printf(void) {