From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Wed, 21 Mar 2018 14:26:47 +0000 (+0100)
Subject: When mangling names, optionally emit a warning (#8400)
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=6895c0be1845203e14b776dd8a15a97780ac6942;p=elogind.git

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 d42d59bd1..1d938f662 100644
--- a/src/basic/unit-name.c
+++ b/src/basic/unit-name.c
@@ -574,28 +574,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;
 }
 
 /**
@@ -605,9 +609,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);
@@ -624,7 +629,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;
@@ -649,13 +654,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 0fcba1153..50ac3e10b 100644
--- a/src/basic/unit-name.h
+++ b/src/basic/unit-name.h
@@ -75,14 +75,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);