From deef8dd68241cdb9826a2293bced5e2257ccea90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 16 Dec 2011 09:29:37 +0100 Subject: [PATCH] Extract the rendering of public exceptions pages into a Rack app. --- actionpack/lib/action_dispatch.rb | 1 + .../middleware/public_exceptions.rb | 30 +++++++++++ .../middleware/show_exceptions.rb | 51 +++++++------------ actionpack/test/abstract_unit.rb | 10 +--- .../test/controller/show_exceptions_test.rb | 2 +- .../test/dispatch/show_exceptions_test.rb | 2 +- railties/lib/rails/application.rb | 2 +- 7 files changed, 53 insertions(+), 45 deletions(-) create mode 100644 actionpack/lib/action_dispatch/middleware/public_exceptions.rb diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 38c361c100..4b821037dc 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -56,6 +56,7 @@ module ActionDispatch autoload :Flash autoload :Head autoload :ParamsParser + autoload :PublicExceptions autoload :Reloader autoload :RemoteIp autoload :Rescue diff --git a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb new file mode 100644 index 0000000000..6600d6232e --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb @@ -0,0 +1,30 @@ +module ActionDispatch + # A simple Rack application that renders exceptions in the given public path. + class PublicExceptions + attr_accessor :public_path + + def initialize(public_path) + @public_path = public_path + end + + def call(env) + status = env["PATH_INFO"][1..-1] + locale_path = "#{public_path}/#{status}.#{I18n.locale}.html" if I18n.locale + path = "#{public_path}/#{status}.html" + + if locale_path && File.exist?(locale_path) + render(status, File.read(locale_path)) + elsif File.exist?(path) + render(status, File.read(path)) + else + render(status, '') + end + end + + private + + def render(status, body) + [status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]] + end + end +end \ No newline at end of file diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 0c95248611..4cfcb0a7db 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -6,8 +6,6 @@ module ActionDispatch # This middleware rescues any exception returned by the application # and wraps them in a format for the end user. class ShowExceptions - RESCUES_TEMPLATE_PATH = File.join(File.dirname(__FILE__), 'templates') - FAILSAFE_RESPONSE = [500, {'Content-Type' => 'text/html'}, ["

500 Internal Server Error

" << "If you are the administrator of this website, then please read this web " << @@ -28,9 +26,19 @@ def rescue_templates end end - def initialize(app, consider_all_requests_local = nil) - ActiveSupport::Deprecation.warn "Passing consider_all_requests_local option to ActionDispatch::ShowExceptions middleware no longer works" unless consider_all_requests_local.nil? + def initialize(app, exceptions_app = nil) + if [true, false].include?(exceptions_app) + ActiveSupport::Deprecation.warn "Passing consider_all_requests_local option to ActionDispatch::ShowExceptions middleware no longer works" + exceptions_app = nil + end + + if exceptions_app.nil? + raise ArgumentError, "You need to pass an exceptions_app when initializing ActionDispatch::ShowExceptions. " \ + "In case you want to render pages from a public path, you can use ActionDispatch::PublicExceptions.new('path/to/public')" + end + @app = app + @exceptions_app = exceptions_app end def call(env) @@ -40,7 +48,7 @@ def call(env) raise exception if env['action_dispatch.show_exceptions'] == false end - response || render_exception_with_failsafe(env, exception) + response || render_exception(env, exception) end private @@ -50,37 +58,14 @@ def call(env) def status_code(*) end - def render_exception_with_failsafe(env, exception) - render_exception(env, exception) + def render_exception(env, exception) + wrapper = ExceptionWrapper.new(env, exception) + env["action_dispatch.exception"] = wrapper.exception + env["PATH_INFO"] = "/#{wrapper.status_code}" + @exceptions_app.call(env) rescue Exception => failsafe_error $stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}" FAILSAFE_RESPONSE end - - def render_exception(env, exception) - wrapper = ExceptionWrapper.new(env, exception) - - status = wrapper.status_code - locale_path = "#{public_path}/#{status}.#{I18n.locale}.html" if I18n.locale - path = "#{public_path}/#{status}.html" - - if locale_path && File.exist?(locale_path) - render(status, File.read(locale_path)) - elsif File.exist?(path) - render(status, File.read(path)) - else - render(status, '') - end - end - - def render(status, body) - [status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]] - end - - # TODO: Make this a middleware initialization parameter once - # we removed the second option (which is deprecated) - def public_path - defined?(Rails.public_path) ? Rails.public_path : 'public_path' - end end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index c95b8221a1..40ca23a39d 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -174,7 +174,7 @@ class ActionDispatch::IntegrationTest < ActiveSupport::TestCase def self.build_app(routes = nil) RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware| - middleware.use "ActionDispatch::ShowExceptions" + middleware.use "ActionDispatch::ShowExceptions", ActionDispatch::PublicExceptions.new("#{FIXTURE_LOAD_PATH}/public") middleware.use "ActionDispatch::DebugExceptions" middleware.use "ActionDispatch::Callbacks" middleware.use "ActionDispatch::ParamsParser" @@ -337,14 +337,6 @@ def to_s end module ActionDispatch - class ShowExceptions - private - remove_method :public_path - def public_path - "#{FIXTURE_LOAD_PATH}/public" - end - end - class DebugExceptions private remove_method :stderr_logger diff --git a/actionpack/test/controller/show_exceptions_test.rb b/actionpack/test/controller/show_exceptions_test.rb index 2f5c268330..ba78559f31 100644 --- a/actionpack/test/controller/show_exceptions_test.rb +++ b/actionpack/test/controller/show_exceptions_test.rb @@ -2,7 +2,7 @@ module ShowExceptions class ShowExceptionsController < ActionController::Base - use ActionDispatch::ShowExceptions + use ActionDispatch::ShowExceptions, ActionDispatch::PublicExceptions.new("#{FIXTURE_LOAD_PATH}/public") use ActionDispatch::DebugExceptions before_filter :only => :another_boom do diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 020cc80f3f..d3c99e4f31 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -18,7 +18,7 @@ def call(env) end end - ProductionApp = ActionDispatch::ShowExceptions.new(Boomer.new) + ProductionApp = ActionDispatch::ShowExceptions.new(Boomer.new, ActionDispatch::PublicExceptions.new("#{FIXTURE_LOAD_PATH}/public")) test 'skip diagnosis if not showing exceptions' do @app = ProductionApp diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 56d11d0b01..54612755b4 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -244,7 +244,7 @@ def default_middleware_stack middleware.use ::Rack::MethodOverride middleware.use ::ActionDispatch::RequestId middleware.use ::Rails::Rack::Logger, config.log_tags # must come after Rack::MethodOverride to properly log overridden methods - middleware.use ::ActionDispatch::ShowExceptions + middleware.use ::ActionDispatch::ShowExceptions, ActionDispatch::PublicExceptions.new(Rails.public_path) middleware.use ::ActionDispatch::DebugExceptions middleware.use ::ActionDispatch::RemoteIp, config.action_dispatch.ip_spoofing_check, config.action_dispatch.trusted_proxies