Merge #4964 from ZyX-I/no-xdg-expand

option: Do not expand options, obtained from XDG vars

9 files changed, 286 insertions, 46 deletions

diff --git a/scripts/genoptions.lua b/scripts/genoptions.lua
index da53d010bd..9f7d94969d 100644
--- a/scripts/genoptions.lua
+++ b/scripts/genoptions.lua
@@ -57,9 +57,14 @@ local get_flags = function(o)
       add_flag(redraw_flags[r_flag])
     end
   end
+  if o.expand then
+    add_flag('P_EXPAND')
+    if o.expand == 'nodefault' then
+      add_flag('P_NO_DEF_EXP')
+    end
+  end
   for _, flag_desc in ipairs({
     {'alloced'},
-    {'expand'},
     {'nodefault'},
     {'no_mkrc'},
     {'vi_def'},
diff --git a/src/nvim/option.c b/src/nvim/option.c
index de53b0b1f4..6baf8c65ce 100644
--- a/src/nvim/option.c
+++ b/src/nvim/option.c
@@ -236,6 +236,7 @@ typedef struct vimoption {
 #define P_NO_ML        0x2000000U  ///< not allowed in modeline
 #define P_CURSWANT     0x4000000U  ///< update curswant required; not needed
                                    ///< when there is a redraw flag
+#define P_NO_DEF_EXP   0x8000000U  ///< Do not expand default value.
 
 #define HIGHLIGHT_INIT \
   "8:SpecialKey,~:EndOfBuffer,z:TermCursor,Z:TermCursorNC,@:NonText," \
@@ -676,15 +677,18 @@ void set_init_1(void)
 #endif
                      false);
 
-  char *backupdir = stdpaths_user_data_subpath("backup", 0);
+  char *backupdir = stdpaths_user_data_subpath("backup", 0, true);
   const size_t backupdir_len = strlen(backupdir);
   backupdir = xrealloc(backupdir, backupdir_len + 3);
   memmove(backupdir + 2, backupdir, backupdir_len + 1);
   memmove(backupdir, ".,", 2);
-  set_string_default("viewdir", stdpaths_user_data_subpath("view", 0), true);
+  set_string_default("viewdir", stdpaths_user_data_subpath("view", 0, true),
+                     true);
   set_string_default("backupdir", backupdir, true);
-  set_string_default("directory", stdpaths_user_data_subpath("swap", 2), true);
-  set_string_default("undodir", stdpaths_user_data_subpath("undo", 0), true);
+  set_string_default("directory", stdpaths_user_data_subpath("swap", 2, true),
+                     true);
+  set_string_default("undodir", stdpaths_user_data_subpath("undo", 0, true),
+                     true);
   // Set default for &runtimepath. All necessary expansions are performed in
   // this function.
   set_runtimepath_default();
@@ -726,6 +730,9 @@ void set_init_1(void)
    * default.
    */
   for (opt_idx = 0; options[opt_idx].fullname; opt_idx++) {
+    if (options[opt_idx].flags & P_NO_DEF_EXP) {
+      continue;
+    }
     char *p;
     if ((options[opt_idx].flags & P_GETTEXT)
         && options[opt_idx].var != NULL) {
@@ -1473,16 +1480,19 @@ do_set (
                * default value was already expanded, only
                * required when an environment variable was set
                * later */
-              if (newval == NULL)
+              new_value_alloced = true;
+              if (newval == NULL) {
                 newval = empty_option;
-              else {
+              } else if (!(options[opt_idx].flags | P_NO_DEF_EXP)) {
                 s = option_expand(opt_idx, newval);
-                if (s == NULL)
+                if (s == NULL) {
                   s = newval;
+                }
                 newval = vim_strsave(s);
+              } else {
+                newval = (char_u *)xstrdup((char *)newval);
               }
-              new_value_alloced = TRUE;
-            } else if (nextchar == '<') {             /* set to global val */
+            } else if (nextchar == '<') {  // set to global val
               newval = vim_strsave(*(char_u **)get_varp_scope(
                       &(options[opt_idx]), OPT_GLOBAL));
               new_value_alloced = TRUE;
@@ -2020,13 +2030,15 @@ static char_u *option_expand(int opt_idx, char_u *val)
   if (!(options[opt_idx].flags & P_EXPAND) || options[opt_idx].var == NULL)
     return NULL;
 
-  /* If val is longer than MAXPATHL no meaningful expansion can be done,
-   * expand_env() would truncate the string. */
-  if (val != NULL && STRLEN(val) > MAXPATHL)
-    return NULL;
-
-  if (val == NULL)
+  if (val == NULL) {
     val = *(char_u **)options[opt_idx].var;
+  }
+
+  // If val is longer than MAXPATHL no meaningful expansion can be done,
+  // expand_env() would truncate the string.
+  if (val == NULL || STRLEN(val) > MAXPATHL) {
+    return NULL;
+  }
 
   /*
    * Expanding this with NameBuff, expand_env() must not be passed IObuff.
diff --git a/src/nvim/option.h b/src/nvim/option.h
index 5c2b2662b5..3a43b859a8 100644
--- a/src/nvim/option.h
+++ b/src/nvim/option.h
@@ -6,17 +6,18 @@
 #define BCO_ALWAYS      2       /* always copy the options */
 #define BCO_NOHELP      4       /* don't touch the help related options */
 
-/*
- * "flags" values for option-setting functions.
- * When OPT_GLOBAL and OPT_LOCAL are both missing, set both local and global
- * values, get local value.
- */
-#define OPT_FREE        1       /* free old value if it was allocated */
-#define OPT_GLOBAL      2       /* use global value */
-#define OPT_LOCAL       4       /* use local value */
-#define OPT_MODELINE    8       /* option in modeline */
-#define OPT_WINONLY     16      /* only set window-local options */
-#define OPT_NOWIN       32      /* don't set window-local options */
+/// Flags for option-setting functions
+///
+/// When OPT_GLOBAL and OPT_LOCAL are both missing, set both local and global
+/// values, get local value.
+typedef enum {
+  OPT_FREE = 1,  ///< Free old value if it was allocated.
+  OPT_GLOBAL = 2,  ///< Use global value.
+  OPT_LOCAL = 4,  ///< Use local value.
+  OPT_MODELINE = 8,  ///< Option in modeline.
+  OPT_WINONLY = 16,  ///< Only set window-local options.
+  OPT_NOWIN = 32,  ///< Don’t set window-local options.
+} OptionFlags;
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "option.h.generated.h"
diff --git a/src/nvim/options.lua b/src/nvim/options.lua
index d19af4f73f..060ec8c1e1 100644
--- a/src/nvim/options.lua
+++ b/src/nvim/options.lua
@@ -165,7 +165,7 @@ return {
       deny_duplicates=true,
       secure=true,
       vi_def=true,
-      expand=true,
+      expand='nodefault',
       varname='p_bdir',
       defaults={if_true={vi=''}}
     },
@@ -616,7 +616,7 @@ return {
       deny_duplicates=true,
       secure=true,
       vi_def=true,
-      expand=true,
+      expand='nodefault',
       varname='p_dir',
       defaults={if_true={vi=''}}
     },
@@ -1891,7 +1891,7 @@ return {
       deny_duplicates=true,
       secure=true,
       vi_def=true,
-      expand=true,
+      expand='nodefault',
       varname='p_rtp',
       defaults={if_true={vi=''}}
     },
@@ -2507,7 +2507,7 @@ return {
       deny_duplicates=true,
       secure=true,
       vi_def=true,
-      expand=true,
+      expand='nodefault',
       varname='p_udir',
       defaults={if_true={vi=''}}
     },
@@ -2568,7 +2568,7 @@ return {
       type='string', scope={'global'},
       secure=true,
       vi_def=true,
-      expand=true,
+      expand='nodefault',
       varname='p_vdir',
       defaults={if_true={vi=''}}
     },
diff --git a/src/nvim/os/stdpaths.c b/src/nvim/os/stdpaths.c
index 81ceb919c4..10b3f4c091 100644
--- a/src/nvim/os/stdpaths.c
+++ b/src/nvim/os/stdpaths.c
@@ -100,18 +100,30 @@ char *stdpaths_user_conf_subpath(const char *fname)
 ///
 /// @param[in]  fname  New component of the path.
 /// @param[in]  trailing_pathseps  Amount of trailing path separators to add.
+/// @param[in]  escape_commas  If true, all commas will be escaped.
 ///
-/// @return [allocated] `$XDG_DATA_HOME/nvim/{fname}`
+/// @return [allocated] `$XDG_DATA_HOME/nvim/{fname}`.
 char *stdpaths_user_data_subpath(const char *fname,
-                                 const size_t trailing_pathseps)
+                                 const size_t trailing_pathseps,
+                                 const bool escape_commas)
   FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL FUNC_ATTR_NONNULL_RET
 {
   char *ret = concat_fnames_realloc(get_xdg_home(kXDGDataHome), fname, true);
-  if (trailing_pathseps) {
-    const size_t len = strlen(ret);
-    ret = xrealloc(ret, len + trailing_pathseps + 1);
-    memset(ret + len, PATHSEP, trailing_pathseps);
-    ret[len + trailing_pathseps] = NUL;
+  const size_t len = strlen(ret);
+  const size_t numcommas = (escape_commas ? memcnt(ret, ',', len) : 0);
+  if (numcommas || trailing_pathseps) {
+    ret = xrealloc(ret, len + trailing_pathseps + numcommas + 1);
+    for (size_t i = 0 ; i < len + numcommas ; i++) {
+      if (ret[i] == ',') {
+        memmove(ret + i + 1, ret + i, len - i + numcommas);
+        ret[i] = '\\';
+        i++;
+      }
+    }
+    if (trailing_pathseps) {
+      memset(ret + len + numcommas, PATHSEP, trailing_pathseps);
+    }
+    ret[len + trailing_pathseps + numcommas] = NUL;
   }
   return ret;
 }
diff --git a/src/nvim/shada.c b/src/nvim/shada.c
index b5921eb810..fe62f06e59 100644
--- a/src/nvim/shada.c
+++ b/src/nvim/shada.c
@@ -1524,7 +1524,7 @@ static const char *shada_get_default_file(void)
   FUNC_ATTR_WARN_UNUSED_RESULT
 {
   if (default_shada_file == NULL) {
-    char *shada_dir = stdpaths_user_data_subpath("shada", 0);
+    char *shada_dir = stdpaths_user_data_subpath("shada", 0, false);
     default_shada_file = concat_fnames_realloc(shada_dir, "main.shada", true);
   }
   return default_shada_file;
diff --git a/test/functional/helpers.lua b/test/functional/helpers.lua
index 02109d0889..6f43ec817c 100644
--- a/test/functional/helpers.lua
+++ b/test/functional/helpers.lua
@@ -216,17 +216,47 @@ local function merge_args(...)
   return argv
 end
 
-local function spawn(argv, merge)
-  local child_stream = ChildProcessStream.spawn(merge and merge_args(prepend_argv, argv) or argv)
+local function spawn(argv, merge, env)
+  local child_stream = ChildProcessStream.spawn(
+      merge and merge_args(prepend_argv, argv) or argv,
+      env)
   return Session.new(child_stream)
 end
 
 local function clear(...)
   local args = {unpack(nvim_argv)}
-  for _, arg in ipairs({...}) do
+  local new_args
+  local env = nil
+  local opts = select(1, ...)
+  if type(opts) == 'table' then
+    if opts.env then
+      local env_tbl = {}
+      for k, v in pairs(opts.env) do
+        assert(type(k) == 'string')
+        assert(type(v) == 'string')
+        env_tbl[k] = v
+      end
+      for _, k in ipairs({
+        'HOME',
+        'ASAN_OPTIONS',
+        'LD_LIBRARY_PATH', 'PATH',
+        'NVIM_LOG_FILE',
+      }) do
+        env_tbl[k] = os.getenv(k)
+      end
+      env = {}
+      for k, v in pairs(env_tbl) do
+        env[#env + 1] = k .. '=' .. v
+      end
+    end
+    new_args = opts.args or {}
+  else
+    new_args = {...}
+  end
+  for _, arg in ipairs(new_args) do
     table.insert(args, arg)
   end
-  set_session(spawn(args))
+  set_session(spawn(args, nil, env))
 end
 
 local function insert(...)
diff --git a/test/functional/options/defaults_spec.lua b/test/functional/options/defaults_spec.lua
index a36939b0bd..8eec6ad8bf 100644
--- a/test/functional/options/defaults_spec.lua
+++ b/test/functional/options/defaults_spec.lua
@@ -1,7 +1,12 @@
 local helpers = require('test.functional.helpers')(after_each)
+
 local Screen = require('test.functional.ui.screen')
-local eval, eq = helpers.eval, helpers.eq
+
+local meths = helpers.meths
 local execute = helpers.execute
+local clear = helpers.clear
+local eval = helpers.eval
+local eq = helpers.eq
 
 local function init_session(...)
   local args = { helpers.nvim_prog, '-i', 'NONE', '--embed',
@@ -77,4 +82,179 @@ describe('startup defaults', function()
   end)
 end)
 
+describe('XDG-based defaults', function()
+  -- Need to be in separate describe() block to not run clear() twice.
+  -- Do not put before_each() here for the same reasons.
+  describe('with too long XDG variables', function()
+    before_each(function()
+      clear({env={
+        XDG_CONFIG_HOME=('/x'):rep(4096),
+        XDG_CONFIG_DIRS=(('/a'):rep(2048)
+                         .. ':' .. ('/b'):rep(2048)
+                         .. (':/c'):rep(512)),
+        XDG_DATA_HOME=('/X'):rep(4096),
+        XDG_DATA_DIRS=(('/A'):rep(2048)
+                       .. ':' .. ('/B'):rep(2048)
+                       .. (':/C'):rep(512)),
+      }})
+    end)
+
+    it('are correctly set', function()
+      eq((('/x'):rep(4096) .. '/nvim'
+          .. ',' .. ('/a'):rep(2048) .. '/nvim'
+          .. ',' .. ('/b'):rep(2048) .. '/nvim'
+          .. (',' .. '/c/nvim'):rep(512)
+          .. ',' .. ('/X'):rep(4096) .. '/nvim/site'
+          .. ',' .. ('/A'):rep(2048) .. '/nvim/site'
+          .. ',' .. ('/B'):rep(2048) .. '/nvim/site'
+          .. (',' .. '/C/nvim/site'):rep(512)
+          .. ',' .. eval('$VIMRUNTIME')
+          .. (',' .. '/C/nvim/site/after'):rep(512)
+          .. ',' .. ('/B'):rep(2048) .. '/nvim/site/after'
+          .. ',' .. ('/A'):rep(2048) .. '/nvim/site/after'
+          .. ',' .. ('/X'):rep(4096) .. '/nvim/site/after'
+          .. (',' .. '/c/nvim/after'):rep(512)
+          .. ',' .. ('/b'):rep(2048) .. '/nvim/after'
+          .. ',' .. ('/a'):rep(2048) .. '/nvim/after'
+          .. ',' .. ('/x'):rep(4096) .. '/nvim/after'
+      ), meths.get_option('runtimepath'))
+      meths.command('set runtimepath&')
+      meths.command('set backupdir&')
+      meths.command('set directory&')
+      meths.command('set undodir&')
+      meths.command('set viewdir&')
+      eq((('/x'):rep(4096) .. '/nvim'
+          .. ',' .. ('/a'):rep(2048) .. '/nvim'
+          .. ',' .. ('/b'):rep(2048) .. '/nvim'
+          .. (',' .. '/c/nvim'):rep(512)
+          .. ',' .. ('/X'):rep(4096) .. '/nvim/site'
+          .. ',' .. ('/A'):rep(2048) .. '/nvim/site'
+          .. ',' .. ('/B'):rep(2048) .. '/nvim/site'
+          .. (',' .. '/C/nvim/site'):rep(512)
+          .. ',' .. eval('$VIMRUNTIME')
+          .. (',' .. '/C/nvim/site/after'):rep(512)
+          .. ',' .. ('/B'):rep(2048) .. '/nvim/site/after'
+          .. ',' .. ('/A'):rep(2048) .. '/nvim/site/after'
+          .. ',' .. ('/X'):rep(4096) .. '/nvim/site/after'
+          .. (',' .. '/c/nvim/after'):rep(512)
+          .. ',' .. ('/b'):rep(2048) .. '/nvim/after'
+          .. ',' .. ('/a'):rep(2048) .. '/nvim/after'
+          .. ',' .. ('/x'):rep(4096) .. '/nvim/after'
+      ), meths.get_option('runtimepath'))
+      eq('.,' .. ('/X'):rep(4096) .. '/nvim/backup',
+         meths.get_option('backupdir'))
+      eq(('/X'):rep(4096) .. '/nvim/swap//', meths.get_option('directory'))
+      eq(('/X'):rep(4096) .. '/nvim/undo', meths.get_option('undodir'))
+      eq(('/X'):rep(4096) .. '/nvim/view', meths.get_option('viewdir'))
+    end)
+  end)
+
+  describe('with XDG variables that can be expanded', function()
+    before_each(function()
+      clear({env={
+        XDG_CONFIG_HOME='$XDG_DATA_HOME',
+        XDG_CONFIG_DIRS='$XDG_DATA_DIRS',
+        XDG_DATA_HOME='$XDG_CONFIG_HOME',
+        XDG_DATA_DIRS='$XDG_CONFIG_DIRS',
+      }})
+    end)
+
+    it('are not expanded', function()
+      eq(('$XDG_DATA_HOME/nvim'
+          .. ',$XDG_DATA_DIRS/nvim'
+          .. ',$XDG_CONFIG_HOME/nvim/site'
+          .. ',$XDG_CONFIG_DIRS/nvim/site'
+          .. ',' .. eval('$VIMRUNTIME')
+          .. ',$XDG_CONFIG_DIRS/nvim/site/after'
+          .. ',$XDG_CONFIG_HOME/nvim/site/after'
+          .. ',$XDG_DATA_DIRS/nvim/after'
+          .. ',$XDG_DATA_HOME/nvim/after'
+      ), meths.get_option('runtimepath'))
+      meths.command('set runtimepath&')
+      meths.command('set backupdir&')
+      meths.command('set directory&')
+      meths.command('set undodir&')
+      meths.command('set viewdir&')
+      eq(('$XDG_DATA_HOME/nvim'
+          .. ',$XDG_DATA_DIRS/nvim'
+          .. ',$XDG_CONFIG_HOME/nvim/site'
+          .. ',$XDG_CONFIG_DIRS/nvim/site'
+          .. ',' .. eval('$VIMRUNTIME')
+          .. ',$XDG_CONFIG_DIRS/nvim/site/after'
+          .. ',$XDG_CONFIG_HOME/nvim/site/after'
+          .. ',$XDG_DATA_DIRS/nvim/after'
+          .. ',$XDG_DATA_HOME/nvim/after'
+      ), meths.get_option('runtimepath'))
+      eq('.,$XDG_CONFIG_HOME/nvim/backup', meths.get_option('backupdir'))
+      eq('$XDG_CONFIG_HOME/nvim/swap//', meths.get_option('directory'))
+      eq('$XDG_CONFIG_HOME/nvim/undo', meths.get_option('undodir'))
+      eq('$XDG_CONFIG_HOME/nvim/view', meths.get_option('viewdir'))
+      meths.command('set all&')
+      eq(('$XDG_DATA_HOME/nvim'
+          .. ',$XDG_DATA_DIRS/nvim'
+          .. ',$XDG_CONFIG_HOME/nvim/site'
+          .. ',$XDG_CONFIG_DIRS/nvim/site'
+          .. ',' .. eval('$VIMRUNTIME')
+          .. ',$XDG_CONFIG_DIRS/nvim/site/after'
+          .. ',$XDG_CONFIG_HOME/nvim/site/after'
+          .. ',$XDG_DATA_DIRS/nvim/after'
+          .. ',$XDG_DATA_HOME/nvim/after'
+      ), meths.get_option('runtimepath'))
+      eq('.,$XDG_CONFIG_HOME/nvim/backup', meths.get_option('backupdir'))
+      eq('$XDG_CONFIG_HOME/nvim/swap//', meths.get_option('directory'))
+      eq('$XDG_CONFIG_HOME/nvim/undo', meths.get_option('undodir'))
+      eq('$XDG_CONFIG_HOME/nvim/view', meths.get_option('viewdir'))
+    end)
+  end)
+
+  describe('with commas', function()
+    before_each(function()
+      clear({env={
+        XDG_CONFIG_HOME=', , ,',
+        XDG_CONFIG_DIRS=',-,-,:-,-,-',
+        XDG_DATA_HOME=',=,=,',
+        XDG_DATA_DIRS=',≡,≡,:≡,≡,≡',
+      }})
+    end)
 
+    it('are escaped properly', function()
+      eq(('\\, \\, \\,/nvim'
+          .. ',\\,-\\,-\\,/nvim'
+          .. ',-\\,-\\,-/nvim'
+          .. ',\\,=\\,=\\,/nvim/site'
+          .. ',\\,≡\\,≡\\,/nvim/site'
+          .. ',≡\\,≡\\,≡/nvim/site'
+          .. ',' .. eval('$VIMRUNTIME')
+          .. ',≡\\,≡\\,≡/nvim/site/after'
+          .. ',\\,≡\\,≡\\,/nvim/site/after'
+          .. ',\\,=\\,=\\,/nvim/site/after'
+          .. ',-\\,-\\,-/nvim/after'
+          .. ',\\,-\\,-\\,/nvim/after'
+          .. ',\\, \\, \\,/nvim/after'
+      ), meths.get_option('runtimepath'))
+      meths.command('set runtimepath&')
+      meths.command('set backupdir&')
+      meths.command('set directory&')
+      meths.command('set undodir&')
+      meths.command('set viewdir&')
+      eq(('\\, \\, \\,/nvim'
+          .. ',\\,-\\,-\\,/nvim'
+          .. ',-\\,-\\,-/nvim'
+          .. ',\\,=\\,=\\,/nvim/site'
+          .. ',\\,≡\\,≡\\,/nvim/site'
+          .. ',≡\\,≡\\,≡/nvim/site'
+          .. ',' .. eval('$VIMRUNTIME')
+          .. ',≡\\,≡\\,≡/nvim/site/after'
+          .. ',\\,≡\\,≡\\,/nvim/site/after'
+          .. ',\\,=\\,=\\,/nvim/site/after'
+          .. ',-\\,-\\,-/nvim/after'
+          .. ',\\,-\\,-\\,/nvim/after'
+          .. ',\\, \\, \\,/nvim/after'
+      ), meths.get_option('runtimepath'))
+      eq('.,\\,=\\,=\\,/nvim/backup', meths.get_option('backupdir'))
+      eq('\\,=\\,=\\,/nvim/swap//', meths.get_option('directory'))
+      eq('\\,=\\,=\\,/nvim/undo', meths.get_option('undodir'))
+      eq('\\,=\\,=\\,/nvim/view', meths.get_option('viewdir'))
+    end)
+  end)
+end)
diff --git a/third-party/cmake/BuildLuarocks.cmake b/third-party/cmake/BuildLuarocks.cmake
index 0041bb1a03..1bff020691 100644
--- a/third-party/cmake/BuildLuarocks.cmake
+++ b/third-party/cmake/BuildLuarocks.cmake
@@ -156,7 +156,7 @@ if(USE_BUNDLED_BUSTED)
 
   add_custom_command(OUTPUT ${HOSTDEPS_LIB_DIR}/luarocks/rocks/nvim-client
     COMMAND ${LUAROCKS_BINARY}
-    ARGS build https://raw.githubusercontent.com/neovim/lua-client/0.0.1-24/nvim-client-0.0.1-24.rockspec ${LUAROCKS_BUILDARGS}
+    ARGS build https://raw.githubusercontent.com/neovim/lua-client/0.0.1-25/nvim-client-0.0.1-25.rockspec ${LUAROCKS_BUILDARGS}
     DEPENDS luv)
   add_custom_target(nvim-client
     DEPENDS ${HOSTDEPS_LIB_DIR}/luarocks/rocks/nvim-client)