diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 179408c399..8b26b36cfe 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -16,22 +16,26 @@ #include "strv.h" #include "utf8.h" -#define VALID_CHARS_ENV_NAME \ +/* We follow bash for the character set. Different shells have different rules. */ +#define VALID_BASH_ENV_NAME_CHARS \ DIGITS LETTERS \ "_" -static bool env_name_is_valid_n(const char *e, size_t n) { - const char *p; +static bool printable_portable_character(char c) { + /* POSIX.1-2008 specifies almost all ASCII characters as "portable". (Only DEL is excluded, and + * additionally NUL and = are not allowed in variable names). We are stricter, and additionally + * reject BEL, BS, HT, CR, LF, VT, FF and SPACE, i.e. all whitespace. */ + return c >= '!' && c <= '~'; +} + +static bool env_name_is_valid_n(const char *e, size_t n) { if (!e) return false; if (n <= 0) return false; - if (e[0] >= '0' && e[0] <= '9') - return false; - /* POSIX says the overall size of the environment block cannot * be > ARG_MAX, an individual assignment hence cannot be * either. Discounting the equal sign and trailing NUL this @@ -40,8 +44,8 @@ static bool env_name_is_valid_n(const char *e, size_t n) { if (n > (size_t) sysconf(_SC_ARG_MAX) - 2) return false; - for (p = e; p < e + n; p++) - if (!strchr(VALID_CHARS_ENV_NAME, *p)) + for (const char *p = e; p < e + n; p++) + if (!printable_portable_character(*p) || *p == '=') return false; return true; @@ -546,7 +550,7 @@ char *replace_env_n(const char *format, size_t n, char **env, unsigned flags) { word = e+1; state = WORD; - } else if (flags & REPLACE_ENV_ALLOW_BRACELESS && strchr(VALID_CHARS_ENV_NAME, *e)) { + } else if (flags & REPLACE_ENV_ALLOW_BRACELESS && strchr(VALID_BASH_ENV_NAME_CHARS, *e)) { k = strnappend(r, word, e-word-1); if (!k) return NULL; @@ -636,7 +640,7 @@ char *replace_env_n(const char *format, size_t n, char **env, unsigned flags) { case VARIABLE_RAW: assert(flags & REPLACE_ENV_ALLOW_BRACELESS); - if (!strchr(VALID_CHARS_ENV_NAME, *e)) { + if (!strchr(VALID_BASH_ENV_NAME_CHARS, *e)) { const char *t; t = strv_env_get_n(env, word+1, e-word-1, flags); diff --git a/src/test/test-env-util.c b/src/test/test-env-util.c index 9eed8e8f47..b0af660e3b 100644 --- a/src/test/test-env-util.c +++ b/src/test/test-env-util.c @@ -263,7 +263,8 @@ static void test_env_clean(void) { "xyz\n=xyz", "xyz=xyz\n", "another=one", - "another=final one"); + "another=final one", + "BASH_FUNC_foo%%=() { echo foo\n}"); assert_se(e); assert_se(!strv_env_is_valid(e)); assert_se(strv_env_clean(e) == e); @@ -272,10 +273,12 @@ static void test_env_clean(void) { assert_se(streq(e[0], "FOOBAR=WALDO")); assert_se(streq(e[1], "X=")); assert_se(streq(e[2], "F=F")); - assert_se(streq(e[3], "abcd=äöüß")); - assert_se(streq(e[4], "xyz=xyz\n")); - assert_se(streq(e[5], "another=final one")); - assert_se(e[6] == NULL); + assert_se(streq(e[3], "0000=000")); + assert_se(streq(e[4], "abcd=äöüß")); + assert_se(streq(e[5], "xyz=xyz\n")); + assert_se(streq(e[6], "another=final one")); + assert_se(streq(e[7], "BASH_FUNC_foo%%=() { echo foo\n}")); + assert_se(e[8] == NULL); } static void test_env_name_is_valid(void) { @@ -288,8 +291,11 @@ static void test_env_name_is_valid(void) { assert_se(!env_name_is_valid("xxx\a")); assert_se(!env_name_is_valid("xxx\007b")); assert_se(!env_name_is_valid("\007\009")); - assert_se(!env_name_is_valid("5_starting_with_a_number_is_wrong")); + assert_se( env_name_is_valid("5_starting_with_a_number_is_unexpected_but_valid")); assert_se(!env_name_is_valid("#¤%&?_only_numbers_letters_and_underscore_allowed")); + assert_se( env_name_is_valid("BASH_FUNC_foo%%")); + assert_se(!env_name_is_valid("with spaces%%")); + assert_se(!env_name_is_valid("with\nnewline%%")); } static void test_env_value_is_valid(void) { @@ -316,9 +322,13 @@ static void test_env_assignment_is_valid(void) { assert_se(!env_assignment_is_valid("a b=")); assert_se(!env_assignment_is_valid("a =")); assert_se(!env_assignment_is_valid(" b=")); - /* no dots or dashes: http://tldp.org/LDP/abs/html/gotchas.html */ - assert_se(!env_assignment_is_valid("a.b=")); - assert_se(!env_assignment_is_valid("a-b=")); + /* Names with dots and dashes makes those variables inaccessible as bash variables (as the syntax + * simply does not allow such variable names, see http://tldp.org/LDP/abs/html/gotchas.html). They + * are still valid variables according to POSIX though. */ + assert_se( env_assignment_is_valid("a.b=")); + assert_se( env_assignment_is_valid("a-b=")); + /* Those are not ASCII, so not valid according to POSIX (though zsh does allow unicode variable + * names…). */ assert_se(!env_assignment_is_valid("\007=głąb kapuściany")); assert_se(!env_assignment_is_valid("c\009=\007\009\011")); assert_se(!env_assignment_is_valid("głąb=printf \"\x1b]0;\x07\"")); diff --git a/src/test/test-load-fragment.c b/src/test/test-load-fragment.c index 799ac6a51a..bf74cbe6e1 100644 --- a/src/test/test-load-fragment.c +++ b/src/test/test-load-fragment.c @@ -748,26 +748,26 @@ static void test_config_parse_pass_environ(void) { _cleanup_strv_free_ char **passenv = NULL; r = config_parse_pass_environ(NULL, "fake", 1, "section", 1, - "PassEnvironment", 0, "A B", - &passenv, NULL); + "PassEnvironment", 0, "A B", + &passenv, NULL); assert_se(r >= 0); assert_se(strv_length(passenv) == 2); assert_se(streq(passenv[0], "A")); assert_se(streq(passenv[1], "B")); r = config_parse_pass_environ(NULL, "fake", 1, "section", 1, - "PassEnvironment", 0, "", - &passenv, NULL); + "PassEnvironment", 0, "", + &passenv, NULL); assert_se(r >= 0); assert_se(strv_isempty(passenv)); r = config_parse_pass_environ(NULL, "fake", 1, "section", 1, - "PassEnvironment", 0, "'invalid name' 'normal_name' A=1 \\", - &passenv, NULL); + "PassEnvironment", 0, "'invalid name' 'normal_name' A=1 'special_name$$' \\", + &passenv, NULL); assert_se(r >= 0); - assert_se(strv_length(passenv) == 1); + assert_se(strv_length(passenv) == 2); assert_se(streq(passenv[0], "normal_name")); - + assert_se(streq(passenv[1], "special_name$$")); } static void test_unit_dump_config_items(void) {