fix(eval): checking for a non-empty string is too strict (#15987)

Sh3Rm4n · web-flow · commit 1dbbaf89bf5d · 2021-10-28T22:13:40.000-04:00
Cherry-pick check_for_nonempty_string() from patch vim-8.2.2133 and apply it on the bases of #13489 vim/vim@2a9d5d3
diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c
@@ -2201,7 +2201,7 @@ static void f_win_execute(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 /// "exepath()" function
 static void f_exepath(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 {
-  if (tv_check_for_string(&argvars[0]) == FAIL) {
+  if (tv_check_for_nonempty_string(&argvars[0]) == FAIL) {
     return;
   }
 
@@ -2661,9 +2661,9 @@ static void f_fnamemodify(typval_T *argvars, typval_T *rettv, FunPtr fptr)
   char buf[NUMBUFLEN];
   const char *fname = tv_get_string_chk(&argvars[0]);
   const char *const mods = tv_get_string_buf_chk(&argvars[1], buf);
-  if (fname == NULL || mods == NULL) {
+  if (fname == NULL) {
     fname = NULL;
-  } else {
+  } else if (mods != NULL && *mods != NUL) {
     len = strlen(fname);
     size_t usedlen = 0;
     if (*mods != NUL) {
diff --git a/src/nvim/eval/typval.c b/src/nvim/eval/typval.c
@@ -3146,19 +3146,31 @@ float_T tv_get_float(const typval_T *const tv)
   return 0;
 }
 
-// Give an error and return FAIL unless "tv" is a non-empty string.
+// Give an error and return FAIL unless "tv" is a string.
 int tv_check_for_string(const typval_T *const tv)
   FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_PURE
 {
-  if (tv->v_type != VAR_STRING
-      || tv->vval.v_string == NULL
-      || *tv->vval.v_string == NUL) {
+  if (tv->v_type != VAR_STRING) {
     EMSG(_(e_stringreq));
     return FAIL;
   }
   return OK;
 }
 
+// Give an error and return FAIL unless "tv" is a non-empty string.
+int tv_check_for_nonempty_string(const typval_T *const tv)
+  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_PURE
+{
+  if (tv_check_for_string(tv) == FAIL) {
+      return FAIL;
+  }
+  if (tv->vval.v_string == NULL || *tv->vval.v_string == NUL) {
+    EMSG(_(e_non_empty_string_required));
+    return FAIL;
+  }
+  return OK;
+}
+
 /// Get the string value of a "stringish" VimL object.
 ///
 /// @param[in]  tv  Object to get value of.
diff --git a/src/nvim/globals.h b/src/nvim/globals.h
@@ -994,6 +994,8 @@ EXTERN char_u e_floatonly[] INIT(=N_(
                                     "E5601: Cannot close window, only floating window would remain"));
 EXTERN char_u e_floatexchange[] INIT(=N_("E5602: Cannot exchange or rotate float"));
 
+EXTERN char e_non_empty_string_required[] INIT(= N_("E1142: Non-empty string required"));
+
 EXTERN char e_cannot_define_autocommands_for_all_events[] INIT(=
                                                                  N_(
                                                                    "E1155: Cannot define autocommands for ALL events"));
diff --git a/test/functional/vimscript/executable_spec.lua b/test/functional/vimscript/executable_spec.lua
@@ -18,7 +18,7 @@ describe('executable()', function()
   end)
 
   it('fails for invalid values', function()
-    for _, input in ipairs({'""', 'v:null', 'v:true', 'v:false', '{}', '[]'}) do
+    for _, input in ipairs({'v:null', 'v:true', 'v:false', '{}', '[]'}) do
       eq('Vim(call):E928: String required', exc_exec('call executable('..input..')'))
     end
     command('let $PATH = fnamemodify("./test/functional/fixtures/bin", ":p")')
@@ -27,6 +27,10 @@ describe('executable()', function()
     end
   end)
 
+  it('returns 0 for empty strings', function()
+    eq(0, call('executable', '""'))
+  end)
+
   it('returns 0 for non-existent files', function()
     eq(0, call('executable', 'no_such_file_exists_209ufq23f'))
   end)
diff --git a/test/functional/vimscript/exepath_spec.lua b/test/functional/vimscript/exepath_spec.lua
@@ -20,9 +20,10 @@ describe('exepath()', function()
   end)
 
   it('fails for invalid values', function()
-    for _, input in ipairs({'""', 'v:null', 'v:true', 'v:false', '{}', '[]'}) do
+    for _, input in ipairs({'v:null', 'v:true', 'v:false', '{}', '[]'}) do
       eq('Vim(call):E928: String required', exc_exec('call exepath('..input..')'))
     end
+    eq('Vim(call):E1142: Non-empty string required', exc_exec('call exepath("")'))
     command('let $PATH = fnamemodify("./test/functional/fixtures/bin", ":p")')
     for _, input in ipairs({'v:null', 'v:true', 'v:false'}) do
       eq('Vim(call):E928: String required', exc_exec('call exepath('..input..')'))
diff --git a/test/functional/vimscript/null_spec.lua b/test/functional/vimscript/null_spec.lua
@@ -161,7 +161,7 @@ describe('NULL', function()
     null_test('does not crash :echomsg', 'echomsg S', 0)
     null_test('does not crash :execute', 'execute S', 0)
     null_expr_test('does not crash execute()', 'execute(S)', 0, '')
-    null_expr_test('makes executable() error out', 'executable(S)', 'E928: String required', 0)
+    null_expr_test('does not crash executable()', 'executable(S)', 0, 0)
     null_expr_test('makes timer_start() error out', 'timer_start(0, S)', 'E921: Invalid callback argument', -1)
     null_expr_test('does not crash filereadable()', 'filereadable(S)', 0, 0)
     null_expr_test('does not crash filewritable()', 'filewritable(S)', 0, 0)
