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

Ticket #8803 (closed defect: fixed)

Opened 1 year ago

Last modified 1 year ago

[PATCH] Filter in the wrong place (Failing test case), now with fix

Reported by: nik.wakelin Assigned to: nzkoz
Priority: normal Milestone: 1.x
Component: ActionPack Version: edge
Severity: normal Keywords: filters,controller
Cc:

Description

Hey,

This is a failing test case for the following bug: 

ActionController::ActionControllerError in PartiesController#index

filter #<ActionController::Filters::ClassMethods::BeforeFilterProxy:0xb60ee668
@filter=#<ActionController::Filters::ClassMethods::SymbolFilter:0xb60ee6cc
@filter=:login_from_cookie>> was in the wrong place!

(see: http://groups.google.com/group/rubyonrails-core/browse_thread/thread/e5016709ae066a1e/33c1875eb3fad3b1?lnk=gst&q=filter+in+the+wrong+place&rnum=1#33c1875eb3fad3b1)

Tracked this down, looks like it is happening when an around filter doesn't yield and *should* (??) abort the filter chain, but instead keeps tracking back up and hits a before filter (Oh noes!)

Happening somewhere around line 701 in call_filters in filters.rb.

Attached is a failing test case.

Attachments

filter_in_the_wrong_place_failing_test_case.diff (1.2 kB) - added by nik.wakelin on 06/29/07 04:55:45.
Diff of a failing test case
filter_in_wrong_place_fix_with_test.patch (3.7 kB) - added by kamal on 07/04/07 02:16:54.
seems related to #8341, use the same approach to remove left over before filters also

Change History

06/29/07 04:55:45 changed by nik.wakelin

  • attachment filter_in_the_wrong_place_failing_test_case.diff added.

Diff of a failing test case

07/04/07 02:01:35 changed by kamal

  • owner changed from stefan to nzkoz.
  • summary changed from Filter in the wrong place (Failing test case) to [PATCH] Filter in the wrong place (Failing test case), now with fix.

I hit Filter in the wrong place error for a different scenario.

My scenario is that I have a before_filter that was raising an exception, and the around_filter rescuing it with a render / redirect. I have included test cases for this scenario (as well as the action itself raising an exception).

I have included a one-liner patch that basically just returns before it gets the chance to raise the Filter in wrong place exception if performed? is true. Seems to work, and all tests pass but I don't know if this is right.

I have also included the original test case above by nik.wakelin to test that this patch works.

07/04/07 02:14:22 changed by nik.wakelin

Is this desired behavior?

Apologies, but I don't see how this solves the problem any better than raising the error. I think the problem is actually in how the filters get queued up (or how they are removed an dequeued during execution).

07/04/07 02:16:54 changed by kamal

  • attachment filter_in_wrong_place_fix_with_test.patch added.

seems related to #8341, use the same approach to remove left over before filters also

07/04/07 02:17:48 changed by kamal

I've updated the patch to use the same approach as the patch in #8341 by skipping over left over before filters.

07/06/07 08:27:13 changed by skaes

There were additional problems. See #8891 for a complete fix.

07/11/07 03:02:19 changed by kamal

  • status changed from new to closed.
  • resolution set to duplicate.

Resolved in [7177].

07/11/07 03:07:41 changed by kamal

  • status changed from closed to reopened.
  • resolution deleted.

Reopen temporarily to properly set the ticket status

07/11/07 03:35:00 changed by kamal

  • status changed from reopened to closed.
  • resolution set to fixed.

fixed in [7177]