From 3dfbc968e8343172faf754a3c81e27f0dbd8f157 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 1 Jul 2017 16:17:12 -0400 Subject: [PATCH 1/2] Revert "bus: when dumping string property values escape the chars we use as end-of-line and end-of-item marks" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 27e9c5af817147ea1c678769e45e83f2e4b4ae96. Property values already use escaping, so escaping them a second time is confusing. It also should be mostly unnecessary: we take care to make property values only contains strings which (after the initial escaping) are printable and parseable without any futher escaping. Before revert: $ systemctl list-dependencies 'dev-mapper-luks\x2d8db85dcf\x2d6230\x2d4e88\x2d940d\x2dba176d062b31.device' dev-mapper-luks\x2d8db85dcf\x2d6230\x2d4e88\x2d940d\x2dba176d062b31.device ● ├─dev-mapper-luks\x2d8db85dcf\x2d6230\x2d4e88\x2d940d\x2dba176d062b31.swap ● └─systemd-cryptsetup@luks\x2d8db85dcf\x2d6230\x2d4e88\x2d940d\x2dba176d062b31.service $ systemctl show -p Wants,Requires 'dev-mapper-luks\x2d8db85dcf\x2d6230\x2d4e88\x2d940d\x2dba176d062b31.device' Requires=systemd-cryptsetup@luks\x5cx2d8db85dcf\x5cx2d6230\x5cx2d4e88\x5cx2d940d\x5cx2dba176d062b31.service Wants=dev-mapper-luks\x5cx2d8db85dcf\x5cx2d6230\x5cx2d4e88\x5cx2d940d\x5cx2dba176d062b31.swap Difference between systemctl show before revert and now: -Slice=system-systemd\x5cx2dcryptsetup.slice +Slice=system-systemd\x2dcryptsetup.slice -Id=systemd-cryptsetup@luks\x5cx2d8db85dcf\x5cx2d6230\x5cx2d4e88\x5cx2d940d\x5cx2dba176d062b31.service +Id=systemd-cryptsetup@luks\x2d8db85dcf\x2d6230\x2d4e88\x2d940d\x2dba176d062b31.service -Names=systemd-cryptsetup@luks\x5cx2d8db85dcf\x5cx2d6230\x5cx2d4e88\x5cx2d940d\x5cx2dba176d062b31.service +Names=systemd-cryptsetup@luks\x2d8db85dcf\x2d6230\x2d4e88\x2d940d\x2dba176d062b31.service -Requires=system-systemd\x5cx2dcryptsetup.slice +Requires=system-systemd\x2dcryptsetup.slice -BindsTo=dev-mapper-luks\x5cx2d8db85dcf\x5cx2d6230\x5cx2d4e88\x5cx2d940d\x5cx2dba176d062b31.device dev-disk-by\x5cx2duuid-8db85dcf\x5cx2d6230\x5cx2d4e88\x5cx2d940d\x5cx2dba176d062b31.device +BindsTo=dev-mapper-luks\x2d8db85dcf\x2d6230\x2d4e88\x2d940d\x2dba176d062b31.device dev-disk-by\x2duuid-8db85dcf\x2d6230\x2d4e88\x2d940d\x2dba176d062b31.device -RequiredBy=dev-mapper-luks\x5cx2d8db85dcf\x5cx2d6230\x5cx2d4e88\x5cx2d940d\x5cx2dba176d062b31.device cryptsetup.target +RequiredBy=dev-mapper-luks\x2d8db85dcf\x2d6230\x2d4e88\x2d940d\x2dba176d062b31.device cryptsetup.target -WantedBy=dev-disk-by\x5cx2duuid-8db85dcf\x5cx2d6230\x5cx2d4e88\x5cx2d940d\x5cx2dba176d062b31.device +WantedBy=dev-disk-by\x2duuid-8db85dcf\x2d6230\x2d4e88\x2d940d\x2dba176d062b31.device --- src/shared/bus-util.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 8ddfb584ea..8705a7b841 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -724,15 +724,8 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b if (r < 0) return r; - if (all || !isempty(s)) { - _cleanup_free_ char *escaped = NULL; - - escaped = xescape(s, "\n"); - if (!escaped) - return -ENOMEM; - - print_property(name, "%s", escaped); - } + if (all || !isempty(s)) + print_property(name, "%s", s); return 1; } @@ -852,16 +845,10 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b return r; while ((r = sd_bus_message_read_basic(property, SD_BUS_TYPE_STRING, &str)) > 0) { - _cleanup_free_ char *escaped = NULL; - if (first && !value) printf("%s=", name); - escaped = xescape(str, "\n "); - if (!escaped) - return -ENOMEM; - - printf("%s%s", first ? "" : " ", escaped); + printf("%s%s", first ? "" : " ", str); first = false; } From 5993d46a1a4472cc5a9a227311f913ff934395a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 1 Jul 2017 16:49:15 -0400 Subject: [PATCH 2/2] bus-util: replace non-printable values with [unprintable] Like I said in the previous commit, such values do not seem to appear in normal use, but it's pretty hard to prove that all paths to assign values properly check that they contain no spaces. So just in case some slip through, replace values with spaces (in case of single-valued properties) or spaces and newlines (in case of array proprties) with "[unprintable]". We were already doing it in case of properties which we didn't know how to print, so this fits in well. The advantage is the previous code which used escaping that a) this is easier to spot, b) does not mess up printing of properties which were properly escaped already. v2: - add comments --- src/shared/bus-util.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 8705a7b841..207b5e66fc 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -724,8 +724,14 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b if (r < 0) return r; - if (all || !isempty(s)) - print_property(name, "%s", s); + if (all || !isempty(s)) { + bool good; + + /* This property has a single value, so we need to take + * care not to print a new line, everything else is OK. */ + good = !strchr(s, '\n'); + print_property(name, "%s", good ? s : "[unprintable]"); + } return 1; } @@ -845,10 +851,16 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b return r; while ((r = sd_bus_message_read_basic(property, SD_BUS_TYPE_STRING, &str)) > 0) { + bool good; + if (first && !value) printf("%s=", name); - printf("%s%s", first ? "" : " ", str); + /* This property has multiple space-seperated values, so + * neither spaces not newlines can be allowed in a value. */ + good = str[strcspn(str, " \n")] == '\0'; + + printf("%s%s", first ? "" : " ", good ? str : "[unprintable]"); first = false; }