186130 - collectstats.pl doesn't work if 'requirelogin' set.

Status: RESOLVED FIXED

(Bugzilla :: Reporting/Charting, defect)

Description

[Dave Miller [:justdave]]

This is something new.  This installation was cvs updated a couple weeks ago,
and was just cvs updated again a couple days ago, and the warning emails just
started with the most-recent update.

Date: Thu, 19 Dec 2002 03:00:04 -0800
From: root (Cron Daemon)
To: root
Subject: Cron <root@sheridan> cd /usr/local/bugzilla ;
/usr/local/bugzilla/collectstats.pl
X-Cron-Env: <SHELL=/bin/sh>
X-Cron-Env: <HOME=/root>
X-Cron-Env: <PATH=/usr/bin:/bin>
X-Cron-Env: <LOGNAME=root>

[Thu Dec 19 03:00:04 2002] duplicates.cgi: Use of uninitialized value in pattern
match (m//) at (eval 13) line 4.
[Thu Dec 19 03:00:04 2002] duplicates.cgi: Use of uninitialized value in pattern
match (m//) at (eval 13) line 4.

Comment 1

[Dave Miller [:justdave]]

I narrowed this down to a specific checkin...  this was caused by the checkin
for bug 173662.

revision 1.25
date: 2002/12/20 07:21:25;  author: bbaetz%student.usyd.edu.au;  state: Exp; 
lines: +1 -1
Bug 173622 - Move template handling into a module. r=justdave, joel, a=justdave
=============================================================================
Index: collectstats.pl
===================================================================
RCS file: /cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v
retrieving revision 1.24
retrieving revision 1.25
diff -u -r1.24 -r1.25
--- collectstats.pl     13 Dec 2002 11:01:03 -0000      1.24
+++ collectstats.pl     20 Dec 2002 07:21:25 -0000      1.25
@@ -54,7 +54,7 @@
 &calculate_dupes();
 
 # Generate a static RDF file containing the default view of the duplicates data.
-open(CGI, "REQUEST_METHOD=GET QUERY_STRING=ctype=rdf ./duplicates.cgi |")
+open(CGI, "GATEWAY_INTERFACE=cmdline REQUEST_METHOD=GET QUERY_STRING=ctype=rdf
./duplicates.cgi |")
   || die "can't fork duplicates.cgi: $!";
 open(RDF, ">data/duplicates.tmp")
   || die "can't write to data/duplicates.tmp: $!";

Comment 2

[Bradley Baetz (:bbaetz)]

Oh. This is likly due to the ip matching code not having a REMOTE_ADDR env 
variable, and the changed code patchs meaning that we now run through that code 
anyway. Hmm. Actually, do we? What os that line - I'm not really in a position 
to test ATM

Someone want to hack that to test for existance, anyway, if it is that line? 
Else I'll look at this in a week or so

Comment 3

[Dave Miller [:justdave]]

*** Bug 187581 has been marked as a duplicate of this bug. ***

Comment 4

[Dave Miller [:justdave]]

I started playing around with this a little bit, and I'm not sure if this is
related or not, but I have "requirelogin" set on my install.  When I run
duplicates.cgi with the same command-line that's used from within
collectstats.pl, I get the login screen in the HTML output, not the expected RDF
file...

Comment 5

[Dave Miller [:justdave]]

confirmed.  If I turn off 'requirelogin' not only do I get the RDF output, but
the warning messages go away, too.

Comment 6

[Joel Peshkin]

Well, this comes down to a simple question....   do you want unauthenticated
users to be able to run duplicates.cgi??    The problem is that the answer to
that question is "NO." So, when collectstats acts as a user to do this, it
really needs to do its own equivalent of logging in first.

Comment 7

[Dave Miller [:justdave]]

Or the real answer is to break out the necessary code into a separate module,
and load that module from collectstats.pl rather than trying to pretend it's a
user accessing duplicates.cgi...

Comment 8

[Gervase Markham [:gerv]]

This is Joel's baby :-)

Gerv

Comment 9

[Dave Miller [:justdave]]

No it's not.  This wasn't broken by requirelogin, it was already broken and
requirelogin exposed it.  This needs architectural changes to the way
collectstats calls duplicates.cgi

That's going to be either you (Gerv), or bbaetz.  Since you obviously don't want
it, I'll give it to bbaetz.

Comment 10

[Gervase Markham [:gerv]]

Oh, OK. Sorry, not paying enough attention. 

Can we fix this in a short-term hacky way by making quietly_check_login() also
check URL parameters (someone else wanted that, and there's probably a bug about
it) so collectstats.pl can supply them?

Gerv

Comment 11

[Bradley Baetz (:bbaetz)]

Can we live with this for the next few weeks? After my db patch goes in, making
user atha  module is next on the list, and thats going to fix this indirectly,
(by moving teh ->create call into each individual script, where we can easily
add an argument to handle this)

Comment 12

[Bradley Baetz (:bbaetz)]

Logins are now separate arguments. We can have duplicates.cgi now detect if its
running via apache, possibly with a different ENV option. Then we can just pass
in LOGIN_OPTIONAL to Bugzilla->login, rather LOGIN_DEFAULT, or not call
Bugzilla->login at all.

Comment 13

[Gervase Markham [:gerv]]

bbaetz: your suggestion fixes the problem. So, how does one detect in
duplicates.cgi that one is running under Apache, as opposed to from collectstats.pl?

Gerv

Comment 14

[Bradley Baetz (:bbaetz)]

exists $::ENV{'GATEWAY_INTERFACE'}

Possibly need defined instead of exists, if someone else (eg CGI.pm) does
|if($::ENV{'GATEWAY_INTERFACE'})| first.

Doen't work if we fork collectstats from the browser (unless we clear it first),
but we don't.

Comment 15

[Gervase Markham [:gerv]]

Attachment: Patch v.1

Actually, for CGI, $::ENV{'GATEWAY_INTERFACE'} is "CGI/1.1", and for
command-line use, it's "cmdline". So I've used that distinction.

Gerv

Comment 16

[Bradley Baetz (:bbaetz)]

Comment on attachment 131027 [details] [diff] [review]
Patch v.1

someone may want to generate this with a specific username/pass. Do what I
suggested, and call Bugzilla->login with LOGIN_OPTIONAL from the cmdline, and
LOGIN_DEFAULT from CGI.

Can we also double check this on a windows system - what does it have for that
ENV variable?

Comment 17

[Gervase Markham [:gerv]]

Attachment: Patch v.2

New patch, as bbaetz requests. Tested, and it removes the two warnings.

Gerv

Comment 18

[Gervase Markham [:gerv]]

Comment on attachment 134650 [details] [diff] [review]
Patch v.2

bbaetz: could you please review? I'd like to fix this for 2.17.5.

Gerv

Comment 19

[Gervase Markham [:gerv]]

Fixed.

Checking in duplicates.cgi;
/cvsroot/mozilla/webtools/bugzilla/duplicates.cgi,v  <--  duplicates.cgi
new revision: 1.40; previous revision: 1.39
done

Gerv