From 8630521c3151c652143673b80fb7f1cc501687a7 Mon Sep 17 00:00:00 2001 From: Dylan William Hardison Date: Fri, 23 Jun 2017 14:34:55 -0400 Subject: Bug 1361890 - Fix problems with current js and css concatenation --- Bugzilla/CGI.pm | 10 +- Bugzilla/Constants.pm | 9 +- Bugzilla/Install/AssetManager.pm | 218 +++++++++++++++++++++++++++++++++++++++ Bugzilla/Install/Filesystem.pm | 42 +++----- Bugzilla/Template.pm | 153 ++++----------------------- 5 files changed, 262 insertions(+), 170 deletions(-) create mode 100644 Bugzilla/Install/AssetManager.pm (limited to 'Bugzilla') 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{(?_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 - - Allow from all + + Deny from all +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; }, -- cgit v1.2.3-24-g4f1b