Bug 1361890 - Fix problems with current js and css concatenation

5 files changed, 262 insertions, 170 deletions

diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm
index 2402e997b..65e36a24f 100644
--- a/Bugzilla/CGI.pm
+++ b/Bugzilla/CGI.pm
@@ -441,12 +441,20 @@ sub header {
         %headers = ('-type' => shift(@_));
     }
     else {
-        %headers = @_;
+        # we always want headers to be in lower case, to avoid
+        # for instance -Cache_Control vs. -cache_control.
+        my %tmp = @_;
+        foreach my $key (keys %tmp) {
+            my $lc_key = lc $key;
+            warn "duplicate header: $key" if exists $headers{$lc_key};
+            $headers{$lc_key} = $tmp{$key};
+        }
     }
 
     if ($self->{'_content_disp'}) {
         $headers{'-content_disposition'} = $self->{'_content_disp'};
     }
+    $headers{'-cache_control'} //= 'no-cache, no-store, must-revalidate';
 
     if (!$user->id && $user->authorizer->can_login
         && !$self->cookie('Bugzilla_login_request_cookie'))
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 9c8f39b4a..4cb838f94 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -26,8 +26,6 @@ use Memoize;
 
     bz_locations
 
-    CONCATENATE_ASSETS
-
     IS_NULL
     NOT_NULL
 
@@ -219,11 +217,6 @@ use constant REST_DOC => "http://www.bugzilla.org/docs/tip/en/html/api/";
 use constant REMOTE_FILE => 'http://updates.bugzilla.org/bugzilla-update.xml';
 use constant LOCAL_FILE  => 'bugzilla-update.xml'; # Relative to datadir.
 
-# When true CSS and JavaScript assets will be concatanted and minified at
-# run-time, to reduce the number of requests required to render a page.
-# Setting this to a false value can help debugging.
-use constant CONCATENATE_ASSETS => 1;
-
 # These are unique values that are unlikely to match a string or a number,
 # to be used in criteria for match() functions and other things. They start
 # and end with spaces because most Bugzilla stuff has trim() called on it,
@@ -702,7 +695,7 @@ sub _bz_locations {
         # The script should really generate these graphs directly...
         'webdotdir'      => "$datadir/webdot",
         'extensionsdir'  => "$libpath/extensions",
-        'assetsdir'      => "$datadir/assets",
+        'assetsdir'      => "$libpath/assets",
         # error_reports store error/warnings destined for sentry
         'error_reports'  => "$libpath/error_reports",
     };
diff --git a/Bugzilla/Install/AssetManager.pm b/Bugzilla/Install/AssetManager.pm
new file mode 100644
index 000000000..e314e2ccb
--- /dev/null
+++ b/Bugzilla/Install/AssetManager.pm
@@ -0,0 +1,218 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+
+package Bugzilla::Install::AssetManager;
+
+use 5.10.1;
+use strict;
+use warnings;
+
+use Moo;
+use MooX::StrictConstructor;
+use Type::Utils;
+use Types::Standard qw(Bool Str ArrayRef);
+
+use Digest::SHA ();
+use File::Copy qw(cp);
+use File::Find qw(find);
+use File::Basename qw(dirname);
+use File::Spec;
+use JSON::XS ();
+use MIME::Base64 qw(encode_base64);
+use File::Slurp;
+use List::MoreUtils qw(any all);
+use Carp;
+
+use Bugzilla::Constants qw(bz_locations);
+
+our $VERSION = 1;
+
+my $SHA_VERSION = '224';
+
+my $ABSOLUTE_DIR = declare as Str, where { File::Spec->file_name_is_absolute($_) && -d $_ }
+message {"must be an absolute path to a directory"};
+
+has 'base_dir'  => ( is => 'lazy', isa => $ABSOLUTE_DIR );
+has 'asset_dir' => ( is => 'lazy', isa => $ABSOLUTE_DIR );
+has 'source_dirs' => ( is => 'lazy' );
+has 'state'       => ( is => 'lazy' );
+has 'state_file'  => ( is => 'lazy' );
+has 'json'        => ( is => 'lazy' );
+
+sub asset_file {
+    my ( $self, $file, $relative_to ) = @_;
+    $relative_to //= $self->base_dir;
+    my $asset_file = $self->state->{asset_map}->{$file}
+        or return $file;
+
+    return File::Spec->abs2rel( File::Spec->catfile( $self->asset_dir, $asset_file ), $relative_to );
+}
+
+sub asset_files {
+    my ( $self, @files ) = @_;
+
+    return \@files;
+}
+
+sub asset_sri {
+    my ( $self, $asset_file ) = @_;
+    my ($hex) = $asset_file =~ m!([[:xdigit:]]+)\.\w+$!;
+    my $data = pack "H*", $hex;
+    return "sha$SHA_VERSION-" . encode_base64( $data, "" );
+}
+
+sub compile_file {
+    my ( $self, $file ) = @_;
+    return unless -f $file;
+    my $base_dir  = $self->base_dir;
+    my $asset_dir = $self->asset_dir;
+    my $asset_map = $self->state->{asset_map};
+
+    my $key = File::Spec->abs2rel( $file, $base_dir );
+    return if $asset_map->{$key};
+
+    if ( $file =~ /\.(jpe?g|png|gif|ico|woff|js)$/i ) {
+        my $ext        = $1;
+        my $digest     = $self->_digest_file($file) or die "invalid digest for $file";
+        my $asset_file = File::Spec->catfile( $asset_dir, "$digest.$ext" );
+        cp( $file, $asset_file ) or die "failed to copy $file to $asset_file: $!";
+        if ( $digest eq $self->_digest_file($asset_file) ) {
+            $asset_map->{$key} = File::Spec->abs2rel( $asset_file, $asset_dir );
+        }
+        else {
+            die "failed to write $asset_file";
+        }
+    }
+    elsif ( $file =~ /\.css$/ ) {
+        my $content = read_file($file);
+
+        $content =~ s{(?<!=)url\(([^\)]+)\)}{$self->_css_url_rewrite($1, $file)}eig;
+        my $digest = $self->_digest_string($content);
+        my $asset_file = File::Spec->catfile( $asset_dir, "$digest.css" );
+        write_file( $asset_file, $content ) or die "failed to write $asset_file: $!";
+        if ( $digest eq $self->_digest_file($asset_file) ) {
+            $asset_map->{$key} = File::Spec->abs2rel( $asset_file, $asset_dir );
+        }
+        else {
+            die "failed to write $asset_file";
+        }
+    }
+}
+
+sub compile_all {
+    my ($self) = @_;
+    my $asset_map = $self->state->{asset_map} = {};
+
+    my $wanted = sub {
+        $self->compile_file($File::Find::name);
+    };
+
+    find( { wanted => $wanted, no_chdir => 1 }, @{ $self->source_dirs } );
+
+    $self->_save_state();
+}
+
+sub _css_url_rewrite {
+    my ( $self, $url, $file ) = @_;
+    my $dir = dirname($file);
+
+    # rewrite relative urls as the unified stylesheet lives in a different
+    # directory from the source
+    $url =~ s/(^['"]|['"]$)//g;
+    if ( $url =~ m!^(/|data:)! ) {
+        return 'url(' . $url . ')';
+    }
+    else {
+        my $url_file = File::Spec->rel2abs( $url, $dir );
+        my $ref_file = File::Spec->abs2rel( $url_file, $self->base_dir );
+        $self->compile_file($url_file);
+        return sprintf( "url(%s)", $self->asset_file( $ref_file, $self->asset_dir ) );
+    }
+}
+
+sub _new_digest { Digest::SHA->new($SHA_VERSION) }
+
+sub _digest_file {
+    my ( $self, $file ) = @_;
+    my $digest = $self->_new_digest;
+    $digest->addfile( $file, "b" );
+    return $digest->hexdigest;
+}
+
+sub _digest_string {
+    my ( $self, $string ) = @_;
+    my $digest = $self->_new_digest;
+    $digest->add($string);
+    return $digest->hexdigest;
+}
+
+sub _build_base_dir {
+    Cwd::realpath( File::Spec->rel2abs( bz_locations->{cgi_path} ) );
+}
+
+sub _build_asset_dir {
+    my ($self) = @_;
+    my $dir
+        = Cwd::realpath( File::Spec->rel2abs( bz_locations->{assetsdir} ) );
+
+    if ( $dir && -d $dir ) {
+        my $version_dir = File::Spec->catdir( $dir, "v" . $self->VERSION );
+        unless ( -d $version_dir ) {
+            mkdir $version_dir or die "mkdir $version_dir failed: $!";
+        }
+        return $version_dir;
+    }
+    else {
+        return $dir;
+    }
+}
+
+sub _build_source_dirs {
+    my ($self)  = @_;
+    my $base    = $self->base_dir;
+    my $ext_dir = "$base/extensions";
+    opendir my $ext_dir_handle, $ext_dir or die "unable to open $ext_dir: $!";
+    my @dirs = grep { -d $_ } map {"$ext_dir/$_/web"} grep { !/^\.\.?$/ } readdir $ext_dir_handle;
+    closedir $ext_dir_handle;
+
+    return [ "$base/skins", "$base/js", grep { -d $_ } @dirs ];
+}
+
+sub _build_state_file {
+    my ($self) = @_;
+    return $self->asset_dir . "/state.json";
+}
+
+sub _build_state {
+    my ($self) = @_;
+    my $state;
+    if ( open my $fh, '<:bytes', $self->state_file ) {
+        local $/ = undef;
+        my $json = <$fh>;
+        close $fh;
+        $state = $self->json->decode($json);
+    }
+    else {
+        $state = {};
+    }
+
+    $state->{asset_map} //= {};
+
+    return $state;
+}
+
+sub _build_json { JSON::XS->new->canonical->utf8 }
+
+sub _save_state {
+    my ($self) = @_;
+    open my $fh, '>:bytes', $self->state_file
+        or die "unable to write state file: $!";
+    print $fh $self->json->encode( $self->state );
+    close $fh;
+}
+
+1;
diff --git a/Bugzilla/Install/Filesystem.pm b/Bugzilla/Install/Filesystem.pm
index 22ec34a95..162e324f7 100644
--- a/Bugzilla/Install/Filesystem.pm
+++ b/Bugzilla/Install/Filesystem.pm
@@ -31,6 +31,7 @@ use File::Path;
 use File::Basename;
 use File::Copy qw(move);
 use File::Spec;
+use File::stat;
 use Cwd ();
 use File::Slurp;
 use IO::File;
@@ -81,12 +82,16 @@ EOT
 
 use constant HT_ASSETS_DIR => <<'EOT';
 # Allow access to .css and js files
-<FilesMatch \.(css|js)$>
-  Allow from all
+<FilesMatch state\.json$>
+    Deny from all
 </FilesMatch>
 
+FileETag None
+Header set Cache-Control "public, immutable, max-age=31536000"
+Header set Content-Security-Policy "default-src 'none';"
+
 # And no directory listings, either.
-Deny from all
+Options -Indexes
 EOT
 
 use constant INDEX_HTML => <<'EOT';
@@ -344,7 +349,7 @@ sub FILESYSTEM {
         $attachdir              => DIR_CGI_WRITE,
         $graphsdir              => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE,
         $webdotdir              => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE,
-        $assetsdir              => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE,
+        $assetsdir              => DIR_WS_SERVE,
         $template_cache         => DIR_CGI_WRITE,
         $error_reports          => DIR_CGI_WRITE,
         # Directories that contain content served directly by the web server.
@@ -446,8 +451,13 @@ sub FILESYSTEM {
         "$webdotdir/.htaccess"       => { perms => WS_SERVE,
                                           contents => HT_WEBDOT_DIR },
         "$assetsdir/.htaccess"       => { perms => WS_SERVE,
-                                          contents => HT_ASSETS_DIR },
+                                          contents  => HT_ASSETS_DIR },
     );
+    my $mtime = stat(__FILE__)->mtime;
+    foreach my $file (keys %htaccess) {
+        my $file_stat = stat($file);
+        $htaccess{$file}{overwrite} = $file_stat && $mtime > $file_stat->mtime;
+    }
 
     Bugzilla::Hook::process('install_filesystem', {
         files            => \%files,
@@ -561,7 +571,6 @@ sub update_filesystem {
 
     _remove_empty_css_files();
     _convert_single_file_skins();
-    _remove_dynamic_assets();
 }
 
 sub _css_url_fix {
@@ -627,27 +636,6 @@ sub _convert_single_file_skins {
     }
 }
 
-# delete all automatically generated css/js files to force recreation at the
-# next request.
-sub _remove_dynamic_assets {
-    my @files = (
-        glob(bz_locations()->{assetsdir} . '/*.css'),
-        glob(bz_locations()->{assetsdir} . '/*.js'),
-    );
-    foreach my $file (@files) {
-        unlink($file);
-    }
-
-    # remove old skins/assets directory
-    my $old_path = bz_locations()->{skinsdir} . '/assets';
-    if (-d $old_path) {
-        foreach my $file (glob("$old_path/*.css")) {
-            unlink($file);
-        }
-        rmdir($old_path);
-    }
-}
-
 sub create_htaccess {
     _create_files(%{FILESYSTEM()->{htaccess}});
 
diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm
index 5bef599d4..d72feea67 100644
--- a/Bugzilla/Template.pm
+++ b/Bugzilla/Template.pm
@@ -401,7 +401,13 @@ sub css_files {
 
     unshift @requested_css, "skins/yui.css" unless $no_yui;
 
-    my @css_sets = map { _css_link_set($_) } @requested_css;
+    my @css_sets = map { _css_link_set($_) } sort {
+        my $first_a = $a =~ m!js/jquery!;
+        my $first_b = $b =~ m!js/jquery!;
+        return -1 if $first_a && !$first_b;
+        return 1 if !$first_a && $first_b;
+        return 0;
+    } @requested_css;
 
     my %by_type = (standard => [], skin => [], custom => []);
     foreach my $set (@css_sets) {
@@ -410,18 +416,13 @@ sub css_files {
         }
     }
 
-    # build unified
-    $by_type{unified_standard_skin} = _concatenate_css($by_type{standard},
-                                                       $by_type{skin});
-    $by_type{unified_custom} = _concatenate_css($by_type{custom});
-
     return \%by_type;
 }
 
 sub _css_link_set {
     my ($file_name) = @_;
 
-    my %set = (standard => mtime_filter($file_name));
+    my %set = (standard => $file_name);
 
     # We use (?:^|/) to allow Extensions to use the skins system if they want.
     if ($file_name !~ m{(?:^|/)skins/standard/}) {
@@ -432,127 +433,19 @@ sub _css_link_set {
     my $cgi_path = bz_locations()->{'cgi_path'};
     my $skin_file_name = $file_name;
     $skin_file_name =~ s{(?:^|/)skins/standard/}{skins/contrib/$skin/};
-    if (my $mtime = _mtime("$cgi_path/$skin_file_name")) {
-        $set{skin} = mtime_filter($skin_file_name, $mtime);
+    if (-f "$cgi_path/$skin_file_name") {
+        $set{skin} = $skin_file_name;
     }
 
     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} = mtime_filter($custom_file_name, $custom_mtime);
+    if (-f "$cgi_path/$custom_file_name") {
+        $set{custom} = $custom_file_name;
     }
 
     return \%set;
 }
 
-sub _concatenate_css {
-    my @sources = map { @$_ } @_;
-    return unless @sources;
-
-    my %files =
-        map {
-            (my $file = $_) =~ s/(^[^\?]+)\?.+/$1/;
-            $_ => $file;
-        } @sources;
-
-    my $cgi_path   = bz_locations()->{cgi_path};
-    my $skins_path = bz_locations()->{assetsdir};
-
-    # build minified files
-    my @minified;
-    foreach my $source (@sources) {
-        next unless -e "$cgi_path/$files{$source}";
-        my $file = $skins_path . '/' . md5_hex($source) . '.css';
-        if (!-e $file) {
-            my $content = read_file("$cgi_path/$files{$source}");
-
-            # minify
-            $content =~ s{/\*.*?\*/}{}sg;   # comments
-            $content =~ s{(^\s+|\s+$)}{}mg; # leading/trailing whitespace
-            $content =~ s{\n}{}g;           # single line
-
-            # rewrite urls
-            $content =~ s{url\(([^\)]+)\)}{_css_url_rewrite($source, $1)}eig;
-
-            write_file($file, "/* $files{$source} */\n" . $content . "\n");
-        }
-        push @minified, $file;
-    }
-
-    # concat files
-    my $file = $skins_path . '/' . md5_hex(join(' ', @sources)) . '.css';
-    if (!-e $file) {
-        my $content = '';
-        foreach my $source (@minified) {
-            $content .= read_file($source);
-        }
-        write_file($file, $content);
-    }
-
-    $file =~ s/^\Q$cgi_path\E\///o;
-    return mtime_filter($file);
-}
-
-sub _css_url_rewrite {
-    my ($source, $url) = @_;
-    # rewrite relative urls as the unified stylesheet lives in a different
-    # directory from the source
-    $url =~ s/(^['"]|['"]$)//g;
-    if (substr($url, 0, 1) eq '/' || substr($url, 0, 5) eq 'data:') {
-        return 'url(' . $url . ')';
-    }
-    return 'url(../../' . dirname($source) . '/' . $url . ')';
-}
-
-sub _concatenate_js {
-    return @_ unless CONCATENATE_ASSETS;
-    my ($sources) = @_;
-    return [] unless $sources;
-    $sources = ref($sources) ? $sources : [ $sources ];
-
-    my %files =
-        map {
-            (my $file = $_) =~ s/(^[^\?]+)\?.+/$1/;
-            $_ => $file;
-        } @$sources;
-
-    my $cgi_path   = bz_locations()->{cgi_path};
-    my $skins_path = bz_locations()->{assetsdir};
-
-    # build minified files
-    my @minified;
-    foreach my $source (@$sources) {
-        next unless -e "$cgi_path/$files{$source}";
-        my $file = $skins_path . '/' . md5_hex($source) . '.js';
-        if (!-e $file) {
-            my $content = read_file("$cgi_path/$files{$source}");
-
-            # minimal minification
-            $content =~ s#/\*.*?\*/##sg;    # block comments
-            $content =~ s#(^ +| +$)##gm;    # leading/trailing spaces
-            $content =~ s#^//.+$##gm;       # single line comments
-            $content =~ s#\n{2,}#\n#g;      # blank lines
-            $content =~ s#(^\s+|\s+$)##g;   # whitespace at the start/end of file
-
-            write_file($file, "/* $files{$source} */\n" . $content . "\n");
-        }
-        push @minified, $file;
-    }
-
-    # concat files
-    my $file = $skins_path . '/' . md5_hex(join(' ', @$sources)) . '.js';
-    if (!-e $file) {
-        my $content = '';
-        foreach my $source (@minified) {
-            $content .= read_file($source);
-        }
-        write_file($file, $content);
-    }
-
-    $file =~ s/^\Q$cgi_path\E\///o;
-    return [ $file ];
-}
-
 # YUI dependency resolution
 sub yui_resolve_deps {
     my ($yui, $yui_deps) = @_;
@@ -900,7 +793,7 @@ sub create {
             html_light => \&Bugzilla::Util::html_light_quote,
 
             email => \&Bugzilla::Util::email_filter,
-            
+
             mtime => \&mtime_filter,
 
             # iCalendar contentline filter
@@ -983,19 +876,12 @@ sub create {
             # Function for retrieving global parameters.
             'Param' => sub { return Bugzilla->params->{$_[0]}; },
 
-            'bugzilla_version' => sub {
-                my $version = Bugzilla->VERSION;
-                if (my @ver = $version =~ /^(\d{4})(\d{2})(\d{2})\.(\d+)$/s) {
-                    if ($ver[3] eq '1') {
-                        return join('.', @ver[0,1,2]);
-                    }
-                    else {
-                        return join('.', @ver);
-                    }
-                }
-                else {
-                    return $version;
-                }
+            asset_file => sub {
+                return Bugzilla->asset_manager->asset_file($_[0]);
+            },
+
+            asset_files => sub {
+                return Bugzilla->asset_manager->asset_files(@{ $_[0] });
             },
 
             json_encode => sub {
@@ -1091,7 +977,6 @@ sub create {
 
             'css_files' => \&css_files,
             yui_resolve_deps => \&yui_resolve_deps,
-            concatenate_js => \&_concatenate_js,
 
             # Whether or not keywords are enabled, in this Bugzilla.
             'use_keywords' => sub { return Bugzilla::Keyword->any_exist; },