Rails - Apr 2006 - Bug in Rails 1.1 implementation of before_filters

I just spent a few hours tracking down a bug in Rails 1.1, so I thought 
I''d post the issue and a workaround just in case anyone else hits it.

I designed a security enhancement so that controller methods can be 
protected by preceding them with a role. The standard Ruby 
method_added() callback is used to detect when controller methods are 
added, and then I manipulate the controller''s before_filter chain 
appropriately. My code worked fine in Rails 1.0, and then stopped 
working in Rails 1.1.

I eventually tracked down the bug to the new implementation of 
ActionController::Filters::before_filters and include_actions, which 
cache their results the first time they are called. This caching 
approach works fine as long as before_filters and include_actions are 
only called *after* all the filters have been registered, but I was 
using them *during* the registration process to figure out how to 
manipulate the filter chain. Since Rails 1.1 caches their value the 
first time they''re called, their return value is out of date if more 
filters are added.

A workaround is to bypass the cache and use 
read_inheritable_attribute("before_filters") instead of
before_filters.
Similarly, use read_inheritable_attribute("included_actions") instead
of
included_actions. This is bad because now my code is tied to an 
implementation detail which would normally be hidden.

Incidentally, the same bug also occurs in after_filters and 
excluded_actions.

Cheers,
Graham

-- 
Posted via http://www.ruby-forum.com/.

Graham Glass wrote:
> I just spent a few hours tracking down a bug in Rails 1.1, so I thought 
> I''d post the issue and a workaround just in case anyone else hits
it.
>
> I designed a security enhancement so that controller methods can be 
> protected by preceding them with a role. The standard Ruby 
> method_added() callback is used to detect when controller methods are 
> added, and then I manipulate the controller''s before_filter chain 
> appropriately. My code worked fine in Rails 1.0, and then stopped 
> working in Rails 1.1.
>
> I eventually tracked down the bug to the new implementation of 
> ActionController::Filters::before_filters and include_actions, which 
> cache their results the first time they are called. This caching 
> approach works fine as long as before_filters and include_actions are 
> only called *after* all the filters have been registered, but I was 
> using them *during* the registration process to figure out how to 
> manipulate the filter chain. Since Rails 1.1 caches their value the 
> first time they''re called, their return value is out of date if
more
> filters are added.
>
> A workaround is to bypass the cache and use 
> read_inheritable_attribute("before_filters") instead of
before_filters.
> Similarly, use read_inheritable_attribute("included_actions")
instead of
> included_actions. This is bad because now my code is tied to an 
> implementation detail which would normally be hidden.
>   
Incidentally, your previous code relied on an implementation detail as 
well: namely, that the values returned by "before_filters" and 
"included_actions" were recomputed on each call.

The named methods are usually only called during request processing. In 
fact, they are pretty much internal to the filter implementation. 
Caching the return values improved performance, sometimes significantly, 
so it was decided to cache them.

If you really need to modify the filter chain in your app, use 
read_inheritable_attribute("before_filters"), or define your own
version
of "before_filters" etc, or simply overwrite the methods with their
old
version, by requiring the following code from your environment.rb file:

 class ActionController::Base

  class << self
    # Returns all the before filters for this class and all its ancestors.
    def before_filters #:nodoc:
      read_inheritable_attribute(''before_filters'') || []
    end

    # Returns all the after filters for this class and all its ancestors.
    def after_filters #:nodoc:
      read_inheritable_attribute(''after_filters'') || []
    end

    # Returns a mapping between filters and the actions that may run them.
    def included_actions #:nodoc:
      read_inheritable_attribute(''included_actions'') || {}
    end

    # Returns a mapping between filters and actions that may not run them.
    def excluded_actions #:nodoc:
      read_inheritable_attribute(''excluded_actions'') || {}
    end
  end
end
 
> Incidentally, the same bug also occurs in after_filters and
excluded_actions.
>   
It''s a feature, not a bug.

-- stefan

-- 
For rails performance tuning, see: http://railsexpress.de/blog
Subscription: http://railsexpress.de/blog/xml/rss20/feed.xml

Stefan Kaes wrote:
> Graham Glass wrote:
>> I eventually tracked down the bug to the new implementation of 
>> read_inheritable_attribute("before_filters") instead of
before_filters.
>> Similarly, use read_inheritable_attribute("included_actions")
instead of
>> included_actions. This is bad because now my code is tied to an 
>> implementation detail which would normally be hidden.
>>   
> 
> Incidentally, your previous code relied on an implementation detail as
> well: namely, that the values returned by "before_filters" and
> "included_actions" were recomputed on each call.
> 
> The named methods are usually only called during request processing. In
> fact, they are pretty much internal to the filter implementation.
> Caching the return values improved performance, sometimes significantly,
> so it was decided to cache them.
> 
> If you really need to modify the filter chain in your app, use
> read_inheritable_attribute("before_filters"), or define your own
version
> of "before_filters" etc, or simply overwrite the methods with
their old
> version, by requiring the following code from your environment.rb file:
> 
>  class ActionController::Base
> 
>   class << self
>     # Returns all the before filters for this class and all its 
> ancestors.
>     def before_filters #:nodoc:
>       read_inheritable_attribute(''before_filters'') || []
>     end
> 
>     # Returns all the after filters for this class and all its 
> ancestors.
>     def after_filters #:nodoc:
>       read_inheritable_attribute(''after_filters'') || []
>     end
> 
>     # Returns a mapping between filters and the actions that may run 
> them.
>     def included_actions #:nodoc:
>       read_inheritable_attribute(''included_actions'') ||
{}
>     end
> 
>     # Returns a mapping between filters and actions that may not run 
> them.
>     def excluded_actions #:nodoc:
>       read_inheritable_attribute(''excluded_actions'') ||
{}
>     end
>   end
> end
> 
>> Incidentally, the same bug also occurs in after_filters and
excluded_actions.
>>   
> 
> It''s a feature, not a bug.
> 
> -- stefan
> 
> --
> For rails performance tuning, see: http://railsexpress.de/blog
> Subscription: http://railsexpress.de/blog/xml/rss20/feed.xml
Hi Stefan,

I think it would be good if there were public apis with well-defined 
semantics that allowed runtime access to the filter chain. The 
approaches that you recommend are not guaranteed to work on future 
versions of Rails.

Right now, "before_filters" does not do what its documentation says it
does, so I consider this to be a bug, not a feature.

Cheers,
Graham

-- 
Posted via http://www.ruby-forum.com/.