From 19a74d6a7ae62ddb2386c105797f4a7b09f1a250 Mon Sep 17 00:00:00 2001 From: Alan Carvalho de Assis Date: Tue, 21 Jul 2026 17:18:30 -0300 Subject: [PATCH] drivers/serial: Modify serial/pty to allow NSH/Telnet line edit Both the UART and PTY serial drivers previously assumed all VT100/ANSI escape sequences were fixed 3-byte CSI sequences, causing longer CSI and SS3 key sequences (such as Home, End, Delete, and modified keys) to leak stray characters into the terminal when local echo was enabled. This patch replaces the fixed-length logic with a state machine that correctly recognizes and suppresses escape sequences of any length, while preserving the data delivered to applications. The change only affects local echo behavior, is fully backward compatible, and has been validated with both interactive NSH sessions and automated PTY tests. Signed-off-by: Alan C. Assis Assisted-by: Claude Code --- drivers/serial/pty.c | 70 +++++++++++++++++++++++++------ drivers/serial/serial.c | 78 ++++++++++++++++++++++++++++++----- include/nuttx/serial/serial.h | 6 ++- 3 files changed, 131 insertions(+), 23 deletions(-) diff --git a/drivers/serial/pty.c b/drivers/serial/pty.c index 59bb59b6c706c..f2b7c091352d1 100644 --- a/drivers/serial/pty.c +++ b/drivers/serial/pty.c @@ -55,6 +55,20 @@ * Pre-processor Definitions ****************************************************************************/ +/* States for the local-echo VT100/ANSI escape sequence detector used by + * pty_read() below (dev->pd_escape). This mirrors the equivalent state + * machine in drivers/serial/serial.c (uart_readv()) -- see the comment + * there for the full rationale. It does not interpret the sequence, it + * only decides how many bytes to swallow from the local echo; the + * sequence bytes themselves are still delivered to the reader + * unmodified either way. + */ + +#define UART_ESCAPE_NONE 0 /* Not in an escape sequence */ +#define UART_ESCAPE_START 1 /* Saw ESC, waiting for '[', 'O', or other */ +#define UART_ESCAPE_CSI 2 /* Saw "ESC [", consuming CSI bytes */ +#define UART_ESCAPE_SS3 3 /* Saw "ESC O", waiting for the final byte */ + /* Maximum number of threads than can be waiting for POLL events */ #ifndef CONFIG_DEV_PTY_NPOLLWAITERS @@ -80,7 +94,10 @@ struct pty_dev_s struct file pd_src; /* Provides data to read() method (pipe output) */ struct file pd_sink; /* Accepts data from write() method (pipe input) */ bool pd_master; /* True: this is the master */ - uint8_t pd_escape; /* Number of the character to be escaped */ + uint8_t pd_escape; /* Escape sequence echo-suppression state + * (see the UART_ESCAPE_* values used in + * pty_read()) + */ #if defined(CONFIG_TTY_SIGINT) || defined(CONFIG_TTY_SIGTSTP) pid_t pd_pid; /* Thread PID to receive signals (-1 if none) */ #endif @@ -480,29 +497,58 @@ static ssize_t pty_read(FAR struct file *filep, FAR char *buffer, size_t len) { ch = buffer[i]; - /* Check for the beginning of a VT100 escape sequence, 3 byte */ + /* Detect a VT100/ANSI escape sequence (CSI "ESC [ ... " + * or SS3 "ESC O ") so it can be swallowed from the + * local echo below. See the UART_ESCAPE_* comment above. + */ if (ch == ASCII_ESC) { - /* Mark that we should skip 2 more bytes */ - - dev->pd_escape = 2; + dev->pd_escape = UART_ESCAPE_START; continue; } - else if (dev->pd_escape == 2 && ch != ASCII_LBRACKET) + else if (dev->pd_escape == UART_ESCAPE_START) { - /* It's not an [x 3 byte sequence, show it */ + if (ch == ASCII_LBRACKET) + { + dev->pd_escape = UART_ESCAPE_CSI; + continue; + } + else if (ch == ASCII_O) + { + dev->pd_escape = UART_ESCAPE_SS3; + continue; + } + + /* Not CSI or SS3 -- an unrecognized two-byte "ESC x" + * sequence. Fall through and consider 'x' for the echo + * batch normally, as before. + */ - dev->pd_escape = 0; + dev->pd_escape = UART_ESCAPE_NONE; } - else if (dev->pd_escape > 0) + else if (dev->pd_escape == UART_ESCAPE_CSI) { - /* Skipping character count down */ + /* Consuming CSI parameter/intermediate bytes (0x20-0x3f); + * the sequence ends with exactly one final byte in + * 0x40-0x7e. + */ - if (--dev->pd_escape > 0) + if (ch >= 0x40 && ch <= 0x7e) { - continue; + dev->pd_escape = UART_ESCAPE_NONE; } + + continue; + } + else if (dev->pd_escape == UART_ESCAPE_SS3) + { + /* The byte following "ESC O" is always the final (and + * only) byte. + */ + + dev->pd_escape = UART_ESCAPE_NONE; + continue; } /* Echo if the character in batch */ diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 9f538edf338cf..c2c76b5249ddc 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -79,6 +79,32 @@ #define POLL_DELAY_USEC 1000 +/* States for the local-echo VT100/ANSI escape sequence detector used by + * uart_readv() below (dev->escape). This does not interpret the + * sequence, it only decides how many bytes to swallow from the local + * echo so that raw escape sequences typed by the user are not rendered + * as visible garbage; the sequence bytes themselves are still delivered + * to the reader unmodified either way. + * + * Two forms are recognized: + * + * CSI: ESC '[' + * + * e.g. ESC[D (left arrow), ESC[3~ (Delete), ESC[1;5D (Ctrl+Left) + * + * SS3: ESC 'O' + * e.g. ESC O F (End, as sent by xterm and many other terminals + * that otherwise use CSI for the plain arrow keys) + * + * Any other byte following ESC is treated as an unrecognized two-byte + * "ESC x" sequence; 'x' itself is echoed normally, as before. + */ + +#define UART_ESCAPE_NONE 0 /* Not in an escape sequence */ +#define UART_ESCAPE_START 1 /* Saw ESC, waiting for '[', 'O', or other */ +#define UART_ESCAPE_CSI 2 /* Saw "ESC [", consuming CSI bytes */ +#define UART_ESCAPE_SS3 3 /* Saw "ESC O", waiting for the final byte */ + /**************************************************************************** * Private Types ****************************************************************************/ @@ -1063,26 +1089,58 @@ static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio) if (dev->tc_lflag & ECHO) { - /* Check for the beginning of a VT100 escape sequence, 3 byte */ - if (ch == ASCII_ESC) { - /* Mark that we should skip 2 more bytes */ + /* Start (or restart) tracking a possible escape + * sequence. + */ - dev->escape = 2; + dev->escape = UART_ESCAPE_START; continue; } - else if (dev->escape == 2 && ch != ASCII_LBRACKET) + else if (dev->escape == UART_ESCAPE_START) + { + /* First byte after ESC selects the sequence type */ + + if (ch == ASCII_LBRACKET) + { + dev->escape = UART_ESCAPE_CSI; + continue; + } + else if (ch == ASCII_O) + { + dev->escape = UART_ESCAPE_SS3; + continue; + } + + /* Not CSI or SS3 -- an unrecognized two-byte "ESC x" + * sequence. Fall through and echo 'x' normally, as + * before. + */ + + dev->escape = UART_ESCAPE_NONE; + } + else if (dev->escape == UART_ESCAPE_CSI) { - /* It's not an [x 3 byte sequence, show it */ + /* Consuming CSI parameter/intermediate bytes + * (0x20-0x3f); the sequence ends with exactly one + * final byte in 0x40-0x7e. + */ + + if (ch >= 0x40 && ch <= 0x7e) + { + dev->escape = UART_ESCAPE_NONE; + } - dev->escape = 0; + continue; } - else if (dev->escape > 0) + else if (dev->escape == UART_ESCAPE_SS3) { - /* Skipping character count down */ + /* The byte following "ESC O" is always the final + * (and only) byte. + */ - dev->escape--; + dev->escape = UART_ESCAPE_NONE; continue; } diff --git a/include/nuttx/serial/serial.h b/include/nuttx/serial/serial.h index ee82a927e1785..8a720399ddb29 100644 --- a/include/nuttx/serial/serial.h +++ b/include/nuttx/serial/serial.h @@ -326,7 +326,11 @@ struct uart_dev_s /* State data */ uint8_t open_count; /* Number of times the device has been opened */ - uint8_t escape; /* Number of the character to be escaped */ + uint8_t escape; /* VT100/ANSI escape sequence echo- + * suppression state (see the + * UART_ESCAPE_* values used in + * uart_readv()) + */ #ifdef CONFIG_SERIAL_REMOVABLE volatile bool disconnected; /* true: Removable device is not connected */ #endif