diff options
Diffstat (limited to 'actionpack')
-rw-r--r-- | actionpack/CHANGELOG | 2 | ||||
-rw-r--r-- | actionpack/lib/action_controller/filters.rb | 9 | ||||
-rw-r--r-- | actionpack/test/controller/filters_test.rb | 44 | ||||
-rw-r--r-- | actionpack/test/controller/verification_test.rb | 2 |
4 files changed, 52 insertions, 5 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index a66c43b850..96493f47cb 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Fix bug that kept any before_filter except the first one from being able to halt the before_filter chain. [Rick Olson] + * strip_links is case-insensitive. #6285 [tagoh, Bob Silva] * Clear the cache of possible controllers whenever Routes are reloaded. [Nicholas Seckar] diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb index 9569672bee..124df970ed 100644 --- a/actionpack/lib/action_controller/filters.rb +++ b/actionpack/lib/action_controller/filters.rb @@ -629,13 +629,12 @@ module ActionController #:nodoc: filter = chain[index] return call_filter(chain, index.next) if filter.excluded_from?(action_name) - called = false + halted = false filter.call(self) do - call_filter(chain, index.next) - called = true + halted = call_filter(chain, index.next) end - halt_filter_chain(filter.filter, :no_yield) if called == false - called + halt_filter_chain(filter.filter, :no_yield) if halted == false + halted end def halt_filter_chain(filter, reason) diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index a8bb11f7eb..421758ac34 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -20,6 +20,26 @@ class FilterTest < Test::Unit::TestCase @ran_after_filter << "clean_up" end end + + class TestMultipleFiltersController < ActionController::Base + before_filter :try_1 + before_filter :try_2 + before_filter :try_3 + + (1..3).each do |i| + define_method "fail_#{i}" do + render :text => i.to_s + end + end + + protected + (1..3).each do |i| + define_method "try_#{i}" do + instance_variable_set :@try, i + action_name != "fail_#{i}" + end + end + end class RenderingController < ActionController::Base before_filter :render_something_else @@ -620,6 +640,30 @@ class YieldingAroundFiltersTest < Test::Unit::TestCase assert_equal 'before around (before yield) around (after yield)',controller.template.assigns['ran_filter'].join(' ') end + def test_first_filter_in_multiple_before_filter_chain_halts + controller = ::FilterTest::TestMultipleFiltersController.new + response = test_process(controller, 'fail_1') + assert_equal '', response.body + assert_equal 1, controller.instance_variable_get(:@try) + assert controller.instance_variable_get(:@before_filter_chain_aborted) + end + + def test_second_filter_in_multiple_before_filter_chain_halts + controller = ::FilterTest::TestMultipleFiltersController.new + response = test_process(controller, 'fail_2') + assert_equal '', response.body + assert_equal 2, controller.instance_variable_get(:@try) + assert controller.instance_variable_get(:@before_filter_chain_aborted) + end + + def test_last_filter_in_multiple_before_filter_chain_halts + controller = ::FilterTest::TestMultipleFiltersController.new + response = test_process(controller, 'fail_3') + assert_equal '', response.body + assert_equal 3, controller.instance_variable_get(:@try) + assert controller.instance_variable_get(:@before_filter_chain_aborted) + end + protected def test_process(controller, action = "show") request = ActionController::TestRequest.new diff --git a/actionpack/test/controller/verification_test.rb b/actionpack/test/controller/verification_test.rb index ee2947ae98..c5ff175d1b 100644 --- a/actionpack/test/controller/verification_test.rb +++ b/actionpack/test/controller/verification_test.rb @@ -3,6 +3,7 @@ require File.dirname(__FILE__) + '/../abstract_unit' class VerificationTest < Test::Unit::TestCase class TestController < ActionController::Base verify :only => :guarded_one, :params => "one", + :add_flash => { :error => 'unguarded' }, :redirect_to => { :action => "unguarded" } verify :only => :guarded_two, :params => %w( one two ), @@ -103,6 +104,7 @@ class VerificationTest < Test::Unit::TestCase def test_guarded_one_without_prereqs get :guarded_one assert_redirected_to :action => "unguarded" + assert_equal 'unguarded', flash[:error] end def test_guarded_with_flash_with_prereqs |