[PATCH] Bug #34671 Incorrect calling convention to cmd.exe

php.internals

Tim Starling

20 years ago
This patch is untested, my apologies for that. I don't have a working Visual C++ environment in which to compile PHP at the moment. I have, however, tested the concepts involved. I can also provide test cases and expected output if that would help. The problem is PHP's incorrect invocation of cmd.exe, which causes no end of trouble to those developing PHP applications on Windows systems. You can find many workarounds for it in the manual comments, but a permanent, robust fix is actually quite simple, at least for NT-based versions of Windows. Here is an extract of the output of "help cmd" on Windows XP: CMD [/A | /U] [/Q] [/D] [/E:ON | /E:OFF] [/F:ON | /F:OFF] [/V:ON | /V:OFF] [[/S] [/C | /K] string] /C Carries out the command specified by string and then terminates /K Carries out the command specified by string but remains [...] 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. [end quote] This provides a simple, robust means to execute a command via cmd.exe: cmd.exe /s /c "...command goes here..." Otherwise, the programmer risks being plunged into a world of heuristic pain that only a Microsoft developer could love. Some instructive examples: C:\>cmd /c "C:\Program Files\ImageMagick-6.2.6-Q16\convert.exe" Version: ImageMagick 6.2.6 01/27/06 Q16 http://www.imagemagick.org Copyright: Copyright (C) 1999-2006 ImageMagick Studio LLC Usage: convert.exe [options ...] file [ [options ...] file ...] [options ...] file [...] C:\>cmd /c "C:\Program Files\ImageMagick-6.2.6-Q16\convert.exe" -version Version: ImageMagick 6.2.6 01/27/06 Q16 http://www.imagemagick.org Copyright: Copyright (C) 1999-2006 ImageMagick Studio LLC C:\>cmd /c "C:\Program Files\ImageMagick-6.2.6-Q16\convert.exe" "some file" 'C:\Program' is not recognized as an internal or external command, operable program or batch file. C:\>cd "C:\Program Files\ImageMagick-6.2.6-Q16" C:\Program Files\ImageMagick-6.2.6-Q16>cmd /c "convert source dest" convert: unable to open image `source': No such file or directory. convert: missing an image filename `dest'. C:\Program Files\ImageMagick-6.2.6-Q16>echo @echo G'day > "convert source dest.bat" C:\Program Files\ImageMagick-6.2.6-Q16>cmd /c "convert source dest" G'day Positively scary. /s restores sanity to the situation: C:\Program Files\ImageMagick-6.2.6-Q16>cmd /s /c "convert source dest" convert: unable to open image `source': No such file or directory. convert: missing an image filename `dest'. Users of COMMAND.COM are not so blessed. There's no handling for double quotes at all, so if you want to execute a program with spaces in the name, you just have to use the 8.3 alias. In my patch, this is the responsibility of the application. C:\>command.com /s /c "echo hello" Invalid switch Bad command or file name The problem as it manifests itself in PHP was reported as Bug #34671, and closed as bogus by a trigger-happy admin. You can't work around it properly in application-level code, because you can't specify /s. -- Tim Starling

Wez Furlong

20 years ago
Is it possible that switching /S on will break existing scripts? If so, it's probably better to make the comspec setting a PHP .ini option that only the admin can change and allow /S to be set there. --Wez. On 2/5/06, Tim Starling <t.starling@physics.unimelb.edu.au> wrote:

Tim Starling

20 years ago
Wez Furlong wrote:
> Is it possible that switching /S on will break existing scripts? > If so, it's probably better to make the comspec setting a PHP .ini > option that only the admin can change and allow /S to be set there. > > --Wez.
Well, the patch doesn't just use /S, it also automatically encloses the argument in double-quotes. It will break workarounds where people have put those double quotes in manually. This was the workaround suggested on bug 34671. It's also the workaround we'll be using in MediaWiki until this patch is applied, and then after it's applied we'll have to put a version switch in. I'm interested in seeing this patch applied in the hopes that years from now, programmers will be able to use shell_exec() on Windows without going through the same tortuous route I went through to determine why it doesn't work and how to make it work. Reading 50 screenfuls of manual comments didn't help. Speaking of which, it would be great if the manual could be amended to precisely document the pre-patch (or compatibility-mode) and post-patch behaviour of the program execution functions. A comspec setting would be nice for people wanting to use, say, cygwin bash as a command interpreter. But to switch automatic quoting and /S on and off, I think you would need a compatibility mode flag. One that's zero to indicate compatibility, so that you can pull a trick like this to get version-independent code: if (ini_get('windows_shell_quoting')) { $result = shell_exec($cmd); } else { $result = shell_exec(ugly_hack($cmd)); } The same php.ini option could put escapeshellarg() into a windows-compatible mode. Maybe I should submit that patch too, for completeness. -- Tim Starling