Permalink
Browse files

patch 8.0.0928: MS-Windows: passing arglist to job has escaping problems

Problem:    MS-Windows: passing arglist to job has escaping problems.
Solution:   Improve escaping. (Yasuhiro Matsumoto, closes #1954)
  • Loading branch information...
brammool committed Aug 13, 2017
1 parent 274a52f commit dcaa61384ca76e42f7feda5640fb85b58cee03e5
Showing with 226 additions and 80 deletions.
  1. +108 −29 src/channel.c
  2. +1 −0 src/proto/channel.pro
  3. +43 −24 src/terminal.c
  4. +65 −27 src/testdir/test_channel.vim
  5. +7 −0 src/testdir/test_terminal.vim
  6. +2 −0 src/version.c
View
@@ -4720,6 +4720,111 @@ job_still_useful(job_T *job)
return job_need_end_check(job) || job_channel_still_useful(job);
}
#if !defined(USE_ARGV) || defined(PROTO)
/*
* Escape one argument for an external command.
* Returns the escaped string in allocated memory. NULL when out of memory.
*/
static char_u *
win32_escape_arg(char_u *arg)
{
int slen, dlen;
int escaping = 0;
int i;
char_u *s, *d;
char_u *escaped_arg;
int has_spaces = FALSE;
/* First count the number of extra bytes required. */
slen = STRLEN(arg);
dlen = slen;
for (s = arg; *s != NUL; MB_PTR_ADV(s))
{
if (*s == '"' || *s == '\\')
++dlen;
if (*s == ' ' || *s == '\t')
has_spaces = TRUE;
}
if (has_spaces)
dlen += 2;
if (dlen == slen)
return vim_strsave(arg);
/* Allocate memory for the result and fill it. */
escaped_arg = alloc(dlen + 1);
if (escaped_arg == NULL)
return NULL;
memset(escaped_arg, 0, dlen+1);
d = escaped_arg;
if (has_spaces)
*d++ = '"';
for (s = arg; *s != NUL;)
{
switch (*s)
{
case '"':
for (i = 0; i < escaping; i++)
*d++ = '\\';
escaping = 0;
*d++ = '\\';
*d++ = *s++;
break;
case '\\':
escaping++;
*d++ = *s++;
break;
default:
escaping = 0;
MB_COPY_CHAR(s, d);
break;
}
}
/* add terminating quote and finish with a NUL */
if (has_spaces)
{
for (i = 0; i < escaping; i++)
*d++ = '\\';
*d++ = '"';
}
*d = NUL;
return escaped_arg;
}
/*
* Build a command line from a list, taking care of escaping.
* The result is put in gap->ga_data.
* Returns FAIL when out of memory.
*/
int
win32_build_cmd(list_T *l, garray_T *gap)
{
listitem_T *li;
char_u *s;
for (li = l->lv_first; li != NULL; li = li->li_next)
{
s = get_tv_string_chk(&li->li_tv);
if (s == NULL)
return FAIL;
s = win32_escape_arg(s);
if (s == NULL)
return FAIL;
ga_concat(gap, s);
vim_free(s);
if (li->li_next != NULL)
ga_append(gap, ' ');
}
return OK;
}
#endif
/*
* NOTE: Must call job_cleanup() only once right after the status of "job"
* changed to JOB_ENDED (i.e. after job_status() returned "dead" first or
@@ -5093,51 +5198,25 @@ job_start(typval_T *argvars, jobopt_T *opt_arg)
else
{
list_T *l = argvars[0].vval.v_list;
#ifdef USE_ARGV
listitem_T *li;
char_u *s;
#ifdef USE_ARGV
/* Pass argv[] to mch_call_shell(). */
argv = (char **)alloc(sizeof(char *) * (l->lv_len + 1));
if (argv == NULL)
goto theend;
#endif
for (li = l->lv_first; li != NULL; li = li->li_next)
{
s = get_tv_string_chk(&li->li_tv);
if (s == NULL)
goto theend;
#ifdef USE_ARGV
argv[argc++] = (char *)s;
#else
/* Only escape when needed, double quotes are not always allowed. */
if (li != l->lv_first && vim_strpbrk(s, (char_u *)" \t\"") != NULL)
{
# ifdef WIN32
int old_ssl = p_ssl;
/* This is using CreateProcess, not cmd.exe. Always use
* double quote and backslashes. */
p_ssl = 0;
# endif
s = vim_strsave_shellescape(s, FALSE, TRUE);
# ifdef WIN32
p_ssl = old_ssl;
# endif
if (s == NULL)
goto theend;
ga_concat(&ga, s);
vim_free(s);
}
else
ga_concat(&ga, s);
if (li->li_next != NULL)
ga_append(&ga, ' ');
#endif
}
#ifdef USE_ARGV
argv[argc] = NULL;
#else
if (win32_build_cmd(l, &ga) == FAIL)
goto theend;
cmd = ga.ga_data;
#endif
}
View
@@ -54,6 +54,7 @@ void free_job_options(jobopt_T *opt);
int get_job_options(typval_T *tv, jobopt_T *opt, int supported, int supported2);
channel_T *get_channel_arg(typval_T *tv, int check_open, int reading, ch_part_T part);
void job_free_all(void);
int win32_build_cmd(list_T *l, garray_T *gap);
void job_cleanup(job_T *job);
int set_ref_in_job(int copyID);
void job_unref(job_T *job);
View
@@ -39,7 +39,6 @@
*
* TODO:
* - Make argument list work on MS-Windows. #1954
* - MS-Windows: no redraw for 'updatetime' #1915
* - To set BS correctly, check get_stty(); Pass the fd of the pty.
* For the GUI fill termios with default values, perhaps like pangoterm:
* http://bazaar.launchpad.net/~leonerd/pangoterm/trunk/view/head:/main.c#L134
@@ -165,7 +164,8 @@ static term_T *in_terminal_loop = NULL;
/*
* Functions with separate implementation for MS-Windows and Unix-like systems.
*/
static int term_and_job_init(term_T *term, int rows, int cols, char_u *cmd, jobopt_T *opt);
static int term_and_job_init(term_T *term, int rows, int cols,
typval_T *argvar, jobopt_T *opt);
static void term_report_winsize(term_T *term, int rows, int cols);
static void term_free_vterm(term_T *term);
@@ -244,7 +244,7 @@ setup_job_options(jobopt_T *opt, int rows, int cols)
}
static void
term_start(char_u *cmd, jobopt_T *opt, int forceit)
term_start(typval_T *argvar, jobopt_T *opt, int forceit)
{
exarg_T split_ea;
win_T *old_curwin = curwin;
@@ -340,16 +340,25 @@ term_start(char_u *cmd, jobopt_T *opt, int forceit)
term->tl_next = first_term;
first_term = term;
if (cmd == NULL || *cmd == NUL)
cmd = p_sh;
if (opt->jo_term_name != NULL)
curbuf->b_ffname = vim_strsave(opt->jo_term_name);
else
{
int i;
size_t len = STRLEN(cmd) + 10;
char_u *p = alloc((int)len);
size_t len;
char_u *cmd, *p;
if (argvar->v_type == VAR_STRING)
cmd = argvar->vval.v_string;
else if (argvar->v_type != VAR_LIST
|| argvar->vval.v_list == NULL
|| argvar->vval.v_list->lv_len < 1)
cmd = (char_u*)"";
else
cmd = get_tv_string_chk(&argvar->vval.v_list->lv_first->li_tv);
len = STRLEN(cmd) + 10;
p = alloc((int)len);
for (i = 0; p != NULL; ++i)
{
@@ -386,7 +395,7 @@ term_start(char_u *cmd, jobopt_T *opt, int forceit)
setup_job_options(opt, term->tl_rows, term->tl_cols);
/* System dependent: setup the vterm and start the job in it. */
if (term_and_job_init(term, term->tl_rows, term->tl_cols, cmd, opt) == OK)
if (term_and_job_init(term, term->tl_rows, term->tl_cols, argvar, opt) == OK)
{
/* Get and remember the size we ended up with. Update the pty. */
vterm_get_size(term->tl_vterm, &term->tl_rows, &term->tl_cols);
@@ -425,6 +434,7 @@ term_start(char_u *cmd, jobopt_T *opt, int forceit)
void
ex_terminal(exarg_T *eap)
{
typval_T argvar;
jobopt_T opt;
char_u *cmd;
@@ -468,7 +478,9 @@ ex_terminal(exarg_T *eap)
opt.jo_term_rows = eap->line2;
}
term_start(cmd, &opt, eap->forceit);
argvar.v_type = VAR_STRING;
argvar.vval.v_string = cmd;
term_start(&argvar, &opt, eap->forceit);
}
/*
@@ -2585,11 +2597,8 @@ f_term_sendkeys(typval_T *argvars, typval_T *rettv)
void
f_term_start(typval_T *argvars, typval_T *rettv)
{
char_u *cmd = get_tv_string_chk(&argvars[0]);
jobopt_T opt;
if (cmd == NULL)
return;
init_job_options(&opt);
/* TODO: allow more job options */
if (argvars[1].v_type != VAR_UNKNOWN
@@ -2603,7 +2612,7 @@ f_term_start(typval_T *argvars, typval_T *rettv)
if (opt.jo_vertical)
cmdmod.split = WSP_VERT;
term_start(cmd, &opt, FALSE);
term_start(&argvars[0], &opt, FALSE);
if (curbuf->b_term != NULL)
rettv->vval.v_number = curbuf->b_fnum;
@@ -2749,20 +2758,32 @@ dyn_winpty_init(void)
* Return OK or FAIL.
*/
static int
term_and_job_init(term_T *term, int rows, int cols, char_u *cmd, jobopt_T *opt)
term_and_job_init(term_T *term, int rows, int cols, typval_T *argvar, jobopt_T *opt)
{
WCHAR *p;
WCHAR *p = NULL;
channel_T *channel = NULL;
job_T *job = NULL;
DWORD error;
HANDLE jo = NULL, child_process_handle, child_thread_handle;
void *winpty_err;
void *spawn_config = NULL;
char buf[MAX_PATH];
garray_T ga;
char_u *cmd;
if (!dyn_winpty_init())
return FAIL;
if (argvar->v_type == VAR_STRING)
cmd = argvar->vval.v_string;
else
{
ga_init2(&ga, (int)sizeof(char*), 20);
if (win32_build_cmd(argvar->vval.v_list, &ga) == FAIL)
goto failed;
cmd = ga.ga_data;
}
p = enc_to_utf16(cmd, NULL);
if (p == NULL)
return FAIL;
@@ -2855,9 +2876,12 @@ term_and_job_init(term_T *term, int rows, int cols, char_u *cmd, jobopt_T *opt)
return OK;
failed:
if (argvar->v_type == VAR_LIST)
vim_free(ga.ga_data);
if (p != NULL)
vim_free(p);
if (spawn_config != NULL)
winpty_spawn_config_free(spawn_config);
vim_free(p);
if (channel != NULL)
channel_clear(channel);
if (job != NULL)
@@ -2924,17 +2948,12 @@ term_report_winsize(term_T *term, int rows, int cols)
* Return OK or FAIL.
*/
static int
term_and_job_init(term_T *term, int rows, int cols, char_u *cmd, jobopt_T *opt)
term_and_job_init(term_T *term, int rows, int cols, typval_T *argvar, jobopt_T *opt)
{
typval_T argvars[2];
create_vterm(term, rows, cols);
/* TODO: if the command is "NONE" only create a pty. */
argvars[0].v_type = VAR_STRING;
argvars[0].vval.v_string = cmd;
term->tl_job = job_start(argvars, opt);
term->tl_job = job_start(argvar, opt);
if (term->tl_job != NULL)
++term->tl_job->jv_refcount;
Oops, something went wrong.

1 comment on commit dcaa613

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Aug 13, 2017

Oops, sorry, :terminal doesn't run default shell.

mattn commented on dcaa613 Aug 13, 2017

Oops, sorry, :terminal doesn't run default shell.

Please sign in to comment.