From c106ca67785f7d578505fe8fa82f1863268a5e89 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 20 Mar 2017 15:59:06 -0400 Subject: [PATCH] FEATURE: fallback asset path for multi host setups --- app/controllers/static_controller.rb | 72 +++++++++++----------- config/discourse_defaults.conf | 6 ++ lib/tasks/assets.rake | 6 +- spec/controllers/static_controller_spec.rb | 24 +++++++- 4 files changed, 66 insertions(+), 42 deletions(-) diff --git a/app/controllers/static_controller.rb b/app/controllers/static_controller.rb index e91bda504c4..b055e011ab3 100644 --- a/app/controllers/static_controller.rb +++ b/app/controllers/static_controller.rb @@ -127,64 +127,62 @@ class StaticController < ApplicationController 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$/ - - begin - response.headers["Last-Modified"] = File.ctime(path).httpdate - response.headers["Content-Length"] = File.size(path).to_s - rescue Errno::ENOENT - response.headers["Expires"] = 5.seconds.from_now.httpdate - response.headers["Cache-Control"] = 'max-age=5, public' - expires_in 5.seconds, public: true, must_revalidate: false - - render text: "missing brotli asset", status: 404 - return + serve_asset(".br") do + response.headers["Content-Encoding"] = 'br' end - - response.headers["Content-Encoding"] = 'br' - - # disable NGINX mucking with transfer - request.env['sendfile.type'] = '' - immutable_for 1.year - send_file(path, opts) end def cdn_asset - path = File.expand_path(Rails.root + "public/assets/" + params[:path]) + serve_asset + end + + protected + + def serve_asset(suffix=nil) + + path = File.expand_path(Rails.root + "public/assets/#{params[:path]}#{suffix}") # SECURITY what if path has /../ raise Discourse::NotFound unless path.start_with?(Rails.root.to_s + "/public/assets") - immutable_for 1.year response.headers["Expires"] = 1.year.from_now.httpdate response.headers["Access-Control-Allow-Origin"] = params[:origin] if params[:origin] begin response.headers["Last-Modified"] = File.ctime(path).httpdate - response.headers["Content-Length"] = File.size(path).to_s rescue Errno::ENOENT - raise Discourse::NotFound + begin + if GlobalSetting.fallback_assets_path.present? + path = File.expand_path("#{GlobalSetting.fallback_assets_path}/#{params[:path]}#{suffix}") + response.headers["Last-Modified"] = File.ctime(path).httpdate + else + raise + end + rescue Errno::ENOENT + response.headers["Expires"] = 5.seconds.from_now.httpdate + response.headers["Cache-Control"] = 'max-age=5, public' + expires_in 1.second, public: true, must_revalidate: false + + render text: "can not find #{params[:path]}", status: 404 + return + end end + response.headers["Content-Length"] = File.size(path).to_s + + yield if block_given? + + immutable_for 1.year + + # disable NGINX mucking with transfer + request.env['sendfile.type'] = '' + opts = { disposition: nil } opts[:type] = "application/javascript" if path =~ /\.js$/ - - immutable_for(1.year) - - # we must disable acceleration otherwise NGINX strips - # access control headers - request.env['sendfile.type'] = '' - # TODO send_file chunks which kills caching, need to render text here send_file(path, opts) + end end diff --git a/config/discourse_defaults.conf b/config/discourse_defaults.conf index 74284c88de3..3d8e096ab9f 100644 --- a/config/discourse_defaults.conf +++ b/config/discourse_defaults.conf @@ -150,3 +150,9 @@ message_bus_max_backlog_size = 100 # must be a 64 byte hex string, anything else will be ignored with a warning secret_key_base = + +# fallback path for all assets which are served via the application +# used by static_controller +# in multi host setups this allows you to have old unicorn instances serve +# newly compiled assets +fallback_assets_path = diff --git a/lib/tasks/assets.rake b/lib/tasks/assets.rake index b0c2f553e29..9612a09a818 100644 --- a/lib/tasks/assets.rake +++ b/lib/tasks/assets.rake @@ -175,11 +175,11 @@ task 'assets:precompile' => 'assets:precompile:before' do # protected manifest.send :save - if backup_path=ENV["BACKUP_ASSET_PATH"] + if GlobalSetting.fallback_assets_path.present? begin - FileUtils.cp_r("#{Rails.root}/public/assets/.", backup_path) + FileUtils.cp_r("#{Rails.root}/public/assets/.", GlobalSetting.fallback_assets_path) rescue => e - STDERR.puts "Failed to backup assets to #{backup_path}" + STDERR.puts "Failed to backup assets to #{GlobalSetting.fallback_assets_path}" STDERR.puts e STDERR.puts e.backtrace end diff --git a/spec/controllers/static_controller_spec.rb b/spec/controllers/static_controller_spec.rb index 6e7f13a4c18..fba85ae8c11 100644 --- a/spec/controllers/static_controller_spec.rb +++ b/spec/controllers/static_controller_spec.rb @@ -3,13 +3,33 @@ require 'rails_helper' describe StaticController do context 'brotli_asset' do - it 'returns a brotli encoded 404 if asset is missing' do + it 'returns a non brotli encoded 404 if asset is missing' do get :brotli_asset, path: 'missing.js' expect(response.status).to eq(404) expect(response.headers['Content-Encoding']).not_to eq('br') - expect(response.headers["Cache-Control"]).to match(/max-age=5/) + expect(response.headers["Cache-Control"]).to match(/max-age=1/) + end + + it 'can handle fallback brotli assets' do + begin + assets_path = Rails.root.join("tmp/backup_assets") + + GlobalSetting.stubs(:fallback_assets_path).returns(assets_path.to_s) + + FileUtils.mkdir_p(assets_path) + + file_path = assets_path.join("test.js.br") + File.write(file_path, 'fake brotli file') + + get :brotli_asset, path: 'test.js' + + expect(response.status).to eq(200) + expect(response.headers["Cache-Control"]).to match(/public/) + ensure + File.delete(file_path) + end end it 'has correct headers for brotli assets' do