patch 9.2.0250: system() does not support bypassing the shell

Problem:  system() and systemlist() only accept a String, requiring
          manual shell escaping for arguments with special characters.
Solution: Accept a List as the first argument and execute the command
          bypassing the shell (Yasuhiro Matsumoto).

fixes:  #19789
closes: #19791

Signed-off-by: Yasuhiro Matsumoto <mattn.jp@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

Author: mattn

runtime/doc/builtin.txt

@@ -699,7 +699,8 @@ synconcealed({lnum}, {col})	List	info about concealing
 synstack({lnum}, {col})		List	stack of syntax IDs at {lnum} and
 					{col}
 system({expr} [, {input}])	String	output of shell command/filter {expr}
-systemlist({expr} [, {input}])	List	output of shell command/filter {expr}
+systemlist({expr} [, {input}])
+				List	output of shell command/filter {expr}
 tabpagebuflist([{arg}])		List	list of buffer numbers in tab page
 tabpagenr([{arg}])		Number	number of current or last tab page
 tabpagewinnr({tabarg} [, {arg}])
@@ -11694,6 +11695,30 @@ system({expr} [, {input}])				*system()* *E677*
 		Get the output of the shell command {expr} as a |String|.  See
 		|systemlist()| to get the output as a |List|.
 
+		{expr} can be a |String| or a |List|.
+		When {expr} is a |String|, the command is executed through the
+		shell (see below for how the command is constructed).
+
+								*E1575*
+		When {expr} is a |List|, the first item is the executable and
+		the remaining items are passed as arguments directly.  The
+		command is executed without using a shell, similar to
+		|job_start()|.  Since no shell is involved, shell features
+		such as redirection, piping, globbing, environment variable
+		expansion and backtick expansion will not work.  Characters
+		like ">" are passed as literal arguments to the command, not
+		interpreted as redirection.  Use this form when arguments may
+		contain special characters that should not be interpreted by
+		the shell.  Example: >
+		    :let out = system(['grep', '-r', 'pattern', '.'])
+<		With the String form ">" would be shell redirection, but
+		with a List it is passed as a literal argument: >
+		    :let out = system(['echo', 'hello', '>', 'world'])
+<		This outputs "hello > world", not redirect to a file.
+
+		To use the shell explicitly with a List: >
+		    :let out = system(['/bin/sh', '-c', 'echo $HOME'])
+<
 		When {input} is given and is a |String| this string is written
 		to a file and passed as stdin to the command.  The string is
 		written as-is, you need to take care of using the correct line
@@ -11719,11 +11744,11 @@ system({expr} [, {input}])				*system()* *E677*
 		being echoed on the screen. >
 			:silent let f = system('ls *.vim')
 <
-		Note: Use |shellescape()| or |::S| with |expand()| or
-		|fnamemodify()| to escape special characters in a command
-		argument.  Newlines in {expr} may cause the command to fail.
-		The characters in 'shellquote' and 'shellxquote' may also
-		cause trouble.
+		Note: When {expr} is a String, use |shellescape()| or |::S|
+		with |expand()| or |fnamemodify()| to escape special
+		characters in a command argument.  Newlines in {expr} may
+		cause the command to fail.  The characters in 'shellquote'
+		and 'shellxquote' may also cause trouble.
 		This is not to be used for interactive commands.
 
 		The result is a String.  Example: >
@@ -11736,7 +11761,8 @@ system({expr} [, {input}])				*system()* *E677*
 		To avoid the string being truncated at a NUL, all NUL
 		characters are replaced with SOH (0x01).
 
-		The command executed is constructed using several options:
+		When {expr} is a String, the command executed is constructed
+		using several options:
 	'shell' 'shellcmdflag' 'shellxquote' {expr} 'shellredir' {tmp} 'shellxquote'
 		({tmp} is an automatically generated file name).
 		For Unix, braces are put around {expr} to allow for
@@ -11763,6 +11789,9 @@ system({expr} [, {input}])				*system()* *E677*
 systemlist({expr} [, {input}])				*systemlist()*
 		Same as |system()|, but returns a |List| with lines (parts of
 		output separated by NL) with NULs transformed into NLs.
+		Like |system()|, {expr} can be a |String| (executed through
+		the shell) or a |List| (executed directly without a shell).
+		See |system()| for details.
 		Output is the same as |readfile()| will output with {binary}
 		argument set to "b", except that there is no extra empty item
 		when the result ends in a NL.

runtime/doc/tags

@@ -4771,6 +4771,7 @@ E1571	builtin.txt	/*E1571*
 E1572	options.txt	/*E1572*
 E1573	channel.txt	/*E1573*
 E1574	channel.txt	/*E1574*
+E1575	builtin.txt	/*E1575*
 E158	sign.txt	/*E158*
 E159	sign.txt	/*E159*
 E16	cmdline.txt	/*E16*

runtime/doc/version9.txt

@@ -52611,6 +52611,8 @@ Other ~
 - New "leadtab" value for the 'listchars' setting.
 - Improved |:set+=|, |:set^=| and |:set-=| handling of comma-separated "key:value"
   pairs individually (e.g. 'listchars', 'fillchars', 'diffopt').
+- |system()| and |systemlist()| functions accept a list as first argument,
+  bypassing the shell completely.
 
 xxd ~
 ---

src/errors.h

@@ -3795,8 +3795,6 @@ EXTERN char e_osc_response_timed_out[]
 #ifdef FEAT_EVAL
 EXTERN char e_cannot_add_listener_in_listener_callback[]
 	INIT(= N_("E1569: Cannot use listener_add in a listener callback"));
-#endif
-#ifdef FEAT_EVAL
 EXTERN char e_cannot_add_redraw_listener_in_listener_callback[]
 	INIT(= N_("E1570: Cannot use redraw_listener_add in a redraw listener callback"));
 EXTERN char e_no_redraw_listener_callbacks_defined[]
@@ -3810,3 +3808,7 @@ EXTERN char e_cannot_listen_on_port[]
 EXTERN char e_gethostbyname_in_channel_listen[]
 	INIT(= N_("E1574: gethostbyname(): cannot resolve hostname in channel_listen()"));
 #endif
+#ifdef FEAT_EVAL
+EXTERN char e_cannot_create_pipes[]
+	INIT(= N_("E1575: Cannot create pipes"));
+#endif

src/evalfunc.c

@@ -1380,7 +1380,7 @@ static argcheck_T arg45_sign_place[] = {arg_number, arg_string, arg_string, arg_
 static argcheck_T arg23_slice[] = {arg_slice1, arg_number, arg_number};
 static argcheck_T arg13_sortuniq[] = {arg_list_any_mod, arg_sort_how, arg_dict_any};
 static argcheck_T arg24_strpart[] = {arg_string, arg_number, arg_number, arg_bool};
-static argcheck_T arg12_system[] = {arg_string, arg_str_or_nr_or_list};
+static argcheck_T arg12_system[] = {arg_string_or_list_any, arg_str_or_nr_or_list};
 static argcheck_T arg23_win_execute[] = {arg_number, arg_string_or_list_string, arg_string};
 static argcheck_T arg23_writefile[] = {arg_list_or_blob, arg_string, arg_string};
 static argcheck_T arg24_match_func[] = {arg_string_or_list_any, arg_string, arg_number, arg_number};

src/misc1.c

@@ -2511,14 +2511,17 @@ get_cmd_output_as_rettv(
     FILE	*fd;
     list_T	*list = NULL;
     int		flags = SHELL_SILENT;
+    int		use_argv = FALSE;
+    char	**argv = NULL;
+    int		argc = 0;
 
     rettv->v_type = VAR_STRING;
     rettv->vval.v_string = NULL;
     if (check_restricted() || check_secure())
 	goto errret;
 
     if (in_vim9script()
-	    && (check_for_string_arg(argvars, 0) == FAIL
+	    && (check_for_string_or_list_arg(argvars, 0) == FAIL
 		|| check_for_opt_string_or_number_or_list_arg(argvars, 1)
 								      == FAIL))
 	return;
@@ -2598,6 +2601,47 @@ get_cmd_output_as_rettv(
 	}
     }
 
+    // When the command is a List, execute directly without the shell.
+    if (argvars[0].v_type == VAR_LIST)
+    {
+	list_T	*l = argvars[0].vval.v_list;
+
+	if (l == NULL || l->lv_len < 1)
+	{
+	    emsg(_(e_invalid_argument));
+	    goto errret;
+	}
+	if (build_argv_from_list(l, &argv, &argc) == FAIL)
+	    goto errret;
+	if (argc == 0 || *skipwhite((char_u *)argv[0]) == NUL)
+	{
+	    emsg(_(e_invalid_argument));
+	    goto errret;
+	}
+	use_argv = TRUE;
+
+	if (p_verbose > 3)
+	{
+	    int		i;
+	    garray_T	ga;
+
+	    verbose_enter();
+	    ga_init2(&ga, 1, 200);
+	    for (i = 0; i < argc; ++i)
+	    {
+		if (i > 0)
+		    ga_append(&ga, ' ');
+		ga_concat(&ga, (char_u *)argv[i]);
+	    }
+	    ga_append(&ga, NUL);
+	    smsg(_("Executing directly: \"%s\""), (char *)ga.ga_data);
+	    msg_putchar_attr('\n', 0);
+	    cursor_on();
+	    verbose_leave();
+	    ga_clear(&ga);
+	}
+    }
+
     // Omit SHELL_COOKED when invoked with ":silent".  Avoids that the shell
     // echoes typeahead, that messes up the display.
     if (!msg_silent)
@@ -2612,7 +2656,10 @@ get_cmd_output_as_rettv(
 	char_u		*end;
 	int		i;
 
-	res = get_cmd_output(tv_get_string(&argvars[0]), infile, flags, &len);
+	if (use_argv)
+	    res = mch_get_cmd_output_direct(argv, infile, flags, &len);
+	else
+	    res = get_cmd_output(tv_get_string(&argvars[0]), infile, flags, &len);
 	if (res == NULL)
 	    goto errret;
 
@@ -2652,7 +2699,10 @@ get_cmd_output_as_rettv(
     }
     else
     {
-	res = get_cmd_output(tv_get_string(&argvars[0]), infile, flags, NULL);
+	if (use_argv)
+	    res = mch_get_cmd_output_direct(argv, infile, flags, NULL);
+	else
+	    res = get_cmd_output(tv_get_string(&argvars[0]), infile, flags, NULL);
 #  ifdef USE_CRNL
 	// translate <CR><NL> into <NL>
 	if (res != NULL)
@@ -2674,6 +2724,13 @@ get_cmd_output_as_rettv(
     }
 
 errret:
+    if (argv != NULL)
+    {
+	int i;
+	for (i = 0; argv[i] != NULL; i++)
+	    vim_free(argv[i]);
+	vim_free(argv);
+    }
     if (infile != NULL)
     {
 	mch_remove(infile);

src/os_unix.c

@@ -5915,6 +5915,154 @@ mch_call_shell(
 #endif
 }
 
+#if defined(FEAT_EVAL)
+/*
+ * Execute "argv" directly without the shell and return the output.
+ * Used by system() and systemlist() when the command is a List.
+ * "infile" is an optional temp file for stdin input.
+ * "flags" is SHELL_SILENT etc.
+ * When "ret_len" is not NULL, set it to the length of the output.
+ * Returns the output in allocated memory (or NULL on error).
+ * Sets v:shell_error to the exit status.
+ */
+    char_u *
+mch_get_cmd_output_direct(
+    char	**argv,
+    char_u	*infile,
+    int		flags UNUSED,
+    int		*ret_len)
+{
+    pid_t	pid;
+    int		fd_out[2] = {-1, -1};
+    int		status = -1;
+    char_u	*buffer = NULL;
+    garray_T	ga;
+    SIGSET_DECL(curset)
+
+    ga_init2(&ga, 1, 4096);
+
+    ch_log(NULL, "directly executing: %s", argv[0]);
+
+    if (pipe(fd_out) < 0)
+    {
+	emsg(_(e_cannot_create_pipes));
+	return NULL;
+    }
+
+    BLOCK_SIGNALS(&curset);
+    pid = fork();
+    if (pid == -1)
+    {
+	UNBLOCK_SIGNALS(&curset);
+	close(fd_out[0]);
+	close(fd_out[1]);
+	emsg(_("\nCannot fork\n"));
+	return NULL;
+    }
+
+    if (pid == 0)
+    {
+	// child process
+	reset_signals();
+	UNBLOCK_SIGNALS(&curset);
+
+	if (ch_log_active())
+	{
+	    ch_log(NULL, "closing channel log in the child process");
+	    ch_logfile((char_u *)"", (char_u *)"");
+	}
+
+	// Set up stdin.
+	if (infile != NULL)
+	{
+	    int fd_in = open((char *)infile, O_RDONLY);
+	    if (fd_in >= 0)
+	    {
+		close(0);
+		vim_ignored = dup(fd_in);
+		close(fd_in);
+	    }
+	}
+	else
+	{
+	    int nullfd = open("/dev/null", O_RDONLY);
+	    if (nullfd >= 0)
+	    {
+		close(0);
+		vim_ignored = dup(nullfd);
+		close(nullfd);
+	    }
+	}
+
+	// Set up stdout: write end of pipe.
+	close(fd_out[0]);
+	close(1);
+	vim_ignored = dup(fd_out[1]);
+	// Also redirect stderr to the pipe.
+	close(2);
+	vim_ignored = dup(fd_out[1]);
+	close(fd_out[1]);
+
+	execvp(argv[0], argv);
+	_exit(127);
+	// NOTREACHED
+    }
+
+    // parent process
+    UNBLOCK_SIGNALS(&curset);
+    close(fd_out[1]);
+
+    // Read output from child.
+    for (;;)
+    {
+	char	buf[4096];
+	int	n;
+
+	n = (int)read(fd_out[0], buf, sizeof(buf));
+	if (n <= 0)
+	    break;
+	ga_grow(&ga, n);
+	mch_memmove((char *)ga.ga_data + ga.ga_len, buf, n);
+	ga.ga_len += n;
+    }
+    close(fd_out[0]);
+
+    // Wait for child to finish.
+    (void)waitpid(pid, &status, 0);
+    if (WIFEXITED(status))
+	status = WEXITSTATUS(status);
+    else
+	status = -1;
+    set_vim_var_nr(VV_SHELL_ERROR, (long)status);
+
+    if (ga.ga_len > 0)
+    {
+	buffer = alloc(ga.ga_len + 1);
+	if (buffer != NULL)
+	{
+	    mch_memmove(buffer, ga.ga_data, ga.ga_len);
+	    if (ret_len == NULL)
+	    {
+		int	i;
+
+		// Change NUL into SOH, otherwise the string is truncated.
+		for (i = 0; i < ga.ga_len; ++i)
+		    if (buffer[i] == NUL)
+			buffer[i] = 1;
+		buffer[ga.ga_len] = NUL;
+	    }
+	    else
+		*ret_len = ga.ga_len;
+	}
+    }
+    else if (ret_len != NULL)
+	*ret_len = 0;
+
+    ga_clear(&ga);
+    return buffer;
+}
+#endif
+
 #if defined(FEAT_JOB_CHANNEL)
     void
 mch_job_start(char **argv, job_T *job, jobopt_T *options, int is_terminal)