mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Fix errors with around_filters which do not yield, restore 1.1 behaviour with after filters. Closes #8891 [skaes]
git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7177 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
This commit is contained in:
parent
f7bd676c8d
commit
e80fabbbf4
3 changed files with 126 additions and 54 deletions
|
@ -1,5 +1,11 @@
|
|||
*SVN*
|
||||
|
||||
* Fix errors with around_filters which do not yield, restore 1.1 behaviour with after filters. Closes #8891 [skaes]
|
||||
|
||||
After filters will *no longer* be run if an around_filter fails to yield, users relying on
|
||||
this behaviour are advised to put the code in question after a yield statement in an around filter.
|
||||
|
||||
|
||||
* Allow you to delete cookies with options. Closes #3685 [josh, Chris Wanstrath]
|
||||
|
||||
* Allow you to render views with periods in the name. Closes #8076 [norbert]
|
||||
|
|
|
@ -214,9 +214,10 @@ module ActionController #:nodoc:
|
|||
# == Filter Chain Halting
|
||||
#
|
||||
# <tt>before_filter</tt> and <tt>around_filter</tt> may halt the request
|
||||
# before controller action is run. This is useful, for example, to deny
|
||||
# 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.
|
||||
#
|
||||
# Around filters halt the request unless the action block is called.
|
||||
# Given these filters
|
||||
|
@ -238,12 +239,12 @@ module ActionController #:nodoc:
|
|||
# . . /
|
||||
# . #around (code after yield)
|
||||
# . /
|
||||
# #after (actual filter code is run)
|
||||
# #after (actual filter code is run, unless the around filter does not yield)
|
||||
#
|
||||
# If #around returns before yielding, only #after will be run. The #before
|
||||
# filter and controller action will not be run. If #before returns false,
|
||||
# the second half of #around and all of #after will still run but the
|
||||
# action will not.
|
||||
# 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,
|
||||
# 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
|
||||
# The passed <tt>filters</tt> will be appended to the filter_chain and
|
||||
# will execute before the action on this controller is performed.
|
||||
|
@ -439,7 +440,7 @@ module ActionController #:nodoc:
|
|||
def run(controller)
|
||||
# only filters returning false are halted.
|
||||
if false == @filter.call(controller)
|
||||
controller.halt_filter_chain(@filter)
|
||||
controller.send :halt_filter_chain, @filter, :returned_false
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -528,7 +529,7 @@ module ActionController #:nodoc:
|
|||
|
||||
def find_filter_append_position(filters, filter_type)
|
||||
# appending an after filter puts it at the end of the call chain
|
||||
# before and around filters goe before the first after filter in the chain
|
||||
# before and around filters go before the first after filter in the chain
|
||||
unless filter_type == :after
|
||||
filter_chain.each_with_index do |f,i|
|
||||
return i if f.after?
|
||||
|
@ -655,7 +656,7 @@ module ActionController #: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
|
||||
controller.send :halt_filter_chain, filter, :returned_false
|
||||
else
|
||||
begin
|
||||
action.call
|
||||
|
@ -675,8 +676,25 @@ module ActionController #:nodoc:
|
|||
end
|
||||
end
|
||||
|
||||
def filter_chain
|
||||
self.class.filter_chain
|
||||
protected
|
||||
|
||||
def process_with_filters(request, response, method = :perform_action, *arguments) #:nodoc:
|
||||
@before_filter_chain_aborted = false
|
||||
process_without_filters(request, response, method, *arguments)
|
||||
end
|
||||
|
||||
def perform_action_with_filters
|
||||
call_filters(self.class.filter_chain, 0, 0)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def call_filters(chain, index, nesting)
|
||||
index = run_before_filters(chain, index, nesting)
|
||||
aborted = @before_filter_chain_aborted
|
||||
perform_action_without_filters unless performed? || aborted
|
||||
return index if nesting != 0 || aborted
|
||||
run_after_filters(chain, index)
|
||||
end
|
||||
|
||||
def skip_excluded_filters(chain, index)
|
||||
|
@ -686,72 +704,54 @@ module ActionController #:nodoc:
|
|||
[filter, index]
|
||||
end
|
||||
|
||||
def call_filters(chain, index, nesting)
|
||||
# run before filters until we find an after filter or around filter
|
||||
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
|
||||
# invoke before filter
|
||||
filter.run(self)
|
||||
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
|
||||
# no before or around filters left
|
||||
break
|
||||
break # no before or around filters left
|
||||
end
|
||||
end
|
||||
index
|
||||
end
|
||||
|
||||
aborted = @before_filter_chain_aborted
|
||||
perform_action_without_filters unless performed? || aborted
|
||||
return index if aborted || nesting != 0
|
||||
|
||||
# if an around filter catches an exception during rendering and handles it, e.g.
|
||||
# by rendering an error page, we might have left over around filters in the filter
|
||||
# chain. so skip over these.
|
||||
index = index.next while (filter = chain[index]) && filter.type == :around
|
||||
|
||||
# run after filters, if any
|
||||
def run_after_filters(chain, index)
|
||||
seen_after_filter = false
|
||||
while chain[index]
|
||||
filter, index = skip_excluded_filters(chain, index)
|
||||
break unless filter
|
||||
break unless filter # end of call chain reached
|
||||
case filter.type
|
||||
when :after
|
||||
filter.run(self)
|
||||
index = index.next
|
||||
seen_after_filter = true
|
||||
filter.run(self) # invoke after filter
|
||||
else
|
||||
raise ActionControllerError, "filter #{filter.inspect} was in the wrong place!"
|
||||
# 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
|
||||
|
||||
def halt_filter_chain(filter)
|
||||
logger.info "Filter chain halted as [#{filter.inspect}] returned false." if logger
|
||||
def halt_filter_chain(filter, reason)
|
||||
@before_filter_chain_aborted = true
|
||||
logger.info "Filter chain halted as [#{filter.inspect}] #{reason}." if logger
|
||||
false
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def process_with_filters(request, response, method = :perform_action, *arguments) #:nodoc:
|
||||
@before_filter_chain_aborted = false
|
||||
process_without_filters(request, response, method, *arguments)
|
||||
end
|
||||
|
||||
private
|
||||
def perform_action_with_filters
|
||||
call_filters(self.class.filter_chain, 0, 0)
|
||||
end
|
||||
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -324,9 +324,9 @@ class FilterTest < Test::Unit::TestCase
|
|||
render :text => 'hello'
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
class ErrorToRescue < Exception; end
|
||||
|
||||
|
||||
class RescuingAroundFilterWithBlock
|
||||
def filter(controller)
|
||||
begin
|
||||
|
@ -336,20 +336,86 @@ class FilterTest < Test::Unit::TestCase
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
class RescuedController < ActionController::Base
|
||||
around_filter RescuingAroundFilterWithBlock.new
|
||||
|
||||
|
||||
def show
|
||||
raise ErrorToRescue.new("Something made the bad noise.")
|
||||
end
|
||||
|
||||
|
||||
private
|
||||
def rescue_action(exception)
|
||||
raise exception
|
||||
end
|
||||
end
|
||||
|
||||
class NonYieldingAroundFilterController < ActionController::Base
|
||||
|
||||
before_filter :filter_one
|
||||
around_filter :non_yielding_filter
|
||||
before_filter :filter_two
|
||||
after_filter :filter_three
|
||||
|
||||
def index
|
||||
render :inline => "index"
|
||||
end
|
||||
|
||||
#make sure the controller complains
|
||||
def rescue_action(e); raise e; end
|
||||
|
||||
private
|
||||
|
||||
def filter_one
|
||||
@filters ||= []
|
||||
@filters << "filter_one"
|
||||
end
|
||||
|
||||
def filter_two
|
||||
@filters << "filter_two"
|
||||
end
|
||||
|
||||
def non_yielding_filter
|
||||
@filters << "zomg it didn't yield"
|
||||
@filter_return_value
|
||||
end
|
||||
|
||||
def filter_three
|
||||
@filters << "filter_three"
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
def test_non_yielding_around_filters_not_returning_false_do_not_raise
|
||||
controller = NonYieldingAroundFilterController.new
|
||||
controller.instance_variable_set "@filter_return_value", true
|
||||
assert_nothing_raised do
|
||||
test_process(controller, "index")
|
||||
end
|
||||
end
|
||||
|
||||
def test_non_yielding_around_filters_returning_false_do_not_raise
|
||||
controller = NonYieldingAroundFilterController.new
|
||||
controller.instance_variable_set "@filter_return_value", false
|
||||
assert_nothing_raised do
|
||||
test_process(controller, "index")
|
||||
end
|
||||
end
|
||||
|
||||
def test_after_filters_are_not_run_if_around_filter_returns_false
|
||||
controller = NonYieldingAroundFilterController.new
|
||||
controller.instance_variable_set "@filter_return_value", false
|
||||
test_process(controller, "index")
|
||||
assert_equal ["filter_one", "zomg it didn't yield"], controller.assigns['filters']
|
||||
end
|
||||
|
||||
def test_after_filters_are_not_run_if_around_filter_does_not_yield
|
||||
controller = NonYieldingAroundFilterController.new
|
||||
controller.instance_variable_set "@filter_return_value", true
|
||||
test_process(controller, "index")
|
||||
assert_equal ["filter_one", "zomg it didn't yield"], controller.assigns['filters']
|
||||
end
|
||||
|
||||
def test_empty_filter_chain
|
||||
assert_equal 0, EmptyFilterChainController.filter_chain.size
|
||||
assert test_process(EmptyFilterChainController).template.assigns['action_executed']
|
||||
|
@ -516,13 +582,13 @@ class FilterTest < Test::Unit::TestCase
|
|||
def test_changing_the_requirements
|
||||
assert_equal nil, test_process(ChangingTheRequirementsController, "go_wild").template.assigns['ran_filter']
|
||||
end
|
||||
|
||||
|
||||
def test_a_rescuing_around_filter
|
||||
response = nil
|
||||
assert_nothing_raised do
|
||||
response = test_process(RescuedController)
|
||||
end
|
||||
|
||||
|
||||
assert response.success?
|
||||
assert_equal("I rescued this: #<FilterTest::ErrorToRescue: Something made the bad noise.>", response.body)
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue