From 6fe3196350fdb8715aabd315f8252c307da6661c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Sep 2020 17:42:46 +0200 Subject: [PATCH 1/4] test-fileio: use test strings that are actually valid in shell --- src/test/test-fileio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index 95dcd4fdb1..3e6413ec47 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -162,8 +162,8 @@ static void test_parse_multiline_env_file(void) { assert_se(fmkostemp_safe(t, "w", &f) == 0); fputs("one=BAR\\\n" - " VAR\\\n" - "\tGAR\n" + "\\ \\ \\ \\ VAR\\\n" + "\\\tGAR\n" "#comment\n" "two=\"bar\\\n" " var\\\n" @@ -173,7 +173,7 @@ static void test_parse_multiline_env_file(void) { " var \\\n" "\tgar \"\n", f); - fflush(f); + assert_se(fflush_and_check(f) >= 0); fclose(f); r = load_env_file(NULL, t, &a); From de008e537dc6e3504f988fa9bd358f783016df8a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Sep 2020 15:14:03 +0200 Subject: [PATCH 2/4] env-file: bring our decoding of double-quoted strings in env files in line with shell In shell, inside of double quotes only a select few chars should be escaped. If other chars are escaped this has no effect. Correct the list of chars that need such escaping. Also, make sure we can read back the stuff we wrote out without loss. Fixes: #16788 --- src/basic/env-file.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/basic/env-file.c b/src/basic/env-file.c index 26470796eb..dc92b13a6f 100644 --- a/src/basic/env-file.c +++ b/src/basic/env-file.c @@ -209,17 +209,21 @@ static int parse_env_file_internal( case DOUBLE_QUOTE_VALUE_ESCAPE: state = DOUBLE_QUOTE_VALUE; - if (c == '"') { + if (strchr(SHELL_NEED_ESCAPE, c)) { + /* If this is a char that needs escaping, just unescape it. */ if (!GREEDY_REALLOC(value, value_alloc, n_value+2)) return -ENOMEM; - value[n_value++] = '"'; - } else if (!strchr(NEWLINE, c)) { + value[n_value++] = c; + } else if (c != '\n') { + /* If other char than what needs escaping, keep the "\" in place, like the + * real shell does. */ if (!GREEDY_REALLOC(value, value_alloc, n_value+3)) return -ENOMEM; value[n_value++] = '\\'; value[n_value++] = c; } + /* Escaped newlines (aka "continuation lines") are eaten up entirely */ break; case COMMENT: From 55f99c26c678b4bf4fb697a57d540b0266aa3f49 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Sep 2020 15:18:07 +0200 Subject: [PATCH 3/4] test-env-file: add test that tests our env file read + writer + shell against each other Should hopefully make regressions on this unlikely. --- src/test/test-env-file.c | 45 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/test/test-env-file.c b/src/test/test-env-file.c index 47f86a5676..861238169b 100644 --- a/src/test/test-env-file.c +++ b/src/test/test-env-file.c @@ -2,6 +2,7 @@ #include "env-file.h" #include "fd-util.h" +#include "fileio.h" #include "fs-util.h" #include "macro.h" #include "strv.h" @@ -132,6 +133,46 @@ static void test_load_env_file_5(void) { assert_se(data[2] == NULL); } +static void test_write_and_load_env_file(void) { + const char *v; + + /* Make sure that our writer, parser and the shell agree on what our env var files mean */ + + FOREACH_STRING(v, + "obbardc-laptop", + "obbardc\\-laptop", + "obbardc-lap\\top", + "obbardc-lap\\top", + "obbardc-lap\\\\top", + "double\"quote", + "single\'quote", + "dollar$dollar", + "newline\nnewline") { + _cleanup_(unlink_and_freep) char *p = NULL; + _cleanup_strv_free_ char **l = NULL; + _cleanup_free_ char *j = NULL, *w = NULL, *cmd = NULL, *from_shell = NULL; + _cleanup_fclose_ FILE *f = NULL; + size_t sz; + + assert_se(tempfn_random_child(NULL, NULL, &p) >= 0); + + assert_se(j = strjoin("TEST=", v)); + assert_se(write_env_file(p, STRV_MAKE(j)) >= 0); + + assert_se(cmd = strjoin(". ", p, " && /bin/echo -n \"$TEST\"")); + assert_se(f = popen(cmd, "re")); + assert_se(read_full_stream(f, &from_shell, &sz) >= 0); + assert_se(sz == strlen(v)); + assert_se(streq(from_shell, v)); + + assert_se(load_env_file(NULL, p, &l) >= 0); + assert_se(strv_equal(l, STRV_MAKE(j))); + + assert_se(parse_env_file(NULL, p, "TEST", &w) >= 0); + assert_se(streq_ptr(w, v)); + } +} + int main(int argc, char *argv[]) { test_setup_logging(LOG_INFO); @@ -140,4 +181,8 @@ int main(int argc, char *argv[]) { test_load_env_file_3(); test_load_env_file_4(); test_load_env_file_5(); + + test_write_and_load_env_file(); + + return 0; } From df8b14b59119c61f660037710bbb3899db441ef7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Sep 2020 17:43:05 +0200 Subject: [PATCH 4/4] test-fileio: test test strings with shell, too --- src/test/test-fileio.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index 3e6413ec47..9c0b3533c2 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -151,6 +151,18 @@ static void test_parse_env_file(void) { assert_se(r >= 0); } +static void test_one_shell_var(const char *file, const char *variable, const char *value) { + _cleanup_free_ char *cmd = NULL, *from_shell = NULL; + _cleanup_fclose_ FILE *f = NULL; + size_t sz; + + assert_se(cmd = strjoin(". ", file, " && /bin/echo -n \"$", variable, "\"")); + assert_se(f = popen(cmd, "re")); + assert_se(read_full_stream(f, &from_shell, &sz) >= 0); + assert_se(sz == strlen(value)); + assert_se(streq(from_shell, value)); +} + static void test_parse_multiline_env_file(void) { _cleanup_(unlink_tempfilep) char t[] = "/tmp/test-fileio-in-XXXXXX", @@ -176,6 +188,10 @@ static void test_parse_multiline_env_file(void) { assert_se(fflush_and_check(f) >= 0); fclose(f); + test_one_shell_var(t, "one", "BAR VAR\tGAR"); + test_one_shell_var(t, "two", "bar var\tgar"); + test_one_shell_var(t, "tri", "bar var \tgar "); + r = load_env_file(NULL, t, &a); assert_se(r >= 0);