300978 - bad output when a non-cgi script dies

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect, P1)

Description

[Frédéric Buclin]

It's a regression due to bug 283989. CGI.pm, line 39, contains:

use CGI::Carp qw(fatalsToBrowser);


This makes non-cgi scripts to display HTML content when dying:

[root@antares bugzilla]# ./collectstats.pl
Content-type: text/html

<h1>Software error:</h1>
<pre>pomme at ./collectstats.pl line 57.</pre>
<p>
For help, please send mail to this site's webmaster, giving this error message
and the time and date of the error.

</p>


collectstats.pl "use Bugzilla;", which "use Bugzilla::CGI;".

Removing this line completely looks like the best solution to me.

Comment 1

[Max Kanat-Alexander]

Ahhh. Hrm... I think the solution is to only do a "require Bugzilla::CGI" when
it's actually needed in Bugzilla.pm, which might be almost never...

Comment 2

[Marc Schumann [:Wurblzap]]

Attachment: Patch

This seems to work. Note removing the dependency on CGI::Carp from
Bugzilla::Bug.pm.

I feel a little uncomfortable about this because the global init code is being
called later now than before. Perhaps the global init code should rather be
moved to Bugzilla.pm?

Comment 3

[Max Kanat-Alexander]

Well, the global init code really does look like it belongs in CGI.pm. I think
we're probably okay. Another thing we could do to fix this (at least, something
we could try) would be to require CGI::Carp and import fatalsToBrowser only if
i_am_cgi().

We'd have to check the CGI::Carp docs to make sure that that would be okay...

Comment 4

[Max Kanat-Alexander]

Oh, but one way or another, whatever solution we decide on will need a comment,
so other developers years from now aren't confused about why we're doing
"require" statements in strange places. :-)

Comment 5

[Frédéric Buclin]

Comment on attachment 189513 [details] [diff] [review]
Patch

run ./collectstats.pl and ./whine.pl with this patch applied. Enjoy!

[root@antares bugzilla]# ./collectstats.pl
Can't locate object method "new" via package "Bugzilla::CGI" at
./collectstats.pl line 489.
[root@antares bugzilla]# ./whine.pl
Can't locate object method "new" via package "Bugzilla::CGI" at ./whine.pl line
461.

Adding use Bugzilla::CGI; at the top of these two files fix the problem, but
the output again displays HTML. :(

Non-cgi scripts should not create CGI objects IMO.

Comment 6

[Frédéric Buclin]

(In reply to comment #5)
> Non-cgi scripts should not create CGI objects IMO.

Note that the culprit is Search.pm which requires a CGI object in order to run
saved queries.

Comment 7

[Marc Schumann [:Wurblzap]]

(In reply to comment #6)
> Note that the culprit is Search.pm which requires a CGI object in order to run
> saved queries.

In this case, I don't think I'll manage to patch this bug any time soon :(

Comment 8

[Max Kanat-Alexander]

I think we'll need to do an "eval use" if i_am_cgi perhaps, on the CGI::Carp
qw(fatalsToBrowser) thing.

Comment 9

[Frédéric Buclin]

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

Comment 10

[Marc Schumann [:Wurblzap]]

Let's have bug 319140 block this one. We can have an i_am_cgi check added easily after a patch along the lines of the current one on bug 319140 has landed.

Comment 11

[Max Kanat-Alexander]

Attachment: v2

Finally, a patch for this bug. :-) I've always suspected this would fix it, I just didn't remember or have the time to do it before now. :-)

Comment 12

[Frédéric Buclin]

Comment on attachment 230649 [details] [diff] [review]
v2

Great fix! :) Tested on both tip and 2.22. r=LpSolit

Comment 13

[Max Kanat-Alexander]

tip:

Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.130; previous revision: 1.129
done
Checking in Bugzilla/CGI.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/CGI.pm,v  <--  CGI.pm
new revision: 1.27; previous revision: 1.26
done

2.22:

Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.104.2.4; previous revision: 1.104.2.3
done
Checking in Bugzilla/CGI.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/CGI.pm,v  <--  CGI.pm
new revision: 1.20.2.3; previous revision: 1.20.2.2
done