fix(input): handle ESC during CSI and SS3 parse states per ECMA-48#1054
Conversation
The input parser's CSI and SS3 state handlers do not check for ESC (0x1B). Per ECMA-48 §5.3.1, ESC is always effective and should restart the escape sequence machine from any intermediate state. Instead, ESC falls into the "bad parse" branch in inpStateCsi and is silently discarded, and in inpStateSs3 it fails the key lookup and is lost. This causes the byte(s) following the discarded ESC to be misinterpreted. In practice, the user's first keystroke after Suspend/Resume can be swallowed: engage() sends escape sequences that may provoke terminal responses, and if those responses leave the parser mid-CSI, the next ESC-prefixed key (arrow keys, function keys) is consumed as the tail of the stale sequence. Add an ESC check to both inpStateCsi and inpStateSs3 that transitions to inpStateEsc with the standard timeout setup, matching the behavior of inpStateInit.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gdamore
left a comment
There was a problem hiding this comment.
No immediate plans for an update on tcell v2 though.
Disclosure: I stumbled across this while working on a personal project. AI tools were used to diagnose and resolve the issue as well as generate this potential upstream fix. Systems programming at this level is not my expertise, but the fix seemed small, contained, and testable so I thought it worthwhile to open a PR.
The input parser's CSI and SS3 state handlers do not check for ESC (0x1B). Per ECMA-48 §5.3.1, ESC is "always effective" and should restart the escape sequence machine from any intermediate state. Instead, ESC falls into the "bad parse" branch in inpStateCsi and is silently discarded, and in inpStateSs3 it fails the key lookup and is similarly lost.
This causes the byte(s) following the discarded ESC to be misinterpreted. In practice, the user's first keystroke after Suspend/Resume can be swallowed: engage() sends escape sequences that may provoke terminal responses, and if those responses leave the parser mid-CSI, the next ESC-prefixed key (arrow keys, function keys) is consumed as the tail of the stale sequence.
The tty input flush added in #1048 reduces the likelihood of stale bytes reaching the parser, but does not eliminate it — terminal responses can arrive after the flush and before inputLoop starts, or bytes can be split across reads leaving a partial sequence in the parser's state machine.
This adds an ESC check to both inpStateCsi and inpStateSs3 that transitions to inpStateEsc with the standard timeout setup, matching the behavior of inpStateInit. Tests verify that a partial CSI or SS3 followed by a new escape sequence produces the correct key event.
Backport of the equivalent fix on main (#1053)