Merge #2789 'system(): Respect shellxescape, shellxquote'

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);
-    }
   }
 
   set_vim_var_nr(VV_SHELL_ERROR, (varnumber_T) retval);

src/nvim/os/shell.c

@@ -59,7 +59,7 @@ 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_xescape_xquote(cmd);  // Copy (and escape) `cmd`.
   }
 
   rv[i] = NULL;
@@ -548,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;
+}
+

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',
@@ -24,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)
@@ -126,5 +116,50 @@ describe('shell functions', function()
           '-c', 'abc  def'},
          shell_build_argv('abc  def', 'ghi  jkl'))
     end)
+
+    it('applies shellxescape (p_sxe) and shellxquote (p_sxq)', function()
+      cimported.p_sxq = to_cstr('(')
+      cimported.p_sxe = to_cstr('"&|<>()@^')
+
+      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="(', 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)
+
+    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))
+      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('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)