system(): Respect 'sxe' and 'sxq' #2789

Fixes #2773

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)