From 9bc2acad8f47e53c1bcd3e36760dce49fd499ded Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 16 Dec 2024 16:23:49 +0900 Subject: [PATCH 1/2] ptyfwd: clean up logic of color state transition in pty_forward_ansi_process() Drop all 'continue', to make the logic easy to understand. No functional change, just refactoring and preparation for later commit. --- src/shared/ptyfwd.c | 55 ++++++++++++++++----------------------------- 1 file changed, 19 insertions(+), 36 deletions(-) diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index 72f91b0b62..331d092c63 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -36,8 +36,6 @@ typedef enum AnsiColorState { ANSI_COLOR_STATE_ESC, ANSI_COLOR_STATE_CSI_SEQUENCE, ANSI_COLOR_STATE_OSC_SEQUENCE, - ANSI_COLOR_STATE_NEWLINE, - ANSI_COLOR_STATE_CARRIAGE_RETURN, _ANSI_COLOR_STATE_MAX, _ANSI_COLOR_STATE_INVALID = -EINVAL, } AnsiColorState; @@ -427,39 +425,33 @@ static int pty_forward_ansi_process(PTYForward *f, size_t offset) { switch (f->ansi_color_state) { case ANSI_COLOR_STATE_TEXT: - break; - - case ANSI_COLOR_STATE_NEWLINE: - case ANSI_COLOR_STATE_CARRIAGE_RETURN: - /* Immediately after a newline (ASCII 10) or carriage return (ASCII 13) insert an - * ANSI sequence set the background color back. */ - - r = insert_background_color(f, i); - if (r < 0) - return r; - - i += r; + if (IN_SET(c, '\n', '\r')) { + /* Immediately after a newline (ASCII 10) or carriage return (ASCII 13) insert an + * ANSI sequence set the background color back. */ + r = insert_background_color(f, i+1); + if (r < 0) + return r; + i += r; + } else if (c == 0x1B) /* ESC */ + f->ansi_color_state = ANSI_COLOR_STATE_ESC; break; case ANSI_COLOR_STATE_ESC: - if (c == '[') { + if (c == '[') f->ansi_color_state = ANSI_COLOR_STATE_CSI_SEQUENCE; - continue; - } else if (c == ']') { + else if (c == ']') f->ansi_color_state = ANSI_COLOR_STATE_OSC_SEQUENCE; - continue; - } else if (c == 'c') { - /* "Full reset" aka "Reset to initial state"*/ + else if (c == 'c') { + /* "Full reset" aka "Reset to initial state" */ r = insert_background_color(f, i+1); if (r < 0) return r; i += r; - f->ansi_color_state = ANSI_COLOR_STATE_TEXT; - continue; - } + } else + f->ansi_color_state = ANSI_COLOR_STATE_TEXT; break; case ANSI_COLOR_STATE_CSI_SEQUENCE: @@ -472,7 +464,7 @@ static int pty_forward_ansi_process(PTYForward *f, size_t offset) { /* Safety check: lets not accept unbounded CSI sequences */ f->csi_sequence = mfree(f->csi_sequence); - break; + f->ansi_color_state = ANSI_COLOR_STATE_TEXT; } else if (!strextend(&f->csi_sequence, CHAR_TO_STR(c))) return -ENOMEM; } else { @@ -498,7 +490,7 @@ static int pty_forward_ansi_process(PTYForward *f, size_t offset) { f->csi_sequence = mfree(f->csi_sequence); f->ansi_color_state = ANSI_COLOR_STATE_TEXT; } - continue; + break; case ANSI_COLOR_STATE_OSC_SEQUENCE: @@ -506,7 +498,7 @@ static int pty_forward_ansi_process(PTYForward *f, size_t offset) { if (strlen_ptr(f->osc_sequence) >= ANSI_SEQUENCE_LENGTH_MAX) { /* Safety check: lets not accept unbounded OSC sequences */ f->osc_sequence = mfree(f->osc_sequence); - break; + f->ansi_color_state = ANSI_COLOR_STATE_TEXT; } else if (!strextend(&f->osc_sequence, CHAR_TO_STR(c))) return -ENOMEM; } else { @@ -534,20 +526,11 @@ static int pty_forward_ansi_process(PTYForward *f, size_t offset) { f->osc_sequence = mfree(f->osc_sequence); f->ansi_color_state = ANSI_COLOR_STATE_TEXT; } - continue; + break; default: assert_not_reached(); } - - if (c == '\n') - f->ansi_color_state = ANSI_COLOR_STATE_NEWLINE; - else if (c == '\r') - f->ansi_color_state = ANSI_COLOR_STATE_CARRIAGE_RETURN; - else if (c == 0x1B) /* ESC */ - f->ansi_color_state = ANSI_COLOR_STATE_ESC; - else - f->ansi_color_state = ANSI_COLOR_STATE_TEXT; } return 0; From a763364cc9183d72d7527ebf9ef853f99966bd5e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 16 Dec 2024 16:25:00 +0900 Subject: [PATCH 2/2] ptyfwd: fix logic of OSC sequence termination Previously, when an OSC sequence is terminated with \x1b\x5c, configured OSC sequence would be inserted between the two characters, rather than after the two. Fixes a bug introduced by d848a9499830c530e804a41ffd8aa1bc942fa735. Fixes #35631. --- src/shared/ptyfwd.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index 331d092c63..00450ca696 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -36,6 +36,7 @@ typedef enum AnsiColorState { ANSI_COLOR_STATE_ESC, ANSI_COLOR_STATE_CSI_SEQUENCE, ANSI_COLOR_STATE_OSC_SEQUENCE, + ANSI_COLOR_STATE_OSC_SEQUENCE_TERMINATING, _ANSI_COLOR_STATE_MAX, _ANSI_COLOR_STATE_INVALID = -EINVAL, } AnsiColorState; @@ -501,33 +502,46 @@ static int pty_forward_ansi_process(PTYForward *f, size_t offset) { f->ansi_color_state = ANSI_COLOR_STATE_TEXT; } else if (!strextend(&f->osc_sequence, CHAR_TO_STR(c))) return -ENOMEM; - } else { + } else if (c == '\x07') { /* Otherwise, the OSC sequence is over * * There are three documented ways to end an OSC sequence: * 1. BEL aka ^G aka \x07 * 2. \x9c * 3. \x1b\x5c - * since we cannot look ahead to see if the Esc is followed by a "\" - * we cut a corner here and assume it will be "\"e. - * * Note that we do not support \x9c here, because that's also a valid UTF8 * codepoint, and that would create ambiguity. Various terminal emulators * similar do not support it. */ - if (IN_SET(c, '\x07', '\x1b')) { - r = insert_window_title_fix(f, i+1); - if (r < 0) - return r; - - i += r; - } + r = insert_window_title_fix(f, i+1); + if (r < 0) + return r; + i += r; + f->osc_sequence = mfree(f->osc_sequence); + f->ansi_color_state = ANSI_COLOR_STATE_TEXT; + } else if (c == '\x1b') + /* See the comment above. */ + f->ansi_color_state = ANSI_COLOR_STATE_OSC_SEQUENCE_TERMINATING; + else { + /* Unexpected or unsupported OSC sequence. */ f->osc_sequence = mfree(f->osc_sequence); f->ansi_color_state = ANSI_COLOR_STATE_TEXT; } break; + case ANSI_COLOR_STATE_OSC_SEQUENCE_TERMINATING: + if (c == '\x5c') { + r = insert_window_title_fix(f, i+1); + if (r < 0) + return r; + i += r; + } + + f->osc_sequence = mfree(f->osc_sequence); + f->ansi_color_state = ANSI_COLOR_STATE_TEXT; + break; + default: assert_not_reached(); }