query.cgi doesn't check for shutdownhtml

RESOLVED FIXED in Bugzilla 2.16

Status

()

Bugzilla
Query/Bug List
P1
blocker
RESOLVED FIXED
16 years ago
5 years ago

People

(Reporter: justdave, Assigned: myk)

Tracking

({regression})

2.15
Bugzilla 2.16
regression

Details

Attachments

(1 attachment, 2 obsolete attachments)

Is this a regression because of the template conversion?  Are any other
templatized pages affected?
Severity: normal → blocker
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.16
Keywords: regression
Probably, because all the template stuff reads before anything happens.

We need a global func: checkForShutdown() which handles this for us.

Comment 2

16 years ago
IIRC, we check for shutdownhtml in PutHeader().  It was done there because then
it only required a few lines of code change rather than changing *.cgi.  This
wasn't a really big deal originally, as all the .cgi's called PutHeader() [most
of them before doing anything critical, see bug 121741 for an exception].  As we
move to templates, this line of thinking changes in that you want to gather all
your data before you output anything.  Also, I'm pretty sure that the templates
don't use PutHeader() anymore.  I know they all include global/header, but I
highly doubt that checks for shutdownhtml and does the right thing.

So, it appears that Bradley is correct and we need to new global routine [such
as checkForShutdown()] that gets called at the top of every .cgi...  As we start
moving toward having Bugzilla be able to output data formats other than HTML
(such as xml.cgi and the patch in bug 103778) we need to be careful that we
output errors appropriate to the format in question, not just generically spit
out some HTML that says Bugzilla is down.
(Assignee)

Comment 3

16 years ago
Why not do this in CGI.pl?  That script gets required by every CGI script, so we
could put a check at the top of it and stop every script dead in its tracks if
Bugzilla is currently shut down.
(Assignee)

Comment 4

16 years ago
Created attachment 66925 [details] [diff] [review]
patch v1: moves shut down message display to beginning of CGI.pl
(Assignee)

Comment 5

16 years ago
Reassigning to myself.
Assignee: endico → myk
(Assignee)

Comment 6

16 years ago
Accepting.
Status: NEW → ASSIGNED
(Assignee)

Updated

16 years ago
Keywords: patch, review
(Assignee)

Updated

16 years ago
Blocks: 121741
Comment on attachment 66925 [details] [diff] [review]
patch v1: moves shut down message display to beginning of CGI.pl

Looks like you have the attachment tracker patch mixed in with this...
(Assignee)

Comment 8

16 years ago
Created attachment 66936 [details] [diff] [review]
patch v2: same as v1 but without extra cruft from another patch
Attachment #66925 - Attachment is obsolete: true
(Assignee)

Comment 9

16 years ago
Created attachment 66959 [details] [diff] [review]
patch v3: uses global variables instead of "our" declaration

This version removes the "our" variable declarations that Perl 5.005 doesn't
support and substitutes good ole "$::varname"s.
Attachment #66936 - Attachment is obsolete: true
Comment on attachment 66959 [details] [diff] [review]
patch v3: uses global variables instead of "our" declaration

r=gerv. This would be very good to have :-)

Gerv
Attachment #66959 - Flags: review+

Comment 11

16 years ago
Comment on attachment 66959 [details] [diff] [review]
patch v3: uses global variables instead of "our" declaration

r=kiko 

except for the Stash functions (they look ok, I just don't understand what they
are for)

The rest of the code should simplify work.

You can mark 2r= if you think it's ok to land (I do)
(Assignee)

Comment 12

16 years ago
Comment on attachment 66959 [details] [diff] [review]
patch v3: uses global variables instead of "our" declaration

Apparently I kiko's r= if I agree with him that he should r= it.  I do agree
with about that, so checking it in.

Checking in CGI.pl;
/cvsroot/mozilla/webtools/bugzilla/CGI.pl,v  <--  CGI.pl
new revision: 1.130; previous revision: 1.129
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.135; previous revision: 1.134
done
Checking in template/default/global/message.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/default/global/message.html.tmpl,v 
<--  message.html.tmpl
initial revision: 1.1
done
Checking in template/default/global/header;
/cvsroot/mozilla/webtools/bugzilla/template/default/global/header,v  <-- 
header
new revision: 1.5; previous revision: 1.4
done
Attachment #66959 - Flags: review+
(Assignee)

Comment 13

16 years ago
Fix checked in.  Resolving fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
So, what's the officially-sanctioned way of using templates in a file now, then?
Here's my guess:

use vars qw($template $vars);
...
$vars->{'foo'} = \@bar;

Is that right? 

I'd very much like to avoid having to do a global s/$vars/$::vars/, if possible. 

Gerv

Comment 15

16 years ago
Clarifying my drunken comment: I just didn't figure out if the Stash part did
what was expected; the code looked correct and good, so if Stash works r=kiko. :)
(Assignee)

Comment 16

16 years ago
>I'd very much like to avoid having to do a global s/$vars/$::vars/, if possible. 

Sorry, but we had to revert to $::vars for compatibility with Perl 5.005 ("our"
was introduced in 5.6).  This nightware will all go away in modularization.  In
the meantime, squeeze your eyes shut and repeat after me: there's no place like
home, there's no place like home. ;-)

Will 

my $vars = $::vars;
my $template = $::template;

work? That would solve a lot of problems.

Gerv
(Assignee)

Comment 18

16 years ago
>Will
>
>my $vars = $::vars;
>my $template = $::template;
>
>work? That would solve a lot of problems.

This probably won't work if you need to process multiple templates without the
variables from earlier templates polluting the variable space of later
templates, since $::vars (and $::template) are references to a hash and an
object, respectively.  Otherwise, they probably will work, but what problems do
$::vars and $::template have that creating local references will solve?
> Otherwise, they probably will work, but what problems do
> $::vars and $::template have that creating local references will solve?

a) They prevent a global search and replace
b) They look far, far nicer :-)

Gerv
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.