diff options
Diffstat (limited to 'actionpack')
| -rw-r--r-- | actionpack/CHANGELOG.md | 8 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/log_subscriber.rb | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/conditional_get.rb | 53 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch.rb | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/middleware/debug_exceptions.rb | 77 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/middleware/exception_wrapper.rb | 78 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/middleware/show_exceptions.rb | 184 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/railtie.rb | 13 | ||||
| -rw-r--r-- | actionpack/lib/action_view/helpers/form_helper.rb | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_view/template/error.rb | 6 | ||||
| -rw-r--r-- | actionpack/lib/sprockets/railtie.rb | 2 | ||||
| -rw-r--r-- | actionpack/test/abstract_unit.rb | 22 | ||||
| -rw-r--r-- | actionpack/test/controller/render_test.rb | 46 | ||||
| -rw-r--r-- | actionpack/test/controller/show_exceptions_test.rb | 1 | ||||
| -rw-r--r-- | actionpack/test/dispatch/debug_exceptions_test.rb | 116 | ||||
| -rw-r--r-- | actionpack/test/dispatch/show_exceptions_test.rb | 95 |
16 files changed, 469 insertions, 238 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index b091d12d2c..0545d36a0a 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,6 +1,12 @@ ## Rails 3.2.0 (unreleased) ## -* Log "Filter chain halted as CALLBACKNAME rendered or redirected" every time a before callback halts. *José Valim* +* Allow rescue responses to be configured through a railtie as in `config.action_dispatch.rescue_responses`. Please look at ActiveRecord::Railtie for an example *José Valim* + +* Allow fresh_when/stale? to take a record instead of an options hash *DHH* + +* Assets should use the request protocol by default or default to relative if no request is available *Jonathan del Strother* + +* Log "Filter chain halted as CALLBACKNAME rendered or redirected" every time a before callback halts *José Valim* * You can provide a namespace for your form to ensure uniqueness of id attributes on form elements. The namespace attribute will be prefixed with underscore on the generate HTML id. *Vasiliy Ermolovich* diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index 8666ecc9c8..de250053d3 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -20,7 +20,7 @@ module ActionController status = payload[:status] if status.nil? && payload[:exception].present? - status = Rack::Utils.status_code(ActionDispatch::ShowExceptions.rescue_responses[payload[:exception].first]) rescue nil + status = Rack::Utils.status_code(ActionDispatch::ExceptionWrapper.new({}, payload[:exception]).status_code) end message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in %.0fms" % event.duration message << " (#{additions.join(" | ")})" unless additions.blank? diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index a5e37172c9..1645400693 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -23,8 +23,27 @@ module ActionController # This will render the show template if the request isn't sending a matching etag or # If-Modified-Since header and just a <tt>304 Not Modified</tt> response if there's a match. # - def fresh_when(options) - options.assert_valid_keys(:etag, :last_modified, :public) + # You can also just pass a record where last_modified will be set by calling updated_at and the etag by passing the object itself. Example: + # + # def show + # @article = Article.find(params[:id]) + # fresh_when(@article) + # end + # + # When passing a record, you can still set whether the public header: + # + # def show + # @article = Article.find(params[:id]) + # fresh_when(@article, :public => true) + # end + def fresh_when(record_or_options, additional_options = {}) + if record_or_options.is_a? Hash + options = record_or_options + options.assert_valid_keys(:etag, :last_modified, :public) + else + record = record_or_options + options = { :etag => record, :last_modified => record.try(:updated_at) }.merge(additional_options) + end response.etag = options[:etag] if options[:etag] response.last_modified = options[:last_modified] if options[:last_modified] @@ -55,8 +74,34 @@ module ActionController # end # end # end - def stale?(options) - fresh_when(options) + # + # You can also just pass a record where last_modified will be set by calling updated_at and the etag by passing the object itself. Example: + # + # def show + # @article = Article.find(params[:id]) + # + # if stale?(@article) + # @statistics = @article.really_expensive_call + # respond_to do |format| + # # all the supported formats + # end + # end + # end + # + # When passing a record, you can still set whether the public header: + # + # def show + # @article = Article.find(params[:id]) + # + # if stale?(@article, :public => true) + # @statistics = @article.really_expensive_call + # respond_to do |format| + # # all the supported formats + # end + # end + # end + def stale?(record_or_options, additional_options = {}) + fresh_when(record_or_options, additional_options) !request.fresh?(response) end diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 1e92d14542..89f515ee3d 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -51,6 +51,8 @@ module ActionDispatch autoload :BestStandardsSupport autoload :Callbacks autoload :Cookies + autoload :DebugExceptions + autoload :ExceptionWrapper autoload :Flash autoload :Head autoload :ParamsParser diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb new file mode 100644 index 0000000000..5da21ddd3d --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -0,0 +1,77 @@ +require 'action_dispatch/http/request' +require 'action_dispatch/middleware/exception_wrapper' + +module ActionDispatch + # This middleware is responsible for logging exceptions and + # showing a debugging page in case the request is local. + class DebugExceptions + RESCUES_TEMPLATE_PATH = File.join(File.dirname(__FILE__), 'templates') + + def initialize(app) + @app = app + end + + def call(env) + begin + response = @app.call(env) + + if response[1]['X-Cascade'] == 'pass' + raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}" + end + rescue Exception => exception + raise exception if env['action_dispatch.show_exceptions'] == false + end + + response ? response : render_exception(env, exception) + end + + private + + def render_exception(env, exception) + wrapper = ExceptionWrapper.new(env, exception) + log_error(env, wrapper) + + if env['action_dispatch.show_detailed_exceptions'] + template = ActionView::Base.new([RESCUES_TEMPLATE_PATH], + :request => Request.new(env), + :exception => wrapper.exception, + :application_trace => wrapper.application_trace, + :framework_trace => wrapper.framework_trace, + :full_trace => wrapper.full_trace + ) + + file = "rescues/#{wrapper.rescue_template}" + body = template.render(:template => file, :layout => 'rescues/layout') + render(wrapper.status_code, body) + else + raise exception + end + end + + def render(status, body) + [status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]] + end + + def log_error(env, wrapper) + logger = logger(env) + return unless logger + + exception = wrapper.exception + + ActiveSupport::Deprecation.silence do + message = "\n#{exception.class} (#{exception.message}):\n" + message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) + message << " " << wrapper.application_trace.join("\n ") + logger.fatal("#{message}\n\n") + end + end + + def logger(env) + env['action_dispatch.logger'] || stderr_logger + end + + def stderr_logger + @stderr_logger ||= Logger.new($stderr) + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb new file mode 100644 index 0000000000..c0532c80c4 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -0,0 +1,78 @@ +require 'action_controller/metal/exceptions' +require 'active_support/core_ext/exception' + +module ActionDispatch + class ExceptionWrapper + cattr_accessor :rescue_responses + @@rescue_responses = Hash.new(:internal_server_error) + @@rescue_responses.merge!( + 'ActionController::RoutingError' => :not_found, + 'AbstractController::ActionNotFound' => :not_found, + 'ActionController::MethodNotAllowed' => :method_not_allowed, + 'ActionController::NotImplemented' => :not_implemented, + 'ActionController::InvalidAuthenticityToken' => :unprocessable_entity + ) + + cattr_accessor :rescue_templates + @@rescue_templates = Hash.new('diagnostics') + @@rescue_templates.merge!( + 'ActionView::MissingTemplate' => 'missing_template', + 'ActionController::RoutingError' => 'routing_error', + 'AbstractController::ActionNotFound' => 'unknown_action', + 'ActionView::Template::Error' => 'template_error' + ) + + attr_reader :env, :exception + + def initialize(env, exception) + @env = env + @exception = original_exception(exception) + end + + def rescue_template + @@rescue_templates[@exception.class.name] + end + + def status_code + Rack::Utils.status_code(@@rescue_responses[@exception.class.name]) + end + + def application_trace + clean_backtrace(:silent) + end + + def framework_trace + clean_backtrace(:noise) + end + + def full_trace + clean_backtrace(:all) + end + + private + + def original_exception(exception) + if registered_original_exception?(exception) + exception.original_exception + else + exception + end + end + + def registered_original_exception?(exception) + exception.respond_to?(:original_exception) && @@rescue_responses.has_key?(exception.original_exception.class.name) + end + + def clean_backtrace(*args) + if backtrace_cleaner + backtrace_cleaner.clean(@exception.backtrace, *args) + else + @exception.backtrace + end + end + + def backtrace_cleaner + @backtrace_cleaner ||= @env['action_dispatch.backtrace_cleaner'] + 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 c850e25507..9efe90a670 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -1,44 +1,33 @@ -require 'active_support/core_ext/exception' -require 'action_controller/metal/exceptions' -require 'active_support/notifications' require 'action_dispatch/http/request' +require 'action_dispatch/middleware/exception_wrapper' require 'active_support/deprecation' module ActionDispatch - # This middleware rescues any exception returned by the application and renders - # nice exception pages if it's being rescued locally. + # 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') - cattr_accessor :rescue_responses - @@rescue_responses = Hash.new(:internal_server_error) - @@rescue_responses.update({ - 'ActionController::RoutingError' => :not_found, - 'AbstractController::ActionNotFound' => :not_found, - 'ActiveRecord::RecordNotFound' => :not_found, - 'ActiveRecord::StaleObjectError' => :conflict, - 'ActiveRecord::RecordInvalid' => :unprocessable_entity, - 'ActiveRecord::RecordNotSaved' => :unprocessable_entity, - 'ActionController::MethodNotAllowed' => :method_not_allowed, - 'ActionController::NotImplemented' => :not_implemented, - 'ActionController::InvalidAuthenticityToken' => :unprocessable_entity - }) - - cattr_accessor :rescue_templates - @@rescue_templates = Hash.new('diagnostics') - @@rescue_templates.update({ - 'ActionView::MissingTemplate' => 'missing_template', - 'ActionController::RoutingError' => 'routing_error', - 'AbstractController::ActionNotFound' => 'unknown_action', - 'ActionView::Template::Error' => 'template_error' - }) - FAILSAFE_RESPONSE = [500, {'Content-Type' => 'text/html'}, ["<html><body><h1>500 Internal Server Error</h1>" << "If you are the administrator of this website, then please read this web " << "application's log file and/or the web server's log file to find out what " << "went wrong.</body></html>"]] + class << self + def rescue_responses + ActiveSupport::Deprecation.warn "ActionDispatch::ShowExceptions.rescue_responses is deprecated. " \ + "Please configure your exceptions using a railtie or in your application config instead." + ExceptionWrapper.rescue_responses + end + + def rescue_templates + ActiveSupport::Deprecation.warn "ActionDispatch::ShowExceptions.rescue_templates is deprecated. " \ + "Please configure your exceptions using a railtie or in your application config instead." + ExceptionWrapper.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? @app = app @@ -46,131 +35,52 @@ module ActionDispatch def call(env) begin - status, headers, body = @app.call(env) - exception = nil - - # Only this middleware cares about RoutingError. So, let's just raise - # it here. - if headers['X-Cascade'] == 'pass' - raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}" - end + response = @app.call(env) rescue Exception => exception raise exception if env['action_dispatch.show_exceptions'] == false end - exception ? render_exception(env, exception) : [status, headers, body] + response ? response : render_exception_with_failsafe(env, exception) end private - def render_exception(env, exception) - log_error(env, exception) - exception = original_exception(exception) - - if env['action_dispatch.show_detailed_exceptions'] == true - rescue_action_diagnostics(env, exception) - else - rescue_action_error_page(exception) - end - rescue Exception => failsafe_error - $stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}" - FAILSAFE_RESPONSE - end - - # Render detailed diagnostics for unhandled exceptions rescued from - # a controller action. - def rescue_action_diagnostics(env, exception) - template = ActionView::Base.new([RESCUES_TEMPLATE_PATH], - :request => Request.new(env), - :exception => exception, - :application_trace => application_trace(env, exception), - :framework_trace => framework_trace(env, exception), - :full_trace => full_trace(env, exception) - ) - file = "rescues/#{@@rescue_templates[exception.class.name]}" - body = template.render(:template => file, :layout => 'rescues/layout') - render(status_code(exception), body) - end - - # Attempts to render a static error page based on the - # <tt>status_code</tt> thrown, or just return headers if no such file - # exists. At first, it will try to render a localized static page. - # For example, if a 500 error is being handled Rails and locale is :da, - # it will first attempt to render the file at <tt>public/500.da.html</tt> - # then attempt to render <tt>public/500.html</tt>. If none of them exist, - # the body of the response will be left empty. - def rescue_action_error_page(exception) - status = status_code(exception) - 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 status_code(exception) - Rack::Utils.status_code(@@rescue_responses[exception.class.name]) - end - - def render(status, body) - [status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]] - end - - def public_path - defined?(Rails.public_path) ? Rails.public_path : 'public_path' - end - - def log_error(env, exception) - return unless logger(env) - - ActiveSupport::Deprecation.silence do - message = "\n#{exception.class} (#{exception.message}):\n" - message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) - message << " " << application_trace(env, exception).join("\n ") - logger(env).fatal("#{message}\n\n") - end - end - - def application_trace(env, exception) - clean_backtrace(env, exception, :silent) - end - - def framework_trace(env, exception) - clean_backtrace(env, exception, :noise) - end - def full_trace(env, exception) - clean_backtrace(env, exception, :all) - end + # Define this method because some plugins were monkey patching it. + # Remove this after 3.2 is out with the other deprecations in this class. + def status_code(*) + end - def clean_backtrace(env, exception, *args) - env['action_dispatch.backtrace_cleaner'] ? - env['action_dispatch.backtrace_cleaner'].clean(exception.backtrace, *args) : - exception.backtrace - end + def render_exception_with_failsafe(env, exception) + render_exception(env, exception) + rescue Exception => failsafe_error + $stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}" + FAILSAFE_RESPONSE + end - def logger(env) - env['action_dispatch.logger'] || stderr_logger - end + def render_exception(env, exception) + wrapper = ExceptionWrapper.new(env, exception) - def stderr_logger - Logger.new($stderr) - end + status = wrapper.status_code + locale_path = "#{public_path}/#{status}.#{I18n.locale}.html" if I18n.locale + path = "#{public_path}/#{status}.html" - def original_exception(exception) - if registered_original_exception?(exception) - exception.original_exception + if locale_path && File.exist?(locale_path) + render(status, File.read(locale_path)) + elsif File.exist?(path) + render(status, File.read(path)) else - exception + render(status, '') end end - def registered_original_exception?(exception) - exception.respond_to?(:original_exception) && @@rescue_responses.has_key?(exception.original_exception.class.name) + 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/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index f18ebabf29..2ba3198ebd 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -9,11 +9,22 @@ module ActionDispatch config.action_dispatch.best_standards_support = true config.action_dispatch.tld_length = 1 config.action_dispatch.ignore_accept_header = false - config.action_dispatch.rack_cache = {:metastore => "rails:/", :entitystore => "rails:/", :verbose => true} + config.action_dispatch.rescue_templates = { } + config.action_dispatch.rescue_responses = { } + + config.action_dispatch.rack_cache = { + :metastore => "rails:/", + :entitystore => "rails:/", + :verbose => true + } + initializer "action_dispatch.configure" do |app| ActionDispatch::Http::URL.tld_length = app.config.action_dispatch.tld_length ActionDispatch::Request.ignore_accept_header = app.config.action_dispatch.ignore_accept_header + ActionDispatch::ExceptionWrapper.rescue_responses.merge!(config.action_dispatch.rescue_responses) + ActionDispatch::ExceptionWrapper.rescue_templates.merge!(config.action_dispatch.rescue_templates) + config.action_dispatch.always_write_cookie = Rails.env.development? if config.action_dispatch.always_write_cookie.nil? ActionDispatch::Cookies::CookieJar.always_write_cookie = config.action_dispatch.always_write_cookie end diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 6c97d75922..e674e12509 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -160,7 +160,7 @@ module ActionView # here a named route directly as well. Defaults to the current action. # * <tt>:namespace</tt> - A namespace for your form to ensure uniqueness of # id attributes on form elements. The namespace attribute will be prefixed - # with underscore on the generate HTML id. + # with underscore on the generated HTML id. # * <tt>:html</tt> - Optional HTML attributes for the form tag. # # Also note that +form_for+ doesn't create an exclusive scope. It's still diff --git a/actionpack/lib/action_view/template/error.rb b/actionpack/lib/action_view/template/error.rb index 587e37a84f..fe27e54037 100644 --- a/actionpack/lib/action_view/template/error.rb +++ b/actionpack/lib/action_view/template/error.rb @@ -89,10 +89,14 @@ module ActionView line_counter = start_on_line return unless source_code = source_code[start_on_line..end_on_line] - source_code.sum do |line| + extract = source_code.sum do |line| line_counter += 1 "#{indent}#{line_counter}: #{line}\n" end + + extract.encode! if extract.respond_to?(:encode!) + + extract end def sub_template_of(template_path) diff --git a/actionpack/lib/sprockets/railtie.rb b/actionpack/lib/sprockets/railtie.rb index 3d330bd91a..a7eb03acaf 100644 --- a/actionpack/lib/sprockets/railtie.rb +++ b/actionpack/lib/sprockets/railtie.rb @@ -10,8 +10,6 @@ module Sprockets # TODO: Get rid of config.assets.enabled class Railtie < ::Rails::Railtie - config.action_controller.default_asset_host_protocol = :relative - rake_tasks do load "sprockets/assets.rake" end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index d191a203dd..c95b8221a1 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -175,6 +175,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::DebugExceptions" middleware.use "ActionDispatch::Callbacks" middleware.use "ActionDispatch::ParamsParser" middleware.use "ActionDispatch::Cookies" @@ -338,16 +339,19 @@ end module ActionDispatch class ShowExceptions private - remove_method :public_path - def public_path - "#{FIXTURE_LOAD_PATH}/public" - end + remove_method :public_path + def public_path + "#{FIXTURE_LOAD_PATH}/public" + end + end - remove_method :stderr_logger - # Silence logger - def stderr_logger - nil - end + class DebugExceptions + private + remove_method :stderr_logger + # Silence logger + def stderr_logger + nil + end end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index aea603b014..243bad8749 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -50,12 +50,28 @@ class TestController < ActionController::Base end end + def conditional_hello_with_record + record = Struct.new(:updated_at, :cache_key).new(Time.now.utc.beginning_of_day, "foo/123") + + if stale?(record) + render :action => 'hello_world' + end + end + def conditional_hello_with_public_header if stale?(:last_modified => Time.now.utc.beginning_of_day, :etag => [:foo, 123], :public => true) render :action => 'hello_world' end end + def conditional_hello_with_public_header_with_record + record = Struct.new(:updated_at, :cache_key).new(Time.now.utc.beginning_of_day, "foo/123") + + if stale?(record, :public => true) + render :action => 'hello_world' + end + end + def conditional_hello_with_public_header_and_expires_at expires_in 1.minute if stale?(:last_modified => Time.now.utc.beginning_of_day, :etag => [:foo, 123], :public => true) @@ -1440,6 +1456,36 @@ class LastModifiedRenderTest < ActionController::TestCase assert_equal @last_modified, @response.headers['Last-Modified'] end + + def test_responds_with_last_modified_with_record + get :conditional_hello_with_record + assert_equal @last_modified, @response.headers['Last-Modified'] + end + + def test_request_not_modified_with_record + @request.if_modified_since = @last_modified + get :conditional_hello_with_record + assert_equal 304, @response.status.to_i + assert_blank @response.body + assert_equal @last_modified, @response.headers['Last-Modified'] + end + + def test_request_not_modified_but_etag_differs_with_record + @request.if_modified_since = @last_modified + @request.if_none_match = "234" + get :conditional_hello_with_record + assert_response :success + end + + def test_request_modified_with_record + @request.if_modified_since = 'Thu, 16 Jul 2008 00:00:00 GMT' + get :conditional_hello_with_record + assert_equal 200, @response.status.to_i + assert_present @response.body + assert_equal @last_modified, @response.headers['Last-Modified'] + end + + def test_request_with_bang_gets_last_modified get :conditional_hello_with_bangs assert_equal @last_modified, @response.headers['Last-Modified'] diff --git a/actionpack/test/controller/show_exceptions_test.rb b/actionpack/test/controller/show_exceptions_test.rb index 74067cb895..5eff1eb09d 100644 --- a/actionpack/test/controller/show_exceptions_test.rb +++ b/actionpack/test/controller/show_exceptions_test.rb @@ -3,6 +3,7 @@ require 'abstract_unit' module ShowExceptions class ShowExceptionsController < ActionController::Base use ActionDispatch::ShowExceptions + use ActionDispatch::DebugExceptions def boom raise 'boom!' diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb new file mode 100644 index 0000000000..e6c0a06878 --- /dev/null +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -0,0 +1,116 @@ +require 'abstract_unit' + +class DebugExceptionsTest < ActionDispatch::IntegrationTest + + class Boomer + def initialize(detailed = false) + @detailed = detailed + end + + def call(env) + env['action_dispatch.show_detailed_exceptions'] = @detailed + req = ActionDispatch::Request.new(env) + case req.path + when "/not_found" + raise ActionController::UnknownAction + when "/runtime_error" + raise RuntimeError + when "/method_not_allowed" + raise ActionController::MethodNotAllowed + when "/not_implemented" + raise ActionController::NotImplemented + when "/unprocessable_entity" + raise ActionController::InvalidAuthenticityToken + when "/not_found_original_exception" + raise ActionView::Template::Error.new('template', {}, AbstractController::ActionNotFound.new) + else + raise "puke!" + end + end + end + + ProductionApp = ActionDispatch::DebugExceptions.new((Boomer.new(false))) + DevelopmentApp = ActionDispatch::DebugExceptions.new((Boomer.new(true))) + + test 'skip diagnosis if not showing detailed exceptions' do + @app = ProductionApp + assert_raise RuntimeError do + get "/", {}, {'action_dispatch.show_exceptions' => true} + end + end + + test 'skip diagnosis if not showing exceptions' do + @app = DevelopmentApp + assert_raise RuntimeError do + get "/", {}, {'action_dispatch.show_exceptions' => false} + end + end + + test "rescue with diagnostics message" do + @app = DevelopmentApp + + get "/", {}, {'action_dispatch.show_exceptions' => true} + assert_response 500 + assert_match(/puke/, body) + + get "/not_found", {}, {'action_dispatch.show_exceptions' => true} + assert_response 404 + assert_match(/#{ActionController::UnknownAction.name}/, body) + + get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} + assert_response 405 + assert_match(/ActionController::MethodNotAllowed/, body) + end + + test "does not show filtered parameters" do + @app = DevelopmentApp + + get "/", {"foo"=>"bar"}, {'action_dispatch.show_exceptions' => true, + 'action_dispatch.parameter_filter' => [:foo]} + assert_response 500 + assert_match(""foo"=>"[FILTERED]"", body) + end + + test "show registered original exception for wrapped exceptions" do + @app = DevelopmentApp + + get "/not_found_original_exception", {}, {'action_dispatch.show_exceptions' => true} + assert_response 404 + assert_match(/AbstractController::ActionNotFound/, body) + end + + test "show the controller name in the diagnostics template when controller name is present" do + @app = DevelopmentApp + get("/runtime_error", {}, { + 'action_dispatch.show_exceptions' => true, + 'action_dispatch.request.parameters' => { + 'action' => 'show', + 'id' => 'unknown', + 'controller' => 'featured_tile' + } + }) + assert_response 500 + assert_match(/RuntimeError\n in FeaturedTileController/, body) + end + + test "sets the HTTP charset parameter" do + @app = DevelopmentApp + + get "/", {}, {'action_dispatch.show_exceptions' => true} + assert_equal "text/html; charset=utf-8", response.headers["Content-Type"] + end + + test 'uses logger from env' do + @app = DevelopmentApp + output = StringIO.new + get "/", {}, {'action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => Logger.new(output)} + assert_match(/puke/, output.rewind && output.read) + end + + test 'uses backtrace cleaner from env' do + @app = DevelopmentApp + cleaner = stub(:clean => ['passed backtrace cleaner']) + get "/", {}, {'action_dispatch.show_exceptions' => true, 'action_dispatch.backtrace_cleaner' => cleaner} + assert_match(/passed backtrace cleaner/, body) + end +end diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 90f13a3bb9..020cc80f3f 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -3,24 +3,13 @@ require 'abstract_unit' class ShowExceptionsTest < ActionDispatch::IntegrationTest class Boomer - def initialize(detailed = false) - @detailed = detailed - end - def call(env) - env['action_dispatch.show_detailed_exceptions'] = @detailed req = ActionDispatch::Request.new(env) case req.path when "/not_found" raise ActionController::UnknownAction - when "/runtime_error" - raise RuntimeError when "/method_not_allowed" raise ActionController::MethodNotAllowed - when "/not_implemented" - raise ActionController::NotImplemented - when "/unprocessable_entity" - raise ActionController::InvalidAuthenticityToken when "/not_found_original_exception" raise ActionView::Template::Error.new('template', {}, AbstractController::ActionNotFound.new) else @@ -29,10 +18,16 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest end end - ProductionApp = ActionDispatch::ShowExceptions.new(Boomer.new(false)) - DevelopmentApp = ActionDispatch::ShowExceptions.new(Boomer.new(true)) + ProductionApp = ActionDispatch::ShowExceptions.new(Boomer.new) + + test 'skip diagnosis if not showing exceptions' do + @app = ProductionApp + assert_raise RuntimeError do + get "/", {}, {'action_dispatch.show_exceptions' => false} + end + end - test "rescue with error page when show_exceptions is false" do + test "rescue with error page" do @app = ProductionApp get "/", {}, {'action_dispatch.show_exceptions' => true} @@ -48,24 +43,7 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest assert_equal "", body end - test "rescue with diagnostics message when show_exceptions is true" do - @app = DevelopmentApp - - get "/", {}, {'action_dispatch.show_exceptions' => true} - assert_response 500 - assert_match(/puke/, body) - - get "/not_found", {}, {'action_dispatch.show_exceptions' => true} - assert_response 404 - assert_match(/#{ActionController::UnknownAction.name}/, body) - - get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} - assert_response 405 - assert_match(/ActionController::MethodNotAllowed/, body) - end - test "localize rescue error page" do - # Change locale old_locale, I18n.locale = I18n.locale, :da begin @@ -83,63 +61,18 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest end end - test "does not show filtered parameters" do - @app = DevelopmentApp - - get "/", {"foo"=>"bar"}, {'action_dispatch.show_exceptions' => true, - 'action_dispatch.parameter_filter' => [:foo]} - assert_response 500 - assert_match(""foo"=>"[FILTERED]"", body) - end - - test "show registered original exception for wrapped exceptions when show_exceptions is false" do - @app = ProductionApp - - get "/not_found_original_exception", {}, {'action_dispatch.show_exceptions' => true} - assert_response 404 - assert_match(/404 error/, body) - end - - test "show registered original exception for wrapped exceptions when show_exceptions is true" do - @app = DevelopmentApp - - get "/not_found_original_exception", {}, {'action_dispatch.show_exceptions' => true} - assert_response 404 - assert_match(/AbstractController::ActionNotFound/, body) - end - - test "show the controller name in the diagnostics template when controller name is present" do - @app = DevelopmentApp - get("/runtime_error", {}, { - 'action_dispatch.show_exceptions' => true, - 'action_dispatch.request.parameters' => { - 'action' => 'show', - 'id' => 'unknown', - 'controller' => 'featured_tile' - } - }) - assert_response 500 - assert_match(/RuntimeError\n in FeaturedTileController/, body) - end - test "sets the HTTP charset parameter" do - @app = DevelopmentApp + @app = ProductionApp get "/", {}, {'action_dispatch.show_exceptions' => true} assert_equal "text/html; charset=utf-8", response.headers["Content-Type"] end - test 'uses logger from env' do + test "show registered original exception for wrapped exceptions" do @app = ProductionApp - output = StringIO.new - get "/", {}, {'action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => Logger.new(output)} - assert_match(/puke/, output.rewind && output.read) - end - test 'uses backtrace cleaner from env' do - @app = DevelopmentApp - cleaner = stub(:clean => ['passed backtrace cleaner']) - get "/", {}, {'action_dispatch.show_exceptions' => true, 'action_dispatch.backtrace_cleaner' => cleaner} - assert_match(/passed backtrace cleaner/, body) + get "/not_found_original_exception", {}, {'action_dispatch.show_exceptions' => true} + assert_response 404 + assert_match(/404 error/, body) end end |