From 5ccc79e880d5913f092e041f1a67530c1d2d6728 Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Tue, 8 Dec 2020 20:19:08 -0500 Subject: [PATCH 1/2] eval: executable(), exepath() accept strings only Cherry-pick f_executable(), f_exepath(), check_for_string() from patch 8.2.2117. Rename check_for_string() to tv_check_for_string(). https://github.com/vim/vim/commit/7bb4e74c38642682cfdd0cb4052adfa5efdd7dd1 Close https://github.com/neovim/neovim/issues/13485 --- src/nvim/eval/funcs.c | 14 +++++++----- src/nvim/eval/typval.c | 13 +++++++++++ src/nvim/globals.h | 1 + test/functional/eval/executable_spec.lua | 11 ++++++++++ test/functional/eval/exepath_spec.lua | 28 ++++++++++++++++++------ test/functional/fixtures/bin/false | 0 test/functional/fixtures/bin/false.cmd | 0 test/functional/fixtures/bin/null | 0 test/functional/fixtures/bin/null.cmd | 0 test/functional/fixtures/bin/true | 0 test/functional/fixtures/bin/true.cmd | 0 11 files changed, 55 insertions(+), 12 deletions(-) create mode 100755 test/functional/fixtures/bin/false create mode 100755 test/functional/fixtures/bin/false.cmd create mode 100755 test/functional/fixtures/bin/null create mode 100755 test/functional/fixtures/bin/null.cmd create mode 100755 test/functional/fixtures/bin/true create mode 100755 test/functional/fixtures/bin/true.cmd diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index c5fa0f12cc..b56034b92d 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -95,7 +95,6 @@ PRAGMA_DIAG_POP static char *e_listarg = N_("E686: Argument of %s must be a List"); -static char *e_stringreq = N_("E928: String required"); static char *e_invalwindow = N_("E957: Invalid window number"); /// Dummy va_list for passing to vim_snprintf @@ -1877,10 +1876,12 @@ static void f_eventhandler(typval_T *argvars, typval_T *rettv, FunPtr fptr) */ static void f_executable(typval_T *argvars, typval_T *rettv, FunPtr fptr) { - const char *name = tv_get_string(&argvars[0]); + if (tv_check_for_string(&argvars[0]) == FAIL) { + return; + } // Check in $PATH and also check directly if there is a directory name - rettv->vval.v_number = os_can_exe(name, NULL, true); + rettv->vval.v_number = os_can_exe(tv_get_string(&argvars[0]), NULL, true); } typedef struct { @@ -1984,10 +1985,13 @@ static void f_execute(typval_T *argvars, typval_T *rettv, FunPtr fptr) /// "exepath()" function static void f_exepath(typval_T *argvars, typval_T *rettv, FunPtr fptr) { - const char *arg = tv_get_string(&argvars[0]); + if (tv_check_for_string(&argvars[0]) == FAIL) { + return; + } + char *path = NULL; - (void)os_can_exe(arg, &path, true); + (void)os_can_exe(tv_get_string(&argvars[0]), &path, true); rettv->v_type = VAR_STRING; rettv->vval.v_string = (char_u *)path; diff --git a/src/nvim/eval/typval.c b/src/nvim/eval/typval.c index b62820fecc..02d32a4f86 100644 --- a/src/nvim/eval/typval.c +++ b/src/nvim/eval/typval.c @@ -2953,6 +2953,19 @@ 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. +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) { + EMSG(_(e_stringreq)); + 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 index e0853e900f..c53c1546a4 100644 --- a/src/nvim/globals.h +++ b/src/nvim/globals.h @@ -935,6 +935,7 @@ EXTERN char_u e_readonly[] INIT(= N_( "E45: 'readonly' option is set (add ! to override)")); EXTERN char_u e_readonlyvar[] INIT(= N_( "E46: Cannot change read-only variable \"%.*s\"")); +EXTERN char_u e_stringreq[] INIT(= N_("E928: String required")); EXTERN char_u e_dictreq[] INIT(= N_("E715: Dictionary required")); EXTERN char_u e_toomanyarg[] INIT(= N_( "E118: Too many arguments for function: %s")); diff --git a/test/functional/eval/executable_spec.lua b/test/functional/eval/executable_spec.lua index a1cf056907..f8e579a8ff 100644 --- a/test/functional/eval/executable_spec.lua +++ b/test/functional/eval/executable_spec.lua @@ -2,6 +2,7 @@ local helpers = require('test.functional.helpers')(after_each) local eq, clear, call, iswin, write_file, command = helpers.eq, helpers.clear, helpers.call, helpers.iswin, helpers.write_file, helpers.command +local exc_exec = helpers.exc_exec local eval = helpers.eval describe('executable()', function() @@ -12,6 +13,16 @@ describe('executable()', function() eq(1, call('executable', exe)) end) + it('fails for invalid values', function() + 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")') + for _, input in ipairs({'v:null', 'v:true', 'v:false'}) do + eq('Vim(call):E928: String required', exc_exec('call executable('..input..')')) + end + end) + it('returns 0 for non-existent files', function() eq(0, call('executable', 'no_such_file_exists_209ufq23f')) end) diff --git a/test/functional/eval/exepath_spec.lua b/test/functional/eval/exepath_spec.lua index 10a11aeacc..b86fea5535 100644 --- a/test/functional/eval/exepath_spec.lua +++ b/test/functional/eval/exepath_spec.lua @@ -1,14 +1,28 @@ local helpers = require('test.functional.helpers')(after_each) local eq, clear, call, iswin = helpers.eq, helpers.clear, helpers.call, helpers.iswin +local command = helpers.command +local exc_exec = helpers.exc_exec -describe('exepath() (Windows)', function() - if not iswin() then return end -- N/A for Unix. +describe('exepath()', function() + before_each(clear) - it('append extension if omitted', function() - local filename = 'cmd' - local pathext = '.exe' - clear({env={PATHEXT=pathext}}) - eq(call('exepath', filename..pathext), call('exepath', filename)) + it('fails for invalid values', function() + for _, input in ipairs({'""', 'v:null', 'v:true', 'v:false', '{}', '[]'}) do + eq('Vim(call):E928: String required', exc_exec('call exepath('..input..')')) + end + 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..')')) + end end) + + if iswin() then + it('append extension if omitted', function() + local filename = 'cmd' + local pathext = '.exe' + clear({env={PATHEXT=pathext}}) + eq(call('exepath', filename..pathext), call('exepath', filename)) + end) + end end) diff --git a/test/functional/fixtures/bin/false b/test/functional/fixtures/bin/false new file mode 100755 index 0000000000..e69de29bb2 diff --git a/test/functional/fixtures/bin/false.cmd b/test/functional/fixtures/bin/false.cmd new file mode 100755 index 0000000000..e69de29bb2 diff --git a/test/functional/fixtures/bin/null b/test/functional/fixtures/bin/null new file mode 100755 index 0000000000..e69de29bb2 diff --git a/test/functional/fixtures/bin/null.cmd b/test/functional/fixtures/bin/null.cmd new file mode 100755 index 0000000000..e69de29bb2 diff --git a/test/functional/fixtures/bin/true b/test/functional/fixtures/bin/true new file mode 100755 index 0000000000..e69de29bb2 diff --git a/test/functional/fixtures/bin/true.cmd b/test/functional/fixtures/bin/true.cmd new file mode 100755 index 0000000000..e69de29bb2 From da58242fb8e432415778e3eeab96a9d63edfdf06 Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Fri, 11 Dec 2020 22:59:43 -0500 Subject: [PATCH 2/2] test/functional/eval: assert that executable() fixtures are executable --- test/functional/eval/executable_spec.lua | 4 ++++ test/functional/eval/exepath_spec.lua | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/test/functional/eval/executable_spec.lua b/test/functional/eval/executable_spec.lua index f8e579a8ff..28aefb72e5 100644 --- a/test/functional/eval/executable_spec.lua +++ b/test/functional/eval/executable_spec.lua @@ -11,6 +11,10 @@ describe('executable()', function() it('returns 1 for commands in $PATH', function() local exe = iswin() and 'ping' or 'ls' eq(1, call('executable', exe)) + command('let $PATH = fnamemodify("./test/functional/fixtures/bin", ":p")') + eq(1, call('executable', 'null')) + eq(1, call('executable', 'true')) + eq(1, call('executable', 'false')) end) it('fails for invalid values', function() diff --git a/test/functional/eval/exepath_spec.lua b/test/functional/eval/exepath_spec.lua index b86fea5535..08d2c59af8 100644 --- a/test/functional/eval/exepath_spec.lua +++ b/test/functional/eval/exepath_spec.lua @@ -3,10 +3,22 @@ local eq, clear, call, iswin = helpers.eq, helpers.clear, helpers.call, helpers.iswin local command = helpers.command local exc_exec = helpers.exc_exec +local matches = helpers.matches describe('exepath()', function() before_each(clear) + it('returns 1 for commands in $PATH', function() + local exe = iswin() and 'ping' or 'ls' + local ext_pat = iswin() and '%.EXE$' or '$' + matches(exe .. ext_pat, call('exepath', exe)) + command('let $PATH = fnamemodify("./test/functional/fixtures/bin", ":p")') + ext_pat = iswin() and '%.CMD$' or '$' + matches('null' .. ext_pat, call('exepath', 'null')) + matches('true' .. ext_pat, call('exepath', 'true')) + matches('false' .. ext_pat, call('exepath', 'false')) + end) + it('fails for invalid values', function() for _, input in ipairs({'""', 'v:null', 'v:true', 'v:false', '{}', '[]'}) do eq('Vim(call):E928: String required', exc_exec('call exepath('..input..')'))