patch 9.1.0048: Abort opening cmdwin if autocmds screw things up

Problem:  Autocmds triggered from opening the cmdwin (in win_split and
          do_ecmd) can cause issues such as E199, as the current checks
          are insufficient.

Solution: Commands executed from the cmdwin apply to the old curwin/buf,
          so they should be kept in a "suspended" state; abort if
          they've changed. Also abort if cmdwin/buf was tampered with,
          and check that curwin is correct. Try to clean up the cmdwin
          buffer (only if hidden and non-current to simplify things; the
          same approach is used when closing cmdwin normally), and add a
          beep. (Sean Dewar)

It'd be nice to also check that curwin was *really* created by win_split, as
autocommands can change curwin before it returns (so it can't be assumed to be
that of the split); for now, this means that the cmdwin may not be the botwin in
that case, which is probably OK.

closes: #12819

Signed-off-by: Sean Dewar <seandewar@users.noreply.github.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

src/errors.h

@@ -471,8 +471,8 @@ EXTERN char e_no_digraphs_version[]
 EXTERN char e_cannot_set_language_to_str[]
 	INIT(= N_("E197: Cannot set language to \"%s\""));
 // E198 unused
-EXTERN char e_active_window_or_buffer_deleted[]
+EXTERN char e_active_window_or_buffer_changed_or_deleted[]
-	INIT(= N_("E199: Active window or buffer deleted"));
+	INIT(= N_("E199: Active window or buffer changed or deleted"));
 EXTERN char e_readpre_autocommands_made_file_unreadable[]
 	INIT(= N_("E200: *ReadPre autocommands made the file unreadable"));
 EXTERN char e_readpre_autocommands_must_not_change_current_buffer[]

src/ex_getln.c

@@ -4458,6 +4458,8 @@ open_cmdwin(void)
 #ifdef FEAT_FOLDING
     int			save_KeyTyped;
 #endif
+    int			newbuf_status;
+    int			cmdwin_valid;
 
     // Can't do this when text or buffer is locked.
     // Can't do this recursively.  Can't do it when typing a password.
@@ -4491,6 +4493,16 @@ open_cmdwin(void)
 	ga_clear(&winsizes);
 	return K_IGNORE;
     }
+    // win_split() autocommands may have messed with the old window or buffer.
+    // Treat it as abandoning this command-line.
+    if (!win_valid(old_curwin) || curwin == old_curwin
+	    || !bufref_valid(&old_curbuf)
+	    || old_curwin->w_buffer != old_curbuf.br_buf)
+    {
+	beep_flush();
+	ga_clear(&winsizes);
+	return Ctrl_C;
+    }
     // Don't let quitting the More prompt make this fail.
     got_int = FALSE;
 
@@ -4498,14 +4510,29 @@ open_cmdwin(void)
     cmdwin_type = get_cmdline_type();
     cmdwin_win = curwin;
 
-    // Create the command-line buffer empty.
+    // Create empty command-line buffer.  Be especially cautious of BufLeave
-    if (do_ecmd(0, NULL, NULL, NULL, ECMD_ONE, ECMD_HIDE, NULL) == FAIL)
+    // autocommands from do_ecmd(), as cmdwin restrictions do not apply to them!
+    newbuf_status = do_ecmd(0, NULL, NULL, NULL, ECMD_ONE, ECMD_HIDE,
+					NULL);
+    cmdwin_valid = win_valid(cmdwin_win);
+    if (newbuf_status == FAIL || !cmdwin_valid || curwin != cmdwin_win ||
+	    !win_valid(old_curwin) || !bufref_valid(&old_curbuf) ||
+	    old_curwin->w_buffer != old_curbuf.br_buf)
     {
-	// Some autocommand messed it up?
+	if (newbuf_status == OK)
-	win_close(curwin, TRUE);
+	    set_bufref(&bufref, curbuf);
-	ga_clear(&winsizes);
+	if (cmdwin_valid && !last_window())
+	    win_close(cmdwin_win, TRUE);
+
+	// win_close() autocommands may have already deleted the buffer.
+	if (newbuf_status == OK && bufref_valid(&bufref) &&
+		bufref.br_buf != curbuf)
+	    close_buffer(NULL, bufref.br_buf, DOBUF_WIPE, FALSE, FALSE);
+
 	cmdwin_type = 0;
 	cmdwin_win = NULL;
+	beep_flush();
+	ga_clear(&winsizes);
 	return Ctrl_C;
     }
     cmdwin_buf = curbuf;
@@ -4622,12 +4649,13 @@ open_cmdwin(void)
     cmdwin_win = NULL;
     exmode_active = save_exmode;
 
-    // Safety check: The old window or buffer was deleted: It's a bug when
+    // Safety check: The old window or buffer was changed or deleted: It's a bug
-    // this happens!
+    // when this happens!
-    if (!win_valid(old_curwin) || !bufref_valid(&old_curbuf))
+    if (!win_valid(old_curwin) || !bufref_valid(&old_curbuf)
+	    || old_curwin->w_buffer != old_curbuf.br_buf)
     {
 	cmdwin_result = Ctrl_C;
-	emsg(_(e_active_window_or_buffer_deleted));
+	emsg(_(e_active_window_or_buffer_changed_or_deleted));
     }
     else
     {

src/proto/window.pro

@@ -18,6 +18,7 @@ void leaving_window(win_T *win);
 void entering_window(win_T *win);
 void curwin_init(void);
 void close_windows(buf_T *buf, int keep_curwin);
+int last_window(void);
 int one_window(void);
 int win_close(win_T *win, int free_buf);
 void snapshot_windows_scroll_size(void);

src/testdir/test_cmdwin.vim

@@ -188,7 +188,7 @@ func Test_cmdwin_tabpage()
   tabclose!
 endfunc
 
-func Test_cmdwin_interrupted()
+func Test_cmdwin_interrupted_more_prompt()
   CheckScreendump
 
   " aborting the :smile output caused the cmdline window to use the current
@@ -498,9 +498,75 @@ func Test_cmdwin_temp_curwin()
 
   call feedkeys("q::call CheckCmdWin()\<CR>:call win_execute(win_getid(winnr('#')), 'call CheckOtherWin()')\<CR>:q<CR>", 'ntx')
 
+  %bwipe!
   delfunc CheckWraps
   delfunc CheckCmdWin
   delfunc CheckOtherWin
 endfunc
 
+func Test_cmdwin_interrupted()
+  func CheckInterrupted()
+    call feedkeys("q::call assert_equal('', getcmdwintype())\<CR>:call assert_equal('', getcmdtype())\<CR>:q<CR>", 'ntx')
+  endfunc
+
+  augroup CmdWin
+
+  " While opening the cmdwin's split:
+  " Close the cmdwin's window.
+  au WinEnter * ++once quit
+  call CheckInterrupted()
+
+  " Close the old window.
+  au WinEnter * ++once execute winnr('#') 'quit'
+  call CheckInterrupted()
+
+  " Switch back to the old window.
+  au WinEnter * ++once wincmd p
+  call CheckInterrupted()
+
+  " Change the old window's buffer.
+  au WinEnter * ++once call win_execute(win_getid(winnr('#')), 'enew')
+  call CheckInterrupted()
+
+  " Using BufLeave autocmds as cmdwin restrictions do not apply to them when
+  " fired from opening the cmdwin...
+  " After opening the cmdwin's split, while creating the cmdwin's buffer:
+  " Delete the cmdwin's buffer.
+  au BufLeave * ++once bwipe
+  call CheckInterrupted()
+
+  " Close the cmdwin's window.
+  au BufLeave * ++once quit
+  call CheckInterrupted()
+
+  " Close the old window.
+  au BufLeave * ++once execute winnr('#') 'quit'
+  call CheckInterrupted()
+
+  " Switch to a different window.
+  au BufLeave * ++once split
+  call CheckInterrupted()
+
+  " Change the old window's buffer.
+  au BufLeave * ++once call win_execute(win_getid(winnr('#')), 'enew')
+  call CheckInterrupted()
+
+  " However, changing the current buffer is OK and does not interrupt.
+  au BufLeave * ++once edit other
+  call feedkeys("q::let t=getcmdwintype()\<CR>:let b=bufnr()\<CR>:clo<CR>", 'ntx')
+  call assert_equal(':', t)
+  call assert_equal(1, bufloaded('other'))
+  call assert_notequal(b, bufnr('other'))
+
+  augroup END
+
+  " No autocmds should remain, but clear the augroup to be sure.
+  augroup CmdWin
+    au!
+  augroup END
+
+  %bwipe!
+  delfunc CheckInterrupted
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

src/version.c

@@ -704,6 +704,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    48,
 /**/
     47,
 /**/

src/window.c

@@ -2485,7 +2485,7 @@ close_windows(
  * "aucmd_win[]").
  * Returns FALSE if there is a window, possibly in another tab page.
  */
-    static int
+    int
 last_window(void)
 {
     return (one_window() && first_tabpage->tp_next == NULL);