Revert "basic/env-util: (mostly) follow POSIX for what variable names…

… are allowed"

This reverts commit b45c068.

I think the idea was generally sound, but didn't take into account the
limitations of show-environment and how it is used. People expect to be able to
eval systemctl show-environment output in bash, and no escaping syntax is
defined for environment *names* (we only do escaping for *values*). We could
skip such problematic variables in 'systemctl show-environment', and only allow
them to be inherited directly. But this would be confusing and ugly.

The original motivation for this change was that various import operations
would fail. a4ccce2 changed systemctl to filter
invalid variables in import-environment.
https://gitlab.gnome.org/GNOME/gnome-session/-/issues/71 does a similar change
in GNOME. So those problematic variables should not cause failures, but just
be silently ignored.

Finally, the environment block is becoming a dumping ground. In my gnome
session 'systemctl show-environment --user' includes stuff like PWD, FPATH
(from zsh), SHLVL=0 (no idea what that is). This is not directly related to
variable names (since all those are allowed under the stricter rules too), but
I think we should start pushing people away from running import-environment and
towards importing only select variables.

systemd/systemd#17188 (comment)

src/basic/env-util.c

@@ -21,21 +21,18 @@
         DIGITS LETTERS                          \
         "_"
 
-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) {
+        const char *p;
+
         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
@@ -44,8 +41,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 (const char *p = e; p < e + n; p++)
-                if (!printable_portable_character(*p) || *p == '=')
+        for (p = e; p < e + n; p++)
+                if (!strchr(VALID_BASH_ENV_NAME_CHARS, *p))
                         return false;
 
         return true;

src/test/test-env-util.c

@@ -274,12 +274,10 @@ 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], "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);
+        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);
 }
 
 static void test_env_name_is_valid(void) {
@@ -292,11 +290,8 @@ 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_unexpected_but_valid"));
+        assert_se(!env_name_is_valid("5_starting_with_a_number_is_wrong"));
         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) {
@@ -325,13 +320,9 @@ 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="));
-        /* 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…). */
+        /* 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="));
         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;<mock-chroot>\x07<mock-chroot>\""));

src/test/test-load-fragment.c

@@ -765,9 +765,8 @@ static void test_config_parse_pass_environ(void) {
                                       "PassEnvironment", 0, "'invalid name' 'normal_name' A=1 'special_name$$' \\",
                                       &passenv, NULL);
         assert_se(r >= 0);
-        assert_se(strv_length(passenv) == 2);
+        assert_se(strv_length(passenv) == 1);
         assert_se(streq(passenv[0], "normal_name"));
-        assert_se(streq(passenv[1], "special_name$$"));
 }
 
 static void test_unit_dump_config_items(void) {