Bug 586244: Make mod_headers and mod_expires optional

.htaccess

@@ -2,14 +2,24 @@
 <FilesMatch ^(.*\.pm|.*\.pl|.*localconfig.*)$>
   deny from all
 </FilesMatch>
-<FilesMatch (\.js|\.css)$>
-  ExpiresActive On
-  # According to RFC 2616, "1 year in the future" means "never expire".
-  # We change the name of the file's URL whenever its modification date
-  # changes, so browsers can cache any individual JS or CSS URL forever.
-  # However, since all JS and CSS URLs involve a ? in them (for the changing
-  # name) we have to explicitly set an Expires header or browsers won't
-  # *ever* cache them.
-  ExpiresDefault "now plus 1 years"
-  Header append Cache-Control "public"
-</FilesMatch>
+<IfModule mod_expires.c>
+<IfModule mod_headers.c>
+<IfModule mod_env.c>
+  <FilesMatch (\.js|\.css)$>
+    ExpiresActive On
+    # According to RFC 2616, "1 year in the future" means "never expire".
+    # We change the name of the file's URL whenever its modification date
+    # changes, so browsers can cache any individual JS or CSS URL forever.
+    # However, since all JS and CSS URLs involve a ? in them (for the changing
+    # name) we have to explicitly set an Expires header or browsers won't
+    # *ever* cache them.
+    ExpiresDefault "now plus 1 years"
+    Header append Cache-Control "public"
+  </FilesMatch>
+
+  # This lets Bugzilla know that we are properly sending Cache-Control
+  # and Expires headers for CSS and JS files.
+  SetEnv BZ_CACHE_CONTROL 1
+</IfModule>
+</IfModule>
+</IfModule>

Bugzilla/DB.pm

@@ -202,16 +202,19 @@ EOT
     # And now check the version of the database server itself.
     my $dbh = _get_no_db_connection();
 
-    printf("Checking for %15s %-9s ", $sql_server, "(v$sql_want)")
-        if $output;
     my $sql_vers = $dbh->bz_server_version;
     $dbh->disconnect;
 
+    my $version_ok = vers_cmp($sql_vers, $sql_want) > -1 ? 1 : 0;
+    if ($output) {
+        Bugzilla::Install::Requirements::_checking_for({
+            package => $sql_server, wanted => $sql_want,
+            found   => $sql_vers, ok => $version_ok });
+    }
+
     # Check what version of the database server is installed and let
     # the user know if the version is too old to be used with Bugzilla.
-    if ( vers_cmp($sql_vers,$sql_want) > -1 ) {
-        print "ok: found v$sql_vers\n" if $output;
-    } else {
+    if (!$version_ok) {
         die <<EOT;
 
 Your $sql_server v$sql_vers is too old. Bugzilla requires version

Bugzilla/Install/Requirements.pm

@@ -26,7 +26,7 @@ package Bugzilla::Install::Requirements;
 use strict;
 
 use Bugzilla::Constants;
-use Bugzilla::Install::Util qw(vers_cmp install_string 
+use Bugzilla::Install::Util qw(vers_cmp install_string bin_loc 
                                extension_requirement_packages);
 use List::Util qw(max);
 use Safe;
@@ -49,6 +49,34 @@ our @EXPORT = qw(
 # by checksetup.pl.
 use constant TABLE_WIDTH => 71;
 
+# Optional Apache modules that have no Perl component to them.
+# If these are installed, Bugzilla has additional functionality.
+#
+# The keys are the names of the modules, the values are what the module
+# is called in the output of "apachectl -t -D DUMP_MODULES".
+use constant APACHE_MODULES => { 
+    mod_headers => 'headers_module',
+    mod_env     => 'env_module',
+    mod_expires => 'expires_module',
+};
+
+# These are all of the binaries that we could possibly use that can
+# give us info about which Apache modules are installed.
+# If we can't use "apachectl", the "httpd" binary itself takes the same
+# parameters. Note that on Debian and Gentoo, there is an "apache2ctl",
+# but it takes different parameters on each of those two distros, so we
+# don't use apache2ctl.
+use constant APACHE => qw(apachectl httpd apache2 apache);
+
+# If we don't find any of the above binaries in the normal PATH,
+# these are extra places we look.
+use constant APACHE_PATH => [qw(
+    /usr/sbin 
+    /usr/local/sbin
+    /usr/libexec
+    /usr/local/libexec
+)];
+
 # The below two constants are subroutines so that they can implement
 # a hook. Other than that they are actually constants.
 
@@ -347,6 +375,8 @@ sub check_requirements {
     print "\n", install_string('checking_optional'), "\n" if $output;
     my $missing_optional = _check_missing(OPTIONAL_MODULES, $output);
 
+    my $missing_apache = _missing_apache_modules(APACHE_MODULES, $output);
+
     # If we're running on Windows, reset the input line terminator so that
     # console input works properly - loading CGI tends to mess it up
     $/ = "\015\012" if ON_WINDOWS;
@@ -357,6 +387,7 @@ sub check_requirements {
         one_dbd  => $have_one_dbd,
         missing  => $missing,
         optional => $missing_optional,
+        apache   => $missing_apache,
         any_missing => !$pass || scalar(@$missing_optional),
     };
 }
@@ -375,6 +406,54 @@ sub _check_missing {
     return \@missing;
 }
 
+sub _missing_apache_modules {
+    my ($modules, $output) = @_;
+    my $apachectl = _get_apachectl();
+    return [] if !$apachectl;
+    my $command = "$apachectl -t -D DUMP_MODULES";
+    my $cmd_info = `$command 2>&1`;
+    # If apachectl returned a value greater than 0, then there was an
+    # error parsing Apache's configuration, and we can't check modules.
+    my $retval = $?;
+    if ($retval > 0) {
+        print STDERR install_string('apachectl_failed', 
+            { command => $command, root => ROOT_USER }), "\n";
+        return [];
+    }
+    my @missing;
+    foreach my $module (keys %$modules) {
+        my $ok = _check_apache_module($module, $modules->{$module}, 
+                                      $cmd_info, $output);
+        push(@missing, $module) if !$ok;
+    }
+    return \@missing;
+}
+
+sub _get_apachectl {
+    foreach my $bin_name (APACHE) {
+        my $bin = bin_loc($bin_name);
+        return $bin if $bin;
+    }
+    # Try again with a possibly different path.
+    foreach my $bin_name (APACHE) {
+        my $bin = bin_loc($bin_name, APACHE_PATH);
+        return $bin if $bin;
+    }
+    return undef;
+}
+
+sub _check_apache_module {
+    my ($module, $config_name, $mod_info, $output) = @_;
+    my $ok;
+    if ($mod_info =~ /^\s+\Q$config_name\E\b/m) {
+        $ok = 1;
+    }
+    if ($output) {
+        _checking_for({ package => $module, ok => $ok });
+    }
+    return $ok;
+}
+
 sub print_module_instructions {
     my ($check_results, $output) = @_;
 
@@ -411,30 +490,41 @@ sub print_module_instructions {
         }
     }
 
+    if (my @missing = @{ $check_results->{apache} }) {
+        print install_string('modules_message_apache');
+        my $missing_string = join(', ', @missing);
+        my $size = TABLE_WIDTH - 7;
+        printf "*    \%-${size}s *\n", $missing_string;
+        my $spaces = TABLE_WIDTH - 2;
+        print "*", (' ' x $spaces), "*\n";
+    }
+
+    my $need_module_instructions =  
+        ( (!$output and @{$check_results->{missing}})
+          or ($output and $check_results->{any_missing}) ) ? 1 : 0;
+
     # We only print the PPM repository note if we have to.
-    if ((!$output && @{$check_results->{missing}})
-        || ($output && $check_results->{any_missing}))
-    {
-        if (ON_ACTIVESTATE) {
-            my $perl_ver = sprintf('%vd', $^V);
+    if ($need_module_instructions and ON_ACTIVESTATE) {
+        my $perl_ver = sprintf('%vd', $^V);
             
-            # URL when running Perl 5.8.x.
-            my $url_to_theory58S = 'http://theoryx5.uwinnipeg.ca/ppms';
-            # Packages for Perl 5.10 are not compatible with Perl 5.8.
-            if (vers_cmp($perl_ver, '5.10') > -1) {
-                $url_to_theory58S = 'http://cpan.uwinnipeg.ca/PPMPackages/10xx/';
-            }
-            print colored(
-                install_string('ppm_repo_add', 
-                               { theory_url => $url_to_theory58S }),
-                COLOR_ERROR);
-
-            # ActivePerls older than revision 819 require an additional command.
-            if (ON_ACTIVESTATE < 819) {
-                print install_string('ppm_repo_up');
-            }
+        # URL when running Perl 5.8.x.
+        my $url_to_theory58S = 'http://theoryx5.uwinnipeg.ca/ppms';
+        # Packages for Perl 5.10 are not compatible with Perl 5.8.
+        if (vers_cmp($perl_ver, '5.10') > -1) {
+            $url_to_theory58S = 'http://cpan.uwinnipeg.ca/PPMPackages/10xx/';
         }
+        print colored(
+            install_string('ppm_repo_add', 
+                           { theory_url => $url_to_theory58S }),
+            COLOR_ERROR);
+
+        # ActivePerls older than revision 819 require an additional command.
+        if (ON_ACTIVESTATE < 819) {
+            print install_string('ppm_repo_up');
+        }
+    }
 
+    if ($need_module_instructions or @{ $check_results->{apache} }) {
         # If any output was required, we want to close the "table"
         print "*" x TABLE_WIDTH . "\n";
     }
@@ -494,15 +584,14 @@ sub check_graphviz {
     my $webdotbase = Bugzilla->params->{'webdotbase'};
     return 1 if $webdotbase =~ /^https?:/;
 
-    my $checking_for = install_string('checking_for');
-    my $any = install_string('any');
-    printf("%s %15s %-9s ", $checking_for, "GraphViz", "($any)") if $output;
+    my $return;
+    $return = 1 if -x $webdotbase;
+
+    if ($output) {
+        _checking_for({ package => 'GraphViz', ok => $return });
+    }
 
-    my $return = 0;
-    if(-x $webdotbase) {
-        print install_string('module_ok'), "\n" if $output;
-        $return = 1;
-    } else {
+    if (!$return) {
         print install_string('bad_executable', { bin => $webdotbase }), "\n";
     }
 
@@ -554,17 +643,6 @@ sub have_vers {
         $vnum = $1;
     }
 
-    my $vstr;
-    if ($vnum eq "-1") { # string compare just in case it's non-numeric
-        $vstr = install_string('module_not_found');
-    }
-    elsif (vers_cmp($vnum,"0") > -1) {
-        $vstr = install_string('module_found', { ver => $vnum });
-    }
-    else {
-        $vstr = install_string('module_unknown_version');
-    }
-
     my $vok = (vers_cmp($vnum,$wanted) > -1);
     my $blacklisted;
     if ($vok && $params->{blacklist}) {
@@ -573,19 +651,52 @@ sub have_vers {
     }
 
     if ($output) {
-        my $ok           = $vok ? install_string('module_ok') : '';
-        my $black_string = $blacklisted ? install_string('blacklisted') : '';
-        my $want_string  = $wanted ? "v$wanted" : install_string('any');
-
-        $ok = "$ok:" if $ok;
-        my $str = sprintf "%s %19s %-9s $ok $vstr $black_string\n",
-                    install_string('checking_for'), $package, "($want_string)";
-        print $vok ? $str : colored($str, COLOR_ERROR);
+        _checking_for({ 
+            package => $package, ok => $vok, wanted => $wanted,
+            found   => $vnum, blacklisted => $blacklisted
+        });
     }
     
     return $vok ? 1 : 0;
 }
 
+sub _checking_for {
+    my ($params) = @_;
+    my ($package, $ok, $wanted, $blacklisted, $found) = 
+        @$params{qw(package ok wanted blacklisted found)};
+
+    my $ok_string = $ok ? install_string('module_ok') : '';
+
+    # If we're actually checking versions (like for Perl modules), then
+    # we have some rather complex logic to determine what we want to 
+    # show. If we're not checking versions (like for GraphViz) we just
+    # show "ok" or "not found".
+    if (exists $params->{found}) {
+        my $found_string;
+        # We do a string compare in case it's non-numeric.
+        if ($found and $found eq "-1") {
+            $found_string = install_string('module_not_found');
+        }
+        elsif ($found) {
+            $found_string = install_string('module_found', { ver => $found });
+        }
+        else {
+            $found_string = install_string('module_unknown_version');
+        }
+        $ok_string = $ok ? "$ok_string: $found_string" : $found_string;
+    }
+    elsif (!$ok) {
+        $ok_string = install_string('module_not_found');
+    }
+
+    my $black_string = $blacklisted ? install_string('blacklisted') : '';
+    my $want_string  = $wanted ? "v$wanted" : install_string('any');
+
+    my $str = sprintf "%s %20s %-11s $ok_string $black_string\n",
+                install_string('checking_for'), $package, "($want_string)";
+    print $ok ? $str : colored($str, COLOR_ERROR);
+}
+
 sub install_command {
     my $module = shift;
     my ($command, $package);
@@ -707,10 +818,12 @@ a hashref in the format of items from L</REQUIRED_MODULES>.
 
 =item C<optional> - The same as C<missing>, but for optional modules.
 
+=item C<apache> - The name of each optional Apache module that is missing.
+
 =item C<have_one_dbd> - True if at least one C<DBD::> module is installed.
 
-=item C<any_missing> - True if there are any missing modules, even optional
-modules.
+=item C<any_missing> - True if there are any missing Perl modules, even
+optional modules.
 
 =back
 

Bugzilla/Install/Util.pm

@@ -29,7 +29,9 @@ use strict;
 use Bugzilla::Constants;
 
 use Encode;
+use ExtUtils::MM ();
 use File::Basename;
+use File::Spec;
 use POSIX qw(setlocale LC_CTYPE);
 use Safe;
 use Scalar::Util qw(tainted);
@@ -54,18 +56,19 @@ our @EXPORT_OK = qw(
 );
 
 sub bin_loc {
-    my ($bin) = @_;
-    return '' if ON_WINDOWS;
-    # Don't print any errors from "which"
-    open(my $saveerr, ">&STDERR");
-    open(STDERR, '>/dev/null');
-    my $loc = `which $bin`;
-    close(STDERR);
-    open(STDERR, ">&", $saveerr);
-    my $exit_code = $? >> 8; # See the perlvar manpage.
-    return '' if $exit_code > 0;
-    chomp($loc);
-    return $loc;
+    my ($bin, $path) = @_;
+    my @path = $path ? @$path : File::Spec->path;
+    
+    foreach my $dir (@path) {
+        next if !-d $dir;
+        my $full_path = File::Spec->catfile($dir, $bin);
+        # MM is an alias for ExtUtils::MM. maybe_command is nice
+        # because it checks .com, .bat, .exe (etc.) on Windows.
+        my $command = MM->maybe_command($full_path);
+        return $command if $command;
+    }
+
+    return '';
 }
 
 sub get_version_and_os {

Bugzilla/Template.pm

@@ -399,10 +399,17 @@ sub multiline_sprintf {
 sub _mtime { return (stat($_[0]))[9] }
 
 sub mtime_filter {
-    my ($file_url) = @_;
-    my $cgi_path = bz_locations()->{'cgi_path'};
-    my $file_path = "$cgi_path/$file_url";
-    return "$file_url?" . _mtime($file_path);
+    my ($file_url, $mtime) = @_;
+    # This environment var is set in the .htaccess if we have mod_headers
+    # and mod_expires installed, to make sure that JS and CSS with "?"
+    # after them will still be cached by clients.
+    return $file_url if !$ENV{BZ_CACHE_CONTROL};
+    if (!$mtime) {
+        my $cgi_path = bz_locations()->{'cgi_path'};
+        my $file_path = "$cgi_path/$file_url";
+        $mtime = _mtime($file_path);
+    }
+    return "$file_url?$mtime";
 }
 
 # Set up the skin CSS cascade:
@@ -453,8 +460,7 @@ sub css_files {
 sub _css_link_set {
     my ($file_name) = @_;
 
-    my $standard_mtime = _mtime($file_name);
-    my %set = (standard => $file_name . "?$standard_mtime");
+    my %set = (standard => mtime_filter($file_name));
     
     # We use (^|/) to allow Extensions to use the skins system if they
     # want.
@@ -471,7 +477,7 @@ sub _css_link_set {
         my $skin_file_name = $file_name;
         $skin_file_name =~ s{(^|/)skins/standard/}{skins/contrib/$option/};
         if (my $mtime = _mtime("$cgi_path/$skin_file_name")) {
-            $skin_urls{$option} = $skin_file_name . "?$mtime";
+            $skin_urls{$option} = mtime_filter($skin_file_name, $mtime);
         }
     }
     $set{alternate} = \%skin_urls;
@@ -484,7 +490,7 @@ sub _css_link_set {
     my $custom_file_name = $file_name;
     $custom_file_name =~ s{(^|/)skins/standard/}{skins/custom/};
     if (my $custom_mtime = _mtime("$cgi_path/$custom_file_name")) {
-        $set{custom} = $custom_file_name . "?$custom_mtime";
+        $set{custom} = mtime_filter($custom_file_name, $custom_mtime);
     }
     
     return \%set;

template/en/default/setup/strings.txt.pl

@@ -28,6 +28,11 @@
 
 %strings = (
     any  => 'any',
+    apachectl_failed => <<END,
+WARNING: We could not check the configuration of Apache. This sometimes
+happens when you are not running checksetup.pl as ##root##. To see the
+problem we ran into, run: ##command##
+END
     bad_executable => 'not a valid executable: ##bin##',
     blacklisted => '(blacklisted)',
     bz_schema_exists_before_220 => <<'END',
@@ -260,6 +265,18 @@ EOT
 # Note: When translating these "modules" messages, don't change the formatting
 # if possible, because there is hardcoded formatting in 
 # Bugzilla::Install::Requirements to match the box formatting.
+    modules_message_apache => <<END,
+***********************************************************************
+* APACHE MODULES                                                      *
+***********************************************************************
+* Normally, when Bugzilla is upgraded, all Bugzilla users have to     *
+* clear their browser cache or Bugzilla will break. If you enable     *
+* certain modules in your Apache configuration (usually called        *
+* httpd.conf or apache2.conf) then your users will not have to clear  *
+* their caches when you upgrade Bugzilla. The modules you need to     *
+* enable are:                                                         *
+*                                                                     *
+END
     modules_message_db => <<EOT,
 ***********************************************************************
 * DATABASE ACCESS                                                     *