To: vim_dev@googlegroups.com Subject: Patch 8.0.0928 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.0.0928 Problem: MS-Windows: passing arglist to job has escaping problems. Solution: Improve escaping. (Yasuhiro Matsumoto, closes #1954) Files: src/testdir/test_channel.vim, src/testdir/test_terminal.vim, src/channel.c, src/proto/channel.pro, src/terminal.c *** ../vim-8.0.0927/src/testdir/test_channel.vim 2017-08-11 19:12:05.060966304 +0200 --- src/testdir/test_channel.vim 2017-08-13 17:01:44.797830888 +0200 *************** *** 1636,1647 **** endif let g:linecount = 0 ! if has('win32') ! " workaround: 'shellescape' does improper escaping double quotes ! let arg = 'import sys;sys.stdout.write(\"1\n2\n3\")' ! else ! let arg = 'import sys;sys.stdout.write("1\n2\n3")' ! endif call job_start([s:python, '-c', arg], {'callback': 'MyLineCountCb'}) call WaitFor('3 <= g:linecount') call assert_equal(3, g:linecount) --- 1636,1642 ---- endif let g:linecount = 0 ! let arg = 'import sys;sys.stdout.write("1\n2\n3")' call job_start([s:python, '-c', arg], {'callback': 'MyLineCountCb'}) call WaitFor('3 <= g:linecount') call assert_equal(3, g:linecount) *************** *** 1653,1664 **** endif let g:linecount = 0 ! if has('win32') ! " workaround: 'shellescape' does improper escaping double quotes ! let arg = 'import os,sys;os.close(1);sys.stderr.write(\"test\n\")' ! else ! let arg = 'import os,sys;os.close(1);sys.stderr.write("test\n")' ! endif call job_start([s:python, '-c', arg], {'callback': 'MyLineCountCb'}) call WaitFor('1 <= g:linecount') call assert_equal(1, g:linecount) --- 1648,1654 ---- endif let g:linecount = 0 ! let arg = 'import os,sys;os.close(1);sys.stderr.write("test\n")' call job_start([s:python, '-c', arg], {'callback': 'MyLineCountCb'}) call WaitFor('1 <= g:linecount') call assert_equal(1, g:linecount) *************** *** 1669,1683 **** return endif ! let s:envstr = '' if has('win32') ! call job_start(['cmd', '/c', 'echo %FOO%'], {'callback': {ch,msg->execute(":let s:envstr .= msg")}, 'env':{'FOO': 'bar'}}) else ! call job_start([&shell, &shellcmdflag, 'echo $FOO'], {'callback': {ch,msg->execute(":let s:envstr .= msg")}, 'env':{'FOO': 'bar'}}) endif ! call WaitFor('"" != s:envstr') ! call assert_equal("bar", s:envstr) ! unlet s:envstr endfunc func Test_cwd() --- 1659,1673 ---- return endif ! let g:envstr = '' if has('win32') ! call job_start(['cmd', '/c', 'echo %FOO%'], {'callback': {ch,msg->execute(":let g:envstr .= msg")}, 'env':{'FOO': 'bar'}}) else ! call job_start([&shell, &shellcmdflag, 'echo $FOO'], {'callback': {ch,msg->execute(":let g:envstr .= msg")}, 'env':{'FOO': 'bar'}}) endif ! call WaitFor('"" != g:envstr') ! call assert_equal("bar", g:envstr) ! unlet g:envstr endfunc func Test_cwd() *************** *** 1685,1706 **** return endif ! let s:envstr = '' if has('win32') let expect = $TEMP ! call job_start(['cmd', '/c', 'echo %CD%'], {'callback': {ch,msg->execute(":let s:envstr .= msg")}, 'cwd': expect}) else let expect = $HOME ! call job_start(['pwd'], {'callback': {ch,msg->execute(":let s:envstr .= msg")}, 'cwd': expect}) endif ! call WaitFor('"" != s:envstr') let expect = substitute(expect, '[/\\]$', '', '') ! let s:envstr = substitute(s:envstr, '[/\\]$', '', '') ! if $CI != '' && stridx(s:envstr, '/private/') == 0 ! let s:envstr = s:envstr[8:] endif ! call assert_equal(expect, s:envstr) ! unlet s:envstr endfunc function Ch_test_close_lambda(port) --- 1675,1696 ---- return endif ! let g:envstr = '' if has('win32') let expect = $TEMP ! call job_start(['cmd', '/c', 'echo %CD%'], {'callback': {ch,msg->execute(":let g:envstr .= msg")}, 'cwd': expect}) else let expect = $HOME ! call job_start(['pwd'], {'callback': {ch,msg->execute(":let g:envstr .= msg")}, 'cwd': expect}) endif ! call WaitFor('"" != g:envstr') let expect = substitute(expect, '[/\\]$', '', '') ! let g:envstr = substitute(g:envstr, '[/\\]$', '', '') ! if $CI != '' && stridx(g:envstr, '/private/') == 0 ! let g:envstr = g:envstr[8:] endif ! call assert_equal(expect, g:envstr) ! unlet g:envstr endfunc function Ch_test_close_lambda(port) *************** *** 1721,1723 **** --- 1711,1761 ---- call ch_log('Test_close_lambda()') call s:run_server('Ch_test_close_lambda') endfunc + + func s:test_list_args(cmd, out, remove_lf) + try + let g:out = '' + call job_start([s:python, '-c', a:cmd], {'callback': {ch, msg -> execute('let g:out .= msg')}, 'out_mode': 'raw'}) + call WaitFor('"" != g:out') + if has('win32') + let g:out = substitute(g:out, '\r', '', 'g') + endif + if a:remove_lf + let g:out = substitute(g:out, '\n$', '', 'g') + endif + call assert_equal(a:out, g:out) + finally + unlet g:out + endtry + endfunc + + func Test_list_args() + if !has('job') + return + endif + + call s:test_list_args('import sys;sys.stdout.write("hello world")', "hello world", 0) + call s:test_list_args('import sys;sys.stdout.write("hello\nworld")', "hello\nworld", 0) + call s:test_list_args('import sys;sys.stdout.write(''hello\nworld'')', "hello\nworld", 0) + call s:test_list_args('import sys;sys.stdout.write(''hello"world'')', "hello\"world", 0) + call s:test_list_args('import sys;sys.stdout.write(''hello^world'')', "hello^world", 0) + call s:test_list_args('import sys;sys.stdout.write("hello&&world")', "hello&&world", 0) + call s:test_list_args('import sys;sys.stdout.write(''hello\\world'')', "hello\\world", 0) + call s:test_list_args('import sys;sys.stdout.write(''hello\\\\world'')', "hello\\\\world", 0) + call s:test_list_args('import sys;sys.stdout.write("hello\"world\"")', 'hello"world"', 0) + call s:test_list_args('import sys;sys.stdout.write("h\"ello worl\"d")', 'h"ello worl"d', 0) + call s:test_list_args('import sys;sys.stdout.write("h\"e\\\"llo wor\\\"l\"d")', 'h"e\"llo wor\"l"d', 0) + call s:test_list_args('import sys;sys.stdout.write("h\"e\\\"llo world")', 'h"e\"llo world', 0) + call s:test_list_args('import sys;sys.stdout.write("hello\tworld")', "hello\tworld", 0) + + " tests which not contain spaces in the argument + call s:test_list_args('print("hello\nworld")', "hello\nworld", 1) + call s:test_list_args('print(''hello\nworld'')', "hello\nworld", 1) + call s:test_list_args('print(''hello"world'')', "hello\"world", 1) + call s:test_list_args('print(''hello^world'')', "hello^world", 1) + call s:test_list_args('print("hello&&world")', "hello&&world", 1) + call s:test_list_args('print(''hello\\world'')', "hello\\world", 1) + call s:test_list_args('print(''hello\\\\world'')', "hello\\\\world", 1) + call s:test_list_args('print("hello\"world\"")', 'hello"world"', 1) + call s:test_list_args('print("hello\tworld")', "hello\tworld", 1) + endfunc *** ../vim-8.0.0927/src/testdir/test_terminal.vim 2017-08-13 14:13:15.252878860 +0200 --- src/testdir/test_terminal.vim 2017-08-13 16:18:38.514106507 +0200 *************** *** 434,436 **** --- 434,443 ---- unlet g:job endfunc + + func Test_terminal_list_args() + let buf = term_start([&shell, &shellcmdflag, 'echo "123"']) + call assert_fails(buf . 'bwipe', 'E517') + exe buf . 'bwipe!' + call assert_equal("", bufname(buf)) + endfunction *** ../vim-8.0.0927/src/channel.c 2017-08-12 16:01:00.682997047 +0200 --- src/channel.c 2017-08-13 16:46:56.519466488 +0200 *************** *** 4720,4725 **** --- 4720,4830 ---- 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,5143 **** else { list_T *l = argvars[0].vval.v_list; 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 cmd = ga.ga_data; #endif } --- 5198,5222 ---- else { list_T *l = argvars[0].vval.v_list; + #ifdef USE_ARGV listitem_T *li; char_u *s; /* Pass argv[] to mch_call_shell(). */ argv = (char **)alloc(sizeof(char *) * (l->lv_len + 1)); if (argv == NULL) goto theend; for (li = l->lv_first; li != NULL; li = li->li_next) { s = get_tv_string_chk(&li->li_tv); if (s == NULL) goto theend; argv[argc++] = (char *)s; } argv[argc] = NULL; #else + if (win32_build_cmd(l, &ga) == FAIL) + goto theend; cmd = ga.ga_data; #endif } *** ../vim-8.0.0927/src/proto/channel.pro 2017-08-11 21:51:18.808637183 +0200 --- src/proto/channel.pro 2017-08-13 16:48:58.574688569 +0200 *************** *** 54,59 **** --- 54,60 ---- 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); *** ../vim-8.0.0927/src/terminal.c 2017-08-13 16:09:27.917624976 +0200 --- src/terminal.c 2017-08-13 16:48:54.198716434 +0200 *************** *** 39,45 **** * * 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 --- 39,44 ---- *************** *** 165,171 **** /* * 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 void term_report_winsize(term_T *term, int rows, int cols); static void term_free_vterm(term_T *term); --- 164,171 ---- /* * Functions with separate implementation for MS-Windows and Unix-like systems. */ ! 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,250 **** } static void ! term_start(char_u *cmd, jobopt_T *opt, int forceit) { exarg_T split_ea; win_T *old_curwin = curwin; --- 244,250 ---- } static void ! term_start(typval_T *argvar, jobopt_T *opt, int forceit) { exarg_T split_ea; win_T *old_curwin = curwin; *************** *** 340,355 **** 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); for (i = 0; p != NULL; ++i) { --- 340,364 ---- term->tl_next = first_term; first_term = term; if (opt->jo_term_name != NULL) curbuf->b_ffname = vim_strsave(opt->jo_term_name); else { int i; ! 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,392 **** 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) { /* Get and remember the size we ended up with. Update the pty. */ vterm_get_size(term->tl_vterm, &term->tl_rows, &term->tl_cols); --- 395,401 ---- 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, 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,430 **** --- 434,440 ---- void ex_terminal(exarg_T *eap) { + typval_T argvar; jobopt_T opt; char_u *cmd; *************** *** 468,474 **** opt.jo_term_rows = eap->line2; } ! term_start(cmd, &opt, eap->forceit); } /* --- 478,486 ---- opt.jo_term_rows = eap->line2; } ! argvar.v_type = VAR_STRING; ! argvar.vval.v_string = cmd; ! term_start(&argvar, &opt, eap->forceit); } /* *************** *** 2585,2595 **** 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 --- 2597,2604 ---- *************** *** 2603,2609 **** if (opt.jo_vertical) cmdmod.split = WSP_VERT; ! term_start(cmd, &opt, FALSE); if (curbuf->b_term != NULL) rettv->vval.v_number = curbuf->b_fnum; --- 2612,2618 ---- if (opt.jo_vertical) cmdmod.split = WSP_VERT; ! term_start(&argvars[0], &opt, FALSE); if (curbuf->b_term != NULL) rettv->vval.v_number = curbuf->b_fnum; *************** *** 2749,2757 **** * Return OK or FAIL. */ static int ! term_and_job_init(term_T *term, int rows, int cols, char_u *cmd, jobopt_T *opt) { ! WCHAR *p; channel_T *channel = NULL; job_T *job = NULL; DWORD error; --- 2758,2766 ---- * Return OK or FAIL. */ static int ! term_and_job_init(term_T *term, int rows, int cols, typval_T *argvar, jobopt_T *opt) { ! WCHAR *p = NULL; channel_T *channel = NULL; job_T *job = NULL; DWORD error; *************** *** 2759,2768 **** --- 2768,2789 ---- 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,2863 **** return OK; failed: if (spawn_config != NULL) winpty_spawn_config_free(spawn_config); - vim_free(p); if (channel != NULL) channel_clear(channel); if (job != NULL) --- 2876,2887 ---- 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); if (channel != NULL) channel_clear(channel); if (job != NULL) *************** *** 2924,2940 **** * Return OK or FAIL. */ static int ! term_and_job_init(term_T *term, int rows, int cols, char_u *cmd, 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); if (term->tl_job != NULL) ++term->tl_job->jv_refcount; --- 2948,2959 ---- * Return OK or FAIL. */ static int ! term_and_job_init(term_T *term, int rows, int cols, typval_T *argvar, jobopt_T *opt) { create_vterm(term, rows, cols); /* TODO: if the command is "NONE" only create a pty. */ ! term->tl_job = job_start(argvar, opt); if (term->tl_job != NULL) ++term->tl_job->jv_refcount; *** ../vim-8.0.0927/src/version.c 2017-08-13 16:09:27.917624976 +0200 --- src/version.c 2017-08-13 16:27:21.590784216 +0200 *************** *** 771,772 **** --- 771,774 ---- { /* Add new patch number below this line */ + /**/ + 928, /**/ -- LETTERS TO THE EDITOR (The Times of London) Dear Sir, I am firmly opposed to the spread of microchips either to the home or to the office.  We have more than enough of them foisted upon us in public places.  They are a disgusting Americanism, and can only result in the farmers being forced to grow smaller potatoes, which in turn will cause massive unemployment in the already severely depressed agricultural industry. Yours faithfully,         Capt. Quinton D'Arcy, J. P.         Sevenoaks /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///