Tighten checking for variable validity

In the future we might want to allow additional syntax (for example
"unset VAR". But let's check that the data we're getting does not contain
anything unexpected.

man/environment.d.xml

@@ -81,6 +81,9 @@
     and <literal>$OTHER_KEY</literal> format. No other elements of shell syntax are supported.
     </para>
 
+    <para>Each<replaceable>KEY</replaceable> must be a valid variable name. Empty lines
+    and lines beginning with the comment character <literal>#</literal> are ignored.</para>
+
     <refsect2>
       <title>Example</title>
       <example>

src/basic/exec-util.c

@@ -279,6 +279,11 @@ static int gather_environment_generate(int fd, void *arg) {
         STRV_FOREACH_PAIR(x, y, new) {
                 char *p;
 
+                if (!env_name_is_valid(*x)) {
+                        log_warning("Invalid variable assignment \"%s=...\", ignoring.", *x);
+                        continue;
+                }
+
                 p = strjoin(*x, "=", *y);
                 if (!p)
                         return -ENOMEM;

src/basic/fileio.c

@@ -773,6 +773,16 @@ static int merge_env_file_push(
 
         assert(env);
 
+        if (!value) {
+                log_error("%s:%u: invalid syntax (around \"%s\"), ignoring.", strna(filename), line, key);
+                return 0;
+        }
+
+        if (!env_name_is_valid(key)) {
+                log_error("%s:%u: invalid variable name \"%s\", ignoring.", strna(filename), line, key);
+                return 0;
+        }
+
         expanded_value = replace_env(value, *env,
                                      REPLACE_ENV_USE_ENVIRONMENT|REPLACE_ENV_ALLOW_BRACELESS);
         if (!expanded_value)

src/test/test-exec-util.c

@@ -305,8 +305,16 @@ static void test_environment_gathering(void) {
                                     "echo A=$A:24\n"
                                     "echo B=12\n"
                                     "echo C=000\n"
-                                    "echo C=001\n"                   /* variable overwriting */
-                                    "echo PATH=$PATH:/no/such/file", /* variable from manager */
+                                    "echo C=001\n"                    /* variable overwriting */
+                                     /* various invalid entries */
+                                    "echo unset A\n"
+                                    "echo unset A=\n"
+                                    "echo unset A=B\n"
+                                    "echo unset \n"
+                                    "echo A B=C\n"
+                                    "echo A\n"
+                                    /* test variable assignment without newline */
+                                    "echo PATH=$PATH:/no/such/file",   /* no newline */
                                     WRITE_STRING_FILE_CREATE) == 0);
 
         assert_se(chmod(name, 0755) == 0);

src/test/test-fileio.c

@@ -263,6 +263,45 @@ static void test_merge_env_file(void) {
         assert_se(a[6] == NULL);
 }
 
+static void test_merge_env_file_invalid(void) {
+        char t[] = "/tmp/test-fileio-XXXXXX";
+        int fd, r;
+        FILE *f;
+        _cleanup_strv_free_ char **a = NULL;
+        char **i;
+
+        fd = mkostemp_safe(t);
+        assert_se(fd >= 0);
+
+        log_info("/* %s (%s) */", __func__, t);
+
+        f = fdopen(fd, "w");
+        assert_se(f);
+
+        r = write_string_stream(f,
+                                "unset one   \n"
+                                "unset one=   \n"
+                                "unset one=1   \n"
+                                "one   \n"
+                                "one =  \n"
+                                "one two =\n"
+                                "\x20two=\n"
+                                "#comment=comment\n"
+                                ";comment2=comment2\n"
+                                "#\n"
+                                "\n\n"                  /* empty line */
+                                , false);
+        assert(r >= 0);
+
+        r = merge_env_file(&a, NULL, t);
+        assert_se(r >= 0);
+
+        STRV_FOREACH(i, a)
+                log_info("Got: <%s>", *i);
+
+        assert_se(strv_isempty(a));
+}
+
 static void test_executable_is_script(void) {
         char t[] = "/tmp/test-executable-XXXXXX";
         int fd, r;
@@ -620,6 +659,7 @@ int main(int argc, char *argv[]) {
         test_parse_env_file();
         test_parse_multiline_env_file();
         test_merge_env_file();
+        test_merge_env_file_invalid();
         test_executable_is_script();
         test_status_field();
         test_capeff();