util: rework cunescape(), improve error handling

Change cunescape() to return a normal error code, so that we can
distuingish OOM errors from parse errors.

This also adds a flags parameter to control whether "relaxed" or normal
parsing shall be done. If set no parse failures are generated, and the
only reason why cunescape() can fail is OOM.

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 07384d3668d056219bfa98b428c7f186db2739f1..e61418db5007292f2d0ab4210822b264f217faf0 100644 (file)
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -507,16 +507,17 @@ int config_parse_exec_oom_score_adjust(const char* unit,
         return 0;
 }
 
-int config_parse_exec(const char *unit,
-                      const char *filename,
-                      unsigned line,
-                      const char *section,
-                      unsigned section_line,
-                      const char *lvalue,
-                      int ltype,
-                      const char *rvalue,
-                      void *data,
-                      void *userdata) {
+int config_parse_exec(
+                const char *unit,
+                const char *filename,
+                unsigned line,
+                const char *section,
+                unsigned section_line,
+                const char *lvalue,
+                int ltype,
+                const char *rvalue,
+                void *data,
+                void *userdata) {
 
         ExecCommand **e = data, *nce;
         char *path, **n;
@@ -568,15 +569,13 @@ int config_parse_exec(const char *unit,
                                                 word ++;
                                         }
                                 }
-                        } else
-                                if (strneq(word, ";", MAX(l, 1U)))
+                        } else if (strneq(word, ";", MAX(l, 1U)))
                                         goto found;
 
                         k++;
                 }
                 if (!isempty(state)) {
-                        log_syntax(unit, LOG_ERR, filename, line, EINVAL,
-                                   "Trailing garbage, ignoring.");
+                        log_syntax(unit, LOG_ERR, filename, line, EINVAL, "Trailing garbage, ignoring.");
                         return 0;
                 }
 
@@ -605,9 +604,10 @@ int config_parse_exec(const char *unit,
                         else
                                 skip = strneq(word, "\\;", MAX(l, 1U));
 
-                        c = cunescape_length(word + skip, l - skip);
-                        if (!c) {
-                                r = log_oom();
+                        r = cunescape_length(word + skip, l - skip, 0, &c);
+                        if (r < 0) {
+                                log_syntax(unit, LOG_ERR, filename, line, r, "Failed to unescape command line, ignoring: %s", rvalue);
+                                r = 0;
                                 goto fail;
                         }
 
@@ -637,8 +637,7 @@ int config_parse_exec(const char *unit,
                 else
                         goto ok;
 
-                log_syntax(unit, LOG_ERR, filename, line, EINVAL,
-                           "%s, ignoring: %s", reason, rvalue);
+                log_syntax(unit, LOG_ERR, filename, line, EINVAL, "%s, ignoring: %s", reason, rvalue);
                 r = 0;
                 goto fail;
 
@@ -1976,8 +1975,7 @@ int config_parse_environ(const char *unit,
         if (u) {
                 r = unit_full_printf(u, rvalue, &k);
                 if (r < 0)
-                        log_syntax(unit, LOG_ERR, filename, line, -r,
-                                   "Failed to resolve specifiers, ignoring: %s", rvalue);
+                        log_syntax(unit, LOG_ERR, filename, line, -r, "Failed to resolve specifiers, ignoring: %s", rvalue);
         }
 
         if (!k)
@@ -1989,13 +1987,14 @@ int config_parse_environ(const char *unit,
                 _cleanup_free_ char *n;
                 char **x;
 
-                n = cunescape_length(word, l);
-                if (!n)
-                        return log_oom();
+                r = cunescape_length(word, l, 0, &n);
+                if (r < 0) {
+                        log_syntax(unit, LOG_ERR, filename, line, r, "Couldn't unescape assignment, ignoring: %s", rvalue);
+                        continue;
+                }
 
                 if (!env_assignment_is_valid(n)) {
-                        log_syntax(unit, LOG_ERR, filename, line, EINVAL,
-                                   "Invalid environment assignment, ignoring: %s", rvalue);
+                        log_syntax(unit, LOG_ERR, filename, line, EINVAL, "Invalid environment assignment, ignoring: %s", rvalue);
                         continue;
                 }
 

diff --git a/src/core/manager.c b/src/core/manager.c
index 73417ab1a86825d2ce125e39c8f72a70de734409..1c912fcdf77af8cad8764f253fef29aaf9d1a337 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -2426,11 +2426,9 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         _cleanup_free_ char *uce = NULL;
                         char **e;
 
-                        uce = cunescape(l+4);
-                        if (!uce) {
-                                r = -ENOMEM;
+                        r = cunescape(l + 4, UNESCAPE_RELAX, &uce);
+                        if (r < 0)
                                 goto finish;
-                        }
 
                         e = strv_env_set(m->environment, uce);
                         if (!e) {

diff --git a/src/core/mount.c b/src/core/mount.c
index 53594cd40b4379f3e5350e48824e53ac6f7ce74d..c4aa810d05ec14e9e19c92b09458d71a444f74c0 100644 (file)
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -1579,7 +1579,7 @@ static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) {
         r = 0;
         for (;;) {
                 const char *device, *path, *options, *fstype;
-                _cleanup_free_ const char *d = NULL, *p = NULL;
+                _cleanup_free_ char *d = NULL, *p = NULL;
                 struct libmnt_fs *fs;
                 int k;
 
@@ -1594,12 +1594,10 @@ static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) {
                 options = mnt_fs_get_options(fs);
                 fstype = mnt_fs_get_fstype(fs);
 
-                d = cunescape(device);
-                if (!d)
+                if (cunescape(device, UNESCAPE_RELAX, &d) < 0)
                         return log_oom();
 
-                p = cunescape(path);
-                if (!p)
+                if (cunescape(path, UNESCAPE_RELAX, &p) < 0)
                         return log_oom();
 
                 (void) device_found_node(m, d, true, DEVICE_FOUND_MOUNT, set_flags);

diff --git a/src/core/swap.c b/src/core/swap.c
index 6b1bdb5c1e49d283e43f8903becbb32c23d5e984..53f127484f6447cb4139750eff8ddb774eba1a5d 100644 (file)
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -1099,8 +1099,7 @@ static int swap_load_proc_swaps(Manager *m, bool set_flags) {
                         continue;
                 }
 
-                d = cunescape(dev);
-                if (!d)
+                if (cunescape(dev, UNESCAPE_RELAX, &d) < 0)
                         return log_oom();
 
                 device_found_node(m, d, true, DEVICE_FOUND_SWAP, set_flags);

diff --git a/src/core/umount.c b/src/core/umount.c
index ceee70fbea6ebe46b1442fc894125275737bf5ec..bee267a5ad866b5d43ec091ccafc479a54994960 100644 (file)
--- a/src/core/umount.c
+++ b/src/core/umount.c
@@ -62,6 +62,7 @@ static void mount_points_list_free(MountPoint **head) {
 static int mount_points_list_get(MountPoint **head) {
         _cleanup_fclose_ FILE *proc_self_mountinfo = NULL;
         unsigned int i;
+        int r;
 
         assert(head);
 
@@ -97,9 +98,9 @@ static int mount_points_list_get(MountPoint **head) {
                         continue;
                 }
 
-                p = cunescape(path);
-                if (!p)
-                        return -ENOMEM;
+                r = cunescape(path, UNESCAPE_RELAX, &p);
+                if (r < 0)
+                        return r;
 
                 /* Ignore mount points we can't unmount because they
                  * are API or because we are keeping them open (like
@@ -133,10 +134,12 @@ static int mount_points_list_get(MountPoint **head) {
 static int swap_list_get(MountPoint **head) {
         _cleanup_fclose_ FILE *proc_swaps = NULL;
         unsigned int i;
+        int r;
 
         assert(head);
 
-        if (!(proc_swaps = fopen("/proc/swaps", "re")))
+        proc_swaps = fopen("/proc/swaps", "re");
+        if (!proc_swaps)
                 return (errno == ENOENT) ? 0 : -errno;
 
         (void) fscanf(proc_swaps, "%*s %*s %*s %*s %*s\n");
@@ -146,19 +149,19 @@ static int swap_list_get(MountPoint **head) {
                 char *dev = NULL, *d;
                 int k;
 
-                if ((k = fscanf(proc_swaps,
-                                "%ms " /* device/file */
-                                "%*s " /* type of swap */
-                                "%*s " /* swap size */
-                                "%*s " /* used */
-                                "%*s\n", /* priority */
-                                &dev)) != 1) {
+                k = fscanf(proc_swaps,
+                           "%ms " /* device/file */
+                           "%*s " /* type of swap */
+                           "%*s " /* swap size */
+                           "%*s " /* used */
+                           "%*s\n", /* priority */
+                           &dev);
 
+                if (k != 1) {
                         if (k == EOF)
                                 break;
 
                         log_warning("Failed to parse /proc/swaps:%u.", i);
-
                         free(dev);
                         continue;
                 }
@@ -168,14 +171,13 @@ static int swap_list_get(MountPoint **head) {
                         continue;
                 }
 
-                d = cunescape(dev);
+                r = cunescape(dev, UNESCAPE_RELAX, &d);
                 free(dev);
+                if (r < 0)
+                        return r;
 
-                if (!d) {
-                        return -ENOMEM;
-                }
-
-                if (!(swap = new0(MountPoint, 1))) {
+                swap = new0(MountPoint, 1);
+                if (!swap) {
                         free(d);
                         return -ENOMEM;
                 }

diff --git a/src/import/pull-common.c b/src/import/pull-common.c
index efd67a29379418669c5cb5c9830097e2b49b2492..57323531e22c3fac4899b4d9fc0e6654bc8f7f4a 100644 (file)
--- a/src/import/pull-common.c
+++ b/src/import/pull-common.c
@@ -92,9 +92,9 @@ int pull_find_old_etags(const char *url, const char *image_root, int dt, const c
                 if (a >= b)
                         continue;
 
-                u = cunescape_length(a, b - a);
-                if (!u)
-                        return -ENOMEM;
+                r = cunescape_length(a, b - a, 0, &u);
+                if (r < 0)
+                        return r;
 
                 if (!http_etag_is_valid(u)) {
                         free(u);

diff --git a/src/journal/journald-kmsg.c b/src/journal/journald-kmsg.c
index c4216c4043370bb8047c99a07faf9336cc9bfe16..80bd9cd1933005ae6a280beb3f76e49b1f4542b8 100644 (file)
--- a/src/journal/journald-kmsg.c
+++ b/src/journal/journald-kmsg.c
@@ -201,8 +201,7 @@ static void dev_kmsg_record(Server *s, const char *p, size_t l) {
 
                 *e = 0;
 
-                m = cunescape_length_with_prefix(k, e - k, "_KERNEL_");
-                if (!m)
+                if (cunescape_length_with_prefix(k, e - k, "_KERNEL_", UNESCAPE_RELAX, &m) < 0)
                         break;
 
                 if (startswith(m, "_KERNEL_DEVICE="))
@@ -299,8 +298,7 @@ static void dev_kmsg_record(Server *s, const char *p, size_t l) {
                 }
         }
 
-        message = cunescape_length_with_prefix(p, pl, "MESSAGE=");
-        if (message)
+        if (cunescape_length_with_prefix(p, pl, "MESSAGE=", UNESCAPE_RELAX, &message) >= 0)
                 IOVEC_SET_STRING(iovec[n++], message);
 
         server_dispatch_message(s, iovec, n, ELEMENTSOF(iovec), NULL, NULL, NULL, 0, NULL, priority, 0);

diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c
index cc0677bdf21c94a72e9ea36d1ac76aebd699f6f4..072191e43f4d7e0023cdc2fcf0df679c90d82bd7 100644 (file)
--- a/src/libsystemd/sd-login/sd-login.c
+++ b/src/libsystemd/sd-login/sd-login.c
@@ -496,9 +496,9 @@ _public_ int sd_session_get_desktop(const char *session, char **desktop) {
         if (r < 0)
                 return r;
 
-        t = cunescape(escaped);
-        if (!t)
-                return -ENOMEM;
+        r = cunescape(escaped, 0, &t);
+        if (r < 0)
+                return r;
 
         *desktop = t;
         return 0;

diff --git a/src/login/logind-acl.c b/src/login/logind-acl.c
index 941fd724a56e8a43c799884d4f9d2e65fa0c6613..d2b3337788920cb7a25507daed69ada64170a4a2 100644 (file)
--- a/src/login/logind-acl.c
+++ b/src/login/logind-acl.c
@@ -253,8 +253,7 @@ int devnode_acl_all(struct udev *udev,
                 FOREACH_DIRENT(dent, dir, return -errno) {
                         _cleanup_free_ char *unescaped_devname = NULL;
 
-                        unescaped_devname = cunescape(dent->d_name);
-                        if (!unescaped_devname)
+                        if (cunescape(dent->d_name, UNESCAPE_RELAX, &unescaped_devname) < 0)
                                 return -ENOMEM;
 
                         n = strappend("/dev/", unescaped_devname);

diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c
index 5d81693e6c0bdbd4e2b03f66ca3ac05f06553fec..5eb1a2ea65f61c003298c3c1e000a0d20f5a4e23 100644 (file)
--- a/src/login/logind-inhibit.c
+++ b/src/login/logind-inhibit.c
@@ -231,18 +231,18 @@ int inhibitor_load(Inhibitor *i) {
         }
 
         if (who) {
-                cc = cunescape(who);
-                if (!cc)
-                        return -ENOMEM;
+                r = cunescape(who, 0, &cc);
+                if (r < 0)
+                        return r;
 
                 free(i->who);
                 i->who = cc;
         }
 
         if (why) {
-                cc = cunescape(why);
-                if (!cc)
-                        return -ENOMEM;
+                r = cunescape(why, 0, &cc);
+                if (r < 0)
+                        return r;
 
                 free(i->why);
                 i->why = cc;

diff --git a/src/shared/util.c b/src/shared/util.c
index 8b011ccfa1d735d545aebe2770eab0d3888a5895..67f66ace21e57d6f6854b9cb845cbdc5e5aba18c 100644 (file)
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -1466,12 +1466,13 @@ static int cunescape_one(const char *p, size_t length, char *ret) {
         return r;
 }
 
-char *cunescape_length_with_prefix(const char *s, size_t length, const char *prefix) {
+int cunescape_length_with_prefix(const char *s, size_t length, const char *prefix, UnescapeFlags flags, char **ret) {
         char *r, *t;
         const char *f;
         size_t pl;
 
         assert(s);
+        assert(ret);
 
         /* Undoes C style string escaping, and optionally prefixes it. */
 
@@ -1479,7 +1480,7 @@ char *cunescape_length_with_prefix(const char *s, size_t length, const char *pre
 
         r = new(char, pl+length+1);
         if (!r)
-                return NULL;
+                return -ENOMEM;
 
         if (prefix)
                 memcpy(r, prefix, pl);
@@ -1491,17 +1492,31 @@ char *cunescape_length_with_prefix(const char *s, size_t length, const char *pre
                 remaining = s + length - f;
                 assert(remaining > 0);
 
-                if (*f != '\\' || remaining == 1) {
-                        /* a literal literal, or a trailing backslash, copy verbatim */
+                if (*f != '\\') {
+                        /* A literal literal, copy verbatim */
                         *(t++) = *f;
                         continue;
                 }
 
+                if (remaining == 1) {
+                        if (flags & UNESCAPE_RELAX) {
+                                /* A trailing backslash, copy verbatim */
+                                *(t++) = *f;
+                                continue;
+                        }
+
+                        return -EINVAL;
+                }
+
                 k = cunescape_one(f + 1, remaining - 1, t);
                 if (k < 0) {
-                        /* Invalid escape code, let's take it literal then */
-                        *(t++) = '\\';
-                        continue;
+                        if (flags & UNESCAPE_RELAX) {
+                                /* Invalid escape code, let's take it literal then */
+                                *(t++) = '\\';
+                                continue;
+                        }
+
+                        return k;
                 }
 
                 f += k;
@@ -1509,17 +1524,17 @@ char *cunescape_length_with_prefix(const char *s, size_t length, const char *pre
         }
 
         *t = 0;
-        return r;
-}
 
-char *cunescape_length(const char *s, size_t length) {
-        return cunescape_length_with_prefix(s, length, NULL);
+        *ret = r;
+        return t - r;
 }
 
-char *cunescape(const char *s) {
-        assert(s);
+int cunescape_length(const char *s, size_t length, UnescapeFlags flags, char **ret) {
+        return cunescape_length_with_prefix(s, length, NULL, flags, ret);
+}
 
-        return cunescape_length(s, strlen(s));
+int cunescape(const char *s, UnescapeFlags flags, char **ret) {
+        return cunescape_length(s, strlen(s), flags, ret);
 }
 
 char *xescape(const char *s, const char *bad) {
@@ -6731,9 +6746,9 @@ int umount_recursive(const char *prefix, int flags) {
                                 continue;
                         }
 
-                        p = cunescape(path);
-                        if (!p)
-                                return -ENOMEM;
+                        r = cunescape(path, UNESCAPE_RELAX, &p);
+                        if (r < 0)
+                                return r;
 
                         if (!path_startswith(p, prefix))
                                 continue;
@@ -6833,9 +6848,9 @@ int bind_remount_recursive(const char *prefix, bool ro) {
                                 continue;
                         }
 
-                        p = cunescape(path);
-                        if (!p)
-                                return -ENOMEM;
+                        r = cunescape(path, UNESCAPE_RELAX, &p);
+                        if (r < 0)
+                                return r;
 
                         /* Let's ignore autofs mounts.  If they aren't
                          * triggered yet, we want to avoid triggering

diff --git a/src/shared/util.h b/src/shared/util.h
index 882355665cf5b62d1dbd820ef54a2f05f3267e00..e7a5d6366ebea4ba4daecfc0a11b1e3fb1de01fa 100644 (file)
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -311,9 +311,14 @@ char decchar(int x) _const_;
 int undecchar(char c) _const_;
 
 char *cescape(const char *s);
-char *cunescape(const char *s);
-char *cunescape_length(const char *s, size_t length);
-char *cunescape_length_with_prefix(const char *s, size_t length, const char *prefix);
+
+typedef enum UnescapeFlags {
+        UNESCAPE_RELAX = 1,
+} UnescapeFlags;
+
+int cunescape(const char *s, UnescapeFlags flags, char **ret);
+int cunescape_length(const char *s, size_t length, UnescapeFlags flags, char **ret);
+int cunescape_length_with_prefix(const char *s, size_t length, const char *prefix, UnescapeFlags flags, char **ret);
 
 char *xescape(const char *s, const char *bad);
 
@@ -1014,7 +1019,7 @@ int take_password_lock(const char *root);
 int is_symlink(const char *path);
 int is_dir(const char *path, bool follow);
 
-typedef enum UnquoteFlags{
+typedef enum UnquoteFlags {
         UNQUOTE_RELAX     = 1,
         UNQUOTE_CUNESCAPE = 2,
 } UnquoteFlags;

diff --git a/src/test/test-util.c b/src/test/test-util.c
index e9d1522a65eb4f9ba373da78a316008d738f51c3..aaf25f88bb17f93bbe4f7fc44f3f04b161de449d 100644 (file)
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -417,23 +417,40 @@ static void test_cescape(void) {
 static void test_cunescape(void) {
         _cleanup_free_ char *unescaped;
 
-        unescaped = cunescape("abc\\\\\\\"\\b\\f\\a\\n\\r\\t\\v\\003\\177\\234\\313\\000\\x00");
-        assert_se(streq_ptr(unescaped, "abc\\\"\b\f\a\n\r\t\v\003\177\234\313\\000\\x00"));
+        assert_se(cunescape("abc\\\\\\\"\\b\\f\\a\\n\\r\\t\\v\\003\\177\\234\\313\\000\\x00", 0, &unescaped) < 0);
+        assert_se(cunescape("abc\\\\\\\"\\b\\f\\a\\n\\r\\t\\v\\003\\177\\234\\313\\000\\x00", UNESCAPE_RELAX, &unescaped) >= 0);
+        const char *x = "abc\\\"\b\f\a\n\r\t\v\003\177\234\313\\000\\x00";
+        assert_se(streq_ptr(unescaped, x));
+        free(unescaped);
+        unescaped = NULL;
 
         /* incomplete sequences */
-        unescaped = cunescape("\\x0");
+        assert_se(cunescape("\\x0", 0, &unescaped) < 0);
+        assert_se(cunescape("\\x0", UNESCAPE_RELAX, &unescaped) >= 0);
         assert_se(streq_ptr(unescaped, "\\x0"));
+        free(unescaped);
+        unescaped = NULL;
 
-        unescaped = cunescape("\\x");
+        assert_se(cunescape("\\x", 0, &unescaped) < 0);
+        assert_se(cunescape("\\x", UNESCAPE_RELAX, &unescaped) >= 0);
         assert_se(streq_ptr(unescaped, "\\x"));
+        free(unescaped);
+        unescaped = NULL;
 
-        unescaped = cunescape("\\");
+        assert_se(cunescape("\\", 0, &unescaped) < 0);
+        assert_se(cunescape("\\", UNESCAPE_RELAX, &unescaped) >= 0);
         assert_se(streq_ptr(unescaped, "\\"));
+        free(unescaped);
+        unescaped = NULL;
 
-        unescaped = cunescape("\\11");
+        assert_se(cunescape("\\11", 0, &unescaped) < 0);
+        assert_se(cunescape("\\11", UNESCAPE_RELAX, &unescaped) >= 0);
         assert_se(streq_ptr(unescaped, "\\11"));
+        free(unescaped);
+        unescaped = NULL;
 
-        unescaped = cunescape("\\1");
+        assert_se(cunescape("\\1", 0, &unescaped) < 0);
+        assert_se(cunescape("\\1", UNESCAPE_RELAX, &unescaped) >= 0);
         assert_se(streq_ptr(unescaped, "\\1"));
 }
 

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 20972f6310324c19cebf47b889c3bbbf53ba6a63..8c89fb33b899258881eed8a89cf69ff81954a5d6 100644 (file)
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -913,13 +913,13 @@ static int write_one_file(Item *i, const char *path) {
         }
 
         if (i->argument) {
-                _cleanup_free_ char *unescaped;
+                _cleanup_free_ char *unescaped = NULL;
 
                 log_debug("%s to \"%s\".", i->type == CREATE_FILE ? "Appending" : "Writing", path);
 
-                unescaped = cunescape(i->argument);
-                if (!unescaped)
-                        return log_oom();
+                r = cunescape(i->argument, 0, &unescaped);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to unescape parameter to write: %s", i->argument);
 
                 r = loop_write(fd, unescaped, strlen(unescaped), false);
                 if (r < 0)