External execution on Win32/AS Perl requires CMD.EXE be in your PATH

RESOLVED WORKSFORME

Status

()

RESOLVED WORKSFORME
17 years ago
6 years ago

People

(Reporter: cedric.caron, Assigned: jouni)

Tracking

2.15
x86
Windows 2000

Details

(Whiteboard: [needed for Win32bz])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

17 years ago
Under windows 2000 the system function is not working properly if the PATH 
environment variable have been deleted.

removing PATH from the deleted variables (in global.pl) fix the problem
which system function?  There are a lot of them, and PATH should be irrelevant
if a full pathname is supplied.

I'm not going to put PATH back, because it breaks taint mode if it's present. 
So we need to find a way to work around it.

Can you provide us an error message, or a code snippet showing which one failed?
(Reporter)

Comment 2

17 years ago
Under windows 2000/NT system is using the shell (cmd.exe) to start the 
requested command. If you delete the PATH variable system failed with the 
folowing error message "Can't spawn "cmd.exe": No such file or directory at 
C:\DL\Devlopment\BugZilla\current\mozilla\webtools\bugzilla\process_bug.cgi 
line 1361*
(Reporter)

Comment 3

17 years ago
I solved the problem by adding the folowing code in global.pl

RCS file: /cvsroot/mozilla/webtools/bugzilla/globals.pl,v
retrieving revision 1.147
diff -r1.147 globals.pl
83a84,86
> if($^O =~ /win32/i) {
> 	$::ENV{'PATH'}=$::ENV{'SystemRoot'}."\\system32";
> }
ya know, I'd be tempted to call that a bug in Perl...  :-)

But as long as we know about it we might as well work around it.

Explicitly setting the PATH gets around the taint checks.  Although I'm not sure
whether $ENV{SystemRoot} will be considered tainted or not, so we may have the
same problem since you're basing the PATH on SystemRoot.

bbaetz: since you're our taint mode expert lately :-)  would you happen to be
able to throw together a quick test script for him to run that does a check on
$::ENV{System_Root} to see if it's tainted or not when the script starts?
Keywords: patch, review
Summary: system function call failed → system() on Win32 requires CMD.EXE be in your PATH
Target Milestone: --- → Bugzilla 2.16

Comment 5

17 years ago
Created attachment 72994 [details]
test.pl -- taint test case

The script is really simple... it uses is_tainted from globals.pl (borrowed
from elsewhere) to determine if the variable is tainted or not.

C:\Mozilla>perl -T test.pl
SystemRoot is tainted

If you uncomment the line that explicitly sets $ENV{SystemRoot}, then it says
it's safe.
OK, so SystemRoot has the same problem PATH does then.  How do we work around
this?  Do we check for known safe locations for SystemRoot or do we just blindly
untaint it and hope they have their system set up right? :-)
(Reporter)

Comment 7

17 years ago
Why not adding a systempath variable to localconfig and setting the path to 
this value ?

during the setup checksetup can copy the content of SystemRoot in the variable 

r you ok with a non taintsafe checksetup ?



That would probably work.  Presumably the operator is in control of his
environment when running checksetup.pl, so we consider that safe.  We do that
already to get the path to mysql in checksetup.pl.
(Reporter)

Comment 9

17 years ago
Created attachment 73011 [details] [diff] [review]
Patch to add a systempath variable in localconfig and set PATH to systempath

I modified checksetup to create a variable caled systempath and set it to the
corect value and modified global.pl to set PATH to this value
(Reporter)

Comment 10

17 years ago
Created attachment 73012 [details] [diff] [review]
Patch to add a systempath variable in localconfig and set PATH to systempath

I modified checksetup to create a variable caled systempath and set it to the
corect value and modified global.pl to set PATH to this value
Comment on attachment 73011 [details] [diff] [review]
Patch to add a systempath variable in localconfig and set PATH to systempath

marking duplicate patch obsolete
Attachment #73011 - Attachment is obsolete: true
Taint mode probably isn't enabled for win32, because the #! lines arne't parsed.
Is that right?
Also see bug 129543 - is that related to this? how is ./ working for you?
I believe Perl parses the #! line itself looking for flags if it isn't given any
flags on the command line when it starts.

I know if I have

#!/usr/bin/perl -wT

on the #! line and I do "perl -wc filename" it tells me "too late for -T at line 1"

Dunno how win32 handles it
(Reporter)

Comment 15

17 years ago
to answer to the question "how is ./ working for you?" than answer is : I 
reported the bug 129543 ;-)
(Reporter)

Comment 16

17 years ago
Under windows the #! line iy not parsed by the system but is parsed by perl.

Making perl under windows work exactly like perl under unix (on this point...).

(Reporter)

Comment 17

17 years ago
I reported this bug in the system function to active state 
(http://bugs.activestate.com/show_bug.cgi?id=19287&submit=Go)
What about teh missing extention for calling processmail? ISTR someone
mentioning that somewhere.

Can't you use $^X ? On unix it shuld be the path from the #! line, which is
absolute, and the wnidows file associateion stuff should make it absolute too -
can someone print $^X from a script with a .pl extention run from a web server
on windows?
How did this ever work on windows (the ./ stuff, I mean)?
(Reporter)

Comment 19

17 years ago
the ./ stuf is not working under windows. To make it working you need to 
write .\

Right, and $^X is tainted. Maybe we should just untaint it?

But how did this work pre-2.14?
(Reporter)

Comment 21

17 years ago
it is not working in 2.14 you have to do the same modification...
It never worked I believe, this is documented in the Win32 install instructinos.
'processmail should be a sub' ;)
*** Bug 130156 has been marked as a duplicate of this bug. ***
*** Bug 130171 has been marked as a duplicate of this bug. ***
Since the original bug filed at ActiveState seemed to be a little light on
information, I went and tried to add some more information to it. 
Unfortunately, ActiveState doesn't seem to allow anyone other than the reporter
and the assignee (and probably people with editbugs privs) to add comments, so I
filed a new bug to give them more information on it.

http://bugs.activestate.com/show_bug.cgi?id=19369
-> 2.18. win32 is not a showstopper for 2.16, and it isn't clear that this is
our bug anyway.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
(Assignee)

Comment 28

17 years ago
Comment on attachment 73012 [details] [diff] [review]
Patch to add a systempath variable in localconfig and set PATH to systempath

This patch is no longer applicable as it's based on more than 
20 revisions old files and is not a unidiff, so the line 
offset is wrong. Please regenerate the patch as a diff -u 
against the current trunk codebase. 

Some nits, then:

>> my $systempath = "$::ENV{'SystemRoot'}\\system32";
>> $systempath =~ s/\\/\\\\/g;
>> LocalVar('systempath', <<"END");

These lines inside the if block should have a four space indentation. 

Do you know if SystemRoot is guaranteed not to end in a slash? 
Though, Windows runs c:\winnt\\system32\cmd.exe without problems,
so maybe that isn't critical anyway.

>> 	$::ENV{'PATH'}=$::systempath;

There is a tab on this line - it should be four spaces of course. 
You could also add spaces around the assignment operator.

I'll test this on my systems when a working patch is ready.
Attachment #73012 - Flags: review-
(Reporter)

Comment 29

17 years ago
Created attachment 86109 [details] [diff] [review]
patch v2

Updated patch to work with current CVS version
Whiteboard: [needed for Win32bz]
Date: 16 Dec 2002 20:05:19 -0000
From: bugzilla <bugzilla@bugs.activestate.com>
To: justdave@syndicomm.com
Subject: [ActivePerl Bug 19369] system() fails with "can't find CMD.EXE" if PATH
has been changed/deleted
X-Bugzilla: http://bugs.activestate.com/


http://bugs.activestate.com/show_bug.cgi?id=19369
Gsar@ActiveState.com changed:

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
           Platform|                            |Any
         Resolution|                            |Not a Bug



------- Comments from Gsar@ActiveState.com  2002-12-16 12:05
I don't consider this a bug.  Perl can't know the absolute location
of cmd.exe without relying on Win32 "shell" interfaces it doesn't
want to rely on without an extremely good reason.  And besides,
one could argue the other way and say Perl should pay attention to
PATH when picking up its default command shell.

If you're clobbering PATH, you should then set the PERL5SHELL
variable in the environment to use the absolute location of
cmd.exe so that perl doesn't have to rely on PATH to look it
up.  See perlrun.pod for exactly what to set it to.
(Reporter)

Comment 31

16 years ago
This need to be documented in the win32 instalation guide !!!

Comment 32

16 years ago
please use only "/" in path on win32... this is perl standard. perl will 
convert this in background to "\\" - if perl runs on win32...

Updated

16 years ago
Blocks: 183753

Comment 33

16 years ago
This works for me
Microsoft Windows 2000 [Version 5.00.2195]
Bugzilla 2.17.4
Perl v5.8.0 built for cygwin-multi-64int

I need more info on what is happening, can i get a Buzilla version for comment 2

here is what I tried to reproduce the claims:

C:\usr\jpyeron\proj\jBugzilla\Bugzilla\tip>set PATH
Path=C:\WINNT\system32;C:\WINNT;C:\WINNT\System32\Wbem;C:\Program Files\Common 
Files\Adaptec Shared\System;c:\cygwin\bin
;C:\cygwin\usr\X11R6\bin;C:\j2sdk1.4.1_01\bin;c:\geda\bin;C:\Program 
Files\Support Tools\;C:\Program Files\Common Files\
Autodesk Shared\;C:\Program Files\Microsoft SQL Server\80
\Tools\Binn\;C:\Program Files\jakarta-ant-1.5.1\bin;C:\Program
Files\Rational\ClearQuest;C:\BC5\BIN;


C:\usr\jpyeron\proj\jBugzilla\Bugzilla\tip>cat test.pl
#!/usr/bin/perl

print "trying\n";
print system("test2.pl"), "\n";
print "done\n";

C:\usr\jpyeron\proj\jBugzilla\Bugzilla\tip>cat test2.pl
#!/usr/bin/perl

print "test2.pl\n";

C:\usr\jpyeron\proj\jBugzilla\Bugzilla\tip>test.pl
trying
test2.pl
0
done

C:\usr\jpyeron\proj\jBugzilla\Bugzilla\tip>set PATH=

C:\usr\jpyeron\proj\jBugzilla\Bugzilla\tip>set PATH

C:\usr\jpyeron\proj\jBugzilla\Bugzilla\tip>test.pl
[popup asking for perl.exe location]
Access is denied.

C:\usr\jpyeron\proj\jBugzilla\Bugzilla\tip>set PATH=c:\cygwin\bin

C:\usr\jpyeron\proj\jBugzilla\Bugzilla\tip>test.pl
trying
test2.pl
0
done
Jason: I believe the problem is actually with ActiveState Perl.  I'd be willing
to bet Cygwin Perl "Does The Right Thing" since it's based more closely on the
unix Perl. :)

Comment 35

16 years ago
given ActiveState's perl is 'iffy' can ActiveState and Cygwin be added to the 
OS list?

Comment 36

15 years ago
hi guys

i'd like to explain you how cmd.exe work - it looks like there are some
missunderstandings. you are trying to set a win environment var with cmd.exe and
SET - this isn't possible!!! if you do a "SET PATH=C:\Test" the environment var
isn't set to the machine environment. the var is only set in the current cmd box
you have opened with *this* syscall. this is a feature of cmd.exe... after
cmd.exe is closed (and this is - after this syscall ends) all setting to the
environment made are lost! if you'd like to change a global machine var on the
box you are forced to use other ways... there are tools like "setx" (microsoft
resource kit) and some other registry ways for setting a machine environment var...

Marc
(Assignee)

Comment 37

15 years ago
Given that system() is afaig (as far as I can grep) nowadays only used in
checksetup.pl's |system("stty", "-echo")| statements which we're anyway going to
eliminate in bug 143490, I'm starting to believe this actually WORKSFORME.
Differing opinions?
(Assignee)

Comment 38

15 years ago
A closer inspection revealed that in addition to system(), also open()ing to a
pipe and using exec() require the path. My current theory is the following:

- AS Perl opens files without a shell, if they are executables pointed to by 
  full paths. 
- If the files are not executables (Perl scripts for example) running 
  them requires association db access. AS Perl spawns cmd.exe to handle the
  association stuff, but fails without the path.

This would explain why my absolute-path-dot.exe works even though it uses an
exec() in showdependencygraph.cgi.

After discussing this with justdave on IRC, the best alternative seems to be to
add a localconfig variable for setting the path to the command interpreter
(Win32 only). In checksetup.pl, we can default to %Systemroot%\System32\cmd.exe
fairly safely, as this works on the NT family (NT/2k/XP). We could then set the
path or PERL5SHELL environment variables following Activestate's recommendation
pasted into comment 30.

Taking.
Assignee: justdave → jouni
Summary: system() on Win32 requires CMD.EXE be in your PATH → External execution on Win32/AS Perl requires CMD.EXE be in your PATH
(Reporter)

Comment 39

15 years ago
you can use the ComSpec environement variable as a default for this 
localgonfig variable.

or directly using comspec without a local config variable

This give a 99.99% safe value...
Jouni: Good to see you get a little free time to work out some (most) of the 
Win32 bugs!

I agree that cmd.exe not being in the path causes problems with opening pipes, 
that's why I put this bug as a blocker for bug 183753. :-)

I think COMSPEC is 100% accurate (if it's set to a wrong location, lots and 
lots of things are going to break when using the command line on Windows), so I 
don't think a localconfig setting is necessary, as it will probably never be 
changed. Although maybe a localconfig setting would be a good thing if the user 
ever installs a new command processor (other than cmd.exe) on the machine and 
sets it as default, so that Bugzilla will still use cmd.exe. (installing a new 
command processor is more common on Win95/98/ME than it is on NT/2k/XP, since 
the former still use DOS as a base OS)

Which reminds me, another good reason to use COMSPEC is that on Win95/98/ME 
platforms, the command processor is command.com and not cmd.exe, and 
%SystemRoot%\System32 normally doesn't contain anything (dlls are in 
%SystemRoot%\System), so the default of %SystemRoot%\System32\cmd.exe wouldn't 
work there.
(Assignee)

Comment 41

15 years ago
Yes, comspec is a good idea. Let's use that. But we'll have to store it in
localconfig, because using ENV{COMSPEC} directly doesn't work because of taint
problems (most env. variables are tainted by default). 

I'll still verify this before doing the patch, but setting ENV{COMSPEC} as the
localconfig default value sounds like the right way to do it.
Status: NEW → ASSIGNED
You could untaint it, although we'll need to think of the security issues in
doing so.

The real fix is to remove all those piped opens - are there that many of them left?
(Assignee)

Comment 43

15 years ago
Showdependencygraph.cgi and collectstats.pl are the only ones if I understand
correctly. But then again, at least in these administrative scripts, a new need
may arise some day. This is going to be an easy fix, so unless you have a good
plan on how to replace dot calls and static duplicates rdf generation with
non-pipe solutions (in 2.18 timeframe), this may be a good road to take. I
welcome discussion, of course :-)
(Assignee)

Comment 44

15 years ago
I played around with this a bit. I'm starting to go for the bbaetz solution
again, as it again looks like I was misled by my previous experience. Currently,
I don't know if there is a single line of code in Bugzilla that would use
system() -- it may be that bug 183753 would, but even there, we are going to
have to fix the issue with opening from a pipe first before we can even get to
debug this.

The fix for this is probably fairly trivial. I've experimented with something
like the following in globals.pl, and it seems to be a feasible approach
(although the final solution may involve localconfig as previously described):

--
if ($^O =~ /MSWin/) {
  my $winshell = $ENV{'COMSPEC'} . ' /c';
  $winshell .= ' /x' if ($winshell =~ /cmd.exe/i);
  $ENV{'PERL5SHELL'} = $winshell;
}
--

For now, finding a good solution for pipe opens comes first. If it ends up being
IPC::Run, the scope of this bug may change.
Comment on attachment 86109 [details] [diff] [review]
patch v2

denying review based on Jouni's comment 44.  I'm assuming this is probably
bitrotted by now anyway.
Attachment #86109 - Flags: review-
Attachment #73012 - Attachment is obsolete: true
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
No longer blocks: 183753
No longer depends on: 225359
(Assignee)

Comment 46

15 years ago
I'm not aware of any callsites in Bugzilla where this would be necessary; time
and patches for other bugs have made this irrelevant. I'm marking this
WORKSFORME; if there still are external executions that would need this, please
file a bug with more information and reproduction instructions (for cvs trunk
tip, please). If this still bothers some configurations globally (on every
execution), let's consider reopening this.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → WORKSFORME
Target Milestone: Bugzilla 2.20 → ---
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.