Bug 1495071 - Mojolicious Cleanup

There are some things that should've been in the first patch but were missed:

1. Calling $c->finish in the finally block should not happen if an exception has
   been raised.
2. Bugzilla->cleanup() should be called at the same time the mojolicious stash
   is cleared.
3. Code referencing the shutdownhtml should be removed
4. The conditionals that ran code in Bugzilla.pm when it was not run under
   mod_perl should instead check where the Bugzilla.pm module was loaded from.
5. Revert the default template from #770
6. Also removed some stuff that manipulates the PATH and signals, which we
   shouldn't do

6 files changed, 65 insertions, 152 deletions

diff --git a/Bugzilla.pm b/Bugzilla.pm
index 6a118d808..403f498f9 100644
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -54,23 +54,6 @@ use parent qw(Bugzilla::CPAN);
 # Constants
 #####################################################################
 
-# Scripts that are not stopped by shutdownhtml being in effect.
-use constant SHUTDOWNHTML_EXEMPT => qw(
-    editparams.cgi
-    checksetup.pl
-    migrate.pl
-    recode.pl
-);
-
-# Non-cgi scripts that should silently exit.
-use constant SHUTDOWNHTML_EXIT_SILENTLY => qw(
-    whine.pl
-);
-
-# shutdownhtml pages are sent as an HTTP 503. After how many seconds
-# should search engines attempt to index the page again?
-use constant SHUTDOWNHTML_RETRY_AFTER => 3600;
-
 # This is identical to Install::Util::_cache so that things loaded
 # into Install::Util::_cache during installation can be read out
 # of request_cache later in installation.
@@ -86,9 +69,6 @@ sub init_page {
     # request cache are very annoying (see bug 1347335)
     # and this is not an expensive operation.
     clear_request_cache();
-    if ($0 =~ /\.t/) {
-        return;
-    }
     if (Bugzilla->usage_mode == USAGE_MODE_CMDLINE) {
         init_console();
     }
@@ -100,97 +80,18 @@ sub init_page {
         Bugzilla::Logging->fields->{remote_ip} = remote_ip();
     }
 
-    if (${^TAINT}) {
-        # Some environment variables are not taint safe
-        delete @::ENV{'PATH', 'IFS', 'CDPATH', 'ENV', 'BASH_ENV'};
-        # Some modules throw undefined errors (notably File::Spec::Win32) if
-        # PATH is undefined.
-        $ENV{'PATH'} = '';
-    }
-
     # Because this function is run live from perl "use" commands of
     # other scripts, we're skipping the rest of this function if we get here
     # during a perl syntax check (perl -c, like we do during the
     # 001compile.t test).
     return if $^C;
 
-    # IIS prints out warnings to the webpage, so ignore them, or log them
-    # to a file if the file exists.
-    if ($ENV{SERVER_SOFTWARE} && $ENV{SERVER_SOFTWARE} =~ /microsoft-iis/i) {
-        $SIG{__WARN__} = sub {
-            my ($msg) = @_;
-            my $datadir = bz_locations()->{'datadir'};
-            if (-w "$datadir/errorlog") {
-                my $warning_log = new IO::File(">>$datadir/errorlog");
-                print $warning_log $msg;
-                $warning_log->close();
-            }
-        };
-    }
-
     my $script = basename($0);
 
     # Because of attachment_base, attachment.cgi handles this itself.
     if ($script ne 'attachment.cgi') {
         do_ssl_redirect_if_required();
     }
-
-    # If Bugzilla is shut down, do not allow anything to run, just display a
-    # message to the user about the downtime and log out.  Scripts listed in
-    # SHUTDOWNHTML_EXEMPT are exempt from this message.
-    #
-    # This code must go here. It cannot go anywhere in Bugzilla::CGI, because
-    # it uses Template, and that causes various dependency loops.
-    if (Bugzilla->params->{"shutdownhtml"}
-        && !grep { $_ eq $script } SHUTDOWNHTML_EXEMPT)
-    {
-        # Allow non-cgi scripts to exit silently (without displaying any
-        # message), if desired. At this point, no DBI call has been made
-        # yet, and no error will be returned if the DB is inaccessible.
-        if (!i_am_cgi()
-            && grep { $_ eq $script } SHUTDOWNHTML_EXIT_SILENTLY)
-        {
-            exit;
-        }
-
-        # For security reasons, log out users when Bugzilla is down.
-        # Bugzilla->login() is required to catch the logincookie, if any.
-        my $user;
-        eval { $user = Bugzilla->login(LOGIN_OPTIONAL); };
-        if ($@) {
-            # The DB is not accessible. Use the default user object.
-            $user = Bugzilla->user;
-            $user->{settings} = {};
-        }
-        my $userid = $user->id;
-        Bugzilla->logout();
-
-        my $template = Bugzilla->template;
-        my $vars = {};
-        $vars->{'message'} = 'shutdown';
-        $vars->{'userid'} = $userid;
-        # Generate and return a message about the downtime, appropriately
-        # for if we're a command-line script or a CGI script.
-        my $extension;
-        if (i_am_cgi() && (!Bugzilla->cgi->param('ctype')
-                           || Bugzilla->cgi->param('ctype') eq 'html')) {
-            $extension = 'html';
-        }
-        else {
-            $extension = 'txt';
-        }
-        if (i_am_cgi()) {
-            # Set the HTTP status to 503 when Bugzilla is down to avoid pages
-            # being indexed by search engines.
-            print Bugzilla->cgi->header(-status => 503,
-                -retry_after => SHUTDOWNHTML_RETRY_AFTER);
-        }
-        my $t_output;
-        $template->process("global/message.$extension.tmpl", $vars, \$t_output)
-            || ThrowTemplateError($template->error);
-        print $t_output . "\n";
-        exit;
-    }
 }
 
 #####################################################################
@@ -870,21 +771,15 @@ sub _cleanup {
     }
     clear_request_cache();
 
-    # These are both set by CGI.pm but need to be undone so that
-    # Apache can actually shut down its children if it needs to.
-    foreach my $signal (qw(TERM PIPE)) {
-        $SIG{$signal} = 'DEFAULT' if $SIG{$signal} && $SIG{$signal} eq 'IGNORE';
-    }
-
     Log::Log4perl::MDC->remove();
 }
 
-sub END {
-    # Bugzilla.pm cannot compile in mod_perl.pl if this runs.
-    _cleanup() unless $ENV{MOD_PERL};
-}
+our ($caller_package, $caller_file) = caller;
+init_page() if $caller_package eq 'main' && $caller_package !~ /^Test/ && $caller_file =~ /\.t$/;
 
-init_page() if !$ENV{MOD_PERL};
+END {
+    cleanup() if $caller_package eq 'main';
+}
 
 1;
 
diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm
index e76b1c609..4be384b67 100644
--- a/Bugzilla/CGI.pm
+++ b/Bugzilla/CGI.pm
@@ -97,12 +97,6 @@ sub _init_bz_cgi_globals {
     # We need to disable output buffering - see bug 179174
     $| = 1;
 
-    # Ignore SIGTERM and SIGPIPE - this prevents DB corruption. If the user closes
-    # their browser window while a script is running, the web server sends these
-    # signals, and we don't want to die half way through a write.
-    $SIG{TERM} = 'IGNORE';
-    $SIG{PIPE} = 'IGNORE';
-
     # We don't precompile any functions here, that's done specially in
     # mod_perl code.
     $invocant->_setup_symbols(qw(:no_xhtml :oldstyle_urls :private_tempfiles
diff --git a/Bugzilla/Quantum.pm b/Bugzilla/Quantum.pm
index 28fbd4bf6..e9e7713e2 100644
--- a/Bugzilla/Quantum.pm
+++ b/Bugzilla/Quantum.pm
@@ -10,6 +10,8 @@ use Mojo::Base 'Mojolicious';
 
 # Needed for its exit() overload, must happen early in execution.
 use CGI::Compile;
+use utf8;
+use Encode;
 
 use Bugzilla          ();
 use Bugzilla::BugMail ();
@@ -26,6 +28,7 @@ use Module::Runtime qw( require_module );
 use Bugzilla::Util ();
 use Cwd qw(realpath);
 use MojoX::Log::Log4perl::Tiny;
+use Bugzilla::WebService::Server::REST;
 
 has 'static' => sub { Bugzilla::Quantum::Static->new };
 
@@ -36,7 +39,7 @@ sub startup {
     $self->plugin('Bugzilla::Quantum::Plugin::Glue');
     $self->plugin('Bugzilla::Quantum::Plugin::Hostage') unless $ENV{BUGZILLA_DISABLE_HOSTAGE};
     $self->plugin('Bugzilla::Quantum::Plugin::BlockIP');
-    $self->plugin('Bugzilla::Quantum::Plugin::BasicAuth');
+    $self->plugin('Bugzilla::Quantum::Plugin::Helpers');
 
     # hypnotoad is weird and doesn't look for MOJO_LISTEN itself.
     $self->config(
@@ -81,13 +84,20 @@ sub startup {
             }
         );
     }
+    Bugzilla::WebService::Server::REST->preload;
+
+    $self->setup_routes;
+
+    Bugzilla::Hook::process( 'app_startup', { app => $self } );
+}
+
+sub setup_routes {
+    my ($self) = @_;
 
     my $r = $self->routes;
     Bugzilla::Quantum::CGI->load_all($r);
     Bugzilla::Quantum::CGI->load_one( 'bzapi_cgi', 'extensions/BzAPI/bin/rest.cgi' );
 
-    Bugzilla::WebService::Server::REST->preload;
-
     $r->any('/')->to('CGI#index_cgi');
     $r->any('/bug/<id:num>')->to('CGI#show_bug_cgi');
     $r->any('/<id:num>')->to('CGI#show_bug_cgi');
@@ -102,36 +112,18 @@ sub startup {
     $r->any('/extensions/BzAPI/bin/rest.cgi/*PATH_INFO')->to('CGI#bzapi_cgi');
     $r->any('/bzapi/*PATH_INFO')->to('CGI#bzapi_cgi');
 
-    $r->get(
-        '/__lbheartbeat__' => sub {
-            my $c = shift;
-            $c->reply->file( $c->app->home->child('__lbheartbeat__') );
-        },
-    );
-
-    $r->get(
-        '/__version__' => sub {
-            my $c = shift;
-            $c->reply->file( $c->app->home->child('version.json') );
-        },
-    );
+    $r->static_file('/__lbheartbeat__');
+    $r->static_file('/__version__' => { file => 'version.json', content_type => 'application/json' });
+    $r->static_file('/version.json', { content_type => 'application/json' });
 
-    $r->get(
-        '/version.json' => sub {
-            my $c = shift;
-            $c->reply->file( $c->app->home->child('version.json') );
-        },
-    );
+    $r->page('/review', 'splinter.html');
+    $r->page('/user_profile', 'user_profile.html');
+    $r->page('/userprofile', 'user_profile.html');
+    $r->page('/request_defer', 'request_defer.html');
 
     $r->get('/__heartbeat__')->to('CGI#heartbeat_cgi');
     $r->get('/robots.txt')->to('CGI#robots_cgi');
-
-    $r->any('/review')->to( 'CGI#page_cgi' => { 'id' => 'splinter.html' } );
-    $r->any('/user_profile')->to( 'CGI#page_cgi' => { 'id' => 'user_profile.html' } );
-    $r->any('/userprofile')->to( 'CGI#page_cgi' => { 'id' => 'user_profile.html' } );
-    $r->any('/request_defer')->to( 'CGI#page_cgi' => { 'id' => 'request_defer.html' } );
     $r->any('/login')->to( 'CGI#index_cgi' => { 'GoAheadAndLogIn' => '1' } );
-
     $r->any( '/:new_bug' => [ new_bug => qr{new[-_]bug} ] )->to('CGI#new_bug_cgi');
 
     my $ses_auth = $r->under(
@@ -143,8 +135,6 @@ sub startup {
         }
     );
     $ses_auth->any('/index.cgi')->to('SES#main');
-
-    Bugzilla::Hook::process( 'app_startup', { app => $self } );
 }
 
 1;
diff --git a/Bugzilla/Quantum/CGI.pm b/Bugzilla/Quantum/CGI.pm
index 0a74f1ee5..945a87d5b 100644
--- a/Bugzilla/Quantum/CGI.pm
+++ b/Bugzilla/Quantum/CGI.pm
@@ -58,16 +58,20 @@ sub load_one {
         local *STDIN;    ## no critic (local)
         open STDIN, '<', $stdin->path or die "STDIN @{[$stdin->path]}: $!" if -s $stdin->path;
         tie *STDOUT, 'Bugzilla::Quantum::Stdout', controller => $c;    ## no critic (tie)
+
+        # the finally block calls cleanup.
+        $c->stash->{cleanup_guard}->dismiss;
         try {
             Bugzilla->init_page();
             $inner->();
         }
         catch {
-            die $_ unless ref $_ eq 'ARRAY' && $_->[0] eq "EXIT\n";
+            die $_ unless _is_exit($_);
         }
         finally {
+            my $error = shift;
             untie *STDOUT;
-            $c->finish;
+            $c->finish if !$error || _is_exit($error);
             Bugzilla->cleanup;
             CGI::initialize_globals();
         };
@@ -157,4 +161,9 @@ sub _file_to_method {
     return $name;
 }
 
+sub _is_exit {
+    my ($error) = @_;
+    return ref $error eq 'ARRAY' && $error->[0] eq "EXIT\n";
+}
+
 1;
diff --git a/Bugzilla/Quantum/Plugin/Glue.pm b/Bugzilla/Quantum/Plugin/Glue.pm
index a38a12657..e9d0056d0 100644
--- a/Bugzilla/Quantum/Plugin/Glue.pm
+++ b/Bugzilla/Quantum/Plugin/Glue.pm
@@ -14,6 +14,7 @@ use Bugzilla::Constants;
 use Bugzilla::Logging;
 use Bugzilla::RNG ();
 use JSON::MaybeXS qw(decode_json);
+use Scope::Guard;
 
 sub register {
     my ( $self, $app, $conf ) = @_;
@@ -42,6 +43,7 @@ sub register {
                 }
             }
             Log::Log4perl::MDC->put( request_id => $c->req->request_id );
+            $c->stash->{cleanup_guard} = Scope::Guard->new( \&Bugzilla::cleanup );
         }
     );
 
@@ -72,7 +74,6 @@ sub register {
               or die $template->error;
         }
     );
-    $app->renderer->default_handler('bugzilla');
 
     $app->log( MojoX::Log::Log4perl::Tiny->new( logger => Log::Log4perl->get_logger( ref $app ) ) );
 }
diff --git a/Bugzilla/Quantum/Plugin/BasicAuth.pm b/Bugzilla/Quantum/Plugin/Helpers.pm
index e17273404..0aedca338 100644
--- a/Bugzilla/Quantum/Plugin/BasicAuth.pm
+++ b/Bugzilla/Quantum/Plugin/Helpers.pm
@@ -4,7 +4,7 @@
 #
 # This Source Code Form is "Incompatible With Secondary Licenses", as
 # defined by the Mozilla Public License, v. 2.0.
-package Bugzilla::Quantum::Plugin::BasicAuth;
+package Bugzilla::Quantum::Plugin::Helpers;
 use 5.10.1;
 use Mojo::Base qw(Mojolicious::Plugin);
 
@@ -14,7 +14,7 @@ use Carp;
 sub register {
     my ( $self, $app, $conf ) = @_;
 
-    $app->renderer->add_helper(
+    $app->helper(
         basic_auth => sub {
             my ( $c, $realm, $auth_user, $auth_pass ) = @_;
             my $req = $c->req;
@@ -35,6 +35,30 @@ sub register {
             return 1;
         }
     );
+    $app->routes->add_shortcut(
+        static_file => sub {
+            my ($r, $path, $option) = @_;
+            my $file = $option->{file};
+            my $content_type = $option->{content_type} // 'text/plain';
+            unless ($file) {
+                $file = $path;
+                $file =~ s!^/!!;
+            }
+
+            return $r->get($path => sub {
+                my ($c) = @_;
+                $c->res->headers->content_type($content_type);
+                $c->reply->file( $c->app->home->child($file) );
+            })
+        }
+    );
+    $app->routes->add_shortcut(
+        page => sub {
+            my ($r, $path, $id) = @_;
+
+            return $r->any($path)->to('CGI#page_cgi' => { id => $id });
+        }
+    );
 }
 
-1;
\ No newline at end of file
+1;