From 9924d3c595958982b331c7e5f326bb3a4c1c7db6 Mon Sep 17 00:00:00 2001 From: Sam Leonard Date: Tue, 27 Feb 2024 11:12:39 +0000 Subject: [PATCH 1/4] basic/terminal-util: add check for poll timeout in get_default_background_color Currently the return value 0 is not checked for, this indicates a timeout and should be handled to prevent doing a blocking read on a file descriptor with no data ready. --- src/basic/terminal-util.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 4c1824bc83f..aa1cf7cfff7 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -1744,6 +1744,10 @@ int get_default_background_color(double *ret_red, double *ret_green, double *ret r = fd_wait_for_event(STDIN_FILENO, POLLIN, usec_sub_unsigned(end, n)); if (r < 0) goto finish; + if (r == 0) { + r = -EOPNOTSUPP; + goto finish; + } ssize_t l; l = read(STDIN_FILENO, buf, sizeof(buf) - buf_full); From 73a72e3a7b0e125dfabdf9e16ccb3a129ecdeab6 Mon Sep 17 00:00:00 2001 From: Sam Leonard Date: Tue, 27 Feb 2024 14:35:14 +0000 Subject: [PATCH 2/4] basic/terminal-util: accept ST or BEL to end escape sequence queries Currently scan_background_color_response only accepts BEL (\x07) to end a response, however some terminals (namely kitty in my case) will reply with the string terminator (ST - https://en.wikipedia.org/wiki/ANSI_escape_code). This commit changes the behaviour to now accept either ending. --- src/basic/terminal-util.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index aa1cf7cfff7..8b5a9fa8c68 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -1581,6 +1581,7 @@ typedef enum BackgroundColorState { BACKGROUND_RED, BACKGROUND_GREEN, BACKGROUND_BLUE, + BACKGROUND_STRING_TERMINATOR, } BackgroundColorState; typedef struct BackgroundColorContext { @@ -1672,7 +1673,9 @@ static int scan_background_color_response( return 1; /* success! */ context->state = BACKGROUND_TEXT; - } else { + } else if (c == '\x1b') + context->state = context->blue_bits > 0 ? BACKGROUND_STRING_TERMINATOR : BACKGROUND_TEXT; + else { int d = unhexchar(c); if (d < 0 || context->blue_bits >= sizeof(context->blue)*8) context->state = BACKGROUND_TEXT; @@ -1682,10 +1685,18 @@ static int scan_background_color_response( } } break; + + case BACKGROUND_STRING_TERMINATOR: + if (c == '\\') + return 1; /* success! */ + + context->state = c == ']' ? BACKGROUND_ESCAPE : BACKGROUND_TEXT; + break; + } /* Reset any colors we might have picked up */ - if (context->state == BACKGROUND_TEXT) { + if (IN_SET(context->state, BACKGROUND_TEXT, BACKGROUND_ESCAPE)) { /* reset color */ context->red = context->green = context->blue = 0; context->red_bits = context->green_bits = context->blue_bits = 0; From 9eb118eea7396cb33b37de0d993091cfab0bf3c5 Mon Sep 17 00:00:00 2001 From: Sam Leonard Date: Tue, 27 Feb 2024 15:08:37 +0000 Subject: [PATCH 3/4] shared/ptyfwd: allow window title but not background color as a valid state Previously if a PTYForward instance had the window title set but no background color set then it would crash in an assertion as pty_forward_ansi_process didn't require both to be present. systemd-vmspawn could get into this state if it failed to get the terminal tint color. Now any method that would have called background_color_sequence now becomes just a NOP if the background color is not set. This allows keeping the functionality to set window titles even if the terminal doesn't support the background coloring. --- src/shared/ptyfwd.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index f910f3aaf91..bf775425e78 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -270,7 +270,9 @@ static int insert_newline_color_erase(PTYForward *f, size_t offset) { _cleanup_free_ char *s = NULL; assert(f); - assert(f->background_color); + + if (!f->background_color) + return 0; /* When we see a newline (ASCII 10) then this sets the background color to the desired one, and erase the rest * of the line with it */ @@ -289,7 +291,9 @@ static int insert_carriage_return_color(PTYForward *f, size_t offset) { _cleanup_free_ char *s = NULL; assert(f); - assert(f->background_color); + + if (!f->background_color) + return 0; /* When we see a carriage return (ASCII 13) this this sets only the background */ From d848a9499830c530e804a41ffd8aa1bc942fa735 Mon Sep 17 00:00:00 2001 From: Sam Leonard Date: Tue, 27 Feb 2024 16:08:09 +0000 Subject: [PATCH 4/4] shared/ptyfwd: detect String Terminator or BEL when parsing an OSC sequence --- src/shared/ptyfwd.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index bf775425e78..17c1549132e 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -507,9 +507,15 @@ static int pty_forward_ansi_process(PTYForward *f, size_t offset) { } else if (!strextend(&f->osc_sequence, CHAR_TO_STR(c))) return -ENOMEM; } else { - /* Otherwise, the OSC sequence is over */ + /* Otherwise, the OSC sequence is over + * + * There are two allowed ways to end an OSC sequence: + * BEL '\x07' + * String Terminator (ST): \ - "\x1b\x5c" + * since we cannot lookahead to see if the Esc is followed by a \ + * we cut a corner here and assume it will be \. */ - if (c == '\x07') { + if (c == '\x07' || c == '\x1b') { r = insert_window_title_fix(f, i+1); if (r < 0) return r;