slave: don't skip setting environment variables when using a custom environment

When launching footclient with -E,--client-environment the environment
variables that should be set by foot, wasn't.

Those variables are:

* TERM
* COLORTERM
* PWD
* SHELL

and all variables defined by the user in the [environment] section in
foot.ini.

In the same way, we did not *unset* TERM_PROGRAM and
TERM_PROGRAM_VERSION.

This patch fixes it by "cloning" the custom environment, making it
mutable, and then adding/removing the variables above from it.

Instead of calling setenv()/unsetenv() directly, we add the wrapper
functions add_to_env() and del_from_env().

When *not* using a custom environment, they simply call
setenv()/unsetenv().

When we *are* using a custom environment, add_to_env() first loops all
existing variables, looking for a match. If a match is found, it's
updated with the new value. If it's not found, a new entry is added.

del_from_env() loops all entries, and removes it when a match is
found. If no match is found, nothing is done.

The mutable environment is allocated on the heap, but never free:d. We
don't need to free it, since it's only allocated after forking, in the
child process.

Closes #1568

CHANGELOG.md

@@ -82,9 +82,12 @@
   `cursor-shape-v1` Wayland protocols ([#1573][1573]).
 * Crash in `--server` mode when one or more environment variables are
   set in `[environment]`.
+* Environment variables normally set by foot lost with `footclient
+  -E,--client-environment` ([#1568][1568]).
 
 [1531]: https://codeberg.org/dnkl/foot/issues/1531
 [1573]: https://codeberg.org/dnkl/foot/issues/1573
+[1568]: https://codeberg.org/dnkl/foot/issues/1568
 
 
 ### Security

doc/foot.1.scd

@@ -554,16 +554,31 @@ In all other cases, the exit code is that of the client application
 	set according to either the *--term* command-line option or the
 	*term* config option in *foot.ini*(5).
 
-*PWD*
-	Current working directory (at the time of launching foot)
-
 *COLORTERM*
 	This variable is set to *truecolor*, to indicate to client
 	applications that 24-bit RGB colors are supported.
 
+*PWD*
+	Current working directory (at the time of launching foot)
+
+*SHELL*
+	Set to the launched shell, if the shell is valid (it is listed in
+	*/etc/shells*).
+
 In addition to the variables listed above, custom environment
 variables may be defined in *foot.ini*(5).
 
+## Variables *unset* in the child process
+
+*TERM_PROGRAM*
+*TERM_PROGRAM_VERSION*
+	These environment variables are set by certain other terminal
+	emulators. We unset them, to prevent applications from
+	misdetecting foot.
+
+In addition to the variables listed above, custom environment
+variables to unset may be defined in *foot.ini*(5).
+
 # BUGS
 
 Please report bugs to https://codeberg.org/dnkl/foot/issues

doc/footclient.1.scd

@@ -73,6 +73,11 @@ terminal has terminated.
 	The child process in the new terminal instance will use
 	footclient's environment, instead of the server's.
 
+	Environment variables listed in the *Variables set in the child
+	process* section will be overwritten by the foot server. For
+	example, the new terminal will use *TERM* from the configuration,
+	not footclient's environment.
+
 *-d*,*--log-level*={*info*,*warning*,*error*,*none*}
 	Log level, used both for log output on stderr as well as
 	syslog. Default: _warning_.
@@ -163,9 +168,27 @@ fallback to the less specific path, with the following priority:
 	This variable is set to *truecolor*, to indicate to client
 	applications that 24-bit RGB colors are supported.
 
+*PWD*
+	Current working directory (at the time of launching foot)
+
+*SHELL*
+	Set to the launched shell, if the shell is valid (it is listed in
+	*/etc/shells*).
+
 In addition to the variables listed above, custom environment
 variables may be defined in *foot.ini*(5).
 
+## Variables *unset* in the child process
+
+*TERM_PROGRAM*
+*TERM_PROGRAM_VERSION*
+	These environment variables are set by certain other terminal
+	emulators. We unset them, to prevent applications from
+	misdetecting foot.
+
+In addition to the variables listed above, custom environment
+variables to unset may be defined in *foot.ini*(5).
+
 # SEE ALSO
 
 *foot*(1)

server.c

@@ -332,7 +332,8 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data)
     instance->terminal = term_init(
         conf != NULL ? conf : server->conf,
         server->fdm, server->reaper, server->wayl, "footclient", cwd, token,
-        cdata.argc, argv, envp, &term_shutdown_handler, instance);
+        cdata.argc, argv, (const char *const *)envp,
+        &term_shutdown_handler, instance);
 
     if (instance->terminal == NULL) {
         LOG_ERR("failed to instantiate new terminal");

slave.c

@@ -25,6 +25,11 @@
 
 extern char **environ;
 
+struct environ {
+    size_t count;
+    char **envp;
+};
+
 #if defined(__FreeBSD__)
 static char *
 find_file_in_path(const char *file)
@@ -303,9 +308,69 @@ err:
     _exit(errno);
 }
 
+static bool
+env_matches_var_name(const char *e, const char *name)
+{
+    const size_t e_len = strlen(e);
+    const size_t name_len = strlen(name);
+
+    if (e_len <= name_len)
+        return false;
+    if (memcmp(e, name, name_len) != 0)
+        return false;
+    if (e[name_len] != '=')
+        return false;
+    return true;
+}
+
+static void
+add_to_env(struct environ *env, const char *name, const char *value)
+{
+    if (env->envp == NULL)
+        setenv(name, value, 1);
+    else {
+        char *e = xasprintf("%s=%s", name, value);
+
+        /* Search for existing variable. If found, replace it with the
+           new value */
+        for (size_t i = 0; i < env->count; i++) {
+            if (env_matches_var_name(env->envp[i], name)) {
+                free(env->envp[i]);
+                env->envp[i] = e;
+                return;
+            }
+        }
+
+        /* If the variable does not already exist, add it */
+        env->envp = xrealloc(env->envp, (env->count + 2) * sizeof(env->envp[0]));
+        env->envp[env->count++] = e;
+        env->envp[env->count] = NULL;
+    }
+}
+
+static void
+del_from_env(struct environ *env, const char *name)
+{
+    if (env->envp == NULL)
+        unsetenv(name);
+    else {
+        for (size_t i = 0; i < env->count; i++) {
+            if (env_matches_var_name(env->envp[i], name)) {
+                free(env->envp[i]);
+                memmove(&env->envp[i],
+                        &env->envp[i + 1],
+                        (env->count - i) * sizeof(env->envp[0]));
+                env->count--;
+                xassert(env->envp[env->count] == NULL);
+                break;
+            }
+        }
+    }
+}
+
 pid_t
 slave_spawn(int ptmx, int argc, const char *cwd, char *const *argv,
-            char *const *envp, const env_var_list_t *extra_env_vars,
+            const char *const *envp, const env_var_list_t *extra_env_vars,
             const char *term_env, const char *conf_shell, bool login_shell,
             const user_notifications_t *notifications)
 {
@@ -350,15 +415,30 @@ slave_spawn(int ptmx, int argc, const char *cwd, char *const *argv,
             _exit(errno_copy);
         }
 
-        setenv("TERM", term_env, 1);
-        setenv("COLORTERM", "truecolor", 1);
-        setenv("PWD", cwd, 1);
+        /* Create a mutable copy of the environment */
+        struct environ custom_env = {0};
+        if (envp != NULL) {
+            for (const char *const *e = envp; *e != NULL; e++)
+                custom_env.count++;
 
-        unsetenv("TERM_PROGRAM");
-        unsetenv("TERM_PROGRAM_VERSION");
+            custom_env.envp = xcalloc(
+                custom_env.count + 1, sizeof(custom_env.envp[0]));
+
+            size_t i = 0;
+            for (const char *const *e = envp; *e != NULL; e++, i++)
+                custom_env.envp[i] = xstrdup(*e);
+            xassert(custom_env.envp[custom_env.count] == NULL);
+        }
+
+        add_to_env(&custom_env, "TERM", term_env);
+        add_to_env(&custom_env, "COLORTERM", "truecolor");
+        add_to_env(&custom_env, "PWD", cwd);
+
+        del_from_env(&custom_env, "TERM_PROGRAM");
+        del_from_env(&custom_env, "TERM_PROGRAM_VERSION");
 
 #if defined(FOOT_TERMINFO_PATH)
-        setenv("TERMINFO", FOOT_TERMINFO_PATH, 1);
+        add_to_env(&custom_envp, "TERMINFO", FOOT_TERMINFO_PATH);
 #endif
 
         if (extra_env_vars != NULL) {
@@ -367,9 +447,9 @@ slave_spawn(int ptmx, int argc, const char *cwd, char *const *argv,
                 const char *value = it->item.value;
 
                 if (strlen(value) == 0)
-                    unsetenv(name);
+                    del_from_env(&custom_env, name);
                 else
-                    setenv(name, value, 1);
+                    add_to_env(&custom_env, name, value);
             }
         }
 
@@ -393,9 +473,10 @@ slave_spawn(int ptmx, int argc, const char *cwd, char *const *argv,
         }
 
         if (is_valid_shell(shell_argv[0]))
-            setenv("SHELL", shell_argv[0], 1);
+            add_to_env(&custom_env, "SHELL", shell_argv[0]);
 
-        slave_exec(ptmx, shell_argv, envp != NULL ? envp : environ,
+        slave_exec(ptmx, shell_argv,
+                   custom_env.envp != NULL ? custom_env.envp : environ,
                    fork_pipe[1], login_shell, notifications);
         BUG("Unexpected return from slave_exec()");
         break;

slave.h

@@ -7,7 +7,7 @@
 #include "user-notification.h"
 
 pid_t slave_spawn(
-    int ptmx, int argc, const char *cwd, char *const *argv, char *const *envp,
+    int ptmx, int argc, const char *cwd, char *const *argv, const char *const *envp,
     const env_var_list_t *extra_env_vars, const char *term_env,
     const char *conf_shell, bool login_shell,
     const user_notifications_t *notifications);

terminal.c

@@ -1061,7 +1061,7 @@ static void fdm_client_terminated(
 struct terminal *
 term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper,
           struct wayland *wayl, const char *foot_exe, const char *cwd,
-          const char *token, int argc, char *const *argv, char *const *envp,
+          const char *token, int argc, char *const *argv, const char *const *envp,
           void (*shutdown_cb)(void *data, int exit_code), void *shutdown_data)
 {
     int ptmx = -1;

terminal.h

@@ -728,7 +728,7 @@ struct config;
 struct terminal *term_init(
     const struct config *conf, struct fdm *fdm, struct reaper *reaper,
     struct wayland *wayl, const char *foot_exe, const char *cwd,
-    const char *token, int argc, char *const *argv, char *const *envp,
+    const char *token, int argc, char *const *argv, const char *const *envp,
     void (*shutdown_cb)(void *data, int exit_code), void *shutdown_data);
 
 bool term_shutdown(struct terminal *term);