[perl #121283] system() on Win32 tries to launch bogus paths+bad cmd parsing

# New Ticket Created by  bulk88 
# Please include the string:  [perl #121283]
# in the subject line of all future correspondence about this issue. 
# <URL: https://rt.perl.org/Ticket/Display.html?id=121283 >


This is a bug report for perl from bulk88@hotmail.com,
generated with the help of perlbug 1.40 running under perl 5.19.9.


-----------------------------------------------------------------
[Please describe your issue here]

While investigating why re/subst.t and other subst*.t tests take 300 
seconds on Win32 while their watchdog process times out, (which is 
because the kill() in the end block is on cmd.exe's PID, not the 
watchdog perl's PID, killing the cmd.exe leaves watchdog perl process 
around, which blocks harness script until the timeout at as high as 300 
seconds expires, why harness blocks on the watchdog proc IDK, and that 
is for another ticket, if appropriate) and other watchdog() from test.pl 
using .ts. TonyC brought up the question of why system() made a cmd.exe 
process when perl is not a shell builtin and should have been directly 
launched by the parent perl process (a perl that called watchdog()). I 
will investigate that in this ticket. This ticket isn't directly about 
the watchdog() bug, it is about system()'s behavior. Fixing system() 
will probably fix the watchdog bug (there are other ways too, like 
investigating why harness process hangs on the watchdog, and switching 
test.pl to a process group kill() instead of single process kill(), 
process group kill on Win32 was broken in 5.17 and is another ticket at 
https://rt.perl.org/Ticket/Display.html?id=121230).

For testing my script is
------------------------------------------------------
system(
1,
'C:\\perl519\\src\\t\\perl.exe "-I../lib" -e "sleep(300);warn qq/# Tes
t process timed out - terminating\\n/;kill(KILL, 12748);" ');
-----------------------------------------------------
I am not sure why the "\\" are there even though its single quote. 
Data::Dumper printed it that way. PID 12748 doesn't exist on my system 
but it doesn't matter.

The callstack on blead generally looks like
-------------------------------------------------------
 > perl519.dll!do_spawnvp_handles(int mode=1, const char * 
cmdname=0x0090c5ec, const char * const * argv=0x008f5614, const int * 
handles=0x00000000) Line 3705 C
perl519.dll!win32_spawnvp(int mode=1, const char * cmdname=0x0090c5ec, 
const char * const * argv=0x008f5614) Line 3698 + 0x13 C
perl519.dll!Perl_do_aspawn(interpreter * my_perl=0x003645ec, sv * 
really=0x00000000, sv * * mark=0x0036aec8, sv * * sp=0x0036aec4) Line 
644 + 0x5b C
perl519.dll!Perl_pp_system(interpreter * my_perl=0x003645ec) Line 4225 + 
0x13 C
perl519.dll!Perl_runops_debug(interpreter * my_perl=0x003645ec) Line 
2420 + 0xd C
perl519.dll!S_run_body(interpreter * my_perl=0x003645ec, long 
oldscope=1) Line 2446 + 0xd C
perl519.dll!perl_run(interpreter * my_perl=0x003645ec) Line 2365 C
perl519.dll!RunPerl(int argc=2, char * * argv=0x00362478, char * * 
env=0x003629e0) Line 270 + 0x9 C++
perl.exe!main(int argc=2, char * * argv=0x00362478, char * * 
env=0x00362d58) Line 23 + 0x12 C
perl.exe!mainCRTStartup() Line 398 + 0xe C
kernel32.dll!_BaseProcessStart@4() + 0x23
---------------------------------------------------------
win32_spawnvp is a wrapper that has no logic of its own nowadays so I 
will ignore it exists.
On the 1st try in do_spawnvp_handles CreateProcess is given 
"C:\perl519\src\t\perl.exe -I../lib -e sleep(300);warn qq/# Test process 
timed out - terminating\n/;kill(KILL, 3824);" (no outer quotes) as 
cname/LPCTSTR lpApplicationName, also notice all `"`s were removed from 
inside the path by perl, and "C:\perl519\src\t\perl.exe "-I../lib" -e 
"sleep(300);warn qq/# Test process timed out - terminating\n/;kill(KILL, 
3824);"" (no outer quotes) as cmd/LPTSTR lpCommandLine.

lpApplicationName's MS docs are
------------------------------------------------------
lpApplicationName
[in] Pointer to a null-terminated string that specifies the module to 
execute. The specified module can be a Windows-based application. It can 
be some other type of module (for example, MS-DOS or OS/2) if the 
appropriate subsystem is available on the local computer.
The string can specify the full path and file name of the module to 
execute or it can specify a partial name. In the case of a partial name, 
the function uses the current drive and current directory to complete 
the specification. The function will not use the search path. If the 
file name does not contain an extension, .exe is assumed. Therefore, if 
the file name extension is .com, this parameter must include the .com 
extension.
The lpApplicationName parameter can be NULL. In that case, the module 
name must be the first white space-delimited token in the lpCommandLine 
string. If you are using a long file name that contains a space, use 
quoted strings to indicate where the file name ends and the arguments 
begin; otherwise, the file name is ambiguous. For example, consider the 
string "c:\program files\sub dir\program name". This string can be 
interpreted in a number of ways. The system tries to interpret the 
possibilities in the following order:
c:\program.exe files\sub dir\program name
c:\program files\sub.exe dir\program name
c:\program files\sub dir\program.exe name
c:\program files\sub dir\program name.exe
If the executable module is a 16-bit application, lpApplicationName 
should be NULL, and the string pointed to by lpCommandLine should 
specify the executable module as well as its arguments.
To run a batch file, you must start the command interpreter; set 
lpApplicationName to cmd.exe and set lpCommandLine to the name of the 
batch file.
------------------------------------------------------

Obviously c string "C:\perl519\src\t\perl.exe -I../lib -e 
sleep(300);warn qq/# Test process timed out - terminating\n/;kill(KILL, 
3824);" is not a file. The 1st CreateProcess fails with GLR == 
3/ERROR_PATH_NOT_FOUND. This is a reasonable error. Then 
do_spawnvp_handles() calls qualified_path() is called on this bogus file 
path. GetFileAttributes is called on file "C:\perl519\src\t\perl.exe 
-I../lib -e sleep(300);warn qq/# Test process timed out - 
terminating\n/;kill(KILL, 3824); .exe", notice the new ".exe" at the 
end. GetFileAttributes fails with GLR == 3/ERROR_PATH_NOT_FOUND. Then 
GetFileAttributes is called with "C:\perl519\src\t\perl.exe -I../lib -e 
sleep(300);warn qq/# Test process timed out - terminating\n/;kill(KILL, 
3824);". Another bogus file. qualified_path() fails by returning NULL at 
this point. Then in do_spawnvp_handles, the following executes
-------------------------------
errno = ENOENT;
ret = -1;
goto RETVAL;
------------------------
And control returns to Perl_do_aspawn in win32.c, do_aspawn then adds 
cmd.exe to argv array and then calls do_spawnvp_handles again with 
"cmd.exe" as cmdname instead of a bogus command line string. This works. 
But the child proc is now wrapped in a cmd.exe process.
system()'s docs say
----------------------------------------
Does exactly the same thing as exec LIST , except that a fork is done 
first and the parent process waits for the child process to exit. Note 
that argument processing varies depending on the number of arguments. If 
there is more than one argument in LIST, or if LIST is an array with 
more than one value, starts the program given by the first element of 
the list with arguments given by the rest of the list. If there is only 
one scalar argument, the argument is checked for shell metacharacters, 
and if there are any, the entire argument is passed to the system's 
command shell for parsing (this is /bin/sh -c on Unix platforms, but 
varies on other platforms). If there are no shell metacharacters in the 
argument, it is split into words and passed directly to execvp , which 
is more efficient.
----------------------------------------
The 2 parts that I dont fully understand is “the argument is checked for 
shell metacharacters” and “if there are no shell metacharacters in the 
argument”. We have a complete command line. The file path name might 
being with a “ or not. The file path name might have a space in it, and 
use “ to group it into a file name. IDK what this code does Unix. There 
is a Perl_do_exec3 which defines “check for shell metacharacters” but I 
dont think it is used on Windows. IDK what system() should be doing, 
TonyC says it should not be calling cmd.exe/the shell. The docs say 1 
arg list to system() is always a shell. Win32 Perl uselessly trys to 
launch the process without shell currently, which needs to be stopped. 
Either Win32 Perl always uses shell/cmd.exe for 1 item system(). Or Perl 
knows how to pull out the file path name from the string and pass that 
to CreateProcess. IDK which it should be.

[Please do not change anything below this line]
-----------------------------------------------------------------
---
Flags:
category=core
severity=low
---
Site configuration information for perl 5.19.9:

Configured by Owner at Wed Feb 12 06:47:30 2014.

Summary of my perl5 (revision 5 version 19 subversion 9) configuration:
Derived from: 633f0fd2ca244ca83cc99b3af3a7d3ac2931850b
Platform:
osname=MSWin32, osvers=5.1, archname=MSWin32-x86-multi-thread
uname=''
config_args='undef'
hint=recommended, useposix=true, d_sigaction=undef
useithreads=define, usemultiplicity=define
use64bitint=undef, use64bitall=undef, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cl', ccflags ='-nologo -GF -W3 -Od -MD -Zi -DDEBUGGING -DWIN32 
-D_CONSOLE -DNO_STRICT -DPERL_TEXTMODE_SCRIPTS 
-DPERL_HASH_FUNC_ONE_AT_A_TIME -DPERL_IMPLICIT_CONTEXT 
-DPERL_IMPLICIT_SYS -DUSE_PERLIO -D_USE_32BIT_TIME_T',
optimize='-Od -MD -Zi -DDEBUGGING',
cppflags='-DWIN32'
ccversion='13.10.6030', gccversion='', gccosandvers=''
intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8
ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64', 
lseeksize=8
alignbytes=8, prototype=define
Linker and Libraries:
ld='link', ldflags ='-nologo -nodefaultlib -debug 
-libpath:"c:\perl519\lib\CORE" -machine:x86'
libpth="C:\Program Files\Microsoft Visual Studio .NET 2003\VC7\lib"
libs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib 
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib 
netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib version.lib 
odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib
perllibs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib 
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib 
netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib version.lib 
odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib
libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl519.lib
gnulibc_version=''
Dynamic Linking:
dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug 
-libpath:"c:\perl519\lib\CORE" -machine:x86'

Locally applied patches:
uncommitted-changes

---
@INC for perl 5.19.9:
C:/perl519/site/lib
C:/perl519/lib
.

---
Environment for perl 5.19.9:
HOME (unset)
LANG (unset)
LANGUAGE (unset)
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=C:\perl519\bin;C:\Program Files\Microsoft Visual Studio .NET 
2003\Common7\IDE;C:\Program Files\Microsoft Visual Studio .NET 
2003\VC7\BIN;C:\Program Files\Microsoft Visual Studio .NET 
2003\Common7\Tools;C:\Program Files\Microsoft Visual Studio .NET 
2003\Common7\Tools\bin\prerelease;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\system32\wbem;
PERL_BADLANG (unset)
SHELL (unset)

• [perl #121283] system() on Win32 tries to launch bogus paths+bad cmd parsing by bulk88
• Re: [perl #121283] system() on Win32 tries to launch bogus paths+badcmd parsing by Eric Brine