terminal-util: change conditioning in terminal_reset_defensive()

So far we conditioned the logic that issues ansi sequences for resetting
the TTY based on whether something is a pty is not (under the assumption
we need no reset on ptys, since they are shortlived).

This is simply wrong though. The pty that a container getty is invoked
on is generally long-lived: as long as the container is up, and it will
be reused between getty instances/sessions all the time. In such a case
we really should reset properly.

Let's instead make the logic dependent on whether TERM is set to
anything other than "dumb". The previous commit made sure we always set
TERM in a sensible way in systemd-run, hence this
*explicit* logic sounds like a much better choice now, as it mea

diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c
index 1ff0dbcedc34c0725d295f6eba3c64621a98ab38..787e698738a252210e38ee3595203d3f79ad0600 100644 (file)
--- a/src/basic/terminal-util.c
+++ b/src/basic/terminal-util.c
@@ -1934,21 +1934,18 @@ int terminal_reset_defensive(int fd, TerminalResetFlags flags) {
         int r = 0;
 
         assert(fd >= 0);
+        assert(!FLAGS_SET(flags, TERMINAL_RESET_AVOID_ANSI_SEQ|TERMINAL_RESET_FORCE_ANSI_SEQ));
 
-        /* Resets the terminal comprehensively, but defensively. i.e. both resets the tty via ioctl()s and
-         * via ANSI sequences, but avoids the latter in case we are talking to a pty. That's a safety measure
-         * because ptys might be connected to shell pipelines where we cannot expect such ansi sequences to
-         * work. Given that ptys are generally short-lived (and not recycled) this restriction shouldn't hurt
-         * much.
-         *
-         * The specified fd should be open for *writing*! */
+        /* Resets the terminal comprehensively, i.e. via both ioctl()s and via ANSI sequences, but do so only
+         * if $TERM is unset or set to "dumb" */
 
         if (!isatty_safe(fd))
                 return -ENOTTY;
 
         RET_GATHER(r, terminal_reset_ioctl(fd, FLAGS_SET(flags, TERMINAL_RESET_SWITCH_TO_TEXT)));
 
-        if (terminal_is_pty_fd(fd) == 0)
+        if (!FLAGS_SET(flags, TERMINAL_RESET_AVOID_ANSI_SEQ) &&
+            (FLAGS_SET(flags, TERMINAL_RESET_FORCE_ANSI_SEQ) || !getenv_terminal_is_dumb()))
                 RET_GATHER(r, terminal_reset_ansi_seq(fd));
 
         return r;

diff --git a/src/basic/terminal-util.h b/src/basic/terminal-util.h
index e2c4a83230467487c580e059b912db59963b0f37..9decf42f2214fd6c84ea8d56fd088a401f784cc8 100644 (file)
--- a/src/basic/terminal-util.h
+++ b/src/basic/terminal-util.h
@@ -43,6 +43,8 @@ bool isatty_safe(int fd);
 
 typedef enum TerminalResetFlags {
         TERMINAL_RESET_SWITCH_TO_TEXT = 1 << 0,
+        TERMINAL_RESET_AVOID_ANSI_SEQ = 1 << 1,
+        TERMINAL_RESET_FORCE_ANSI_SEQ = 1 << 2,
 } TerminalResetFlags;
 
 int terminal_reset_defensive(int fd, TerminalResetFlags flags);

diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c
index 8d48d1f75a14acb3d06cb7337a72c3d5bfc39596..490b3a522ae1197739519133f2b01ecdf3ac616d 100644 (file)
--- a/src/core/exec-invoke.c
+++ b/src/core/exec-invoke.c
@@ -4367,6 +4367,10 @@ static void prepare_terminal(
               p->stdout_fd >= 0))
                 return;
 
+        /* Let's explicitly determine whether to reset via ANSI sequences or not, taking our ExecContext
+         * information into account */
+        bool use_ansi = exec_context_shall_ansi_seq_reset(context);
+
         if (context->tty_reset) {
                 /* When we are resetting the TTY, then let's create a lock first, to synchronize access. This
                  * in particular matters as concurrent resets and the TTY size ANSI DSR logic done by the
@@ -4375,7 +4379,10 @@ static void prepare_terminal(
                 if (lock_fd < 0)
                         log_exec_debug_errno(context, p, lock_fd, "Failed to lock /dev/console, ignoring: %m");
 
-                (void) terminal_reset_defensive(STDOUT_FILENO, /* flags= */ 0);
+                /* We explicitly control whether to send ansi sequences or not here, since we want to consult
+                 * the env vars explicitly configured in the ExecContext, rather than our own environment
+                 * block. */
+                (void) terminal_reset_defensive(STDOUT_FILENO, use_ansi ? TERMINAL_RESET_FORCE_ANSI_SEQ : TERMINAL_RESET_AVOID_ANSI_SEQ);
         }
 
         (void) exec_context_apply_tty_size(context, STDIN_FILENO, STDOUT_FILENO, /* tty_path= */ NULL);

diff --git a/src/core/execute.c b/src/core/execute.c
index 62b9e6e221beb9bdb925268cb461d4a1cfdb661b..7d99e72328852a87409e8b67c20dd2eee7e4b0f0 100644 (file)
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -129,11 +129,12 @@ int exec_context_apply_tty_size(
                                 rows == UINT_MAX ? &rows : NULL,
                                 cols == UINT_MAX ? &cols : NULL);
 
-        /* If we got nothing so far and we are talking to a physical device, and the TTY reset logic is on,
-         * then let's query dimensions from the ANSI driver. */
+        /* If we got nothing so far and we are talking to a physical device, then let's query dimensions from
+         * the ANSI terminal driver. Note that we will not bother with this in case terminal reset via ansi
+         * sequences is not enabled, as the DSR logic relies on ANSI sequences after all, and if we shall not
+         * use those during initialization we need to skip it. */
         if (rows == UINT_MAX && cols == UINT_MAX &&
-            context->tty_reset &&
-            terminal_is_pty_fd(output_fd) == 0 &&
+            exec_context_shall_ansi_seq_reset(context) &&
             isatty_safe(input_fd)) {
                 r = terminal_get_size_by_dsr(input_fd, output_fd, &rows, &cols);
                 if (r < 0)
@@ -178,7 +179,10 @@ void exec_context_tty_reset(const ExecContext *context, const ExecParameters *p)
                 log_warning_errno(lock_fd, "Failed to lock /dev/console, proceeding without lock: %m");
 
         if (context->tty_reset)
-                (void) terminal_reset_defensive(fd, TERMINAL_RESET_SWITCH_TO_TEXT);
+                (void) terminal_reset_defensive(
+                                fd,
+                                TERMINAL_RESET_SWITCH_TO_TEXT |
+                                (exec_context_shall_ansi_seq_reset(context) ? TERMINAL_RESET_FORCE_ANSI_SEQ : TERMINAL_RESET_AVOID_ANSI_SEQ));
 
         r = exec_context_apply_tty_size(context, fd, fd, path);
         if (r < 0)
@@ -974,6 +978,22 @@ bool exec_context_may_touch_console(const ExecContext *ec) {
                tty_may_match_dev_console(exec_context_tty_path(ec));
 }
 
+bool exec_context_shall_ansi_seq_reset(const ExecContext *c) {
+        assert(c);
+
+        /* Determines whether ANSI sequences shall be used during any terminal initialisation:
+         *
+         * 1. If the reset logic is enabled at all, this is an immediate no.
+         *
+         * 2. If $TERM is set to anything other than "dumb", it's a yes.
+         */
+
+        if (!c->tty_reset)
+                return false;
+
+        return !streq_ptr(strv_env_get(c->environment, "TERM"), "dumb");
+}
+
 static void strv_fprintf(FILE *f, char **l) {
         assert(f);
 

diff --git a/src/core/execute.h b/src/core/execute.h
index 6421f19cc447aace46cccbcefb149e5e2b6e6876..abfa31afcb60cfabe0005ab8a21a5cbc6bdfb837 100644 (file)
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -530,6 +530,7 @@ const char* exec_context_fdname(const ExecContext *c, int fd_index);
 
 bool exec_context_may_touch_console(const ExecContext *c);
 bool exec_context_maintains_privileges(const ExecContext *c);
+bool exec_context_shall_ansi_seq_reset(const ExecContext *c);
 
 int exec_context_get_effective_ioprio(const ExecContext *c);
 bool exec_context_get_effective_mount_apivfs(const ExecContext *c);