Created attachment 7581 [details] Input used to reproduce the bug. vi is using read_key() to obtain input. The read_key() __may__ process ESCape sequences and __may__ silently drop invalid ESCape sequences. This can result in vi not seeing a valid ESCape keystroke such as to exit 'insert' mode. The read_key() function opportunistically buffers and processes input. If an ESCape key is seen, read_key() will try to read the sequence completion in order to process the sequence. If the key following the ESCape is not a valid ESCape sequence completion, the ESCape and any buffered data that follows is silently dropped. This issue typically will not show up during manual editing through vi as there is very little chance that someone can type fast enough to get input buffered. However, if input is streamed, this problem is very common. To reproduce: Use a default busybox config to build. Using the attached file, execute the following. rm -f foo && busybox vi < test_vi_input The command will terminate with the error 'vi: can't read user input'. It should terminate with no error and successfully create the file 'foo'.
What fix do you propose?
I would propose breaking out the read/buffering functionality into read_key_raw(). Keep the ESC sequence processing in read_key() and use read_key_raw() for the buffered input. Then change vi to use read_key_raw() rather than read_key().
(In reply to Mike Berger from comment #2) > I would propose breaking out the read/buffering functionality > into read_key_raw(). Keep the ESC sequence processing in read_key() > and use read_key_raw() for the buffered input. I meant: how do you propose to handle the sequence of "<ESC><chars which are not a valid esc sequence>"?
I think silently dropping all buffered input when an invalid ESC sequence is seen is a bug. I think there are a few possible reactions. I'll defer to more knowledgeable folks as to which which is best. 1. Silently drop the ESC and continue processing the following character as if the ESC had not been received. 2. Silently drop the ESC and the following invalid key. Continue processing as if the ESC-<bad> had not been received. 3. Mimic the behavior of read_key() if no input had been buffered. Return the ESC, and continue processing the following character as if the ESC had not been received. Note that #3 would fix this particular bug without needing to do the read_key()/read_key_raw() split, but I still think vi should not be calling a function that may or may not process an ESC sequence.
(In reply to Mike Berger from comment #4) > I think silently dropping all buffered input when an invalid ESC sequence is seen is a bug. It's intended. I coded that myself. I got sick and tired of having accidental presses of unrecognized keys (e.g. Ctrl-Shift-key) resulting in unexpected actions. I prefer when in this case nothing happens. > 1. Silently drop the ESC and continue processing the following character as if the ESC had not been received. I don't think it's good. See above. > 3. Mimic the behavior of read_key() if no input had been buffered. Return the ESC, and continue processing the following character as if the ESC had not been received. I don't think it's good. See above. > 2. Silently drop the ESC and the following invalid key. Continue processing as if the ESC-<bad> had not been received. This is what happens now, right? The entire sequence is ignored.
(In reply to Denys Vlasenko from comment #5) No, it's more than the entire sequence being ignored - read_key() silently drops the bad ESC sequence PLUS any other data that happens to be buffered. I really don't want to make this bug about the behavior of read_key(). I think read_key() behavior is suitable for any application that does not perform it's own ESC processing. It is unsuitable for any application that must perform its own ESC handling - like vi. The point of this bug is that vi is not behaving correctly in some corner cases because it is using an input function that processes ESC keys in a non-deterministic way. In most cases, the ESC is merely passed through and everything works fine. But, I've provided a case where vi does not behave properly. The vi applet needs to see the ESC key, not a processed ESC sequence.
(In reply to Mike Berger from comment #6) > No, it's more than the entire sequence being ignored - read_key() silently drops the bad ESC sequence PLUS any other data that happens to be buffered. Yes, that's what it does. Because the dropped sequence is _unknown_ to the code, it can't know where it ends. So it drops all buffered data until there is nothing. > The point of this bug is that vi is not behaving correctly in some corner cases In what corner cases? "vi <test_vi_input" is not a way how "vi" is supposed to be used. If you want to edit a file with a script, use sed/awk/grep/...
Please just close off this bug. I'll rely on local changes so that my automation environment continues to behave as before these "improvements" were made.