[PATCH v3] cli: Consume invalid escape sequences early
Unexpected 'Esc' key presses are accumulated internally, even if it is already clear that the current escape sequence is invalid. This results in weird behaviour. For example, the next character after 'Esc' key simply disappears from input and 'Unknown command' is printed after 'Enter'.
This commit fixes some issues with extra 'Esc' keys entered by user:
1. Sequence <Esc><Esc><Enter> right after autoboot stop gives: => nknown command 'ry 'help' => 2. Sequence <Esc><p><r><i><Enter> gives: => ri Unknown command 'ri' - try 'help' => 3. Extra 'Esc' key presses break backspace functionality.
Signed-off-by: Yurii Monakov monakov.y@gmail.com --- Changes for v2: - add tests and reword commit message Changes for v3: - fix indentation
common/cli_getch.c | 2 ++ test/common/cread.c | 12 ++++++++++++ 2 files changed, 14 insertions(+)
diff --git a/common/cli_getch.c b/common/cli_getch.c index 61d4cb261b..0ee7908777 100644 --- a/common/cli_getch.c +++ b/common/cli_getch.c @@ -46,6 +46,8 @@ static int cli_ch_esc(struct cli_ch_state *cch, int ichar, case 1: if (ichar == '[' || ichar == 'O') act = ESC_SAVE; + else + act = ESC_CONVERTED; break; case 2: switch (ichar) { diff --git a/test/common/cread.c b/test/common/cread.c index 2fdd29a265..4edc773960 100644 --- a/test/common/cread.c +++ b/test/common/cread.c @@ -43,6 +43,12 @@ static int cli_ch_test(struct unit_test_state *uts) ut_asserteq('a', cli_ch_process(cch, 'a')); ut_asserteq(0, cli_ch_process(cch, 0));
+ /* unexpected 'Esc' */ + ut_asserteq('a', cli_ch_process(cch, 'a')); + ut_asserteq(0, cli_ch_process(cch, '\e')); + ut_asserteq('b', cli_ch_process(cch, 'b')); + ut_asserteq(0, cli_ch_process(cch, 0)); + return 0; } COMMON_TEST(cli_ch_test, 0); @@ -80,6 +86,12 @@ static int cread_test(struct unit_test_state *uts) ut_asserteq(7, cli_readline_into_buffer("-> ", buf, 1)); ut_asserteq_str("abc\e[Xx", buf);
+ /* unexpected 'Esc' */ + *buf = '\0'; + ut_asserteq(7, console_in_puts("abc\eXx\n")); + ut_asserteq(5, cli_readline_into_buffer("-> ", buf, 1)); + ut_asserteq_str("abcXx", buf); + /* check timeout, should be between 1000 and 1050ms */ start = get_timer(0); *buf = '\0';
On Tue, 10 Oct 2023 at 02:16, Yurii Monakov monakov.y@gmail.com wrote:
Unexpected 'Esc' key presses are accumulated internally, even if it is already clear that the current escape sequence is invalid. This results in weird behaviour. For example, the next character after 'Esc' key simply disappears from input and 'Unknown command' is printed after 'Enter'.
This commit fixes some issues with extra 'Esc' keys entered by user:
- Sequence <Esc><Esc><Enter> right after autoboot stop gives:
=> nknown command 'ry 'help' => 2. Sequence <Esc><p><r><i><Enter> gives: => ri Unknown command 'ri' - try 'help' => 3. Extra 'Esc' key presses break backspace functionality.
Signed-off-by: Yurii Monakov monakov.y@gmail.com
Changes for v2:
- add tests and reword commit message
Changes for v3:
- fix indentation
common/cli_getch.c | 2 ++ test/common/cread.c | 12 ++++++++++++ 2 files changed, 14 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Unfortunately this shows a design flaw, one which is hard to fix in general. But this does improve it.
The flaw is that we assume that character sequences have a time gap between them, which allows figuring out when a sequence has finished.
But when starting up there may be buffered output with no gaps. I don't think there is a general fix for this problem. One option is to have a special mode at the start, where escape sequences are ignored. But the user may press an arrow key on startup.
So I don't have anything much to suggest here. Let's see how this fix goes. Perhaps it is enough.
Regards, Simon
Simon,
On Tue, Oct 10, 2023 at 5:58 PM Simon Glass sjg@google.com wrote:
On Tue, 10 Oct 2023 at 02:16, Yurii Monakov monakov.y@gmail.com wrote:
Unexpected 'Esc' key presses are accumulated internally, even if it is already clear that the current escape sequence is invalid. This results in weird behaviour. For example, the next character after 'Esc' key simply disappears from input and 'Unknown command' is printed after 'Enter'.
This commit fixes some issues with extra 'Esc' keys entered by user:
- Sequence <Esc><Esc><Enter> right after autoboot stop gives:
=> nknown command 'ry 'help' => 2. Sequence <Esc><p><r><i><Enter> gives: => ri Unknown command 'ri' - try 'help' => 3. Extra 'Esc' key presses break backspace functionality.
Signed-off-by: Yurii Monakov monakov.y@gmail.com
Changes for v2:
- add tests and reword commit message
Changes for v3:
- fix indentation
common/cli_getch.c | 2 ++ test/common/cread.c | 12 ++++++++++++ 2 files changed, 14 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Unfortunately this shows a design flaw, one which is hard to fix in general. But this does improve it.
The flaw is that we assume that character sequences have a time gap between them, which allows figuring out when a sequence has finished.
But when starting up there may be buffered output with no gaps. I don't think there is a general fix for this problem. One option is to have a special mode at the start, where escape sequences are ignored. But the user may press an arrow key on startup.
So I don't have anything much to suggest here. Let's see how this fix goes. Perhaps it is enough.
Regards, Simon
One option is to have a special mode at the start, where escape sequences are ignored
I think that there is no reason to emit parts of invalid escape sequences, as current code does. There is a very little chance (at least for a human being) to input such sequences by intent.
But the user may press an arrow key on startup.
As a fix, autoboot code can check for 'Esc' and keep it as a part of input string.
Best Regards, Yurii
On Tue, Oct 10, 2023 at 11:16:39AM +0300, Yurii Monakov wrote:
Unexpected 'Esc' key presses are accumulated internally, even if it is already clear that the current escape sequence is invalid. This results in weird behaviour. For example, the next character after 'Esc' key simply disappears from input and 'Unknown command' is printed after 'Enter'.
This commit fixes some issues with extra 'Esc' keys entered by user:
- Sequence <Esc><Esc><Enter> right after autoboot stop gives:
=> nknown command 'ry 'help' => 2. Sequence <Esc><p><r><i><Enter> gives: => ri Unknown command 'ri' - try 'help' => 3. Extra 'Esc' key presses break backspace functionality.
Signed-off-by: Yurii Monakov monakov.y@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
On 10/10/2023 10.16, Yurii Monakov wrote:
Unexpected 'Esc' key presses are accumulated internally, even if it is already clear that the current escape sequence is invalid. This results in weird behaviour. For example, the next character after 'Esc' key simply disappears from input and 'Unknown command' is printed after 'Enter'.
This commit fixes some issues with extra 'Esc' keys entered by user:
- Sequence <Esc><Esc><Enter> right after autoboot stop gives:
=> nknown command 'ry 'help' => 2. Sequence <Esc><p><r><i><Enter> gives: => ri Unknown command 'ri' - try 'help' => 3. Extra 'Esc' key presses break backspace functionality.
Thank you! This has been bugging me for years, since we have
CONFIG_AUTOBOOT_DELAY_STR=" " CONFIG_AUTOBOOT_STOP_STR="\x1b"
and pressing <esc> to stop autoboot did have that side-effect of swallowing the first following char. But I never found the time to dig into why or if it was even fixable.
Tom has already applied this, but nevertheless
Tested-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Rasmus
participants (4)
-
Rasmus Villemoes -
Simon Glass -
Tom Rini -
Yurii Monakov