A while back I wrote about private methods in ActiveRecord objects, and how Rails 2.2 makes them behave as they should. ActiveRecord associations will no longer respond to private methods defined on their targets; however, my colleague Joseph pointed out that they also no longer respond to methods defined via #method_missing on their targets. Which sucks horse poop through a straw, to some extent.
In my original post I wrote this, fully aware that I was writing lies, but mostly in a rush to get on to my point at the time:
Sometimes I’m forced to use
#send:
method_name = extract_method_name_from_the_aether
some_object.send(method_name)
In order to make this code correct with regard to access control I have to add cruft:
method_name = extract_method_name_from_the_aether
some_object.send(method_name) if some_object.respond_to?(method_name)
The astute reader will note that the second code block doesn’t actually approximate the behavior of calling a private method via function call syntax. It should look more like this:
method_name = extract_method_name_from_the_aether
raise NoMethodError if some_object.private_methods.include?(method_name)
some_object.send(method_name)
Two things to notice here: first, the original code didn’t throw an exception if it failed to call the method; second, and much more importantly, the set of private methods is not the complement of the set of methods an object will respond to. So, using !#respond_to? as the condition for preventing the method call is too restrictive. The fact that this code will prevent calling a non-private method defined via #method_missing clearly shows this.
Now, I know the Rails core team chose to use #respond_to? in this particular case for a good reason (which my original patch did not take into consideration): performance. Some simple investigation quickly shows that #include? is an order of magnitude slower than #respond_to? Method calls happen quite a lot, so slow is bad.
I’ve submitted another patch that should correct the #method_missing behavior without dragging everything to a halt. Building on my previous examples, the code looks something like this:
method_name = extract_method_name_from_the_aether
raise NoMethodError if !some_object.respond_to?(method_name) && some_object.private_methods.include?(method_name)
some_object.send(method_name)
Note that the conditional expression is now redundant; the first condition cannot be false if the second condition is true. However, an object will likely respond to the majority of method calls sent to it. The first conditions will (quickly) evaluate to false in these cases, short-circuiting the conditional. In the minority of cases where the first condition evaluates to true, the second condition will (slowly, but correctly) determine if the method call violates access control.
Unfortunately, sensible as it may seem, this change actually breaks has_one :through associations, because of collection-specific functionality they inherit from has_many :through associations. (Oh yes, has_one :through associations are collections, based on their place in the ActiveRecord inheritance hierarchy. Or, they were; read more about that here).
And, finally, I think it’s important to note that none of these machinations in Rails would be necessary if #send (and #send!) actually worked as it should. Heads up, Matz.
