Patch 7.3.445

[Bram Moolenaar]

Patch 7.3.445 (after 7.3.443)
Problem: Can't properly escape commands for cmd.exe.
Solution: Default 'shellxquote' to '('. Append ')' to make '(command)'.
No need to use "/s" for 'shellcmdflag'.
Files: src/misc2.c, src/option.c, src/os_win32.c

*** ../vim-7.3.444/src/misc2.c 2012-01-20 17:15:47.000000000 +0100
--- src/misc2.c 2012-02-16 05:34:37.000000000 +0100
***************
*** 3230,3236 ****
{
STRCPY(ncmd, p_sxq);
STRCAT(ncmd, cmd);
! STRCAT(ncmd, p_sxq);
retval = mch_call_shell(ncmd, opt);
vim_free(ncmd);
}
--- 3230,3240 ----
{
STRCPY(ncmd, p_sxq);
STRCAT(ncmd, cmd);
! /* 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 = mch_call_shell(ncmd, opt);
vim_free(ncmd);
}
*** ../vim-7.3.444/src/option.c 2012-02-12 23:23:25.000000000 +0100
--- src/option.c 2012-02-19 18:08:48.000000000 +0100
***************
*** 3933,3959 ****
* my path/to/echo" "my args to echo
* when executed.
*
! * To avoid this, use the /s argument in addition to /c to force the
! * stripping behavior, and also set shellxquote to automatically
! * surround the entire command in quotes (which get stripped as
! * noted).
*/
-
- /* Set shellxquote default to add the quotes to be stripped. */
idx3 = findoption((char_u *)"sxq");
if (idx3 >= 0 && !(options[idx3].flags & P_WAS_SET))
{
! p_sxq = (char_u *)"\"";
options[idx3].def_val[VI_DEFAULT] = p_sxq;
}

- /* Set shellcmdflag default to always strip the quotes, note the order
- * between /s and /c is important or cmd.exe will treat the /s as part
- * of the command to be executed. */
idx3 = findoption((char_u *)"shcf");
if (idx3 >= 0 && !(options[idx3].flags & P_WAS_SET))
{
! p_shcf = (char_u *)"/s /c";
options[idx3].def_val[VI_DEFAULT] = p_shcf;
}
}
--- 3933,3954 ----
* my path/to/echo" "my args to echo
* when executed.
*
! * To avoid this, set shellxquote to surround the command in
! * parenthesis. This appears to make most commands work, without
! * breaking commands that worked previously, such as
! * '"path with spaces/cmd" "a&b"'.
*/
idx3 = findoption((char_u *)"sxq");
if (idx3 >= 0 && !(options[idx3].flags & P_WAS_SET))
{
! p_sxq = (char_u *)"(";
options[idx3].def_val[VI_DEFAULT] = p_sxq;
}

idx3 = findoption((char_u *)"shcf");
if (idx3 >= 0 && !(options[idx3].flags & P_WAS_SET))
{
! p_shcf = (char_u *)"/c";
options[idx3].def_val[VI_DEFAULT] = p_shcf;
}
}
*** ../vim-7.3.444/src/os_win32.c 2011-08-27 15:10:00.000000000 +0200
--- src/os_win32.c 2012-02-19 18:11:23.000000000 +0100
***************
*** 3908,3915 ****
newcmd = lalloc(cmdlen, TRUE);
if (newcmd != NULL)
{
! char_u *cmdbase = (*cmd == '"' ? cmd + 1 : cmd);

if ((STRNICMP(cmdbase, "start", 5) == 0) && vim_iswhite(cmdbase[5]))
{
STARTUPINFO si;
--- 3908,3920 ----
newcmd = lalloc(cmdlen, TRUE);
if (newcmd != NULL)
{
! char_u *cmdbase = cmd;

+ /* Skip a leading ", ( and "(. */
+ if (*cmdbase == '"' )
+ ++cmdbase;
+ if (*cmdbase == '(')
+ ++cmdbase;
if ((STRNICMP(cmdbase, "start", 5) == 0) && vim_iswhite(cmdbase[5]))
{
STARTUPINFO si;
***************
*** 3953,3968 ****
* empty, keep the double quotes around the command.
* Otherwise remove the double quotes, they aren't needed
* here, because we don't use a shell to run the command. */
! if (*cmd == '"' && *p_sxq == NUL)
{
! newcmd[0] = '"';
! STRCPY(newcmd + 1, cmdbase);
! }
! else
! {
! STRCPY(newcmd, cmdbase);
! if (*cmd == '"' && *newcmd != NUL)
! newcmd[STRLEN(newcmd) - 1] = NUL;
}

/*
--- 3958,3983 ----
* empty, keep the double quotes around the command.
* Otherwise remove the double quotes, they aren't needed
* here, because we don't use a shell to run the command. */
! if (cmdbase > cmd)
{
! if (STRNCMP(cmd, p_sxq, cmd - cmdbase) != 0)
! {
! STRCPY(newcmd, cmd);
! }
! else
! {
! char_u *p;
!
! STRCPY(newcmd, cmdbase);
! /* Remove a trailing ", ) and )" if they have a match
! * at the start of the command. */
! p = newcmd + STRLEN(newcmd);
! if (p > newcmd && p[-1] == '"' && *cmd == '"')
! *--p = NUL;
! if (p > newcmd && p[-1] == ')'
! && (*cmd =='(' || cmd[1] == '('))
! *--p = NUL;
! }
}

/*
***************
*** 3970,3976 ****
* inherit our handles which causes unpleasant dangling swap
* files if we exit before the spawned process
*/
! if (CreateProcess (NULL, // Executable name
newcmd, // Command to execute
NULL, // Process security attributes
NULL, // Thread security attributes
--- 3985,3991 ----
* inherit our handles which causes unpleasant dangling swap
* files if we exit before the spawned process
*/
! if (CreateProcess(NULL, // Executable name
newcmd, // Command to execute
NULL, // Process security attributes
NULL, // Thread security attributes
*** ../vim-7.3.444/src/version.c 2012-02-13 00:01:38.000000000 +0100
--- src/version.c 2012-02-19 18:01:46.000000000 +0100
***************
*** 716,717 ****
--- 716,719 ----
{ /* Add new patch number below this line */
+ /**/
+ 445,
/**/

--
hundred-and-one symptoms of being an internet addict:
80. At parties, you introduce your spouse as your "service provider."

/// Bram Moolenaar -- Br...@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 ///

[mattn]

I got an error after applying this patch.

:echo system("cat | grep foo", "foo")
grep: ): No such file or directory

[nanashi]

vim7.3.445 does not working :!start in MS Windows.

gvim.exe -u NONE
:!start explorer.exe
:!start C:\cygwin-1.7\bin\mintty.exe -e /bin/bash.exe -l -c "cd $HOME; exec /bin/bash.exe"

this command return E371: Command not found.

vim7.3.443
default option
set shellcmdflag=/s\ /c
set shellxquote=\"

and

vim7.3.442
default option
set shellcmdflag=/c
set shellxquote=

is working.

[Ben Fritz]

Similar test, using Windows commands only:

:echo system('type mercurial.ini | findstr gvim')
FINDSTR: cannot open )

However,
:echo system('type mercurial.ini ^| findstr gvim')
editor = gvim
editor = gvim

As discussed in the thread responding to 7.3.443, both surrounding the
command in parentheses AND escaping the special characters are needed.

Otherwise, the shell invoking cmd.exe treats something like type file
| findstr as two commands, whereas escaping the | with ^ allows it to
invoke cmd.exe with the entire line as a command, which the invoked
cmd.exe then parses as two commands.

Probably a new option is needed telling the shell what to escape, and
maybe with what.

[mattn]

>  However

> :echo system('type mercurial.ini ^| findstr gvim')

It seems degrade :-( 

It should be escaped automatically. If new option should be needed, it must behave before 7.4.443 I think.

Thanks.

[Bram Moolenaar]

Nanashi wrote:

What is your 'shell' option set to?

--
hundred-and-one symptoms of being an internet addict:

83. Batteries in the TV remote now last for months.

[nanashi]

my 'shell' option is vim7.3.445's default value.

:verbose set shell?
shell=C:\WINDOWS\system32\cmd.exe
shellcmdflag=/c
shellxquote=(

[mattn]

It seems to be expanded like following.

(cat <inputfile | grep foo >outputfile 2>&1)

grep: ): No such file or directory

This should be:

(cat <inputfile | grep foo >outputfile) 2>&1

[Ben Fritz]

Yes, the idea of 7.3.443 was to fix commands like:

:!"exe with spaces.exe" "arg with spaces"

which Vim executes as:

cmd /c "exe with spaces.exe" "arg with spaces"

which then executes in the shell as:

exe with spaces.exe" "arg with spaces

But unfortunately, special characters cause problems, e.g.
:!somecmd "arg & morearg"

As discussed in response to 7.3.443, the way to correctly pass all
strings would be to escape special characters and group all commands
before passing to the cmd.exe shell, e.g.

cmd /c (^"cmd with spaces.exe^" ^"arg ^& morearg^" ^| somethingelse)

In this way, the invoking shell sees one long string of text (all
special characters escaped), which it then passes to a new cmd.exe to
parse as intended.

Vim should automatically add the ^ escapes because the user should not
be expected to know to do this.

But, my point in needing a new option, is that if the user explicitly
sets shellxquote back to " or even an empty string, adding the escapes
will cause problems. E.g.

:!echo "A&B"

will currently work if shellxquote is empty, but if we add
unconditional automatic escaping, it will instead echo "A^&B" in a
shell window.

So I propose that we add another option which defaults to escaping all
special characters with ^ if shellxquote is ( and shell is cmd.exe
(both the default).

[Ben Fritz]

The first is actually (mostly) correct, Vim doesn't expand to

(cat <inputfile | grep foo >outputfile 2>&1)

it expands to

cmd /c (cat <inputfile | grep foo >outputfile 2>&1)

As discussed, this should be:

cmd /c (cat <inputfile ^| grep foo >outputfile 2>&1)

which will in turn invoke:

cat <inputfile | grep foo >outputfile 2>&1

in a new shell.

Unfortunately, instead what happens now because we aren't escaping the
|, is first a new shell invokes:

cat <inputfile

Which the invoking shell then pipes to:

grep foo >outputfile 2>&1)

We need the parentheses to avoid stripping the first and last quote if
the command and argument both are quoted, and we need the escapes to
fix the handling of special characters. Using parentheses instead of
extra surrounding quotes as added in 7.3.443 allows us to ALWAYS
escape special characters instead of complicated logic to determine
when this is needed.

[h_east]

I think mattn is right.

Talk of 'shellxquote' option. (Default: ')' in cmd.exe)

Enclosed in parentheses is to grouping. It same as that of the Bash.

And be enclosed in quotation marks is a different meaning.

Talk of 'shellredir' and 'shellpipe' option . (Default: '2> & 1' in cmd.exe)

This option is used to obtain the results of the entire command.

So, it is not have to resort to take the entire command.

In other words, it should not ever put inside the parentheses.

However, it is put inside of parentheses in Vim 7.3.445. Yes, This is BAD point.

So that, additional processing of this option should be changed after 'shellxquote' process.

Regards,

Hirohito Higashi

[Ben Fritz]

On Feb 20, 10:57 am, h_east <h.east....@gmail.com> wrote:
> I think mattn is right.
>
> Talk of 'shellxquote' option. (Default: ')' in cmd.exe)
> Enclosed in parentheses is to grouping. It same as that of the Bash.
> And be enclosed in quotation marks is a different meaning.
>

Yes, parentheses are intended to be for grouping in cmd.exe. However,
they also work for single commands, and cmd.exe's quote handling is so
broken that common use-cases break no matter how we use them. See the
7.3.443 thread:

http://groups.google.com/group/vim_dev/browse_thread/thread/bd53b29c5e5f2a50

I don't care that we're using parentheses as a hack. It's a hack
that's needed to workaround broken cmd.exe behavior. Namely, cmd.exe
will automatically strip the first quote character and the last quote
character, even if these are not paired together, so that "abc def"
"ghi jkl" becomes abc def" "jkl. With 7.3.443, we added another pair
of quotes, but this broke any commands which have special characters
like & or | in their arguments, because then these characters do not
get included in a quote pair and get interpreted by the invoking shell
rather than the invoked shell. The final solution proposed in the
above thread was to surround in (...) and escape ALL special
characters within the (...) in order to pass everything to the invoked
shell.

> Talk of 'shellredir' and 'shellpipe' option . (Default: '2> & 1' in cmd.exe)
> This option is used to obtain the results of the entire command.

I'm with you so far.

> So, it is not have to resort to take the entire command.
> In other words, it should not ever put inside the parentheses.

You've lost me. I don't see why it is a problem to put the redirect
inside of parentheses. The redirect still applies to the command
executed by the invoked shell. Outside the paretheses, it will apply
to the result of invoking the shell. Are you saying we should use
'shellquote' rather than 'shellxquote'? I'm not certain which is
better in this case. Either seems like it ought to work. What are
scenarios where shellxquote will cause problems? Or is this a
conceptual argument?

> However, it is put inside of parentheses in Vim 7.3.445. Yes, This is BAD
> point.
> So that, additional processing of this option should be changed after
> 'shellxquote' process.
>

What option? What additional processing?

[Bram Moolenaar]

Ben Fritz wrote:

Unfortunately it's not that simple. With 7.3.4xx this command worked:

echo system('ls "a&b" | grep "a&b"')

After patch 7.3.445 we get the error for the trailing ).
With escaping all special characters, which can currently be simulated
with:

echo system('ls "a^&b" ^| grep "a^&b"')

Doesn't work, the ^& is not un-escaped. This works:

echo system('ls "a&b" ^| grep "a&b"')

So, what are the rules for escaping? Do we need to detect
inside/outside double quotes? Nope, this doesn't work:

echo system('ls a^&b ^| grep a^&b')

And this doesn't work either:

echo system('ls a&b ^| grep a&b')

Ah, Ben's suggestion to also escape the double quotes works:

echo system('ls ^"a^&b^" ^| grep ^"a^&b^"')

Well, if we tell users that special characters in arguments, such as &
and | are to be enclosed in double quotes, then it does work escape all
the special characters.

--
If your nose runs, and your feet smell, you might be upside down.