env-util,fileio: immediately replace variables in load_env_file_push()

strv_env_replace was calling env_match(), which in effect allowed multiple
values for the same key to be inserted into the environment block. That's
pointless, because APIs to access variables only return a single value (the
latest entry), so it's better to keep the block clean, i.e. with just a single
entry for each key.

Add a new helper function that simply tests if the part before '=' is equal in
two strings and use that in strv_env_replace.

In load_env_file_push, use strv_env_replace to immediately replace the previous
assignment with a matching name.

Afaict, none of the callers are materially affected by this change, but it
seems like some pointless work was being done, if the same value was set
multiple times. We'd go through parsing and assigning the value for each
entry. With this change, we handle just the last one.

3 files changed, 34 insertions, 7 deletions

diff --git a/src/basic/env-util.c b/src/basic/env-util.c
index 4645ec781e..05b90e499e 100644
--- a/src/basic/env-util.c
+++ b/src/basic/env-util.c
@@ -274,6 +274,19 @@ _pure_ static bool env_match(const char *t, const char *pattern) {
         return false;
 }
 
+static bool env_entry_has_name(const char *entry, const char *name) {
+        const char *t;
+
+        assert(entry);
+        assert(name);
+
+        t = startswith(entry, name);
+        if (!t)
+                return false;
+
+        return *t == '=';
+}
+
 char **strv_env_delete(char **x, unsigned n_lists, ...) {
         size_t n, i = 0;
         char **k, **r;
@@ -387,18 +400,24 @@ char **strv_env_unset_many(char **l, ...) {
 
 int strv_env_replace(char ***l, char *p) {
         char **f;
+        const char *t, *name;
 
         assert(p);
 
         /* Replace first occurrence of the env var or add a new one in the
          * string list. Drop other occurences. Edits in-place. Does not copy p.
+         * p must be a valid key=value assignment.
          */
 
+        t = strchr(p, '=');
+        assert(t);
+
+        name = strndupa(p, t - p);
+
         for (f = *l; f && *f; f++)
-                if (env_match(*f, p)) {
-                        free(*f);
-                        *f = p;
-                        strv_env_unset(f + 1, p);
+                if (env_entry_has_name(*f, name)) {
+                        free_and_replace(*f, p);
+                        strv_env_unset(f + 1, *f);
                         return 0;
                 }
 
@@ -676,5 +695,8 @@ int deserialize_environment(char ***environment, const char *line) {
         if (r < 0)
                 return r;
 
+        if (!env_assignment_is_valid(uce))
+                return -EINVAL;
+
         return strv_env_replace(environment, uce);
 }
diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index fb41431ec9..a1e4978125 100644
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -30,6 +30,7 @@
 
 #include "alloc-util.h"
 #include "ctype.h"
+#include "env-util.h"
 #include "escape.h"
 #include "fd-util.h"
 #include "fileio.h"
@@ -678,13 +679,15 @@ static int load_env_file_push(
         if (r < 0)
                 return r;
 
-        p = strjoin(key, "=", strempty(value));
+        p = strjoin(key, "=", value);
         if (!p)
                 return -ENOMEM;
 
-        r = strv_consume(m, p);
-        if (r < 0)
+        r = strv_env_replace(m, p);
+        if (r < 0) {
+                free(p);
                 return r;
+        }
 
         if (n_pushed)
                 (*n_pushed)++;
diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index 56316904a3..a38bb874a9 100644
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -71,6 +71,8 @@ static void test_parse_env_file(void) {
               "seven=\"sevenval\" #nocomment\n"
               "eight=eightval #nocomment\n"
               "export nine=nineval\n"
+              "ten=ignored\n"
+              "ten=ignored\n"
               "ten=", f);
 
         fflush(f);