Permalink
Browse files

Changed before_filter halting to happen automatically on render or re…

…direct but no longer on simply returning false [DHH]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7984 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent 7042163 commit f777ff72f931983946cbccbf2c64270922e93d84 @dhh dhh committed Oct 21, 2007
Showing with 36 additions and 23 deletions.
  1. +2 −0 actionpack/CHANGELOG
  2. +28 −19 actionpack/lib/action_controller/filters.rb
  3. +6 −4 actionpack/test/controller/filters_test.rb
View
2 actionpack/CHANGELOG
@@ -1,5 +1,7 @@
*SVN*
+* Changed before_filter halting to happen automatically on render or redirect but no longer on simply returning false [DHH]
+
* Ensure that cookies handle array values correctly. Closes #9937 [queso]
* Make sure resource routes don't clash with internal helpers like javascript_path, image_path etc. #9928 [gbuesing]
View
47 actionpack/lib/action_controller/filters.rb
@@ -7,7 +7,7 @@ def self.included(base)
end
end
- # Filters enable controllers to run shared pre and post processing code for its actions. These filters can be used to do
+ # Filters enable controllers to run shared pre- and post-processing code for its actions. These filters can be used to do
# authentication, caching, or auditing before the intended action is performed. Or to do localization or output
# compression after the action has been performed. Filters have access to the request, response, and all the instance
# variables set by other filters in the chain or by the action (in the case of after filters).
@@ -36,7 +36,7 @@ def self.included(base)
# end
#
# Now any actions performed on the BankController will have the audit method called before. On the VaultController,
- # first the audit method is called, then the verify_credentials method. If the audit method returns false, then
+ # first the audit method is called, then the verify_credentials method. If the audit method renders or redirects, then
# verify_credentials and the intended action are never called.
#
# == Filter types
@@ -65,7 +65,7 @@ def self.included(base)
# Or just as a quick test. It works like this:
#
# class WeblogController < ActionController::Base
- # before_filter { |controller| false if controller.params["stop_action"] }
+ # before_filter { |controller| head(400) if controller.params["stop_action"] }
# end
#
# As you can see, the block expects to be passed the controller after it has assigned the request to the internal variables.
@@ -90,7 +90,7 @@ def self.included(base)
# prepend_before_filter :ensure_items_in_cart, :ensure_items_in_stock
#
# The filter chain for the CheckoutController is now <tt>:ensure_items_in_cart, :ensure_items_in_stock,</tt>
- # <tt>:verify_open_shop</tt>. So if either of the ensure filters return false, we'll never get around to see if the shop
+ # <tt>:verify_open_shop</tt>. So if either of the ensure filters renders or redirects, we'll never get around to see if the shop
# is open or not.
#
# You may pass multiple filter arguments of each type as well as a filter block.
@@ -142,23 +142,20 @@ def self.included(base)
# around_filter Authorizer.new
#
# class Authorizer
- # # This will run before the action. Returning false aborts the action.
+ # # This will run before the action. Redirecting aborts the action.
# def before(controller)
- # if user.authorized?
- # return true
- # else
- # redirect_to login_url
- # return false
+ # unless user.authorized?
+ # redirect_to(login_url)
# end
# end
#
- # # This will run after the action if and only if before returned true.
+ # # This will run after the action if and only if before did not render or redirect.
# def after(controller)
# end
# end
#
# If the filter has before and after methods, the before method will be
- # called before the action. If before returns false, the filter chain is
+ # called before the action. If before renders or redirects, the filter chain is
# halted and after will not be run. See Filter Chain Halting below for
# an example.
#
@@ -218,8 +215,8 @@ def self.included(base)
# <tt>before_filter</tt> and <tt>around_filter</tt> may halt the request
# before a controller action is run. This is useful, for example, to deny
# access to unauthenticated users or to redirect from http to https.
- # Simply return false from the filter or call render or redirect.
- # After filters will not be executed if the filter chain is halted.
+ # Simply call render or redirect. After filters will not be executed if the filter
+ # chain is halted.
#
# Around filters halt the request unless the action block is called.
# Given these filters
@@ -244,7 +241,7 @@ def self.included(base)
# #after (actual filter code is run, unless the around filter does not yield)
#
# If #around returns before yielding, #after will still not be run. The #before
- # filter and controller action will not be run. If #before returns false,
+ # filter and controller action will not be run. If #before renders or redirects,
# the second half of #around and will still run but #after and the
# action will not. If #around fails to yield, #after will not be run.
module ClassMethods
@@ -441,8 +438,9 @@ def type
def run(controller)
# only filters returning false are halted.
- if false == @filter.call(controller)
- controller.send! :halt_filter_chain, @filter, :returned_false
+ @filter.call(controller)
+ if controller.send!(:performed?)
+ controller.send!(:halt_filter_chain, @filter, :rendered_or_redirected)
end
end
@@ -657,8 +655,10 @@ def filter_responds_to_before_and_after(filter) #:nodoc:
def proxy_before_and_after_filter(filter) #:nodoc:
return filter unless filter_responds_to_before_and_after(filter)
Proc.new do |controller, action|
- if filter.before(controller) == false
- controller.send! :halt_filter_chain, filter, :returned_false
+ filter.before(controller)
+
+ if controller.send!(:performed?)
+ controller.send!(:halt_filter_chain, filter, :rendered_or_redirected)
else
begin
action.call
@@ -710,32 +710,39 @@ def run_before_filters(chain, index, nesting)
while chain[index]
filter, index = skip_excluded_filters(chain, index)
break unless filter # end of call chain reached
+
case filter.type
when :before
filter.run(self) # invoke before filter
index = index.next
break if @before_filter_chain_aborted
when :around
yielded = false
+
filter.call(self) do
yielded = true
# all remaining before and around filters will be run in this call
index = call_filters(chain, index.next, nesting.next)
end
+
halt_filter_chain(filter, :did_not_yield) unless yielded
+
break
else
break # no before or around filters left
end
end
+
index
end
def run_after_filters(chain, index)
seen_after_filter = false
+
while chain[index]
filter, index = skip_excluded_filters(chain, index)
break unless filter # end of call chain reached
+
case filter.type
when :after
seen_after_filter = true
@@ -744,8 +751,10 @@ def run_after_filters(chain, index)
# implementation error or someone has mucked with the filter chain
raise ActionControllerError, "filter #{filter.inspect} was in the wrong place!" if seen_after_filter
end
+
index = index.next
end
+
index.next
end
View
10 actionpack/test/controller/filters_test.rb
@@ -44,7 +44,9 @@ class TestMultipleFiltersController < ActionController::Base
(1..3).each do |i|
define_method "try_#{i}" do
instance_variable_set :@try, i
- action_name != "fail_#{i}"
+ if action_name == "fail_#{i}"
+ head(404)
+ end
end
end
end
@@ -823,23 +825,23 @@ def test_filter_order_with_skip_filter_method
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 ' ', 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 ' ', 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 ' ', response.body
assert_equal 3, controller.instance_variable_get(:@try)
assert controller.instance_variable_get(:@before_filter_chain_aborted)
end

0 comments on commit f777ff7

Please sign in to comment.