[git commit master] read_key,lineeedit: parse position answerback faster; sanitize its use

Denys Vlasenko vda.linux at googlemail.com
Mon Oct 26 14:23:32 UTC 2009


commit: http://git.busybox.net/busybox/commit/?id=727e1b536e5478b8f28c93990a5bf34091144608
branch: http://git.busybox.net/busybox/commit/?id=refs/heads/master

it's still not reliable, and probably cannot be made so...
added comment with explanation.

function                                             old     new   delta
put_prompt                                            52     110     +58
read_key                                             601     607      +6
lineedit_read_key                                    201     207      +6
win_changed                                          108     104      -4
read_line_input                                     4824    4809     -15
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/2 up/down: 70/-19)             Total: 51 bytes

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 libbb/lineedit.c |   38 +++++++++++++++++++++++++++++++++-----
 libbb/read_key.c |   53 +++++++++++++++++++++++++----------------------------
 2 files changed, 58 insertions(+), 33 deletions(-)

diff --git a/libbb/lineedit.c b/libbb/lineedit.c
index bfd0e33..2e1bc5d 100644
--- a/libbb/lineedit.c
+++ b/libbb/lineedit.c
@@ -370,6 +370,7 @@ static void input_backward(unsigned num)
 static void put_prompt(void)
 {
 	out1str(cmdedit_prompt);
+	fflush(NULL);
 	if (ENABLE_FEATURE_EDITING_ASK_TERMINAL) {
 		/* Ask terminal where is the cursor now.
 		 * lineedit_read_key handles response and corrects
@@ -380,7 +381,33 @@ static void put_prompt(void)
 		 * Note: we print it _after_ prompt, because
 		 * prompt may contain CR. Example: PS1='\[\r\n\]\w '
 		 */
-		out1str("\033" "[6n");
+		/* Problem: if there is buffered input on stdin,
+		 * the response will be delivered later,
+		 * possibly to an unsuspecting application.
+		 * Testcase: "sleep 1; busybox ash" + press and hold [Enter].
+		 * Result:
+		 * ~/srcdevel/bbox/fix/busybox.t4 #
+		 * ~/srcdevel/bbox/fix/busybox.t4 #
+		 * ^[[59;34~/srcdevel/bbox/fix/busybox.t4 #  <-- garbage
+		 * ~/srcdevel/bbox/fix/busybox.t4 #
+		 *
+		 * Checking for input with poll only makes the race narrower,
+		 * I still can trigger it. Strace:
+		 *
+		 * write(1, "~/srcdevel/bbox/fix/busybox.t4 # ", 33) = 33
+		 * poll([{fd=0, events=POLLIN}], 1, 0) = 0 (Timeout)  <-- no input exists
+		 * write(1, "\33[6n", 4) = 4  <-- send the ESC sequence, quick!
+		 * poll([{fd=0, events=POLLIN}], 1, 4294967295) = 1 ([{fd=0, revents=POLLIN}])
+		 * read(0, "\n", 1)      = 1  <-- oh crap, user's input got in first
+		 */
+		struct pollfd pfd;
+
+		pfd.fd = STDIN_FILENO;
+		pfd.events = POLLIN;
+		if (safe_poll(&pfd, 1, 0) == 0) {
+			out1str("\033" "[6n");
+			fflush(NULL); /* make terminal see it ASAP! */
+		}
 	}
 	cursor = 0;
 	{
@@ -1603,7 +1630,7 @@ static void cmdedit_setwidth(unsigned w, int redraw_flg)
 		int new_y = (cursor + cmdedit_prmt_len) / w;
 		/* redraw */
 		redraw((new_y >= cmdedit_y ? new_y : cmdedit_y), command_len - cursor);
-		fflush(stdout);
+		fflush(NULL);
 	}
 }
 
@@ -1640,6 +1667,7 @@ static int lineedit_read_key(char *read_key_buffer)
 		ic = read_key(STDIN_FILENO, read_key_buffer);
 
 		if (ENABLE_FEATURE_EDITING_ASK_TERMINAL
+		 && cursor == 0 /* otherwise it may be bogus */
 		 && (int32_t)ic == KEYCODE_CURSOR_POS
 		) {
 			int col = ((ic >> 32) & 0x7fff) - 1;
@@ -1708,7 +1736,7 @@ int FAST_FUNC read_line_input(const char *prompt, char *command, int maxsize, li
 	) {
 		/* Happens when e.g. stty -echo was run before */
 		parse_and_put_prompt(prompt);
-		fflush(stdout);
+		/* fflush(stdout); - done by parse_and_put_prompt */
 		if (fgets(command, maxsize, stdin) == NULL)
 			len = -1; /* EOF or error */
 		else
@@ -2190,7 +2218,7 @@ int FAST_FUNC read_line_input(const char *prompt, char *command, int maxsize, li
 	tcsetattr_stdin_TCSANOW(&initial_settings);
 	/* restore SIGWINCH handler */
 	signal(SIGWINCH, previous_SIGWINCH_handler);
-	fflush(stdout);
+	fflush(NULL);
 
 	len = command_len;
 	DEINIT_S();
@@ -2204,7 +2232,7 @@ int FAST_FUNC read_line_input(const char *prompt, char *command, int maxsize, li
 int FAST_FUNC read_line_input(const char* prompt, char* command, int maxsize)
 {
 	fputs(prompt, stdout);
-	fflush(stdout);
+	fflush(NULL);
 	fgets(command, maxsize, stdin);
 	return strlen(command);
 }
diff --git a/libbb/read_key.c b/libbb/read_key.c
index ec1b3a4..a2253ce 100644
--- a/libbb/read_key.c
+++ b/libbb/read_key.c
@@ -202,6 +202,31 @@ int64_t FAST_FUNC read_key(int fd, char *buffer)
 			break;
 		}
 		n++;
+		/* Try to decipher "ESC [ NNN ; NNN R" sequence */
+		if (ENABLE_FEATURE_EDITING_ASK_TERMINAL
+		 && n >= 5
+		 && buffer[0] == '['
+		 && buffer[n-1] == 'R'
+		 && isdigit(buffer[1])
+		) {
+			char *end;
+			unsigned long row, col;
+
+			row = strtoul(buffer + 1, &end, 10);
+			if (*end != ';' || !isdigit(end[1]))
+				continue;
+			col = strtoul(end + 1, &end, 10);
+			if (*end != 'R')
+				continue;
+			if (row < 1 || col < 1 || (row | col) > 0x7fff)
+				continue;
+
+			buffer[-1] = 0;
+			/* Pack into "1 <row15bits> <col16bits>" 32-bit sequence */
+			col |= (((-1 << 15) | row) << 16);
+			/* Return it in high-order word */
+			return ((int64_t) col << 32) | (uint32_t)KEYCODE_CURSOR_POS;
+		}
 	}
  got_all:
 
@@ -213,34 +238,6 @@ int64_t FAST_FUNC read_key(int fd, char *buffer)
 		return 27;
 	}
 
-	/* Try to decipher "ESC [ NNN ; NNN R" sequence */
-	if (ENABLE_FEATURE_EDITING_ASK_TERMINAL
-	 && n >= 5
-	 && buffer[0] == '['
-	 && isdigit(buffer[1])
-	 && buffer[n-1] == 'R'
-	) {
-		char *end;
-		unsigned long row, col;
-
-		row = strtoul(buffer + 1, &end, 10);
-		if (*end != ';' || !isdigit(end[1]))
-			goto not_R;
-		col = strtoul(end + 1, &end, 10);
-		if (*end != 'R')
-			goto not_R;
-		if (row < 1 || col < 1 || (row | col) > 0x7fff)
-			goto not_R;
-
-		buffer[-1] = 0;
-
-		/* Pack into "1 <row15bits> <col16bits>" 32-bit sequence */
-		col |= (((-1 << 15) | row) << 16);
-		/* Return it in high-order word */
-		return ((int64_t) col << 32) | (uint32_t)KEYCODE_CURSOR_POS;
-	}
- not_R:
-
 	/* We were doing "buffer[-1] = n; return c;" here, but this results
 	 * in unknown key sequences being interpreted as ESC + garbage.
 	 * This was not useful. Pretend there was no key pressed,
-- 
1.6.3.3



More information about the busybox-cvs mailing list