From b4346b9a77bc6129dd3e73db0ea41e3ccd2b763b Mon Sep 17 00:00:00 2001 From: Filipe Brandenburger Date: Fri, 16 Mar 2018 13:41:54 -0700 Subject: [PATCH 1/2] basic/env-util: Allow newlines in values of environment variables They are allowed by the shell and the EnvironmentFile parsing passes them through, so we should just accept them, same as we accept tabs. --- src/basic/env-util.c | 6 +++--- src/test/test-env-util.c | 9 +++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 0b1d086394..a44bb32a82 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -84,9 +84,9 @@ bool env_value_is_valid(const char *e) { if (!utf8_is_valid(e)) return false; - /* bash allows tabs in environment variables, and so should - * we */ - if (string_has_cc(e, "\t")) + /* bash allows tabs and newlines in environment variables, and so + * should we */ + if (string_has_cc(e, "\t\n")) return false; /* POSIX says the overall size of the environment block cannot diff --git a/src/test/test-env-util.c b/src/test/test-env-util.c index b1e69d4a5a..e212e37b21 100644 --- a/src/test/test-env-util.c +++ b/src/test/test-env-util.c @@ -277,8 +277,9 @@ static void test_env_clean(void) { assert_se(streq(e[1], "X=")); assert_se(streq(e[2], "F=F")); assert_se(streq(e[3], "abcd=äöüß")); - assert_se(streq(e[4], "another=final one")); - assert_se(e[5] == NULL); + assert_se(streq(e[4], "xyz=xyz\n")); + assert_se(streq(e[5], "another=final one")); + assert_se(e[6] == NULL); } static void test_env_name_is_valid(void) { @@ -297,6 +298,8 @@ static void test_env_value_is_valid(void) { assert_se(env_value_is_valid("")); assert_se(env_value_is_valid("głąb kapuściany")); assert_se(env_value_is_valid("printf \"\\x1b]0;\\x07\"")); + assert_se(env_value_is_valid("tab\tcharacter")); + assert_se(env_value_is_valid("new\nline")); } static void test_env_assignment_is_valid(void) { @@ -304,6 +307,8 @@ static void test_env_assignment_is_valid(void) { assert_se(env_assignment_is_valid("b=głąb kapuściany")); assert_se(env_assignment_is_valid("c=\\007\\009\\011")); assert_se(env_assignment_is_valid("e=printf \"\\x1b]0;\\x07\"")); + assert_se(env_assignment_is_valid("f=tab\tcharacter")); + assert_se(env_assignment_is_valid("g=new\nline")); assert_se(!env_assignment_is_valid("=")); assert_se(!env_assignment_is_valid("a b=")); From 9b796f3523aac1c55f39ee8eb07a1b2965aa884b Mon Sep 17 00:00:00 2001 From: Filipe Brandenburger Date: Fri, 16 Mar 2018 16:30:42 -0700 Subject: [PATCH 2/2] test-execute: Introduce tests for environment values containing newlines Also fix one case where the presence of a newline was used to generate an invalid environment assignment. Tested: with mkosi, which builds the local tree and run ninja tests. --- src/test/test-execute.c | 5 ++++- test/test-execute/exec-environmentfile.service | 2 +- test/test-execute/exec-passenvironment-absent.service | 4 ++-- test/test-execute/exec-passenvironment-empty.service | 4 ++-- test/test-execute/exec-passenvironment-repeated.service | 3 ++- test/test-execute/exec-passenvironment.service | 4 ++-- 6 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 954080df36..db8a9c75a9 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -466,7 +466,8 @@ static void test_exec_environmentfile(Manager *m) { "; comment2\n" " ; # comment3\n" "line without an equal\n" - "VAR3='$word 5 6'\n"; + "VAR3='$word 5 6'\n" + "VAR4='new\nline'\n"; int r; r = write_string_file("/tmp/test-exec_environmentfile.conf", e, WRITE_STRING_FILE_CREATE); @@ -492,12 +493,14 @@ static void test_exec_passenvironment(Manager *m) { assert_se(setenv("VAR1", "word1 word2", 1) == 0); assert_se(setenv("VAR2", "word3", 1) == 0); assert_se(setenv("VAR3", "$word 5 6", 1) == 0); + assert_se(setenv("VAR4", "new\nline", 1) == 0); test(m, "exec-passenvironment.service", 0, CLD_EXITED); test(m, "exec-passenvironment-repeated.service", 0, CLD_EXITED); test(m, "exec-passenvironment-empty.service", 0, CLD_EXITED); assert_se(unsetenv("VAR1") == 0); assert_se(unsetenv("VAR2") == 0); assert_se(unsetenv("VAR3") == 0); + assert_se(unsetenv("VAR4") == 0); test(m, "exec-passenvironment-absent.service", 0, CLD_EXITED); } diff --git a/test/test-execute/exec-environmentfile.service b/test/test-execute/exec-environmentfile.service index f6b8462719..cd4747aa20 100644 --- a/test/test-execute/exec-environmentfile.service +++ b/test/test-execute/exec-environmentfile.service @@ -2,6 +2,6 @@ Description=Test for EnvironmentFile [Service] -ExecStart=/bin/sh -x -c 'test "$$VAR1" = "word1 word2" && test "$$VAR2" = word3 && test "$$VAR3" = "\\$$word 5 6"' +ExecStart=/bin/sh -x -c 'test "$$VAR1" = "word1 word2" && test "$$VAR2" = word3 && test "$$VAR3" = "\\$$word 5 6" && test "$$VAR4" = "new\nline"' Type=oneshot EnvironmentFile=/tmp/test-exec_environmentfile.conf diff --git a/test/test-execute/exec-passenvironment-absent.service b/test/test-execute/exec-passenvironment-absent.service index 7d5e32a4eb..4a82206a2c 100644 --- a/test/test-execute/exec-passenvironment-absent.service +++ b/test/test-execute/exec-passenvironment-absent.service @@ -2,6 +2,6 @@ Description=Test for PassEnvironment with variables absent from the execution environment [Service] -ExecStart=/bin/sh -x -c 'test "$${VAR1-unset}" = "unset" && test "$${VAR2-unset}" = "unset" && test "$${VAR3-unset}" = "unset"' +ExecStart=/bin/sh -x -c 'test "$${VAR1-unset}" = "unset" && test "$${VAR2-unset}" = "unset" && test "$${VAR3-unset}" = "unset" && test "$${VAR4-unset}" = "unset"' Type=oneshot -PassEnvironment=VAR1 VAR2 VAR3 +PassEnvironment=VAR1 VAR2 VAR3 VAR4 diff --git a/test/test-execute/exec-passenvironment-empty.service b/test/test-execute/exec-passenvironment-empty.service index c93c197c10..8716cdf6bb 100644 --- a/test/test-execute/exec-passenvironment-empty.service +++ b/test/test-execute/exec-passenvironment-empty.service @@ -2,7 +2,7 @@ Description=Test for PassEnvironment and erasing the variable list [Service] -ExecStart=/bin/sh -x -c 'test "$${VAR1-unset}" = "unset" && test "$${VAR2-unset}" = "unset" && test "$${VAR3-unset}" = "unset"' +ExecStart=/bin/sh -x -c 'test "$${VAR1-unset}" = "unset" && test "$${VAR2-unset}" = "unset" && test "$${VAR3-unset}" = "unset" && test "$${VAR4-unset}" = "unset"' Type=oneshot -PassEnvironment=VAR1 VAR2 VAR3 +PassEnvironment=VAR1 VAR2 VAR3 VAR4 PassEnvironment= diff --git a/test/test-execute/exec-passenvironment-repeated.service b/test/test-execute/exec-passenvironment-repeated.service index 5e8c56f26a..d518d7d8a5 100644 --- a/test/test-execute/exec-passenvironment-repeated.service +++ b/test/test-execute/exec-passenvironment-repeated.service @@ -2,7 +2,8 @@ Description=Test for PassEnvironment with a variable name repeated [Service] -ExecStart=/bin/sh -x -c 'test "$$VAR1" = "word1 word2" && test "$$VAR2" = word3 && test "$$VAR3" = "\\$$word 5 6"' +ExecStart=/bin/sh -x -c 'test "$$VAR1" = "word1 word2" && test "$$VAR2" = word3 && test "$$VAR3" = "\\$$word 5 6" && test "$$VAR4" = "new\nline"' Type=oneshot PassEnvironment=VAR1 VAR2 PassEnvironment=VAR1 VAR3 +PassEnvironment=VAR1 VAR4 diff --git a/test/test-execute/exec-passenvironment.service b/test/test-execute/exec-passenvironment.service index b4a9909682..cca44ebf4a 100644 --- a/test/test-execute/exec-passenvironment.service +++ b/test/test-execute/exec-passenvironment.service @@ -2,6 +2,6 @@ Description=Test for PassEnvironment [Service] -ExecStart=/bin/sh -x -c 'test "$$VAR1" = "word1 word2" && test "$$VAR2" = word3 && test "$$VAR3" = "\\$$word 5 6"' +ExecStart=/bin/sh -x -c 'test "$$VAR1" = "word1 word2" && test "$$VAR2" = word3 && test "$$VAR3" = "\\$$word 5 6" && test "$$VAR4" = "new\nline"' Type=oneshot -PassEnvironment=VAR1 VAR2 VAR3 +PassEnvironment=VAR1 VAR2 VAR3 VAR4