vim-patch:9.2.0418: wildcards in expanded env vars reinterpreted by glob (#39517)

Problem:  With $d='[dir]', `:e $d/file.txt` opens the wrong file,
          `:e $d/<Tab>` fails to complete, and `glob('$d/*')` returns
          nothing. Wildcard characters inside expanded environment
          variables get picked up by globbing again.
Solution: Turn the 4th parameter of expand_env_esc() from a bool into a
          string of characters to escape in each expanded value. Callers
          that pass the result to wildcard expansion should include
          PATH_ESC_WILDCARDS in addition to " \t" (glepnir).

closes: vim/vim#20053

https://github.com/vim/vim/commit/20e98ff1cc3ffc13d42579d2bf9b9567f98595a2

src/nvim/cmdexpand.c

@@ -1071,7 +1071,7 @@ static void showmatches_oneline(expand_T *xp, char **matches, int numMatches, in
         // Expansion was done before and special characters
         // were escaped, need to halve backslashes.  Also
         // $HOME has been replaced with ~/.
-        char *exp_path = expand_env_save_opt(matches[j], true);
+        char *exp_path = expand_env_save_opt(matches[j], true, NULL);
         char *path = exp_path != NULL ? exp_path : matches[j];
         char *halved_slash = backslash_halve_save(path);
         isdir = os_isdir(halved_slash);

src/nvim/ex_docmd.c

@@ -4108,7 +4108,8 @@ int expand_filename(exarg_T *eap, char **cmdlinep, const char **errormsgp)
       // if there are still wildcards present.
       if (vim_strchr(eap->arg, '$') != NULL
           || vim_strchr(eap->arg, '~') != NULL) {
-        expand_env_esc(eap->arg, NameBuff, MAXPATHL, true, true, NULL);
+        expand_env_esc(eap->arg, NameBuff, MAXPATHL, (char *)(" \t" PATH_ESC_WILDCARDS), true,
+                       NULL);
         has_wildcards = path_has_wildcard(NameBuff);
         p = NameBuff;
       } else {

src/nvim/file_search.c

@@ -1437,7 +1437,7 @@ char *find_file_in_path_option(char *ptr, size_t len, int options, int first, ch
     // copy file name into NameBuff, expanding environment variables
     char save_char = ptr[len];
     ptr[len] = NUL;
-    file_to_findlen = expand_env_esc(ptr, NameBuff, MAXPATHL, false, true, NULL);
+    file_to_findlen = expand_env_esc(ptr, NameBuff, MAXPATHL, NULL, true, NULL);
     ptr[len] = save_char;
 
     xfree(*file_to_find);

src/nvim/option.c

@@ -1785,7 +1785,7 @@ static char *option_expand(OptIndex opt_idx, const char *val)
   // For 'spellsuggest' expand after "file:".
   char **var = (char **)options[opt_idx].var;
   bool esc = var == &p_tags || var == &p_path;
-  expand_env_esc(val, NameBuff, MAXPATHL, esc, false,
+  expand_env_esc(val, NameBuff, MAXPATHL, esc ? (char *)" \t" : NULL, false,
                  (char **)options[opt_idx].var == &p_sps ? "file:" : NULL);
   if (strcmp(NameBuff, val) == 0) {   // they are the same
     return NULL;

src/nvim/os/env.c

@@ -510,18 +510,19 @@ void free_homedir(void)
 /// @see {expand_env}
 char *expand_env_save(char *src)
 {
-  return expand_env_save_opt(src, false);
+  return expand_env_save_opt(src, false, NULL);
 }
 
 /// Similar to expand_env_save() but when "one" is `true` handle the string as
 /// one file name, i.e. only expand "~" at the start.
 /// @param src String containing environment variables to expand
 /// @param one Should treat as only one file name
+/// @param esc_chars chars to escape in expanded vars
 /// @see {expand_env}
-char *expand_env_save_opt(char *src, bool one)
+char *expand_env_save_opt(char *src, bool one, char *esc_chars)
 {
   char *p = xmalloc(MAXPATHL);
-  expand_env_esc(src, p, MAXPATHL, false, one, NULL);
+  expand_env_esc(src, p, MAXPATHL, esc_chars, one, NULL);
   return p;
 }
 
@@ -535,7 +536,7 @@ char *expand_env_save_opt(char *src, bool one)
 /// @param dstlen     Maximum length of the result
 size_t expand_env(char *src, char *dst, int dstlen)
 {
-  return expand_env_esc(src, dst, dstlen, false, false, NULL);
+  return expand_env_esc(src, dst, dstlen, NULL, false, NULL);
 }
 
 /// Expand environment variable with path name and escaping.
@@ -544,11 +545,11 @@ size_t expand_env(char *src, char *dst, int dstlen)
 /// @param srcp       Input string e.g. "$HOME/vim.hlp"
 /// @param dst[out]   Where to put the result
 /// @param dstlen     Maximum length of the result
-/// @param esc        Escape spaces in expanded variables
+/// @param esc_chars  chars to escape in expanded vars
 /// @param one        `srcp` is a single filename
 /// @param prefix     Start again after this (can be NULL)
-size_t expand_env_esc(const char *restrict srcp, char *restrict dst, int dstlen, bool esc, bool one,
-                      char *prefix)
+size_t expand_env_esc(const char *restrict srcp, char *restrict dst, int dstlen, char *esc_chars,
+                      bool one, char *prefix)
   FUNC_ATTR_NONNULL_ARG(1, 2)
 {
   char *tail;
@@ -661,10 +662,11 @@ size_t expand_env_esc(const char *restrict srcp, char *restrict dst, int dstlen,
 #endif  // UNIX
       }
 
-      // If "var" contains white space, escape it with a backslash.
-      // Required for ":e ~/tt" when $HOME includes a space.
-      if (esc && var != NULL && strpbrk(var, " \t") != NULL) {
-        char *p = vim_strsave_escaped(var, " \t");
+      // If "var" contains any character from "esc_chars", escape it
+      // with a backslash.  The historical use is escaping spaces so
+      // that ":e ~/tt" works when $HOME contains a space.
+      if (esc_chars != NULL && var != NULL && strpbrk(var, esc_chars) != NULL) {
+        char *p = vim_strsave_escaped(var, esc_chars);
 
         if (mustfree) {
           xfree(var);

src/nvim/path.c

@@ -1328,7 +1328,7 @@ int gen_expand_wildcards(int num_pat, char **pat, int *num_file, char ***file, i
     } else {
       // First expand environment variables, "~/" and "~user/".
       if ((has_env_var(p) && !(flags & EW_NOTENV)) || *p == '~') {
-        p = expand_env_save_opt(p, true);
+        p = expand_env_save_opt(p, true, (char *)PATH_ESC_WILDCARDS);
         if (p == NULL) {
           p = pat[i];
         } else {

src/nvim/path.h

@@ -51,4 +51,10 @@ typedef enum file_comparison {
 # define TO_BACKSLASH(...)
 #endif
 
+#ifdef MSWIN
+# define PATH_ESC_WILDCARDS "*?["
+#else
+# define PATH_ESC_WILDCARDS "*?[{"
+#endif
+
 #include "path.h.generated.h"

src/nvim/profile.c

@@ -295,7 +295,7 @@ void ex_profile(exarg_T *eap)
 
   if (len == 5 && strncmp(eap->arg, "start", 5) == 0 && *e != NUL) {
     xfree(profile_fname);
-    profile_fname = expand_env_save_opt(e, true);
+    profile_fname = expand_env_save_opt(e, true, NULL);
     do_profiling = PROF_YES;
     profile_set_wait(profile_zero());
     set_vim_var_nr(VV_PROFILING, 1);

test/old/testdir/test_cmdline.vim

@@ -5427,4 +5427,31 @@ func Test_wildmode_noinsert()
   delfunc T
 endfunc
 
+func Test_cmdline_compl_env_var_wildcard()
+  CheckUnix
+
+  let d = tempname()
+  call mkdir(d .. '/[x]', 'pR')
+  call writefile(['hello'], d .. '/[x]/file.txt')
+  let $XWILD = d .. '/[x]'
+
+  call feedkeys(":e $XWILD/fi\<Tab>\<C-B>\"\<CR>", 'xt')
+  call assert_match('\[x\]/file\.txt$', @:)
+  call assert_equal([d .. '/[x]/file.txt'], glob('$XWILD/*', 0, 1))
+
+  edit $XWILD/file.txt
+  call assert_equal('hello', getline(1))
+  bwipe!
+
+  if has('profile')
+    let prof = d .. '/[x]/prof.out'
+    profile start $XWILD/prof.out
+    profile stop
+    call assert_true(filereadable(prof))
+    call delete(prof)
+  endif
+
+  unlet $XWILD
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

test/unit/os/env_spec.lua

@@ -335,8 +335,8 @@ describe('env.c', function()
       local output_buff1 = cstr(255, '')
       local output_buff2 = cstr(255, '')
       local output_expected = 'NVIM_UNIT_TEST_EXPAND_ENV_ESCV/test'
-      cimp.expand_env_esc(input1, output_buff1, 255, false, true, NULL)
-      cimp.expand_env_esc(input2, output_buff2, 255, false, true, NULL)
+      cimp.expand_env_esc(input1, output_buff1, 255, NULL, true, NULL)
+      cimp.expand_env_esc(input2, output_buff2, 255, NULL, true, NULL)
       eq(output_expected, ffi.string(output_buff1))
       eq(output_expected, ffi.string(output_buff2))
     end)
@@ -344,21 +344,21 @@ describe('env.c', function()
     itp('expands ~ once when `one` is true', function()
       local input = '~/foo ~ foo'
       local homedir = cstr(255, '')
-      cimp.expand_env_esc(to_cstr('~'), homedir, 255, false, true, NULL)
+      cimp.expand_env_esc(to_cstr('~'), homedir, 255, NULL, true, NULL)
       local output_expected = ffi.string(homedir) .. '/foo ~ foo'
       local output = cstr(255, '')
-      cimp.expand_env_esc(to_cstr(input), output, 255, false, true, NULL)
+      cimp.expand_env_esc(to_cstr(input), output, 255, NULL, true, NULL)
       eq(ffi.string(output), ffi.string(output_expected))
     end)
 
     itp('expands ~ every time when `one` is false', function()
       local input = to_cstr('~/foo ~ foo')
       local dst = cstr(255, '')
-      cimp.expand_env_esc(to_cstr('~'), dst, 255, false, true, NULL)
+      cimp.expand_env_esc(to_cstr('~'), dst, 255, NULL, true, NULL)
       local homedir = ffi.string(dst)
       local output_expected = homedir .. '/foo ' .. homedir .. ' foo'
       local output = cstr(255, '')
-      cimp.expand_env_esc(input, output, 255, false, false, NULL)
+      cimp.expand_env_esc(input, output, 255, NULL, false, NULL)
       eq(output_expected, ffi.string(output))
     end)
 
@@ -370,7 +370,7 @@ describe('env.c', function()
       local src =
         to_cstr('~' .. curuser .. '/Vcs/django-rest-framework/rest_framework/renderers.py')
       local dst = cstr(256, '~' .. curuser)
-      cimp.expand_env_esc(src, dst, 256, false, false, NULL)
+      cimp.expand_env_esc(src, dst, 256, NULL, false, NULL)
       local len = string.len(ffi.string(dst))
       assert.True(len > 56)
       assert.True(len < 256)
@@ -381,7 +381,7 @@ describe('env.c', function()
       -- The buffer is long enough to actually contain the full input in case the
       -- test fails, but we don't tell expand_env_esc that
       local output = cstr(255, '')
-      cimp.expand_env_esc(input, output, 5, false, true, NULL)
+      cimp.expand_env_esc(input, output, 5, NULL, true, NULL)
       -- Make sure the first few characters are copied properly and that there is a
       -- terminating null character
       for i = 0, 3 do
@@ -400,7 +400,7 @@ describe('env.c', function()
       -- The buffer is long enough to actually contain the full input in case the
       -- test fails, but we don't tell expand_env_esc that
       local output = cstr(255, '')
-      cimp.expand_env_esc(input, output, 5, false, true, NULL)
+      cimp.expand_env_esc(input, output, 5, NULL, true, NULL)
       -- Make sure the first few characters are copied properly and that there is a
       -- terminating null character
       -- expand_env_esc SHOULD NOT expand the variable if there is not enough space to