Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #8383 (new defect)

Opened 2 years ago

Last modified 2 years ago

[PATCH] filter bug in update_filter_chain in edge

Reported by: kookster Assigned to: core
Priority: normal Milestone: 2.x
Component: ActionPack Version: edge
Severity: normal Keywords: filter
Cc:

Description

Found a bug in ActionController::Filters::Filter.update_filter_chain. If you have a before filter that, when called in call_filters, adds another filter to the filter_chain, you'll see this show up.

The update_filter_chain method calls insert on the filter_chain which in spite of not having a ! does in fact mutate the filter_chain, and the result is that it ends up with the updated filters inserted into the filter_chain as an array in the array (not flattened). The correct begavior is to not change the filter chain at all - so it is a one line code change to fix. 

Index: actionpack/lib/action_controller/filters.rb
===================================================================
--- actionpack/lib/action_controller/filters.rb (revision 5644)
+++ actionpack/lib/action_controller/filters.rb (working copy)
@@ -522,7 +522,7 @@
 
         def update_filter_chain(filters, filter_type, pos, &block)
           new_filters = create_filters(filters, filter_type, &block)
-          new_chain = filter_chain.insert(pos, new_filters).flatten
+          new_chain = Array.new(filter_chain).insert(pos, new_filters).flatten!
           write_inheritable_attribute('filter_chain', new_chain)
         end

When the error from this defect does occur, here is the error: 

ActionController::ActionControllerError (filter [#<ActionController::Filters::ClassMethods::BeforeFilterProxy:0x36f5d94 @filter=#<ActionController::Filters::ClassMethods::ProcFilter:0x36f5e34 @filter=#<Proc:0x024fc930@/Users/akuklewicz/dev/mediajoint/vendor/rails/actionpack/lib/action_controller/verification.rb:74>>>] was in the wrong place!):
    /vendor/rails/actionpack/lib/action_controller/filters.rb:726:in `call_filters'
    /vendor/rails/actionpack/lib/action_controller/filters.rb:748:in `perform_action_without_benchmark'
    /vendor/rails/actionpack/lib/action_controller/benchmarking.rb:68:in `perform_action_without_rescue'
    /usr/local/lib/ruby/1.8/benchmark.rb:293:in `measure'
    /vendor/rails/actionpack/lib/action_controller/benchmarking.rb:68:in `perform_action_without_rescue'
    /vendor/rails/actionpack/lib/action_controller/rescue.rb:125:in `perform_action_without_caching'
    /vendor/rails/actionpack/lib/action_controller/caching.rb:649:in `perform_action'
    /vendor/rails/activerecord/lib/active_record/query_cache.rb:99:in `cache'
    /vendor/rails/actionpack/lib/action_controller/caching.rb:648:in `perform_action'
    /vendor/rails/actionpack/lib/action_controller/base.rb:473:in `send'
    /vendor/rails/actionpack/lib/action_controller/base.rb:473:in `process_without_filters'
    /vendor/rails/actionpack/lib/action_controller/filters.rb:743:in `process_without_session_management_support'
    /vendor/rails/actionpack/lib/action_controller/session_management.rb:122:in `process'
    /vendor/rails/actionpack/lib/action_controller/components.rb:66:in `process_with_components'
    /vendor/plugins/active_scaffold/lib/extensions/action_controller.rb:57:in `component_response'
    /vendor/rails/actionpack/lib/action_controller/components.rb:88:in `render_component_as_string'
    /vendor/rails/actionpack/lib/action_controller/components.rb:148:in `component_logging'
    /vendor/rails/actionpack/lib/action_controller/components.rb:87:in `render_component_as_string'
    /vendor/rails/actionpack/lib/action_controller/components.rb:44:in `send'
    /vendor/rails/actionpack/lib/action_controller/components.rb:44:in `render_component'
    /vendor/plugins/active_scaffold/lib/extensions/action_view.rb:43:in `render_otherwise'
    /vendor/plugins/comatose/lib/support/inline_rendering.rb:9:in `render'
    /app/views/../../vendor/plugins/active_scaffold/frontends/default/views/_nested.rhtml:19:in `_run_rhtml_47vendor47plugins47active_scaffold47frontends47default47views47_nested46rhtml'
    /app/views/../../vendor/plugins/active_scaffold/frontends/default/views/_nested.rhtml:8:in `each'
    /app/views/../../vendor/plugins/active_scaffold/frontends/default/views/_nested.rhtml:8:in `_run_rhtml_47vendor47plugins47active_scaffold47frontends47default47views47_nested46rhtml'
    /vendor/rails/actionpack/lib/action_view/base.rb:346:in `send'
    /vendor/rails/actionpack/lib/action_view/base.rb:346:in `compile_and_render_template'
    /vendor/rails/actionpack/lib/action_view/base.rb:322:in `render_template'
    /vendor/rails/actionpack/lib/action_view/base.rb:282:in `render_file'
    /vendor/rails/actionpack/lib/action_view/base.rb:297:in `render_without_active_scaffold'
    /vendor/plugins/active_scaffold/lib/extensions/action_view.rb:45:in `render_otherwise'
    /vendor/plugins/comatose/lib/support/inline_rendering.rb:9:in `render'
    /vendor/rails/actionpack/lib/action_view/partials.rb:60:in `render_partial_without_active_scaffold'
    /vendor/rails/actionpack/lib/action_controller/benchmarking.rb:26:in `benchmark'
    /usr/local/lib/ruby/1.8/benchmark.rb:293:in `measure'
    /usr/local/lib/ruby/1.8/benchmark.rb:307:in `realtime'
    /vendor/rails/actionpack/lib/action_controller/benchmarking.rb:26:in `benchmark'
    /vendor/rails/actionpack/lib/action_view/partials.rb:59:in `render_partial_without_active_scaffold'
    /vendor/plugins/active_scaffold/lib/extensions/action_view.rb:55:in `render_partial'
    /vendor/rails/actionpack/lib/action_controller/base.rb:898:in `send'
    /vendor/rails/actionpack/lib/action_controller/base.rb:898:in `render_partial'
    /vendor/rails/actionpack/lib/action_controller/base.rb:810:in `render_with_no_layout'
    /vendor/rails/actionpack/lib/action_controller/layout.rb:258:in `render_without_benchmark'
    /vendor/rails/actionpack/lib/action_controller/benchmarking.rb:51:in `render_without_active_scaffold'
    /usr/local/lib/ruby/1.8/benchmark.rb:293:in `measure'
    /vendor/rails/actionpack/lib/action_controller/benchmarking.rb:51:in `render_without_active_scaffold'
    /vendor/plugins/active_scaffold/lib/extensions/action_controller.rb:13:in `render'
    /vendor/rails/actionpack/lib/action_controller/base.rb:834:in `render_to_string'
    /vendor/rails/activesupport/lib/active_support/deprecation.rb:44:in `silence'
    /vendor/rails/actionpack/lib/action_controller/base.rb:834:in `render_to_string'
    /vendor/plugins/active_scaffold/lib/extensions/action_controller.rb:9:in `render'
    /vendor/plugins/active_scaffold/lib/actions/nested.rb:17:in `nested'
    /vendor/rails/actionpack/lib/action_controller/mime_responds.rb:145:in `call'
    /vendor/rails/actionpack/lib/action_controller/mime_responds.rb:145:in `custom'
    /vendor/rails/actionpack/lib/action_controller/mime_responds.rb:177:in `call'
    /vendor/rails/actionpack/lib/action_controller/mime_responds.rb:177:in `respond'
    /vendor/rails/actionpack/lib/action_controller/mime_responds.rb:171:in `each'
    /vendor/rails/actionpack/lib/action_controller/mime_responds.rb:171:in `respond'
    /vendor/rails/actionpack/lib/action_controller/mime_responds.rb:105:in `respond_to'
    /vendor/plugins/active_scaffold/lib/actions/nested.rb:15:in `nested'
    /vendor/rails/actionpack/lib/action_controller/base.rb:1115:in `send'
    /vendor/rails/actionpack/lib/action_controller/base.rb:1115:in `perform_action_without_filters'
    /vendor/rails/actionpack/lib/action_controller/filters.rb:714:in `call_filters'
    /vendor/rails/actionpack/lib/action_controller/filters.rb:748:in `perform_action_without_benchmark'
    /vendor/rails/actionpack/lib/action_controller/benchmarking.rb:68:in `perform_action_without_rescue'
    /usr/local/lib/ruby/1.8/benchmark.rb:293:in `measure'
    /vendor/rails/actionpack/lib/action_controller/benchmarking.rb:68:in `perform_action_without_rescue'
    /vendor/rails/actionpack/lib/action_controller/rescue.rb:125:in `perform_action_without_caching'
    /vendor/rails/actionpack/lib/action_controller/caching.rb:649:in `perform_action'
    /vendor/rails/activerecord/lib/active_record/query_cache.rb:99:in `cache'
    /vendor/rails/actionpack/lib/action_controller/caching.rb:648:in `perform_action'
    /vendor/rails/actionpack/lib/action_controller/base.rb:473:in `send'
    /vendor/rails/actionpack/lib/action_controller/base.rb:473:in `process_without_filters'
    /vendor/rails/actionpack/lib/action_controller/filters.rb:743:in `process_without_session_management_support'
    /vendor/rails/actionpack/lib/action_controller/session_management.rb:122:in `process'
    /vendor/rails/actionpack/lib/action_controller/base.rb:326:in `process'
    /vendor/rails/railties/lib/dispatcher.rb:39:in `dispatch'

Attachments

filters.patch (0.6 kB) - added by kookster on 05/16/07 18:51:00.
patch

Change History

05/16/07 18:51:00 changed by kookster

  • attachment filters.patch added.

patch

05/16/07 18:52:07 changed by kookster

  • summary changed from filter bug in update_filter_chain in v 6736 to [PATCH] filter bug in update_filter_chain in edge.

06/24/07 23:30:04 changed by nik.wakelin

Hey,

Can you tell me what kind of filters you are using, so that I can reproduce this bug and write a test case for it?

I'm experiencing this in production and I'd really like to help you get this patch applied :)

Cheers.

06/25/07 12:39:59 changed by kookster

It happened to me using active scaffold - some of the code to handle habtm relationships in active scaffold adds additional filters.

So I don't have a simple case unfortunately (which is pretty much why I never wrote a test case).

You can see more details about how to reproduce from my original defect report to active scaffold (before I figured out it was a rails edge bug).

http://code.google.com/p/activescaffold/issues/detail?id=303