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

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'