From fe074611cd5b3319a3f639f68289df6a718e64eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jurica=20Bradari=C4=87?= Date: Sun, 6 Oct 2019 05:35:48 +0200 Subject: [PATCH] vim-patch:8.1.1371: cannot recover from a swap file #11081 Problem: Cannot recover from a swap file. Solution: Do not expand environment variables in the swap file name. Do not check the extension when we already know a file is a swap file. (Ken Takata, closes 4415, closes vim/vim#4369) https://github.com/vim/vim/commit/99499b1c05f85f83876b828eea3f6e14f0f407b4 --- src/nvim/buffer.c | 5 +- src/nvim/ex_cmds.c | 4 +- src/nvim/ex_cmds2.c | 4 +- src/nvim/ex_docmd.c | 11 +++-- src/nvim/main.c | 16 ++++--- src/nvim/memline.c | 14 +++--- src/nvim/path.c | 11 +++-- src/nvim/path.h | 11 +++-- src/nvim/search.c | 3 +- src/nvim/spell.c | 10 ++-- src/nvim/spellfile.c | 5 +- src/nvim/tag.c | 3 +- src/nvim/testdir/test_swap.vim | 84 ++++++++++++++++++++++++++++++++++ test/unit/path_spec.lua | 4 +- 14 files changed, 143 insertions(+), 42 deletions(-) diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c index e5b80693a4..b81ffd09e1 100644 --- a/src/nvim/buffer.c +++ b/src/nvim/buffer.c @@ -929,7 +929,7 @@ void handle_swap_exists(bufref_T *old_curbuf) // User selected Recover at ATTENTION prompt. msg_scroll = true; - ml_recover(); + ml_recover(false); MSG_PUTS("\n"); // don't overwrite the last message cmdline_row = msg_row; do_modelines(0); @@ -4629,7 +4629,8 @@ do_arg_all( if (i < alist->al_ga.ga_len && (AARGLIST(alist)[i].ae_fnum == buf->b_fnum || path_full_compare(alist_name(&AARGLIST(alist)[i]), - buf->b_ffname, true) & kEqualFiles)) { + buf->b_ffname, + true, true) & kEqualFiles)) { int weight = 1; if (old_curtab == curtab) { diff --git a/src/nvim/ex_cmds.c b/src/nvim/ex_cmds.c index 16487ce447..a3a08a5884 100644 --- a/src/nvim/ex_cmds.c +++ b/src/nvim/ex_cmds.c @@ -5023,7 +5023,7 @@ void fix_help_buffer(void) copy_option_part(&p, NameBuff, MAXPATHL, ","); char_u *const rt = (char_u *)vim_getenv("VIMRUNTIME"); if (rt != NULL - && path_full_compare(rt, NameBuff, false) != kEqualFiles) { + && path_full_compare(rt, NameBuff, false, true) != kEqualFiles) { int fcount; char_u **fnames; char_u *s; @@ -5233,7 +5233,7 @@ static void helptags_one(char_u *const dir, const char_u *const ext, ga_init(&ga, (int)sizeof(char_u *), 100); if (add_help_tags || path_full_compare((char_u *)"$VIMRUNTIME/doc", - dir, false) == kEqualFiles) { + dir, false, true) == kEqualFiles) { s = xmalloc(18 + STRLEN(tagfname)); sprintf((char *)s, "help-tags\t%s\t1\n", tagfname); GA_APPEND(char_u *, &ga, s); diff --git a/src/nvim/ex_cmds2.c b/src/nvim/ex_cmds2.c index 813d1a9b0b..87eae2dd4f 100644 --- a/src/nvim/ex_cmds2.c +++ b/src/nvim/ex_cmds2.c @@ -1743,7 +1743,7 @@ static bool editing_arg_idx(win_T *win) && (win->w_buffer->b_ffname == NULL || !(path_full_compare( alist_name(&WARGLIST(win)[win->w_arg_idx]), - win->w_buffer->b_ffname, true) & kEqualFiles)))); + win->w_buffer->b_ffname, true, true) & kEqualFiles)))); } /// Check if window "win" is editing the w_arg_idx file in its argument list. @@ -1761,7 +1761,7 @@ void check_arg_idx(win_T *win) && (win->w_buffer->b_fnum == GARGLIST[GARGCOUNT - 1].ae_fnum || (win->w_buffer->b_ffname != NULL && (path_full_compare(alist_name(&GARGLIST[GARGCOUNT - 1]), - win->w_buffer->b_ffname, true) + win->w_buffer->b_ffname, true, true) & kEqualFiles)))) { arg_had_last = true; } diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index 34b4c10d3e..a6042b0e8c 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -6708,17 +6708,18 @@ static void ex_preserve(exarg_T *eap) /// ":recover". static void ex_recover(exarg_T *eap) { - /* Set recoverymode right away to avoid the ATTENTION prompt. */ - recoverymode = TRUE; + // Set recoverymode right away to avoid the ATTENTION prompt. + recoverymode = true; if (!check_changed(curbuf, (p_awa ? CCGD_AW : 0) | CCGD_MULTWIN | (eap->forceit ? CCGD_FORCEIT : 0) | CCGD_EXCMD) && (*eap->arg == NUL - || setfname(curbuf, eap->arg, NULL, TRUE) == OK)) - ml_recover(); - recoverymode = FALSE; + || setfname(curbuf, eap->arg, NULL, true) == OK)) { + ml_recover(true); + } + recoverymode = false; } /* diff --git a/src/nvim/main.c b/src/nvim/main.c index be1f08bb46..ba15dcedad 100644 --- a/src/nvim/main.c +++ b/src/nvim/main.c @@ -1460,12 +1460,13 @@ static void create_windows(mparm_T *parmp) } else parmp->window_count = 1; - if (recoverymode) { /* do recover */ - msg_scroll = TRUE; /* scroll message up */ - ml_recover(); - if (curbuf->b_ml.ml_mfp == NULL) /* failed */ + if (recoverymode) { // do recover + msg_scroll = true; // scroll message up + ml_recover(true); + if (curbuf->b_ml.ml_mfp == NULL) { // failed getout(1); - do_modelines(0); /* do modelines */ + } + do_modelines(0); // do modelines } else { // Open a buffer for windows that don't have one yet. // Commands in the vimrc might have loaded a file or split the window. @@ -1778,7 +1779,8 @@ static bool do_user_initialization(void) if (do_source(user_vimrc, true, DOSO_VIMRC) != FAIL) { do_exrc = p_exrc; if (do_exrc) { - do_exrc = (path_full_compare((char_u *)VIMRC_FILE, user_vimrc, false) + do_exrc = (path_full_compare((char_u *)VIMRC_FILE, user_vimrc, + false, true) != kEqualFiles); } xfree(user_vimrc); @@ -1805,7 +1807,7 @@ static bool do_user_initialization(void) do_exrc = p_exrc; if (do_exrc) { do_exrc = (path_full_compare((char_u *)VIMRC_FILE, (char_u *)vimrc, - false) != kEqualFiles); + false, true) != kEqualFiles); } xfree(vimrc); xfree(config_dirs); diff --git a/src/nvim/memline.c b/src/nvim/memline.c index 15dd2767a2..f1d6ee064c 100644 --- a/src/nvim/memline.c +++ b/src/nvim/memline.c @@ -738,10 +738,10 @@ static void add_b0_fenc(ZERO_BL *b0p, buf_T *buf) } -/* - * Try to recover curbuf from the .swp file. - */ -void ml_recover(void) +/// Try to recover curbuf from the .swp file. +/// @param checkext If true, check the extension and detect whether it is a +/// swap file. +void ml_recover(bool checkext) { buf_T *buf = NULL; memfile_T *mfp = NULL; @@ -785,7 +785,7 @@ void ml_recover(void) if (fname == NULL) /* When there is no file name */ fname = (char_u *)""; len = (int)STRLEN(fname); - if (len >= 4 + if (checkext && len >= 4 && STRNICMP(fname + len - 4, ".s", 2) == 0 && vim_strchr((char_u *)"abcdefghijklmnopqrstuvw", TOLOWER_ASC(fname[len - 2])) != NULL @@ -1375,7 +1375,9 @@ recover_names ( if (curbuf->b_ml.ml_mfp != NULL && (p = curbuf->b_ml.ml_mfp->mf_fname) != NULL) { for (int i = 0; i < num_files; i++) { - if (path_full_compare(p, files[i], true) & kEqualFiles) { + // Do not expand wildcards, on Windows would try to expand + // "%tmp%" in "%tmp%file" + if (path_full_compare(p, files[i], true, false) & kEqualFiles) { // Remove the name from files[i]. Move further entries // down. When the array becomes empty free it here, since // FreeWild() won't be called below. diff --git a/src/nvim/path.c b/src/nvim/path.c index 1c787e3a1d..62d5d69d1a 100644 --- a/src/nvim/path.c +++ b/src/nvim/path.c @@ -51,9 +51,10 @@ /// expanded. /// @param s2 Second file name. /// @param checkname When both files don't exist, only compare their names. +/// @param expandenv Whether to expand environment variables in file names. /// @return Enum of type FileComparison. @see FileComparison. FileComparison path_full_compare(char_u *const s1, char_u *const s2, - const bool checkname) + const bool checkname, const bool expandenv) { assert(s1 && s2); char_u exp1[MAXPATHL]; @@ -61,7 +62,11 @@ FileComparison path_full_compare(char_u *const s1, char_u *const s2, char_u full2[MAXPATHL]; FileID file_id_1, file_id_2; - expand_env(s1, exp1, MAXPATHL); + if (expandenv) { + expand_env(s1, exp1, MAXPATHL); + } else { + xstrlcpy((char *)exp1, (const char *)s1, MAXPATHL - 1); + } bool id_ok_1 = os_fileid((char *)exp1, &file_id_1); bool id_ok_2 = os_fileid((char *)s2, &file_id_2); if (!id_ok_1 && !id_ok_2) { @@ -1203,7 +1208,7 @@ int gen_expand_wildcards(int num_pat, char_u **pat, int *num_file, } } else { // First expand environment variables, "~/" and "~user/". - if (has_env_var(p) || *p == '~') { + if ((has_env_var(p) && !(flags & EW_NOTENV)) || *p == '~') { p = expand_env_save_opt(p, true); if (p == NULL) p = pat[i]; diff --git a/src/nvim/path.h b/src/nvim/path.h index 4e466d1b71..15abd19646 100644 --- a/src/nvim/path.h +++ b/src/nvim/path.h @@ -20,11 +20,12 @@ #define EW_KEEPDOLLAR 0x800 /* do not escape $, $var is expanded */ /* Note: mostly EW_NOTFOUND and EW_SILENT are mutually exclusive: EW_NOTFOUND * is used when executing commands and EW_SILENT for interactive expanding. */ -#define EW_ALLLINKS 0x1000 // also links not pointing to existing file -#define EW_SHELLCMD 0x2000 // called from expand_shellcmd(), don't check - // if executable is in $PATH -#define EW_DODOT 0x4000 // also files starting with a dot -#define EW_EMPTYOK 0x8000 // no matches is not an error +#define EW_ALLLINKS 0x1000 // also links not pointing to existing file +#define EW_SHELLCMD 0x2000 // called from expand_shellcmd(), don't check + // if executable is in $PATH +#define EW_DODOT 0x4000 // also files starting with a dot +#define EW_EMPTYOK 0x8000 // no matches is not an error +#define EW_NOTENV 0x10000 // do not expand environment variables /// Return value for the comparison of two files. Also @see path_full_compare. typedef enum file_comparison { diff --git a/src/nvim/search.c b/src/nvim/search.c index 7d1c19d68c..85c0d7eb48 100644 --- a/src/nvim/search.c +++ b/src/nvim/search.c @@ -4458,7 +4458,8 @@ find_pattern_in_path( if (i == max_path_depth) { break; } - if (path_full_compare(new_fname, files[i].name, true) & kEqualFiles) { + if (path_full_compare(new_fname, files[i].name, + true, true) & kEqualFiles) { if (type != CHECK_PATH && action == ACTION_SHOW_ALL && files[i].matched) { msg_putchar('\n'); // cursor below last one */ diff --git a/src/nvim/spell.c b/src/nvim/spell.c index 724a0332bc..ab40355a8a 100644 --- a/src/nvim/spell.c +++ b/src/nvim/spell.c @@ -2031,7 +2031,8 @@ char_u *did_set_spelllang(win_T *wp) // Check if we loaded this language before. for (slang = first_lang; slang != NULL; slang = slang->sl_next) { - if (path_full_compare(lang, slang->sl_fname, false) == kEqualFiles) { + if (path_full_compare(lang, slang->sl_fname, false, true) + == kEqualFiles) { break; } } @@ -2076,7 +2077,7 @@ char_u *did_set_spelllang(win_T *wp) // Loop over the languages, there can be several files for "lang". for (slang = first_lang; slang != NULL; slang = slang->sl_next) { if (filename - ? path_full_compare(lang, slang->sl_fname, false) == kEqualFiles + ? path_full_compare(lang, slang->sl_fname, false, true) == kEqualFiles : STRICMP(lang, slang->sl_name) == 0) { region_mask = REGION_ALL; if (!filename && region != NULL) { @@ -2129,7 +2130,7 @@ char_u *did_set_spelllang(win_T *wp) for (c = 0; c < ga.ga_len; ++c) { p = LANGP_ENTRY(ga, c)->lp_slang->sl_fname; if (p != NULL - && path_full_compare(spf_name, p, false) == kEqualFiles) { + && path_full_compare(spf_name, p, false, true) == kEqualFiles) { break; } } @@ -2139,7 +2140,8 @@ char_u *did_set_spelllang(win_T *wp) // Check if it was loaded already. for (slang = first_lang; slang != NULL; slang = slang->sl_next) { - if (path_full_compare(spf_name, slang->sl_fname, false) == kEqualFiles) { + if (path_full_compare(spf_name, slang->sl_fname, false, true) + == kEqualFiles) { break; } } diff --git a/src/nvim/spellfile.c b/src/nvim/spellfile.c index 01daafa09e..eeec5be120 100644 --- a/src/nvim/spellfile.c +++ b/src/nvim/spellfile.c @@ -1787,7 +1787,7 @@ spell_reload_one ( bool didit = false; for (slang = first_lang; slang != NULL; slang = slang->sl_next) { - if (path_full_compare(fname, slang->sl_fname, false) == kEqualFiles) { + if (path_full_compare(fname, slang->sl_fname, false, true) == kEqualFiles) { slang_clear(slang); if (spell_load_file(fname, NULL, slang, false) == NULL) // reloading failed, clear the language @@ -4719,7 +4719,8 @@ static void spell_make_sugfile(spellinfo_T *spin, char_u *wfname) // of the code for the soundfolding stuff. // It might have been done already by spell_reload_one(). for (slang = first_lang; slang != NULL; slang = slang->sl_next) { - if (path_full_compare(wfname, slang->sl_fname, false) == kEqualFiles) { + if (path_full_compare(wfname, slang->sl_fname, false, true) + == kEqualFiles) { break; } } diff --git a/src/nvim/tag.c b/src/nvim/tag.c index 91f3da1793..6fe3efbaae 100644 --- a/src/nvim/tag.c +++ b/src/nvim/tag.c @@ -2666,7 +2666,8 @@ static int test_for_current(char_u *fname, char_u *fname_end, char_u *tag_fname, *fname_end = NUL; } fullname = expand_tag_fname(fname, tag_fname, true); - retval = (path_full_compare(fullname, buf_ffname, true) & kEqualFiles); + retval = (path_full_compare(fullname, buf_ffname, true, true) + & kEqualFiles); xfree(fullname); *fname_end = c; } diff --git a/src/nvim/testdir/test_swap.vim b/src/nvim/testdir/test_swap.vim index 11eb324488..e072e9ed7f 100644 --- a/src/nvim/testdir/test_swap.vim +++ b/src/nvim/testdir/test_swap.vim @@ -221,3 +221,87 @@ func Test_swapfile_delete() augroup END augroup! test_swapfile_delete endfunc + +func Test_swap_recover() + autocmd! SwapExists + augroup test_swap_recover + autocmd! + autocmd SwapExists * let v:swapchoice = 'r' + augroup END + + + call mkdir('Xswap') + let $Xswap = 'foo' " Check for issue #4369. + set dir=Xswap// + " Create a valid swapfile by editing a file. + split Xswap/text + call setline(1, ['one', 'two', 'three']) + write " file is written, not modified + " read the swapfile as a Blob + let swapfile_name = swapname('%') + let swapfile_bytes = readfile(swapfile_name, 'B') + + " Close the file and recreate the swap file. + quit + call writefile(swapfile_bytes, swapfile_name) + " Edit the file again. This triggers recovery. + try + split Xswap/text + catch + " E308 should be caught, not E305. + call assert_exception('E308:') " Original file may have been changed + endtry + " The file should be recovered. + call assert_equal(['one', 'two', 'three'], getline(1, 3)) + quit! + + call delete('Xswap/text') + call delete(swapfile_name) + call delete('Xswap', 'd') + unlet $Xswap + set dir& + augroup test_swap_recover + autocmd! + augroup END + augroup! test_swap_recover +endfunc + +func Test_swap_recover_ext() + autocmd! SwapExists + augroup test_swap_recover_ext + autocmd! + autocmd SwapExists * let v:swapchoice = 'r' + augroup END + + + " Create a valid swapfile by editing a file with a special extension. + split Xtest.scr + call setline(1, ['one', 'two', 'three']) + write " file is written, not modified + write " write again to make sure the swapfile is created + " read the swapfile as a Blob + let swapfile_name = swapname('%') + let swapfile_bytes = readfile(swapfile_name, 'B') + + " Close and delete the file and recreate the swap file. + quit + call delete('Xtest.scr') + call writefile(swapfile_bytes, swapfile_name) + " Edit the file again. This triggers recovery. + try + split Xtest.scr + catch + " E308 should be caught, not E306. + call assert_exception('E308:') " Original file may have been changed + endtry + " The file should be recovered. + call assert_equal(['one', 'two', 'three'], getline(1, 3)) + quit! + + call delete('Xtest.scr') + call delete(swapfile_name) + augroup test_swap_recover_ext + autocmd! + augroup END + augroup! test_swap_recover_ext +endfunc diff --git a/test/unit/path_spec.lua b/test/unit/path_spec.lua index da52af1bf9..356c4997fa 100644 --- a/test/unit/path_spec.lua +++ b/test/unit/path_spec.lua @@ -66,10 +66,10 @@ describe('path.c', function() end) describe('path_full_compare', function() - local function path_full_compare(s1, s2, cn) + local function path_full_compare(s1, s2, cn, ee) s1 = to_cstr(s1) s2 = to_cstr(s2) - return cimp.path_full_compare(s1, s2, cn or 0) + return cimp.path_full_compare(s1, s2, cn or 0, ee or 1) end local f1 = 'f1.o'