From 0991041ae72e866add2a820a6b0401d21b9a8fab Mon Sep 17 00:00:00 2001 From: Zhaosheng Pan Date: Wed, 3 Jun 2015 19:01:17 +0800 Subject: [PATCH 1/2] system(): Respect 'sxe' and 'sxq' #2789 Fixes #2773 --- src/nvim/misc2.c | 24 +-------------- src/nvim/os/shell.c | 48 +++++++++++++++++++++++++++++- test/unit/os/shell_spec.lua | 58 +++++++++++++++++++++++++++++-------- 3 files changed, 94 insertions(+), 36 deletions(-) diff --git a/src/nvim/misc2.c b/src/nvim/misc2.c index 368f83cfb5..8b4d8c7c3e 100644 --- a/src/nvim/misc2.c +++ b/src/nvim/misc2.c @@ -281,7 +281,6 @@ int default_fileformat(void) // Call shell. Calls os_call_shell, with 'shellxquote' added. int call_shell(char_u *cmd, ShellOpts opts, char_u *extra_shell_arg) { - char_u *ncmd; int retval; proftime_T wait_time; @@ -303,28 +302,7 @@ int call_shell(char_u *cmd, ShellOpts opts, char_u *extra_shell_arg) /* The external command may update a tags file, clear cached tags. */ tag_freematch(); - if (cmd == NULL || *p_sxq == NUL) - retval = os_call_shell(cmd, opts, extra_shell_arg); - else { - char_u *ecmd = cmd; - - if (*p_sxe != NUL && STRCMP(p_sxq, "(") == 0) { - ecmd = vim_strsave_escaped_ext(cmd, p_sxe, '^', FALSE); - } - ncmd = xmalloc(STRLEN(ecmd) + STRLEN(p_sxq) * 2 + 1); - STRCPY(ncmd, p_sxq); - STRCAT(ncmd, ecmd); - /* When 'shellxquote' is ( append ). - * When 'shellxquote' is "( append )". */ - STRCAT(ncmd, STRCMP(p_sxq, "(") == 0 ? (char_u *)")" - : STRCMP(p_sxq, "\"(") == 0 ? (char_u *)")\"" - : p_sxq); - retval = os_call_shell(ncmd, opts, extra_shell_arg); - xfree(ncmd); - - if (ecmd != cmd) - xfree(ecmd); - } + retval = os_call_shell(cmd, opts, extra_shell_arg); } set_vim_var_nr(VV_SHELL_ERROR, (varnumber_T) retval); diff --git a/src/nvim/os/shell.c b/src/nvim/os/shell.c index ba52b9f661..5cc15e5a2e 100644 --- a/src/nvim/os/shell.c +++ b/src/nvim/os/shell.c @@ -37,6 +37,51 @@ typedef struct { # include "os/shell.c.generated.h" #endif +/// Process command string with 'shellxescape' (p_sxe) and 'shellxquote' +/// (p_sxq) +/// +/// @param cmd Command string +/// @return NULL if `cmd` is NULL. Otherwise, a newly allocated command string. +/// It must be freed with `xfree` when no longer needed. +static char *shell_escape(const char *cmd) + FUNC_ATTR_MALLOC FUNC_ATTR_WARN_UNUSED_RESULT +{ + char *ncmd; + + if (cmd == NULL) { + ncmd = NULL; + } else if (*p_sxq == NUL) { + ncmd = xstrdup(cmd); + } else { + const char *ecmd; + size_t ncmd_size; + + if (*p_sxe != NUL && STRCMP(p_sxq, "(") == 0) { + ecmd = (char *)vim_strsave_escaped_ext((char_u *)cmd, p_sxe, '^', false); + } else { + ecmd = cmd; + } + ncmd_size = strlen(ecmd) + STRLEN(p_sxq) * 2 + 1; + ncmd = xmalloc(ncmd_size); + + // When 'shellxquote' is '(', append ')'. + // When 'shellxquote' is '"(', append ')"'. + if (STRCMP(p_sxq, "(") == 0) { + vim_snprintf(ncmd, ncmd_size, "(%s)", ecmd); + } else if (STRCMP(p_sxq, "\"(") == 0) { + vim_snprintf(ncmd, ncmd_size, "\"(%s)\"", ecmd); + } else { + vim_snprintf(ncmd, ncmd_size, "%s%s%s", p_sxq, ecmd, p_sxq); + } + + if (ecmd != (const char *)cmd) { + xfree((void *)ecmd); + } + } + + return ncmd; +} + /// Builds the argument vector for running the user-configured 'shell' (p_sh) /// with an optional command prefixed by 'shellcmdflag' (p_shcf). /// @@ -59,7 +104,8 @@ char **shell_build_argv(const char *cmd, const char *extra_args) if (cmd) { i += tokenize(p_shcf, rv + i); // Split 'shellcmdflag' - rv[i++] = xstrdup(cmd); // Push a copy of the command. + rv[i++] = shell_escape(cmd); // Process command string with + // 'shellxescape' and 'shellxquote' } rv[i] = NULL; diff --git a/test/unit/os/shell_spec.lua b/test/unit/os/shell_spec.lua index 906f950308..613a2cfc2c 100644 --- a/test/unit/os/shell_spec.lua +++ b/test/unit/os/shell_spec.lua @@ -1,15 +1,3 @@ --- not all operating systems support the system()-tests, as of yet. -local allowed_os = { - Linux = true, - OSX = true, - BSD = true, - POSIX = true -} - -if allowed_os[jit.os] ~= true then - return -end - local helpers = require('test.unit.helpers') local cimported = helpers.cimport( './src/nvim/os/shell.h', @@ -47,6 +35,13 @@ describe('shell functions', function() return ret end + after_each(function() + cimported.p_sxq = to_cstr('') + cimported.p_sxe = to_cstr('') + cimported.p_sh = to_cstr('/bin/bash') + cimported.p_shcf = to_cstr('-c') + end) + local function os_system(cmd, input) local input_or = input and to_cstr(input) or NULL local input_len = (input ~= nil) and string.len(input) or 0 @@ -127,4 +122,43 @@ describe('shell functions', function() shell_build_argv('abc def', 'ghi jkl')) end) end) + + describe('shell_build_argv can deal with sxe and sxq', function() + setup(function() + cimported.p_sxq = to_cstr('(') + cimported.p_sxe = to_cstr('"&|<>()@^') + end) + + it('applies shellxescape and shellxquote', function() + local argv = ffi.cast('char**', + cimported.shell_build_argv(to_cstr('echo &|<>()@^'), nil)) + eq(ffi.string(argv[0]), '/bin/bash') + eq(ffi.string(argv[1]), '-c') + eq(ffi.string(argv[2]), '(echo ^&^|^<^>^(^)^@^^)') + eq(nil, argv[3]) + end) + + it('applies shellxquote when shellxquote is "\\"("', function() + cimported.p_sxq = to_cstr('"(') + + local argv = ffi.cast('char**', cimported.shell_build_argv( + to_cstr('echo -n some text'), nil)) + eq(ffi.string(argv[0]), '/bin/bash') + eq(ffi.string(argv[1]), '-c') + eq(ffi.string(argv[2]), '"(echo -n some text)"') + eq(nil, argv[3]) + end) + + it('applies shellxquote when shellxquote is "\\""', function() + cimported.p_sxq = to_cstr('"') + cimported.p_sxe = to_cstr('') + + local argv = ffi.cast('char**', cimported.shell_build_argv( + to_cstr('echo -n some text'), nil)) + eq(ffi.string(argv[0]), '/bin/bash') + eq(ffi.string(argv[1]), '-c') + eq(ffi.string(argv[2]), '"echo -n some text"') + eq(nil, argv[3]) + end) + end) end) From 395ef5642e6e61d8f59d1ef67dd7d203b9bb72e6 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Sun, 11 Sep 2016 01:50:58 +0200 Subject: [PATCH 2/2] shell_escape: rename; refactor - rename to shell_xescape_xquote - move to os/shell.c - disallow NULL argument - eliminate casts, nesting - test: empty shellxquote/shellxescape --- src/nvim/os/shell.c | 88 ++++++++++++++++--------------------- test/unit/os/shell_spec.lua | 33 +++++++------- 2 files changed, 56 insertions(+), 65 deletions(-) diff --git a/src/nvim/os/shell.c b/src/nvim/os/shell.c index 5cc15e5a2e..99ae3af221 100644 --- a/src/nvim/os/shell.c +++ b/src/nvim/os/shell.c @@ -37,51 +37,6 @@ typedef struct { # include "os/shell.c.generated.h" #endif -/// Process command string with 'shellxescape' (p_sxe) and 'shellxquote' -/// (p_sxq) -/// -/// @param cmd Command string -/// @return NULL if `cmd` is NULL. Otherwise, a newly allocated command string. -/// It must be freed with `xfree` when no longer needed. -static char *shell_escape(const char *cmd) - FUNC_ATTR_MALLOC FUNC_ATTR_WARN_UNUSED_RESULT -{ - char *ncmd; - - if (cmd == NULL) { - ncmd = NULL; - } else if (*p_sxq == NUL) { - ncmd = xstrdup(cmd); - } else { - const char *ecmd; - size_t ncmd_size; - - if (*p_sxe != NUL && STRCMP(p_sxq, "(") == 0) { - ecmd = (char *)vim_strsave_escaped_ext((char_u *)cmd, p_sxe, '^', false); - } else { - ecmd = cmd; - } - ncmd_size = strlen(ecmd) + STRLEN(p_sxq) * 2 + 1; - ncmd = xmalloc(ncmd_size); - - // When 'shellxquote' is '(', append ')'. - // When 'shellxquote' is '"(', append ')"'. - if (STRCMP(p_sxq, "(") == 0) { - vim_snprintf(ncmd, ncmd_size, "(%s)", ecmd); - } else if (STRCMP(p_sxq, "\"(") == 0) { - vim_snprintf(ncmd, ncmd_size, "\"(%s)\"", ecmd); - } else { - vim_snprintf(ncmd, ncmd_size, "%s%s%s", p_sxq, ecmd, p_sxq); - } - - if (ecmd != (const char *)cmd) { - xfree((void *)ecmd); - } - } - - return ncmd; -} - /// Builds the argument vector for running the user-configured 'shell' (p_sh) /// with an optional command prefixed by 'shellcmdflag' (p_shcf). /// @@ -99,13 +54,12 @@ char **shell_build_argv(const char *cmd, const char *extra_args) size_t i = tokenize(p_sh, rv); if (extra_args) { - rv[i++] = xstrdup(extra_args); // Push a copy of `extra_args` + rv[i++] = xstrdup(extra_args); // Push a copy of `extra_args` } if (cmd) { - i += tokenize(p_shcf, rv + i); // Split 'shellcmdflag' - rv[i++] = shell_escape(cmd); // Process command string with - // 'shellxescape' and 'shellxquote' + i += tokenize(p_shcf, rv + i); // Split 'shellcmdflag' + rv[i++] = shell_xescape_xquote(cmd); // Copy (and escape) `cmd`. } rv[i] = NULL; @@ -594,3 +548,39 @@ static void shell_write_cb(Stream *stream, void *data, int status) { stream_close(stream, NULL, NULL); } + +/// Applies 'shellxescape' (p_sxe) and 'shellxquote' (p_sxq) to a command. +/// +/// @param cmd Command string +/// @return Escaped/quoted command string (allocated). +static char *shell_xescape_xquote(const char *cmd) + FUNC_ATTR_NONNULL_ALL FUNC_ATTR_MALLOC FUNC_ATTR_WARN_UNUSED_RESULT +{ + if (*p_sxq == NUL) { + return xstrdup(cmd); + } + + const char *ecmd = cmd; + if (*p_sxe != NUL && STRCMP(p_sxq, "(") == 0) { + ecmd = (char *)vim_strsave_escaped_ext((char_u *)cmd, p_sxe, '^', false); + } + size_t ncmd_size = strlen(ecmd) + STRLEN(p_sxq) * 2 + 1; + char *ncmd = xmalloc(ncmd_size); + + // When 'shellxquote' is ( append ). + // When 'shellxquote' is "( append )". + if (STRCMP(p_sxq, "(") == 0) { + vim_snprintf(ncmd, ncmd_size, "(%s)", ecmd); + } else if (STRCMP(p_sxq, "\"(") == 0) { + vim_snprintf(ncmd, ncmd_size, "\"(%s)\"", ecmd); + } else { + vim_snprintf(ncmd, ncmd_size, "%s%s%s", p_sxq, ecmd, p_sxq); + } + + if (ecmd != cmd) { + xfree((void *)ecmd); + } + + return ncmd; +} + diff --git a/test/unit/os/shell_spec.lua b/test/unit/os/shell_spec.lua index 613a2cfc2c..3603403daf 100644 --- a/test/unit/os/shell_spec.lua +++ b/test/unit/os/shell_spec.lua @@ -12,10 +12,12 @@ local to_cstr = helpers.to_cstr local NULL = ffi.cast('void *', 0) describe('shell functions', function() - setup(function() + before_each(function() -- os_system() can't work when the p_sh and p_shcf variables are unset cimported.p_sh = to_cstr('/bin/bash') cimported.p_shcf = to_cstr('-c') + cimported.p_sxq = to_cstr('') + cimported.p_sxe = to_cstr('') end) local function shell_build_argv(cmd, extra_args) @@ -35,13 +37,6 @@ describe('shell functions', function() return ret end - after_each(function() - cimported.p_sxq = to_cstr('') - cimported.p_sxe = to_cstr('') - cimported.p_sh = to_cstr('/bin/bash') - cimported.p_shcf = to_cstr('-c') - end) - local function os_system(cmd, input) local input_or = input and to_cstr(input) or NULL local input_len = (input ~= nil) and string.len(input) or 0 @@ -121,15 +116,11 @@ describe('shell functions', function() '-c', 'abc def'}, shell_build_argv('abc def', 'ghi jkl')) end) - end) - - describe('shell_build_argv can deal with sxe and sxq', function() - setup(function() + + it('applies shellxescape (p_sxe) and shellxquote (p_sxq)', function() cimported.p_sxq = to_cstr('(') cimported.p_sxe = to_cstr('"&|<>()@^') - end) - it('applies shellxescape and shellxquote', function() local argv = ffi.cast('char**', cimported.shell_build_argv(to_cstr('echo &|<>()@^'), nil)) eq(ffi.string(argv[0]), '/bin/bash') @@ -138,8 +129,9 @@ describe('shell functions', function() eq(nil, argv[3]) end) - it('applies shellxquote when shellxquote is "\\"("', function() + it('applies shellxquote="(', function() cimported.p_sxq = to_cstr('"(') + cimported.p_sxe = to_cstr('"&|<>()@^') local argv = ffi.cast('char**', cimported.shell_build_argv( to_cstr('echo -n some text'), nil)) @@ -149,7 +141,7 @@ describe('shell functions', function() eq(nil, argv[3]) end) - it('applies shellxquote when shellxquote is "\\""', function() + it('applies shellxquote="', function() cimported.p_sxq = to_cstr('"') cimported.p_sxe = to_cstr('') @@ -160,5 +152,14 @@ describe('shell functions', function() eq(ffi.string(argv[2]), '"echo -n some text"') eq(nil, argv[3]) end) + + it('with empty shellxquote/shellxescape', function() + local argv = ffi.cast('char**', cimported.shell_build_argv( + to_cstr('echo -n some text'), nil)) + eq(ffi.string(argv[0]), '/bin/bash') + eq(ffi.string(argv[1]), '-c') + eq(ffi.string(argv[2]), 'echo -n some text') + eq(nil, argv[3]) + end) end) end)