From 9f9e989704b5274afba6a1de0b7bdcc6e7296314 Mon Sep 17 00:00:00 2001 From: Dylan William Hardison Date: Mon, 27 Mar 2017 21:05:10 -0400 Subject: Bug 1350909 - Make index.cgi cache-friendly for logged out requests --- Bugzilla/Template.pm | 5 + .../auth/login-small-additional_methods.html.tmpl | 12 +- index.cgi | 67 ++++++++---- js/global.js | 52 ++++++++- template/en/default/global/header.html.tmpl | 121 ++++++++++----------- .../en/default/global/per-bug-queries.html.tmpl | 40 ------- template/en/default/index.html.tmpl | 16 +-- 7 files changed, 158 insertions(+), 155 deletions(-) diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index 2887f0138..1f0892a32 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -38,6 +38,7 @@ use File::Spec; use IO::Dir; use List::MoreUtils qw(firstidx); use Scalar::Util qw(blessed); +use JSON::XS qw(encode_json); use base qw(Template); @@ -976,6 +977,10 @@ sub create { # Function for retrieving global parameters. 'Param' => sub { return Bugzilla->params->{$_[0]}; }, + json_encode => sub { + return encode_json($_[0]); + }, + # Function to create date strings 'time2str' => \&Date::Format::time2str, diff --git a/extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl b/extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl index 801d3d5fa..a472156f9 100644 --- a/extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl +++ b/extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl @@ -8,21 +8,11 @@ [% USE Bugzilla %] [% IF Param('user_info_class').split(',').contains('GitHubAuth') %] -
- or
diff --git a/index.cgi b/index.cgi index 56513aff7..420a162b6 100755 --- a/index.cgi +++ b/index.cgi @@ -16,43 +16,72 @@ use Bugzilla; use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Update; +use Digest::MD5 qw(md5_hex); +use List::MoreUtils qw(any); # Check whether or not the user is logged in my $user = Bugzilla->login(LOGIN_OPTIONAL); my $cgi = Bugzilla->cgi; -my $template = Bugzilla->template; my $vars = {}; +# Yes, I really want to avoid two calls to the id method. +my $user_id = $user->id; + +# We only cache unauthenticated requests now, because invalidating is harder for logged in users. +my $can_cache = $user_id == 0; + # And log out the user if requested. We do this first so that nothing # else accidentally relies on the current login. if ($cgi->param('logout')) { Bugzilla->logout(); $user = Bugzilla->user; + $user_id = 0; + $can_cache = 0; $vars->{'message'} = "logged_out"; # Make sure that templates or other code doesn't get confused about this. $cgi->delete('logout'); } -$cgi->content_security_policy(script_src => ['self', 'nonce']); +# our weak etag is based on the bugzilla version parameter (BMO customization) and the announcehtml +# if either change, the cache will be considered invalid. +my @etag_parts = (Bugzilla->params->{bugzilla_version}, Bugzilla->params->{announcehtml}); +my $weak_etag = q{W/"} . md5_hex(@etag_parts) . q{"}; +my $if_none_match = $cgi->http('If-None-Match'); + +# load balancer (or client) will check back with us after max-age seconds +# If the etag in If-None-Match is unchanged, we quickly respond without doing much work. +my @cache_control = ( + $can_cache ? 'public' : 'no-cache', + sprintf('max-age=%d', 60 * 5), +); -############################################################################### -# Main Body Execution -############################################################################### +if ($can_cache && $if_none_match && any { $_ eq $weak_etag } split(/,\s*/, $if_none_match)) { + print $cgi->header(-status => '304 Not Modified', -ETag => $weak_etag); +} +else { + my $template = Bugzilla->template; + $cgi->content_security_policy(script_src => ['self']); -# Return the appropriate HTTP response headers. -print $cgi->header(); + # Return the appropriate HTTP response headers. + print $cgi->header( + -Cache_Control => join(', ', @cache_control), + $can_cache ? (-ETag => $weak_etag) : (), + ); -if ($user->in_group('admin')) { - # If 'urlbase' is not set, display the Welcome page. - unless (Bugzilla->params->{'urlbase'}) { - $template->process('welcome-admin.html.tmpl') - || ThrowTemplateError($template->error()); - exit; + if ($user_id && $user->in_group('admin')) { + # If 'urlbase' is not set, display the Welcome page. + unless (Bugzilla->params->{'urlbase'}) { + $template->process('welcome-admin.html.tmpl') + or ThrowTemplateError($template->error()); + exit; + } + # Inform the administrator about new releases, if any. + $vars->{'release'} = Bugzilla::Update::get_notifications(); } - # Inform the administrator about new releases, if any. - $vars->{'release'} = Bugzilla::Update::get_notifications(); -} -# Generate and return the UI (HTML page) from the appropriate template. -$template->process("index.html.tmpl", $vars) - || ThrowTemplateError($template->error()); + $vars->{use_login_page} = 1; + + # Generate and return the UI (HTML page) from the appropriate template. + $template->process("index.html.tmpl", $vars) + or ThrowTemplateError( $template->error() ); +} \ No newline at end of file diff --git a/js/global.js b/js/global.js index a997821f4..651d10241 100644 --- a/js/global.js +++ b/js/global.js @@ -16,6 +16,8 @@ * */ +var BUGZILLA = $('head').data('bugzilla'); + $(function () { $('.show_mini_login_form').on("click", function (event) { return show_mini_login_form($(this).data('qs-suffix')); @@ -32,15 +34,55 @@ $(function () { $('.check_mini_login_fields').on("click", function (event) { return check_mini_login_fields($(this).data('qs-suffix')); }); - $('form .quicksearch_check_empty').on("submit", function (event) { + $('.quicksearch_check_empty').on("submit", function (event) { if (this.quicksearch.value == '') { - alert('Please enter one or more search terms first.'); - return false; + alert('Please enter one or more search terms first.'); + event.preventDefault(); } - return true; }); + + unhide_language_selector(); + $("#lob_action").on("change", update_text); + $("#lob_newqueryname").on("keyup", manage_old_lists); }); +function unhide_language_selector() { + $('#lang_links_container').removeClass('bz_default_hidden'); +} + +function update_text() { + // 'lob' means list_of_bugs. + var lob_action = document.getElementById('lob_action'); + var action = lob_action.options[lob_action.selectedIndex].value; + var text = document.getElementById('lob_direction'); + var new_query_text = document.getElementById('lob_new_query_text'); + + if (action == "add") { + text.innerHTML = "to"; + new_query_text.style.display = 'inline'; + } + else { + text.innerHTML = "from"; + new_query_text.style.display = 'none'; + } +} + +function manage_old_lists() { + var old_lists = document.getElementById('lob_oldqueryname'); + // If there is no saved searches available, returns. + if (!old_lists) return; + + var new_query = document.getElementById('lob_newqueryname').value; + + if (new_query != "") { + old_lists.disabled = true; + } + else { + old_lists.disabled = false; + } +} + + function show_mini_login_form( suffix ) { $('#login_link' + suffix).addClass('bz_default_hidden'); $('#mini_login' + suffix).removeClass('bz_default_hidden'); @@ -148,4 +190,4 @@ $().ready(function() { $(window).on('pageshow', function(event) { $('.bz_autocomplete').attr('autocomplete', 'off'); }); -}); +}); \ No newline at end of file diff --git a/template/en/default/global/header.html.tmpl b/template/en/default/global/header.html.tmpl index 2e08a461d..07a980050 100644 --- a/template/en/default/global/header.html.tmpl +++ b/template/en/default/global/header.html.tmpl @@ -38,6 +38,7 @@ # generate_api_token: generate a token which can be used to make authenticated webservice calls # no_body: if true the body element will not be generated # allow_mobile: allow special CSS and viewport for detected mobile useragents + # use_login_page: display a link to the full login page, rather than an inline login. #%] [% IF message %] @@ -116,11 +117,37 @@ # value of title anyway. To get around that problem we explicitly # set header's default value here only if it is undefined. %] [% IF !header.defined %][% header = title %][% END %] - +[%- js_BUGZILLA = { + param => { + cookiepath => Param('cookiepath'), + maxusermatches => Param('maxusermatches'), + }, + constant => { + COMMENT_COLS => constants.COMMENT_COLS, + }, + string => { + # Please keep these in alphabetical order. + + attach_desc_required => + 'You must enter a Description for this attachment.', + component_required => + "You must select a Component for this $terms.bug", + description_required => + "You must enter a Description for this $terms.bug", + short_desc_required => + "You must enter a Summary for this $terms.bug", + version_required => + "You must select a Version for this $terms.bug" + } + }; + IF generate_api_token; + js_BUGZILLA.api_token = get_api_token(); + END; +%] - + [% Hook.process("start") %] [% title %] @@ -181,65 +208,18 @@ [% PROCESS format_js_link %] [% END %] - + [% END %] + [% IF inline_javascript.search("\\S") %] + + [% END %] [% FOREACH asset_url = concatenate_js(javascript_urls) %] [% PROCESS format_js_link %] @@ -347,14 +331,21 @@
  • New Account
    • [% END %] - [%# Only display one login form when we're on a LOGIN_REQUIRED page. That - # way, we're guaranteed that the user will use the form that has - # hidden_fields in it (the center form) instead of this one. Also, it's - # less confusing to have one form (as opposed to three) when you're - # required to log in. - #%] - [% IF user.authorizer.can_login && !Bugzilla.page_requires_login %] - [% PROCESS "account/auth/login-small.html.tmpl" qs_suffix = "_top" %] + [% IF use_login_page %] +
  • + | + Log In +
    • + [% ELSE %] + [%# Only display one login form when we're on a LOGIN_REQUIRED page. That + # way, we're guaranteed that the user will use the form that has + # hidden_fields in it (the center form) instead of this one. Also, it's + # less confusing to have one form (as opposed to three) when you're + # required to log in. + #%] + [% IF user.authorizer.can_login && !Bugzilla.page_requires_login %] + [% PROCESS "account/auth/login-small.html.tmpl" qs_suffix = "_top" %] + [% END %] [% END %] [% END %] diff --git a/template/en/default/global/per-bug-queries.html.tmpl b/template/en/default/global/per-bug-queries.html.tmpl index 71723c178..b5c2431e1 100644 --- a/template/en/default/global/per-bug-queries.html.tmpl +++ b/template/en/default/global/per-bug-queries.html.tmpl @@ -15,46 +15,6 @@ [% IF user.id && user.settings.per_bug_queries.value == "on" %]