From 39a524aac87eb33d48787c06c6d2305b48f52d58 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 5 Dec 2016 13:57:09 +1100 Subject: [PATCH] FEATURE: brotli cdn bypass for assets Allow CDNS that strip out brotli encoding to use brotli regardless --- app/controllers/static_controller.rb | 27 ++++++++++++++++++- app/helpers/application_helper.rb | 10 ++++--- config/routes.rb | 1 + lib/middleware/anonymous_cache.rb | 11 +++++++- .../middleware/anonymous_cache_spec.rb | 10 +++++++ 5 files changed, 53 insertions(+), 6 deletions(-) diff --git a/app/controllers/static_controller.rb b/app/controllers/static_controller.rb index d7dee2377bd..9da1201c79d 100644 --- a/app/controllers/static_controller.rb +++ b/app/controllers/static_controller.rb @@ -4,7 +4,7 @@ require_dependency 'file_helper' class StaticController < ApplicationController skip_before_filter :check_xhr, :redirect_to_login_if_required - skip_before_filter :verify_authenticity_token, only: [:cdn_asset, :enter, :favicon] + skip_before_filter :verify_authenticity_token, only: [:brotli_asset, :cdn_asset, :enter, :favicon] PAGES_WITH_EMAIL_PARAM = ['login', 'password_reset', 'signup'] @@ -123,7 +123,32 @@ class StaticController < ApplicationController response.headers["Last-Modified"] = Time.new('2000-01-01').httpdate render text: data, content_type: "image/png" end + end + def brotli_asset + path = File.expand_path(Rails.root + "public/assets/" + params[:path]) + path += ".br" + + # SECURITY what if path has /../ + raise Discourse::NotFound unless path.start_with?(Rails.root.to_s + "/public/assets") + + opts = { disposition: nil } + opts[:type] = "application/javascript" if path =~ /\.js.br$/ + + response.headers["Expires"] = 1.year.from_now.httpdate + response.headers["Content-Encoding"] = 'br' + begin + response.headers["Last-Modified"] = File.ctime(path).httpdate + response.headers["Content-Length"] = File.size(path).to_s + rescue Errno::ENOENT + raise Discourse::NotFound + end + + if File.exists?(path) + send_file(path, opts) + else + raise Discourse::NotFound + end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index d79ace3f343..e1400e9883a 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -46,10 +46,12 @@ module ApplicationHelper end def script(*args) - if SiteSetting.enable_cdn_js_debugging && GlobalSetting.cdn_url - tags = javascript_include_tag(*args, "crossorigin" => "anonymous") - tags.gsub!("/assets/", "/cdn_asset/#{Discourse.current_hostname.tr(".","_")}/") - tags.gsub!(".js\"", ".js?v=1&origin=#{CGI.escape request.base_url}\"") + if GlobalSetting.cdn_url && + GlobalSetting.cdn_url.start_with?("https") && + ENV["COMPRESS_BROTLI"] == "1" && + request.env["ACCEPT_ENCODING"] =~ /br/ + tags = javascript_include_tag(*args) + tags.gsub!("#{GlobalSetting.cdn_url}/assets/", "#{GlobalSetting.cdn_url}/brotli_asset/") tags.html_safe else javascript_include_tag(*args) diff --git a/config/routes.rb b/config/routes.rb index f465e56a380..770d8199455 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -650,6 +650,7 @@ Discourse::Application.routes.draw do delete "draft" => "draft#destroy" get "cdn_asset/:site/*path" => "static#cdn_asset", format: false + get "brotli_asset/*path" => "static#brotli_asset", format: false get "favicon/proxied" => "static#favicon", format: false diff --git a/lib/middleware/anonymous_cache.rb b/lib/middleware/anonymous_cache.rb index a79b4225663..149ac3f009e 100644 --- a/lib/middleware/anonymous_cache.rb +++ b/lib/middleware/anonymous_cache.rb @@ -11,6 +11,7 @@ module Middleware class Helper USER_AGENT = "HTTP_USER_AGENT".freeze RACK_SESSION = "rack.session".freeze + ACCEPT_ENCODING = "HTTP_ACCEPT_ENCODING".freeze def initialize(env) @env = env @@ -35,6 +36,14 @@ module Middleware @is_mobile == :true end + def has_brotli? + @has_brotli ||= + begin + @env[ACCEPT_ENCODING].to_s =~ /br/ ? :true : :false + end + @has_brotli == :true + end + def is_crawler? @is_crawler ||= begin @@ -45,7 +54,7 @@ module Middleware end def cache_key - @cache_key ||= "ANON_CACHE_#{@env["HTTP_ACCEPT"]}_#{@env["HTTP_HOST"]}#{@env["REQUEST_URI"]}|m=#{is_mobile?}|c=#{is_crawler?}" + @cache_key ||= "ANON_CACHE_#{@env["HTTP_ACCEPT"]}_#{@env["HTTP_HOST"]}#{@env["REQUEST_URI"]}|m=#{is_mobile?}|c=#{is_crawler?}|b=#{has_brotli?}" end def cache_key_body diff --git a/spec/components/middleware/anonymous_cache_spec.rb b/spec/components/middleware/anonymous_cache_spec.rb index 8dce0e5bcc0..db0c481c4a8 100644 --- a/spec/components/middleware/anonymous_cache_spec.rb +++ b/spec/components/middleware/anonymous_cache_spec.rb @@ -45,6 +45,16 @@ describe Middleware::AnonymousCache::Helper do crawler.clear_cache end + it "handles brotli switching" do + helper.cache([200, {"HELLO" => "WORLD"}, ["hello ", "my world"]]) + + helper = new_helper("ANON_CACHE_DURATION" => 10) + expect(helper.cached).to eq([200, {"X-Discourse-Cached" => "true", "HELLO" => "WORLD"}, ["hello my world"]]) + + helper = new_helper("ANON_CACHE_DURATION" => 10, "HTTP_ACCEPT_ENCODING" => "gz, br") + expect(helper.cached).to eq(nil) + end + it "returns cached data for cached requests" do helper.is_mobile = true expect(helper.cached).to eq(nil)