From: Zbigniew Jędrzejewski-Szmek Date: Wed, 3 Mar 2021 12:35:40 +0000 (+0100) Subject: Flagsify EscapeStyle and make ESCAPE_BACKSLASH_ONELINE implicit X-Git-Tag: v249-rc1~274^2~19 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9e53c10a0f64bc1ab70877c245d5a8e08c509575;p=thirdparty%2Fsystemd.git Flagsify EscapeStyle and make ESCAPE_BACKSLASH_ONELINE implicit I want to tweak behaviour further, and that'll be easier when "style" is converted to a bitfield. Some callers used ESCAPE_BACKSLASH_ONELINE, and others not. But the ones that didn't, simply didn't care, because the argument was assumed to be one-line anyway (e.g. a service name). In environment-generator, this could make a difference. But I think it's better to escape the newlines there too. So newlines are now always escaped, to simplify the code and the test matrix. --- diff --git a/src/basic/escape.c b/src/basic/escape.c index af785ecfa45..7a9a3b56166 100644 --- a/src/basic/escape.c +++ b/src/basic/escape.c @@ -494,7 +494,7 @@ char* shell_escape(const char *s, const char *bad) { return r; } -char* shell_maybe_quote(const char *s, EscapeStyle style) { +char* shell_maybe_quote(const char *s, ShellEscapeFlags flags) { const char *p; char *r, *t; @@ -513,36 +513,27 @@ char* shell_maybe_quote(const char *s, EscapeStyle style) { if (!*p) return strdup(s); - r = new(char, (style == ESCAPE_POSIX) + 1 + strlen(s)*2 + 1 + 1); + r = new(char, FLAGS_SET(flags, SHELL_ESCAPE_POSIX) + 1 + strlen(s)*2 + 1 + 1); if (!r) return NULL; t = r; - switch (style) { - case ESCAPE_BACKSLASH: - case ESCAPE_BACKSLASH_ONELINE: - *(t++) = '"'; - break; - case ESCAPE_POSIX: + if (FLAGS_SET(flags, SHELL_ESCAPE_POSIX)) { *(t++) = '$'; *(t++) = '\''; - break; - default: - assert_not_reached("Bad EscapeStyle"); - } + } else + *(t++) = '"'; t = mempcpy(t, s, p - s); - if (IN_SET(style, ESCAPE_BACKSLASH, ESCAPE_BACKSLASH_ONELINE)) - t = strcpy_backslash_escaped(t, p, SHELL_NEED_ESCAPE, - style == ESCAPE_BACKSLASH_ONELINE); - else - t = strcpy_backslash_escaped(t, p, SHELL_NEED_ESCAPE_POSIX, true); + t = strcpy_backslash_escaped(t, p, + FLAGS_SET(flags, SHELL_ESCAPE_POSIX) ? SHELL_NEED_ESCAPE_POSIX : SHELL_NEED_ESCAPE, + true); - if (IN_SET(style, ESCAPE_BACKSLASH, ESCAPE_BACKSLASH_ONELINE)) - *(t++) = '"'; - else + if (FLAGS_SET(flags, SHELL_ESCAPE_POSIX)) *(t++) = '\''; + else + *(t++) = '"'; *t = 0; return r; diff --git a/src/basic/escape.h b/src/basic/escape.h index 691b6d802c2..7ecae49e10d 100644 --- a/src/basic/escape.h +++ b/src/basic/escape.h @@ -33,15 +33,12 @@ typedef enum UnescapeFlags { UNESCAPE_ACCEPT_NUL = 1 << 1, } UnescapeFlags; -typedef enum EscapeStyle { - ESCAPE_BACKSLASH = 1, /* Add shell quotes ("") so the shell will consider this a single - argument, possibly multiline. Tabs and newlines are not escaped. */ - ESCAPE_BACKSLASH_ONELINE = 2, /* Similar to ESCAPE_BACKSLASH, but always produces a single-line - string instead. Shell escape sequences are produced for tabs and - newlines. */ - ESCAPE_POSIX = 3, /* Similar to ESCAPE_BACKSLASH_ONELINE, but uses POSIX shell escape - * syntax (a string enclosed in $'') instead of plain quotes. */ -} EscapeStyle; +typedef enum ShellEscapeFlags { + /* The default is to add shell quotes ("") so the shell will consider this a single argument. + * Tabs and newlines are escaped. */ + + SHELL_ESCAPE_POSIX = 1 << 1, /* Use POSIX shell escape syntax (a string enclosed in $'') instead of plain quotes. */ +} ShellEscapeFlags; char* cescape(const char *s); char* cescape_length(const char *s, size_t n); @@ -64,4 +61,4 @@ char* octescape(const char *s, size_t len); char* escape_non_printable_full(const char *str, size_t console_width, bool eight_bit); char* shell_escape(const char *s, const char *bad); -char* shell_maybe_quote(const char *s, EscapeStyle style); +char* shell_maybe_quote(const char *s, ShellEscapeFlags flags); diff --git a/src/core/job.c b/src/core/job.c index 56c99f93eb1..639dc352a6c 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -846,7 +846,7 @@ static void job_print_done_status_message(Unit *u, JobType t, JobResult result) if (t == JOB_START && result == JOB_FAILED) { _cleanup_free_ char *quoted; - quoted = shell_maybe_quote(u->id, ESCAPE_BACKSLASH); + quoted = shell_maybe_quote(u->id, 0); manager_status_printf(u->manager, STATUS_TYPE_NORMAL, NULL, "See 'systemctl status %s' for details.", strna(quoted)); } } diff --git a/src/environment-d-generator/environment-d-generator.c b/src/environment-d-generator/environment-d-generator.c index 1c51cf6b2cf..5372f6cec28 100644 --- a/src/environment-d-generator/environment-d-generator.c +++ b/src/environment-d-generator/environment-d-generator.c @@ -70,7 +70,7 @@ static int load_and_print(void) { t = strchr(*i, '='); assert(t); - q = shell_maybe_quote(t + 1, ESCAPE_BACKSLASH); + q = shell_maybe_quote(t + 1, 0); if (!q) return log_oom(); diff --git a/src/shared/bus-print-properties.c b/src/shared/bus-print-properties.c index 2a712be7339..dc16c2f7635 100644 --- a/src/shared/bus-print-properties.c +++ b/src/shared/bus-print-properties.c @@ -270,7 +270,7 @@ static int bus_print_property(const char *name, const char *expected_value, sd_b while ((r = sd_bus_message_read_basic(m, SD_BUS_TYPE_STRING, &str)) > 0) { _cleanup_free_ char *e = NULL; - e = shell_maybe_quote(str, ESCAPE_BACKSLASH_ONELINE); + e = shell_maybe_quote(str, 0); if (!e) return -ENOMEM; diff --git a/src/shared/bus-wait-for-jobs.c b/src/shared/bus-wait-for-jobs.c index 51b71ecc2c1..f325dd4e6d0 100644 --- a/src/shared/bus-wait-for-jobs.c +++ b/src/shared/bus-wait-for-jobs.c @@ -181,7 +181,7 @@ static void log_job_error_with_service_result(const char* service, const char *r assert(service); - service_shell_quoted = shell_maybe_quote(service, ESCAPE_BACKSLASH); + service_shell_quoted = shell_maybe_quote(service, 0); if (!strv_isempty((char**) extra_args)) { _cleanup_free_ char *t; diff --git a/src/systemctl/systemctl-set-environment.c b/src/systemctl/systemctl-set-environment.c index bfd87c0cdd0..a19e031dd38 100644 --- a/src/systemctl/systemctl-set-environment.c +++ b/src/systemctl/systemctl-set-environment.c @@ -17,7 +17,7 @@ static int print_variable(const char *s) { return log_error_errno(SYNTHETIC_ERRNO(EUCLEAN), "Invalid environment block"); - esc = shell_maybe_quote(sep + 1, ESCAPE_POSIX); + esc = shell_maybe_quote(sep + 1, SHELL_ESCAPE_POSIX); if (!esc) return log_oom(); diff --git a/src/test/test-escape.c b/src/test/test-escape.c index 3e410ca299e..1e653688420 100644 --- a/src/test/test-escape.c +++ b/src/test/test-escape.c @@ -129,56 +129,43 @@ static void test_shell_escape(void) { test_shell_escape_one("foo:bar,baz", ",:", "foo\\:bar\\,baz"); } -static void test_shell_maybe_quote_one(const char *s, - EscapeStyle style, - const char *expected) { +static void test_shell_maybe_quote_one(const char *s, ShellEscapeFlags flags, const char *expected) { _cleanup_free_ char *ret = NULL; - assert_se(ret = shell_maybe_quote(s, style)); + assert_se(ret = shell_maybe_quote(s, flags)); log_debug("[%s] → [%s] (%s)", s, ret, expected); assert_se(streq(ret, expected)); } static void test_shell_maybe_quote(void) { - test_shell_maybe_quote_one("", ESCAPE_BACKSLASH, ""); - test_shell_maybe_quote_one("", ESCAPE_BACKSLASH_ONELINE, ""); - test_shell_maybe_quote_one("", ESCAPE_POSIX, ""); - test_shell_maybe_quote_one("\\", ESCAPE_BACKSLASH, "\"\\\\\""); - test_shell_maybe_quote_one("\\", ESCAPE_BACKSLASH_ONELINE, "\"\\\\\""); - test_shell_maybe_quote_one("\\", ESCAPE_POSIX, "$'\\\\'"); - test_shell_maybe_quote_one("\"", ESCAPE_BACKSLASH, "\"\\\"\""); - test_shell_maybe_quote_one("\"", ESCAPE_BACKSLASH_ONELINE, "\"\\\"\""); - test_shell_maybe_quote_one("\"", ESCAPE_POSIX, "$'\"'"); - test_shell_maybe_quote_one("foobar", ESCAPE_BACKSLASH, "foobar"); - test_shell_maybe_quote_one("foobar", ESCAPE_BACKSLASH_ONELINE, "foobar"); - test_shell_maybe_quote_one("foobar", ESCAPE_POSIX, "foobar"); - test_shell_maybe_quote_one("foo bar", ESCAPE_BACKSLASH, "\"foo bar\""); - test_shell_maybe_quote_one("foo bar", ESCAPE_BACKSLASH_ONELINE, "\"foo bar\""); - test_shell_maybe_quote_one("foo bar", ESCAPE_POSIX, "$'foo bar'"); - test_shell_maybe_quote_one("foo\tbar", ESCAPE_BACKSLASH, "\"foo\tbar\""); - test_shell_maybe_quote_one("foo\tbar", ESCAPE_BACKSLASH_ONELINE, "\"foo\\tbar\""); - test_shell_maybe_quote_one("foo\tbar", ESCAPE_POSIX, "$'foo\\tbar'"); - test_shell_maybe_quote_one("foo\nbar", ESCAPE_BACKSLASH, "\"foo\nbar\""); - test_shell_maybe_quote_one("foo\nbar", ESCAPE_BACKSLASH_ONELINE, "\"foo\\nbar\""); - test_shell_maybe_quote_one("foo\nbar", ESCAPE_POSIX, "$'foo\\nbar'"); - test_shell_maybe_quote_one("foo \"bar\" waldo", ESCAPE_BACKSLASH, "\"foo \\\"bar\\\" waldo\""); - test_shell_maybe_quote_one("foo \"bar\" waldo", ESCAPE_BACKSLASH_ONELINE, "\"foo \\\"bar\\\" waldo\""); - test_shell_maybe_quote_one("foo \"bar\" waldo", ESCAPE_POSIX, "$'foo \"bar\" waldo'"); - test_shell_maybe_quote_one("foo$bar", ESCAPE_BACKSLASH, "\"foo\\$bar\""); - test_shell_maybe_quote_one("foo$bar", ESCAPE_BACKSLASH_ONELINE, "\"foo\\$bar\""); - test_shell_maybe_quote_one("foo$bar", ESCAPE_POSIX, "$'foo$bar'"); + test_shell_maybe_quote_one("", 0, ""); + test_shell_maybe_quote_one("", SHELL_ESCAPE_POSIX, ""); + test_shell_maybe_quote_one("\\", 0, "\"\\\\\""); + test_shell_maybe_quote_one("\\", SHELL_ESCAPE_POSIX, "$'\\\\'"); + test_shell_maybe_quote_one("\"", 0, "\"\\\"\""); + test_shell_maybe_quote_one("\"", SHELL_ESCAPE_POSIX, "$'\"'"); + test_shell_maybe_quote_one("foobar", 0, "foobar"); + test_shell_maybe_quote_one("foobar", SHELL_ESCAPE_POSIX, "foobar"); + test_shell_maybe_quote_one("foo bar", 0, "\"foo bar\""); + test_shell_maybe_quote_one("foo bar", SHELL_ESCAPE_POSIX, "$'foo bar'"); + test_shell_maybe_quote_one("foo\tbar", 0, "\"foo\\tbar\""); + test_shell_maybe_quote_one("foo\tbar", SHELL_ESCAPE_POSIX, "$'foo\\tbar'"); + test_shell_maybe_quote_one("foo\nbar", 0, "\"foo\\nbar\""); + test_shell_maybe_quote_one("foo\nbar", SHELL_ESCAPE_POSIX, "$'foo\\nbar'"); + test_shell_maybe_quote_one("foo \"bar\" waldo", 0, "\"foo \\\"bar\\\" waldo\""); + test_shell_maybe_quote_one("foo \"bar\" waldo", SHELL_ESCAPE_POSIX, "$'foo \"bar\" waldo'"); + test_shell_maybe_quote_one("foo$bar", 0, "\"foo\\$bar\""); + test_shell_maybe_quote_one("foo$bar", SHELL_ESCAPE_POSIX, "$'foo$bar'"); /* Note that current users disallow control characters, so this "test" * is here merely to establish current behaviour. If control characters * were allowed, they should be quoted, i.e. \001 should become \\001. */ - test_shell_maybe_quote_one("a\nb\001", ESCAPE_BACKSLASH, "\"a\nb\001\""); - test_shell_maybe_quote_one("a\nb\001", ESCAPE_BACKSLASH_ONELINE, "\"a\\nb\001\""); - test_shell_maybe_quote_one("a\nb\001", ESCAPE_POSIX, "$'a\\nb\001'"); + test_shell_maybe_quote_one("a\nb\001", 0, "\"a\\nb\001\""); + test_shell_maybe_quote_one("a\nb\001", SHELL_ESCAPE_POSIX, "$'a\\nb\001'"); - test_shell_maybe_quote_one("foo!bar", ESCAPE_BACKSLASH, "\"foo!bar\""); - test_shell_maybe_quote_one("foo!bar", ESCAPE_BACKSLASH_ONELINE, "\"foo!bar\""); - test_shell_maybe_quote_one("foo!bar", ESCAPE_POSIX, "$'foo!bar'"); + test_shell_maybe_quote_one("foo!bar", 0, "\"foo!bar\""); + test_shell_maybe_quote_one("foo!bar", SHELL_ESCAPE_POSIX, "$'foo!bar'"); } int main(int argc, char *argv[]) {