Skip to content
This repository
Browse code

Fix bug that kept any before_filter except the first one from being a…

…ble to halt the before_filter chain. [Rick Olson]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5196 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
commit e537de00d806368f21cc22bde118fb90c1b6fe2d 1 parent 6a8dcc8
risk danger olson authored September 27, 2006
2  actionpack/CHANGELOG
... ...
@@ -1,5 +1,7 @@
1 1
 *SVN*
2 2
 
  3
+* Fix bug that kept any before_filter except the first one from being able to halt the before_filter chain.  [Rick Olson]
  4
+
3 5
 * strip_links is case-insensitive.  #6285 [tagoh, Bob Silva]
4 6
 
5 7
 * Clear the cache of possible controllers whenever Routes are reloaded. [Nicholas Seckar]
9  actionpack/lib/action_controller/filters.rb
@@ -629,13 +629,12 @@ def call_filter(chain, index)
629 629
         filter = chain[index]
630 630
         return call_filter(chain, index.next) if filter.excluded_from?(action_name)
631 631
 
632  
-        called = false
  632
+        halted = false
633 633
         filter.call(self) do
634  
-          call_filter(chain, index.next)
635  
-          called = true
  634
+          halted = call_filter(chain, index.next)
636 635
         end
637  
-        halt_filter_chain(filter.filter, :no_yield) if called == false
638  
-        called
  636
+        halt_filter_chain(filter.filter, :no_yield) if halted == false
  637
+        halted
639 638
       end
640 639
 
641 640
       def halt_filter_chain(filter, reason)
44  actionpack/test/controller/filters_test.rb
@@ -20,6 +20,26 @@ def clean_up
20 20
         @ran_after_filter << "clean_up"
21 21
       end
22 22
   end
  23
+  
  24
+  class TestMultipleFiltersController < ActionController::Base
  25
+    before_filter :try_1
  26
+    before_filter :try_2
  27
+    before_filter :try_3
  28
+
  29
+    (1..3).each do |i|
  30
+      define_method "fail_#{i}" do
  31
+        render :text => i.to_s
  32
+      end
  33
+    end
  34
+
  35
+    protected
  36
+    (1..3).each do |i|
  37
+      define_method "try_#{i}" do
  38
+        instance_variable_set :@try, i
  39
+        action_name != "fail_#{i}"
  40
+      end
  41
+    end
  42
+  end
23 43
 
24 44
   class RenderingController < ActionController::Base
25 45
     before_filter :render_something_else
@@ -620,6 +640,30 @@ def test_filter_order_with_skip_filter_method
620 640
     assert_equal 'before around (before yield) around (after yield)',controller.template.assigns['ran_filter'].join(' ')
621 641
   end
622 642
 
  643
+  def test_first_filter_in_multiple_before_filter_chain_halts
  644
+    controller = ::FilterTest::TestMultipleFiltersController.new
  645
+    response = test_process(controller, 'fail_1')
  646
+    assert_equal '', response.body
  647
+    assert_equal 1, controller.instance_variable_get(:@try)
  648
+    assert controller.instance_variable_get(:@before_filter_chain_aborted)
  649
+  end
  650
+
  651
+  def test_second_filter_in_multiple_before_filter_chain_halts
  652
+    controller = ::FilterTest::TestMultipleFiltersController.new
  653
+    response = test_process(controller, 'fail_2')
  654
+    assert_equal '', response.body
  655
+    assert_equal 2, controller.instance_variable_get(:@try)
  656
+    assert controller.instance_variable_get(:@before_filter_chain_aborted)
  657
+  end
  658
+
  659
+  def test_last_filter_in_multiple_before_filter_chain_halts
  660
+    controller = ::FilterTest::TestMultipleFiltersController.new
  661
+    response = test_process(controller, 'fail_3')
  662
+    assert_equal '', response.body
  663
+    assert_equal 3, controller.instance_variable_get(:@try)
  664
+    assert controller.instance_variable_get(:@before_filter_chain_aborted)
  665
+  end
  666
+
623 667
   protected
624 668
     def test_process(controller, action = "show")
625 669
       request = ActionController::TestRequest.new
2  actionpack/test/controller/verification_test.rb
@@ -3,6 +3,7 @@
3 3
 class VerificationTest < Test::Unit::TestCase
4 4
   class TestController < ActionController::Base
5 5
     verify :only => :guarded_one, :params => "one",
  6
+           :add_flash => { :error => 'unguarded' },
6 7
            :redirect_to => { :action => "unguarded" }
7 8
 
8 9
     verify :only => :guarded_two, :params => %w( one two ),
@@ -103,6 +104,7 @@ def test_guarded_one_with_prereqs
103 104
   def test_guarded_one_without_prereqs
104 105
     get :guarded_one
105 106
     assert_redirected_to :action => "unguarded"
  107
+    assert_equal 'unguarded', flash[:error]
106 108
   end
107 109
 
108 110
   def test_guarded_with_flash_with_prereqs

0 notes on commit e537de0

Please sign in to comment.
Something went wrong with that request. Please try again.