bug#6784: 24.0.50; cmdproxy incosistency with command pathnames

bug#6784: 24.0.50; cmdproxy incosistency with command pathnames

[Óscar Fuentes]

A command path name that contains slashes (instead of backslashes) will
work fine with cmdproxy as far as it doesn't require to be executed
through a shell. Otherwise only backslashes work. Example:

cmdproxy.exe -c "c:/foo/bar.exe"

which executes bar.exe through CreateProces, works fine, but

cmdproxy.exe -c "c:/foo/bar.exe | zoo.exe"

which invokes the shell for executing the command, fails with an error
message that comes from cmd.exe and that says "c:" is not recognized as
a command.

OTOH,

cmdproxy.exe -c "c:\foo\bar.exe | zoo.exe"

works fine.

cmd.exe has no problem with commands that uses the slash as directory
separator, as this works OK:

cmd /c c:/foo/bar.exe

so the problem must be in cmdproxy, which probably splits the command as

"c:" "/foo/bar.exe"

and passes it as separate arguments to the shell.

Although this bug possibly is not hard to fix, maybe we should consider
the larger scenario: is it worth the trouble having two separate
execution paths on cmdproxy? Inconsistencies like this among the two
separate ways of executing commands may arise on the future, confusing
the users. Maybe we should remove the CreateProcess method and do
everything through the underlying shell.

bug#6784: 24.0.50; cmdproxy incosistency with command pathnames

[Eli Zaretskii]

> From: Óscar Fuentes <ofv@wanadoo.es>
> Date: Tue, 03 Aug 2010 17:56:52 +0200
> Cc: 
> 
> A command path name that contains slashes (instead of backslashes) will
> work fine with cmdproxy as far as it doesn't require to be executed
> through a shell. Otherwise only backslashes work. Example:
> 
> cmdproxy.exe -c "c:/foo/bar.exe"
> 
> which executes bar.exe through CreateProces, works fine, but
> 
> cmdproxy.exe -c "c:/foo/bar.exe | zoo.exe"
> 
> which invokes the shell for executing the command, fails with an error
> message that comes from cmd.exe and that says "c:" is not recognized as
> a command.

Please show the full recipe to reproduce this problem.  You don't
invoke cmdproxy from the command line, do you?

> Maybe we should remove the CreateProcess method and do everything
> through the underlying shell.

That's not a good idea if the shell is command.com, for sure.

For cmd.exe, the problem is a lesser one, but it still exists; e.g.,
cmd.exe is limited to 4K long commands, while CreateProcess can handle
32K.

bug#6784: 24.0.50; cmdproxy incosistency with command pathnames

[Óscar Fuentes]

Eli Zaretskii <eliz@gnu.org> writes:

> Please show the full recipe to reproduce this problem.  You don't
> invoke cmdproxy from the command line, do you?

Put this on .emacs, or start with emacs -Q and evaluate it:

(setq find-program "c:/apps/gnuwin32/bin/find.exe")

Now, do a rgrep.

There is no problem if you use the backslash as the path separator.

>> Maybe we should remove the CreateProcess method and do everything
>> through the underlying shell.
>
> That's not a good idea if the shell is command.com, for sure.

Okay.

> For cmd.exe, the problem is a lesser one, but it still exists; e.g.,
> cmd.exe is limited to 4K long commands, while CreateProcess can handle
> 32K.

That's one more incosistency: a long command works fine, then you put
that command as part of a pipe chain and it stops working. I guess the
current cmdproxy approach is the lesser evil.

bug#6784: 24.0.50; cmdproxy incosistency with command pathnames

[Eli Zaretskii]

> From: Óscar Fuentes <ofv@wanadoo.es>
> Cc: 6784@debbugs.gnu.org
> Date: Tue, 03 Aug 2010 19:52:22 +0200
> 
> > For cmd.exe, the problem is a lesser one, but it still exists; e.g.,
> > cmd.exe is limited to 4K long commands, while CreateProcess can handle
> > 32K.
> 
> That's one more incosistency: a long command works fine, then you put
> that command as part of a pipe chain and it stops working.

Perhaps we should bite the bullet and implement redirection and pipes
within cmdproxy.

bug#6784: 24.0.50; cmdproxy incosistency with command pathnames

[Laimonas Vėbra]

Óscar Fuentes wrote:

> That's one more incosistency: a long command works fine, then you put
> that command as part of a pipe chain and it stops working. I guess the
> current cmdproxy approach is the lesser evil.

It's a CreateProcess() (in cmdproxy.c) _valid_ path requirement/problem; 
valid path/directory separator is (should be) '\':

http://msdn.microsoft.com/en-us/library/aa365247(VS.85).aspx

(only File I/O API converts '/' to '\')

p.s. cmd.exe /c accepts both variants.

bug#6784: 24.0.50; cmdproxy incosistency with command pathnames

[Eli Zaretskii]

> From: Laimonas Vėbra <laimonas.vebra@gmail.com>
> Cc: 6784@debbugs.gnu.org
> 
> Óscar Fuentes wrote:
> 
> > That's one more incosistency: a long command works fine, then you put
> > that command as part of a pipe chain and it stops working. I guess the
> > current cmdproxy approach is the lesser evil.
> 
> It's a CreateProcess() (in cmdproxy.c) _valid_ path requirement/problem; 
> valid path/directory separator is (should be) '\':

But the name of the shell, which is the only file name CreateProcess
should care about in this case, _is_ converted to use backslashes:

      progname = getenv ("COMSPEC");
      if (!progname)
	fail ("error: COMSPEC is not set\n");

      canon_filename (progname);

And canon_filename is

    char *
    canon_filename (char *fname)
    {
      char *p = fname;

      while (*p)
	{
	  if (*p == '/')
	    *p = '\\';
	  p++;
	}

      return fname;
    }

bug#6784: 24.0.50; cmdproxy incosistency with command pathnames

[Laimonas Vėbra]

Eli Zaretskii wrote:
>> From: Laimonas Vėbra<laimonas.vebra@gmail.com>
>> Cc: 6784@debbugs.gnu.org
>>
>> Óscar Fuentes wrote:
>>
>>> That's one more incosistency: a long command works fine, then you put
>>> that command as part of a pipe chain and it stops working. I guess the
>>> current cmdproxy approach is the lesser evil.
>>
>> It's a CreateProcess() (in cmdproxy.c) _valid_ path requirement/problem;
>> valid path/directory separator is (should be) '\':
>
> But the name of the shell, which is the only file name CreateProcess
> should care about in this case, _is_ converted to use backslashes:

The problem is poor CreateProcess() description and some flawless aspects.
Why do we have to call CreateProcess like this:

  rv = CreateProcess (progname, cmdline, &sec_attrs, NULL, TRUE,
			0, envblock, dir, &si, &child);

where progname is 'cmd.exe' and cmdline is 'cmd.exe /c command'

Somewhere i read, that this way it just works (with less problems).

The problem doesn't occur, when we call CreateProcess() like
this:
CreateProcess ('C:\cygwin\bin\ls.exe', 'C:/cygwin/bin/ls.exe'...)

That's actually the case when we are not invoking a shell (command 
without pipe or redirection), e.g.:
M-x grep
c:/cygwin/bin/ls

Then progname gets correctly backslashed by:
  if (!get_next_token (path, &args))
	  canon_filename (path);
	  progname = make_absolute (path);

My guess is that in this case CreateProcess() takes correctly 
backslashed progname (LPCTSTR lpApplicationName),
as the executable module name (program name), _ignores_ first token of 
cmdline (LPTSTR lpCommandLine) as it is internally treated to be the 
same executable module name (although with wrong slashes) and 
takes/passes remaining tokens of cmdline as command line args of progname.



Actual bug problem arises/occurs, then we are invoking a shell (e.g. 
c:/cygwin/bin/ls | grep foo); then CreateProcess() gets likes this:

CreateProcess('C:\WINDOWS\system32\cmd.exe', 
'"C:\WINDOWS\system32\cmd.exe" /c c:/cygwin/bin/ls | grep foo'...)

I guess, that CreateProcess(), in this case, internally processes 
command line args, i.e. program names/paths ('c:/cygwin/bin/ls', 
'grep'), that it passes to cmd.exe and because command name/path is not 
correctly/appropriately constructed (should be backslashed), it (chain 
CreateProcess()->cmd.exe) fails.

bug#6784: 24.0.50; cmdproxy incosistency with command pathnames

[Laimonas Vėbra]

Laimonas Vėbra wrote:

> I guess, that CreateProcess(), in this case, internally processes
> command line args, i.e. program names/paths ('c:/cygwin/bin/ls',
> 'grep'), that it passes to cmd.exe and because command name/path is not
> correctly/appropriately constructed (should be backslashed), it (chain
> CreateProcess()->cmd.exe) fails.

Wrong. Actually it's cmd.exe that fails if command string contains pipe 
and (subsequent slashed, not backslashed) program names are not quoted.

Works:
cmd.exe /c c:/cygwin/bin/ls | grep foo
cmd.exe /c c:/cygwin/bin/ls | C:\cygwin\bin\grep foo
cmd.exe /c c:/cygwin/bin/ls | "C:/cygwin/bin/grep" foo
cmd.exe /c "c:/cygwin/bin/ls" | "C:/cygwin/bin/grep" foo

Fails:
cmd.exe /c c:/cygwin/bin/ls | C:/cygwin/bin/grep foo



So it could be fixed in two ways:
1. using windows path naming rules when calling CreateProcess()
2. quoting program names (all or if it contains slashes '/', i.e. is not 
backslashed) args when calling CreateProcess()

I'm not sure which one would be easier and more error prone.

bug#6784: 24.0.50; cmdproxy incosistency with command pathnames

[Eli Zaretskii]

> Date: Tue, 03 Aug 2010 23:57:16 +0300
> From: Laimonas Vėbra <laimonas.vebra@gmail.com>
> CC: 6784@debbugs.gnu.org
> 
> Laimonas Vėbra wrote:
> 
> > I guess, that CreateProcess(), in this case, internally processes
> > command line args, i.e. program names/paths ('c:/cygwin/bin/ls',
> > 'grep'), that it passes to cmd.exe and because command name/path is not
> > correctly/appropriately constructed (should be backslashed), it (chain
> > CreateProcess()->cmd.exe) fails.
> 
> Wrong. Actually it's cmd.exe that fails if command string contains pipe 
> and (subsequent slashed, not backslashed) program names are not quoted.
> 
> Works:
> cmd.exe /c c:/cygwin/bin/ls | grep foo
> cmd.exe /c c:/cygwin/bin/ls | C:\cygwin\bin\grep foo
> cmd.exe /c c:/cygwin/bin/ls | "C:/cygwin/bin/grep" foo
> cmd.exe /c "c:/cygwin/bin/ls" | "C:/cygwin/bin/grep" foo
> 
> Fails:
> cmd.exe /c c:/cygwin/bin/ls | C:/cygwin/bin/grep foo

In the case in point, the problem was with the executable _before_
the pipe.

bug#6784: 24.0.50; cmdproxy incosistency with command pathnames

[Laimonas Vėbra]

Eli Zaretskii wrote:

>> Laimonas Vėbra wrote:
>>
>>> <...>
>>
>> Works:
>> cmd.exe /c c:/cygwin/bin/ls | grep foo
>> cmd.exe /c c:/cygwin/bin/ls | C:\cygwin\bin\grep foo
>> cmd.exe /c c:/cygwin/bin/ls | "C:/cygwin/bin/grep" foo
>> cmd.exe /c "c:/cygwin/bin/ls" | "C:/cygwin/bin/grep" foo
>>
>> Fails:
>> cmd.exe /c c:/cygwin/bin/ls | C:/cygwin/bin/grep foo


> In the case in point, the problem was with the executable _before_
> the pipe.

Or with subsequent _slashed_ _and_ _unquoted_ executable(s).
Quote whole command line (with slashed programs/paths single quoted):

M-x shell-command
""c:/cygwin/bin/ls" | grep foo"
M-x shell-command
""c:/cygwin/bin/ls" | "c:/cygwin/bin/grep" foo"


or:

cmdproxy -c """c:/cygwin/bin/ls" | grep o""
cmdproxy -c """c:/cygwin/bin/ls" | "c:/cygwin/bin/grep" o""

and it _works_.

cmd /?:

If /C or /K is specified, then the remainder of the command line after
the switch is processed as a command line, where the following logic is
used to process quote (") characters:

     1.  If all of the following conditions are met, then quote 
characters on the command line are preserved:

         - no /S switch
         - exactly two quote characters
         - no special characters between the two quote characters,
           where special is one of: &<>()@^|
         - there are one or more whitespace characters between the
           the two quote characters
         - the string between the two quote characters is the name
           of an executable file.

     2.  Otherwise, old behavior is to see if the first character is
         a quote character and if so, strip the leading character and
         remove the last quote character on the command line, preserving
         any text after the last quote character.