From df2b9bf67f8553e5fec5914d55f3330e02967b77 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Fri, 26 Feb 2021 01:30:08 +0000 Subject: [PATCH] vi: fix buffer corruption after filename completion (re: 4cecde1d) This bug was backported along with a fix from 93v-. An inconsistent state occurred if you caused a file name completion menu to appear with two TABs (which also puts you in command mode) but then re-enter insert mode (e.g. with 'a') instead of entering a number. $ set -o vi $ cd / $ bin/p [press TAB twice] 1) pax 2) ps 3) pwd [now type 'a', 'wd', return] $ bin/pwd > [PS2 prompt wrongly appears; press return] / $ Here's another reproducer, suggesting the problem is a write past the end of the screen buffer: $ set -o vi $ cd / $ bin/p [press TAB twice] 1) pax 2) ps 3) pwd [press '0', then '$'] $ bin/p [cursor is one too far to the right, past the 'p'!] [Further operations show random evidence of memory corruption] Harald van Dijk found the cause (thanks!): > In vi.c's textmod there is > > case '=': /** list file name expansions **/ > ... > ++last_virt; > ... > if(ed_expand(vp->ed,(char*)virtual, &cur_virt, &last_virt, ch, vp->repeat_set?vp->repeat:-1)<0) > { > ... > last_virt = i; > ... > } > else if((c=='=' || (c=='\\'&&virtual[last_virt]=='/')) && !vp->repeat_set) > { > ... > } > else > { > ... > --last_virt; > ... > } > break; > > That middle block does not restore last_virt, and everything goes > wrong after that. That function used to restore last_virt until > commit 4cecde1 (#41). The commit message says it was taken from > ksh93v- and indeed this bug is also present in that version too. > If I restore the last_virt = i; that was there originally, like > below, then this bug seems to be fixed. I do not know why it was > taken out, taking it out does not seem to be necessary to fix the > original bug. src/cmd/ksh93/edit/vi.c: textmod(): - Restore the missing restore of last_virt. src/cmd/ksh93/tests/pty.sh: - Add test that checks basic completion menu functionality works and runs modified versions of the two reproducers above. Resolves: https://github.com/ksh93/ksh/issues/195 --- src/cmd/ksh93/edit/vi.c | 1 + src/cmd/ksh93/tests/pty.sh | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/cmd/ksh93/edit/vi.c b/src/cmd/ksh93/edit/vi.c index 97bb5767c..d3c6fa4f2 100644 --- a/src/cmd/ksh93/edit/vi.c +++ b/src/cmd/ksh93/edit/vi.c @@ -2485,6 +2485,7 @@ addin: } else if((c=='=' || (c=='\\'&&virtual[last_virt]=='/')) && !vp->repeat_set) { + last_virt = i; vi_redraw((void*)vp); return(GOOD); } diff --git a/src/cmd/ksh93/tests/pty.sh b/src/cmd/ksh93/tests/pty.sh index 31720769c..733a269d5 100755 --- a/src/cmd/ksh93/tests/pty.sh +++ b/src/cmd/ksh93/tests/pty.sh @@ -677,5 +677,40 @@ w true \\\cC r true \^C ! +# err_exit # +((SHOPT_VSH)) && touch vi_completion_A_file vi_completion_B_file && tst $LINENO <<"!" +L vi filename completion menu + +d 10 +c ls vi_co\t\t +r ls vi_completion\r\n$ +r ^1) vi_completion_A_file\r\n$ +r ^2) vi_completion_B_file\r\n$ +w 2\t +r ^:test-1: ls vi_completion_B_file \r\n$ +r ^vi_completion_B_file\r\n$ + +# 93v- bug: tab completion writes past input buffer +# https://github.com/ksh93/ksh/issues/195 + +# ...reproducer 1 +c ls vi_compl\t\t +r ls vi_completion\r\n$ +r ^1) vi_completion_A_file\r\n$ +r ^2) vi_completion_B_file\r\n$ +w aB_file +r ^:test-2: ls vi_completion_B_file\r\n$ +r ^vi_completion_B_file\r\n$ + +# ...reproducer 2 +c \rls vi_comple\t\t +u ls vi_completion\r\n$ +r ^1) vi_completion_A_file\r\n$ +r ^2) vi_completion_B_file\r\n$ +w 0$aA_file +r ^:test-3: ls vi_completion_A_file\r\n$ +r ^vi_completion_A_file\r\n$ +! + # ====== exit $((Errors<125?Errors:125))