Fix bug that kept any before_filter except the first one from being able to halt the before_filter chain. [Rick Olson]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5196 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

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