SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

core/exec-invoke: rework $TERM fallback logic (#37647)

Browse Source 
Inspired by https://github.com/systemd/systemd/pull/37538, see a
detailed rationale in
https://github.com/systemd/systemd/pull/37538#discussion_r2110229075.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2025-05-30 12:39:51 +02:00
committed by GitHub
parent 9521222222 6131dc0aad
commit 2e8d47f21c
4 changed files with 128 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/env-util.c
+22 -11
View File
@@ -33,7 +33,7 @@ static bool env_name_is_valid_n(const char *e, size_t n) {
if (n == SIZE_MAX)
n = strlen_ptr(e);
if (n <= 0)
if (n == 0)
return false;
assert(e);
@@ -44,7 +44,7 @@ static bool env_name_is_valid_n(const char *e, size_t n) {
/* 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 hence leaves ARG_MAX-2 as
* longest possible variable name. */
if (n > (size_t) sysconf(_SC_ARG_MAX) - 2)
if (_unlikely_(n > sc_arg_max() - 2))
return false;
for (const char *p = e; p < e + n; p++)
@@ -55,7 +55,7 @@ static bool env_name_is_valid_n(const char *e, size_t n) {
}
bool env_name_is_valid(const char *e) {
return env_name_is_valid_n(e, strlen_ptr(e));
return env_name_is_valid_n(e, SIZE_MAX);
}
bool env_value_is_valid(const char *e) {
@@ -72,7 +72,7 @@ bool env_value_is_valid(const char *e) {
/* POSIX says the overall size of the environment block cannot be > ARG_MAX, an individual assignment
* hence cannot be either. Discounting the shortest possible variable name of length 1, the equal
* sign and trailing NUL this hence leaves ARG_MAX-3 as longest possible variable value. */
if (strlen(e) > sc_arg_max() - 3)
if (_unlikely_(strlen(e) > sc_arg_max() - 3))
return false;
return true;
@@ -81,6 +81,8 @@ bool env_value_is_valid(const char *e) {
bool env_assignment_is_valid(const char *e) {
const char *eq;
assert(e);
eq = strchr(e, '=');
if (!eq)
return false;
@@ -93,13 +95,13 @@ bool env_assignment_is_valid(const char *e) {
/* POSIX says the overall size of the environment block cannot be > ARG_MAX, hence the individual
* variable assignments cannot be either, but let's leave room for one trailing NUL byte. */
if (strlen(e) > sc_arg_max() - 1)
if (_unlikely_(strlen(e) > sc_arg_max() - 1))
return false;
return true;
}
bool strv_env_is_valid(char **e) {
bool strv_env_is_valid(char * const *e) {
STRV_FOREACH(p, e) {
size_t k;
@@ -116,7 +118,7 @@ bool strv_env_is_valid(char **e) {
return true;
}
bool strv_env_name_is_valid(char **l) {
bool strv_env_name_is_valid(char * const *l) {
STRV_FOREACH(p, l) {
if (!env_name_is_valid(*p))
return false;
@@ -128,7 +130,7 @@ bool strv_env_name_is_valid(char **l) {
return true;
}
bool strv_env_name_or_assignment_is_valid(char **l) {
bool strv_env_name_or_assignment_is_valid(char * const *l) {
STRV_FOREACH(p, l) {
if (!env_assignment_is_valid(*p) && !env_name_is_valid(*p))
return false;
@@ -373,6 +375,7 @@ int strv_env_replace_consume(char ***l, char *p) {
const char *t, *name;
int r;
assert(l);
assert(p);
/* Replace first occurrence of the env var or add a new one in the string list. Drop other
@@ -406,6 +409,9 @@ int strv_env_replace_consume(char ***l, char *p) {
int strv_env_replace_strdup(char ***l, const char *assignment) {
/* Like strv_env_replace_consume(), but copies the argument. */
assert(l);
assert(assignment);
char *p = strdup(assignment);
if (!p)
return -ENOMEM;
@@ -414,11 +420,14 @@ int strv_env_replace_strdup(char ***l, const char *assignment) {
}
int strv_env_replace_strdup_passthrough(char ***l, const char *assignment) {
/* Like strv_env_replace_strdup(), but pulls the variable from the environment of
* the calling program, if a variable name without value is specified.
*/
char *p;
/* Like strv_env_replace_strdup(), but pulls the variable from the environment of
* the calling program, if a variable name without value is specified. */
assert(l);
assert(assignment);
if (strchr(assignment, '=')) {
if (!env_assignment_is_valid(assignment))
return -EINVAL;
@@ -440,6 +449,8 @@ int strv_env_replace_strdup_passthrough(char ***l, const char *assignment) {
}
int strv_env_assign(char ***l, const char *key, const char *value) {
assert(l);
if (!env_name_is_valid(key))
return -EINVAL;
src/basic/env-util.h
+5 -5
View File
@@ -22,12 +22,12 @@ static inline int replace_env(const char *format, char **env, ReplaceEnvFlags fl
int replace_env_argv(char **argv, char **env, char ***ret, char ***ret_unset_variables, char ***ret_bad_variables);
bool strv_env_is_valid(char **e);
#define strv_env_clean(l) strv_env_clean_with_callback(l, NULL, NULL)
char** strv_env_clean_with_callback(char **l, void (*invalid_callback)(const char *p, void *userdata), void *userdata);
bool strv_env_is_valid(char * const *e);
bool strv_env_name_is_valid(char * const *l);
bool strv_env_name_or_assignment_is_valid(char * const *l);
bool strv_env_name_is_valid(char **l);
bool strv_env_name_or_assignment_is_valid(char **l);
char** strv_env_clean_with_callback(char **l, void (*invalid_callback)(const char *p, void *userdata), void *userdata);
#define strv_env_clean(l) strv_env_clean_with_callback(l, NULL, NULL)
char** _strv_env_merge(char **first, ...);
#define strv_env_merge(first, ...) _strv_env_merge(first, __VA_ARGS__, POINTER_MAX)
src/core/exec-invoke.c
+100 -84
View File
@@ -137,23 +137,6 @@ static bool is_kmsg_output(ExecOutput o) {
EXEC_OUTPUT_KMSG_AND_CONSOLE);
}
static bool exec_context_needs_term(const ExecContext *c) {
assert(c);
/* Return true if the execution context suggests we should set $TERM to something useful. */
if (is_terminal_input(c->std_input))
return true;
if (is_terminal_output(c->std_output))
return true;
if (is_terminal_output(c->std_error))
return true;
return !!c->tty_path;
}
static int open_null_as(int flags, int nfd) {
int fd;
@@ -1932,8 +1915,8 @@ static int build_environment(
assert(cgroup_context);
assert(ret);
#define N_ENV_VARS 22
our_env = new0(char*, N_ENV_VARS + _EXEC_DIRECTORY_TYPE_MAX);
#define N_ENV_VARS 19
our_env = new0(char*, N_ENV_VARS + _EXEC_DIRECTORY_TYPE_MAX + 1);
if (!our_env)
return -ENOMEM;
@@ -2037,69 +2020,6 @@ static int build_environment(
our_env[n_env++] = x;
}
if (exec_context_needs_term(c)) {
_cleanup_free_ char *cmdline = NULL, *dcs_term = NULL;
const char *tty_path, *term = NULL;
tty_path = exec_context_tty_path(c);
/* If we are forked off PID 1 and we are supposed to operate on /dev/console, then let's try
* to inherit the $TERM set for PID 1. This is useful for containers so that the $TERM the
* container manager passes to PID 1 ends up all the way in the console login shown. */
if (path_equal(tty_path, "/dev/console") && getppid() == 1)
term = getenv("TERM");
else if (tty_path && in_charset(skip_dev_prefix(tty_path), ALPHANUMERICAL)) {
_cleanup_free_ char *key = NULL;
key = strjoin("systemd.tty.term.", skip_dev_prefix(tty_path));
if (!key)
return -ENOMEM;
r = proc_cmdline_get_key(key, 0, &cmdline);
if (r < 0)
log_debug_errno(r, "Failed to read %s from kernel cmdline, ignoring: %m", key);
else if (r > 0)
term = cmdline;
}
if (!term && tty_path) {
/* This handles real virtual terminals (returning "linux") and
* any terminals which support the DCS +q query sequence. */
r = query_term_for_tty(tty_path, &dcs_term);
if (r >= 0)
term = dcs_term;
}
if (!term) {
/* If $TERM is not known and we pick a fallback default, then let's also set
* $COLORTERM=truecolor. That's because our fallback default is vt220, which is
* generally a safe bet (as it supports PageUp/PageDown unlike vt100, and is quite
* universally available in terminfo/termcap), except for the fact that real DEC
* vt220 gear never actually supported color. Most tools these days generate color on
* vt220 anyway, ignoring the physical capabilities of the real hardware, but some
* tools actually believe in the historical truth. Which is unfortunate since *we*
* *don't* care about the historical truth, we just want sane defaults if nothing
* better is explicitly configured. It's 2025 after all, at the time of writing,
* pretty much all terminal emulators actually *do* support color, hence if we don't
* know any better let's explicitly claim color support via $COLORTERM. Or in other
* words: we now explicitly claim to be connected to a franken-vt220 with true color
* support. */
x = strdup("COLORTERM=truecolor");
if (!x)
return -ENOMEM;
our_env[n_env++] = x;
term = FALLBACK_TERM;
}
x = strjoin("TERM=", term);
if (!x)
return -ENOMEM;
our_env[n_env++] = x;
}
if (journal_stream_dev != 0 && journal_stream_ino != 0) {
if (asprintf(&x, "JOURNAL_STREAM=" DEV_FMT ":" INO_FMT, journal_stream_dev, journal_stream_ino) < 0)
return -ENOMEM;
@@ -2215,7 +2135,7 @@ static int build_environment(
our_env[n_env++] = x;
}
assert(n_env < N_ENV_VARS + _EXEC_DIRECTORY_TYPE_MAX);
assert(n_env <= N_ENV_VARS + _EXEC_DIRECTORY_TYPE_MAX);
#undef N_ENV_VARS
*ret = TAKE_PTR(our_env);
@@ -4599,6 +4519,96 @@ static void prepare_terminal(
(void) osc_context_open_service(p->unit_id, p->invocation_id, /* ret_seq= */ NULL);
}
static int setup_term_environment(const ExecContext *context, char ***env) {
int r;
assert(context);
assert(env);
/* Already specified by user? */
if (strv_env_get(*env, "TERM"))
return 0;
/* Do we need $TERM at all? */
if (!is_terminal_input(context->std_input) &&
!is_terminal_output(context->std_output) &&
!is_terminal_output(context->std_error) &&
!context->tty_path)
return 0;
const char *tty_path = exec_context_tty_path(context);
if (tty_path) {
/* If we are forked off PID 1 and we are supposed to operate on /dev/console, then let's try
* to inherit the $TERM set for PID 1. This is useful for containers so that the $TERM the
* container manager passes to PID 1 ends up all the way in the console login shown.
*
* Note that if this doesn't work out we won't bother with querying systemd.tty.term.console
* kernel cmdline option or DCS anymore either, because pid1 also imports $TERM based on those
* and it should have showed up as our $TERM if there were anything. */
if (tty_is_console(tty_path) && getppid() == 1) {
const char *term = strv_find_prefix(environ, "TERM=");
if (term) {
r = strv_env_replace_strdup(env, term);
if (r < 0)
return r;
FOREACH_STRING(i, "COLORTERM=", "NO_COLOR=") {
const char *s = strv_find_prefix(environ, i);
if (!s)
continue;
r = strv_env_replace_strdup(env, s);
if (r < 0)
return r;
}
return 1;
}
} else {
if (in_charset(skip_dev_prefix(tty_path), ALPHANUMERICAL)) {
_cleanup_free_ char *key = NULL, *cmdline = NULL;
key = strjoin("systemd.tty.term.", skip_dev_prefix(tty_path));
if (!key)
return -ENOMEM;
r = proc_cmdline_get_key(key, /* flags = */ 0, &cmdline);
if (r > 0)
return strv_env_assign(env, "TERM", cmdline);
if (r < 0)
log_debug_errno(r, "Failed to read '%s' from kernel cmdline, ignoring: %m", key);
}
/* This handles real virtual terminals (returning "linux") and
* any terminals which support the DCS +q query sequence. */
_cleanup_free_ char *dcs_term = NULL;
r = query_term_for_tty(tty_path, &dcs_term);
if (r >= 0)
return strv_env_assign(env, "TERM", dcs_term);
}
}
/* If $TERM is not known and we pick a fallback default, then let's also set
* $COLORTERM=truecolor. That's because our fallback default is vt220, which is
* generally a safe bet (as it supports PageUp/PageDown unlike vt100, and is quite
* universally available in terminfo/termcap), except for the fact that real DEC
* vt220 gear never actually supported color. Most tools these days generate color on
* vt220 anyway, ignoring the physical capabilities of the real hardware, but some
* tools actually believe in the historical truth. Which is unfortunate since *we*
* *don't* care about the historical truth, we just want sane defaults if nothing
* better is explicitly configured. It's 2025 after all, at the time of writing,
* pretty much all terminal emulators actually *do* support color, hence if we don't
* know any better let's explicitly claim color support via $COLORTERM. Or in other
* words: we now explicitly claim to be connected to a franken-vt220 with true color
* support. */
r = strv_env_replace_strdup(env, "COLORTERM=truecolor");
if (r < 0)
return r;
return strv_env_replace_strdup(env, "TERM=" FALLBACK_TERM);
}
int exec_invoke(
const ExecCommand *command,
const ExecContext *context,
@@ -5254,10 +5264,16 @@ int exec_invoke(
*exit_status = EXIT_MEMORY;
return log_oom();
}
accum_env = strv_env_clean(accum_env);
strv_env_clean(accum_env);
(void) umask(context->umask);
r = setup_term_environment(context, &accum_env);
if (r < 0) {
*exit_status = EXIT_MEMORY;
return log_error_errno(r, "Failed to construct $TERM: %m");
}
r = setup_keyring(context, params, uid, gid);
if (r < 0) {
*exit_status = EXIT_KEYRING;
src/core/execute.c
+1 -1
View File
@@ -934,7 +934,7 @@ static int exec_context_load_environment(const Unit *unit, const ExecContext *c,
.path = *path,
};
p = strv_env_clean_with_callback(p, invalid_env, &info);
strv_env_clean_with_callback(p, invalid_env, &info);
}
if (!v)