From 77cfaf03cb501819417179b53ad25ec68ee41a52 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 20 May 2013 15:34:21 +0300 Subject: [PATCH 1/3] Ticket #2988: When an unknown key is pressed, it is interpreted as garbage. keyboard input: simplify code, no logic changes This change slightly simplifies and rearranges the code in get_key_code(), reduces indentation levels there, adds a few comments. The logic remains the same. This is a preparatory patch for subsequent changes. Signed-off-by: Denys Vlasenko Signed-off-by: Slava Zanko --- lib/tty/key.c | 128 ++++++++++++++++++++++++-------------------------- 1 file changed, 62 insertions(+), 66 deletions(-) diff --git a/lib/tty/key.c b/lib/tty/key.c index 0d15a7960..ca9f1ac73 100644 --- a/lib/tty/key.c +++ b/lib/tty/key.c @@ -2,7 +2,7 @@ Keyboard support routines. Copyright (C) 1994, 1995, 1998, 1999, 2000, 2001, 2002, 2003, 2004, - 2005, 2006, 2007, 2009, 2010, 2011 + 2005, 2006, 2007, 2009, 2010, 2011, 2013 The Free Software Foundation, Inc. Written by: @@ -10,6 +10,8 @@ Janne Kukonlehto, 1994, 1995 Jakub Jelinek, 1995 Norbert Warmuth, 1997 + Denys Vlasenko , 2013 + Slava Zanko , 2013 This file is part of the Midnight Commander. @@ -1747,7 +1749,7 @@ get_key_code (int no_delay) int d; d = *pending_keys++; - while (d == ESC_CHAR && *pending_keys != '\0') + while (d == ESC_CHAR) d = ALT (*pending_keys++); if (*pending_keys == '\0') @@ -1828,49 +1830,12 @@ get_key_code (int no_delay) this = keys->child; } } + while (this != NULL) { if (c == this->ch) { - if (this->child) - { - if (!push_char (c)) - { - pending_keys = seq_buffer; - goto pend_send; - } - parent = this; - this = this->child; - if (parent->action == MCKEY_ESCAPE && old_esc_mode) - { - if (no_delay) - { - GET_TIME (esctime); - if (this == NULL) - { - /* Shouldn't happen */ - fputs ("Internal error\n", stderr); - exit (EXIT_FAILURE); - } - goto nodelay_try_again; - } - esctime.tv_sec = -1; - c = xgetch_second (); - if (c == -1) - { - pending_keys = seq_append = NULL; - this = NULL; - return ESC_CHAR; - } - } - else - { - if (no_delay) - goto nodelay_try_again; - c = tty_lowlevel_getch (); - } - } - else + if (!this->child) { /* We got a complete match, return and reset search */ int code; @@ -1880,36 +1845,67 @@ get_key_code (int no_delay) this = NULL; return correct_key_code (code); } - } - else - { - if (this->next != NULL) - this = this->next; - else + /* No match yet, but it may be a prefix for a valid seq */ + if (!push_char (c)) { - if ((parent != NULL) && (parent->action == MCKEY_ESCAPE)) - { - /* Convert escape-digits to F-keys */ - if (g_ascii_isdigit (c)) - c = KEY_F (c - '0'); - else if (c == ' ') - c = ESC_CHAR; - else - c = ALT (c); - - pending_keys = seq_append = NULL; - this = NULL; - return correct_key_code (c); - } - /* Did not find a match or {c} was changed in the if above, - so we have to return everything we had skipped - */ - push_char (c); pending_keys = seq_buffer; goto pend_send; } + parent = this; + this = this->child; + if (parent->action == MCKEY_ESCAPE && old_esc_mode) + { + if (no_delay) + { + GET_TIME (esctime); + goto nodelay_try_again; + } + esctime.tv_sec = -1; + c = xgetch_second (); + if (c == -1) + { + pending_keys = seq_append = NULL; + this = NULL; + return ESC_CHAR; + } + continue; + } + if (no_delay) + goto nodelay_try_again; + c = tty_lowlevel_getch (); + continue; } - } + + /* c != this->ch. Try other keys with this prefix */ + if (this->next != NULL) + { + this = this->next; + continue; + } + + /* No match found. Is it one of our ESC specials? */ + if ((parent != NULL) && (parent->action == MCKEY_ESCAPE)) + { + /* Convert escape-digits to F-keys */ + if (g_ascii_isdigit (c)) + c = KEY_F (c - '0'); + else if (c == ' ') + c = ESC_CHAR; + else + c = ALT (c); + + pending_keys = seq_append = NULL; + this = NULL; + return correct_key_code (c); + } + + /* Unknown sequence. Maybe a prefix of a longer one. Save it. */ + push_char (c); + pending_keys = seq_buffer; + goto pend_send; + + } /* while (this != NULL) */ + this = NULL; return correct_key_code (c); } From ecd3b62b8efea4d94fe8162eb0b50d223494efd0 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 21 May 2013 14:47:30 +0300 Subject: [PATCH 2/3] This change prevents misinterpreting an unknown ESC sequence's tail as a garbage input. To reproduce, run "sleep 3" and hold down Down_Arrow key until sleep runs. With debugging log enabled, the following can be seen: entered get_key_code(no_delay:0) c=tty_lowlevel_getch()=27 push_char(27) !0 c=xgetch_second()=91 push_char(91) !0 2 c=tty_lowlevel_getch()=66 push_char(66) seq_buffer[0]:27 <---- the saved Down Arrow sequence "ESC [ B" seq_buffer[1]:91 seq_buffer[2]:66 seq_buffer[3]:0 pending_keys!=NULL. m=-1 d=*pending_keys++=27 d=ALT(*pending_keys++)=ALT(91)=8283 ^^^^^^^^^^^^^^^^^^^^^^^ we misinterpret "ESC [ B" as "ESC [" return correct_key_code(8283) entered get_key_code(no_delay:0) pending_keys!=NULL. m=-1 d=*pending_keys++=66 ^^^^^^^^^^^^ we think user pressed "B" return correct_key_code(66) With this patch, no bogus "input" is generated. Longer unknown sequences need an additional fix, coming next. Signed-off-by: Denys Vlasenko Signed-off-by: Slava Zanko --- lib/tty/key.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/tty/key.c b/lib/tty/key.c index ca9f1ac73..4d07ffd6f 100644 --- a/lib/tty/key.c +++ b/lib/tty/key.c @@ -1747,14 +1747,19 @@ get_key_code (int no_delay) if (pending_keys != NULL) { int d; + gboolean bad_seq; d = *pending_keys++; while (d == ESC_CHAR) d = ALT (*pending_keys++); - if (*pending_keys == '\0') + bad_seq = (*pending_keys != ESC_CHAR && *pending_keys != 0); + if (*pending_keys == '\0' || bad_seq) pending_keys = seq_append = NULL; + if (bad_seq) + goto nodelay_try_again; + if (d > 127 && d < 256 && use_8th_bit_as_meta) d = ALT (d & 0x7f); @@ -1835,7 +1840,7 @@ get_key_code (int no_delay) { if (c == this->ch) { - if (!this->child) + if (this->child == NULL) { /* We got a complete match, return and reset search */ int code; From 664e7d31b6c073154580ff913950fc393ae34623 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 21 May 2013 15:35:54 +0300 Subject: [PATCH 3/3] When we see an unknown sequence, it is not enough to drop already received part - there can be more of it coming over e.g. a serial line. To prevent interpreting it as a random garbage, eat and discard all chars that follow. Small, but non-zero timeout is needed to reconnect escape sequence split up by a serial line. Before this change, Ctrl-Alt-Shift-Right_Arrow generates "1;8C" bogus "input" in MC on my machine; after the change, nothing is generated. Signed-off-by: Denys Vlasenko Signed-off-by: Slava Zanko --- lib/tty/key.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/tty/key.c b/lib/tty/key.c index 4d07ffd6f..c58043e0b 100644 --- a/lib/tty/key.c +++ b/lib/tty/key.c @@ -1131,14 +1131,14 @@ correct_key_code (int code) /* --------------------------------------------------------------------------------------------- */ static int -xgetch_second (void) +getch_with_timeout (unsigned int delay_us) { fd_set Read_FD_Set; int c; struct timeval time_out; - time_out.tv_sec = old_esc_mode_timeout / 1000000; - time_out.tv_usec = old_esc_mode_timeout % 1000000; + time_out.tv_sec = delay_us / 1000000u; + time_out.tv_usec = delay_us % 1000000u; tty_nodelay (TRUE); FD_ZERO (&Read_FD_Set); FD_SET (input_fd, &Read_FD_Set); @@ -1758,7 +1758,19 @@ get_key_code (int no_delay) pending_keys = seq_append = NULL; if (bad_seq) + { + /* This is an unknown ESC sequence. + * To prevent interpreting its tail as a random garbage, + * eat and discard all buffered and quickly following chars. + * Small, but non-zero timeout is needed to reconnect + * escape sequence split up by e.g. a serial line. + */ + int paranoia = 20; + + while (getch_with_timeout (old_esc_mode_timeout) >= 0 && --paranoia != 0) + ; goto nodelay_try_again; + } if (d > 127 && d < 256 && use_8th_bit_as_meta) d = ALT (d & 0x7f); @@ -1866,7 +1878,7 @@ get_key_code (int no_delay) goto nodelay_try_again; } esctime.tv_sec = -1; - c = xgetch_second (); + c = getch_with_timeout (old_esc_mode_timeout); if (c == -1) { pending_keys = seq_append = NULL;