ActiveRecord::AttributeMethods #respond_to?

116 views
Skip to first unread message

Gaius

unread,
Sep 2, 2008, 1:51:33 PM9/2/08
to Ruby on Rails: Core
I have a model class

class Foo < ActiveRecord::Base
attr_protected :bar
end

I would think that Foo.new.respond_to?(:bar) would be false, but it
returns true. This is because the definition of :respond_to? checks
whether @attributes.include?(method_name), but doesn't take attribute
protection into account.

Foo thus does not abide by the general contract of respond_to?:
"respond_to?(:baz) returns true if and only if call(:baz) does not
raise a NoMethodError."

Will fixing this break tons and tons of code?

Jonathan Weiss

unread,
Sep 2, 2008, 2:00:17 PM9/2/08
to rubyonra...@googlegroups.com
> I would think that Foo.new.respond_to?(:bar) would be false, but it
> returns true.

It should be true as the Foo instance responds to a method called bar.

> This is because the definition of :respond_to? checks
> whether @attributes.include?(method_name), but doesn't take attribute
> protection into account.
>

It doesn't check attributes, it checks the actual methods on the
object. It is independent of Rails.

> Foo thus does not abide by the general contract of respond_to?:
> "respond_to?(:baz) returns true if and only if call(:baz) does not
> raise a NoMethodError."


Hu? Foo.new *has* a bar method. It just doesn't allow you to set it in
mass-assignment.

Jonathan

--
Jonathan Weiss
http://blog.innerewut.de
http://twitter.com/jweiss

James Rosen

unread,
Sep 2, 2008, 2:06:10 PM9/2/08
to rubyonra...@googlegroups.com
Sorry, you're right. I phrased my question wrong. Let's try this:

class Foo < ActiveRecord::Base

private

# really protect attribute bar
def bar
read_attribute :bar
end
def bar=(val)
write_attribute :bar, val
end

end

Now Foo.new.respond_to?(:bar) really _should_ return false, but it
doesn't. This method isn't checked before @attributes is.

-Gaius

Jim Lindley

unread,
Sep 2, 2008, 2:13:55 PM9/2/08
to rubyonra...@googlegroups.com
> class Foo < ActiveRecord::Base
>
> private
>
> # really protect attribute bar
> def bar
> read_attribute :bar
> end
> def bar=(val)
> write_attribute :bar, val
> end
>
> end
>
> Now Foo.new.respond_to?(:bar) really _should_ return false

respond_to? is a Ruby core object method, it's not implemented by Rails.

James Rosen

unread,
Sep 2, 2008, 2:20:32 PM9/2/08
to rubyonra...@googlegroups.com
It _is_ a core Ruby object, with a contract defined in Object. The
problem is that Rails overrides that definition, accounting for
ActiveRecord attributes. (It _should_ override, since it overrides
method_missing.)

All I'm saying is that the override devined in
ActiveRecord::AttributeMethods is inconsistent with the core Ruby
contract.

-Gaius

Damian Janowski

unread,
Sep 2, 2008, 2:21:09 PM9/2/08
to rubyonra...@googlegroups.com
On Tue, Sep 2, 2008 at 3:00 PM, Jonathan Weiss <j...@innerewut.de> wrote:
>> This is because the definition of :respond_to? checks
>> whether @attributes.include?(method_name), but doesn't take attribute
>> protection into account.
>>
>
> It doesn't check attributes, it checks the actual methods on the
> object. It is independent of Rails.

Well, not really. ActiveRecord does override #respond_to? to take
attributes into account (the "actual methods" are defined lazily).

>> Foo thus does not abide by the general contract of respond_to?:
>> "respond_to?(:baz) returns true if and only if call(:baz) does not
>> raise a NoMethodError."
>
>
> Hu? Foo.new *has* a bar method. It just doesn't allow you to set it in
> mass-assignment.

I agree. The current behavior is the same as the pure-Ruby one:

$ irb
irb(main):001:0> class Foo
irb(main):002:1> def bar
irb(main):003:2> end
irb(main):004:1> protected :bar
irb(main):005:1> end
=> Foo
irb(main):006:0> Foo.new.respond_to? :bar
=> true

James Rosen

unread,
Sep 2, 2008, 3:07:31 PM9/2/08
to rubyonra...@googlegroups.com
Ok, but what about

$ irb
irb(main):001:0> class Foo
irb(main):002:1> def bar
irb(main):003:2> end

irb(main):004:1> private:bar


irb(main):005:1> end
=> Foo
irb(main):006:0> Foo.new.respond_to? :bar

=> false

-Gaius

Damian Janowski

unread,
Sep 2, 2008, 3:08:44 PM9/2/08
to rubyonra...@googlegroups.com
On Tue, Sep 2, 2008 at 4:07 PM, James Rosen <james....@gmail.com> wrote:
>
> Ok, but what about
>
> $ irb
> irb(main):001:0> class Foo
> irb(main):002:1> def bar
> irb(main):003:2> end
> irb(main):004:1> private:bar
> irb(main):005:1> end
> => Foo
> irb(main):006:0> Foo.new.respond_to? :bar
> => false

Well, we are talking about attr_protected, right?

James Rosen

unread,
Sep 2, 2008, 3:12:56 PM9/2/08
to rubyonra...@googlegroups.com
No, sorry. I'm trying to _truly_ make an attribute private. I want
nobody but my instance (self) to be able to access this attribute.
There's nothing like attr_private to my knowledge, though, so I
declared private methods.

The problem is the basic order of the way ActiveRecord redefines :respond_to?

1. if regular respond_to? true, then return true
2. if matches an attribute, return true
3. return regular respond_to?

There's no way for me to _prevent_ an attribute from being exposed
publicly as far as respond_to? is concerned.

-Gaius

Michael Koziarski

unread,
Sep 3, 2008, 3:37:55 AM9/3/08
to rubyonra...@googlegroups.com
> No, sorry. I'm trying to _truly_ make an attribute private. I want
> nobody but my instance (self) to be able to access this attribute.
> There's nothing like attr_private to my knowledge, though, so I
> declared private methods.

I'd question *why* you want that to be completely private, what is it
you're hoping to achieve. Users of your code will *always* be able to
call your method, there's simply no way for you to stop them:

@object.__send__(:your_secret_method) # PWN3D!!!!

There's currently no way to mark an attribute as private, and we could
probably add that as an enhancement, but I'm a little confused what
the use case is.

--
Cheers

Koz

acechase

unread,
Sep 3, 2008, 12:27:55 PM9/3/08
to Ruby on Rails: Core
This is slightly tangential, but since we're on the subject, I'd like
to point out a related issue with private methods. It's early, and I'm
still working on my first cup of coffee, so please just ignore me if
I'm too off topic here :). The problem I am seeing is that a private
method works as expected when called directly on an object, but when
the object is accessed through an association_proxy 'private' is
ignored (because the association_proxy is 'send'ing on
method_missing).

Here's an example:

Class User < AR::Base
private
def secret_stuff ; "foo" end
end

Class Purchase < AR::Base
has_one :user
end

>> User.first.secret_stuff
=> NoMethodError: private method `secret_stuff' called for #<User:
0x3d446dc> # as expected
>> Purchase.first.user.secret_stuff
=> "foo" # d'oh!

Because the association_proxy does a blind send, the privacy check is
bypassed. When I first came across this, it really threw me for a loop
-- in one context my 'private' declaration was working fine and dandy,
and in another, it was being ignored! I wanted to use 'private'
because I had just modified my class in such a way that it made sense,
encapsulation-wise, to only have the class itself access the
particular method. I wasn't looking for true protection against
malicious code, I just wanted to verify my encapsulation properties
and find code that was violating those properties. In the end I
decided it wasn't worth the effort to make the two code paths behave
the same so instead I did a manual code comb to find outside classes
who were accessing my private method.

So, I guess I'm just offering this as another data point. It's another
place where the rails metaprogramming slightly modifies the expected
behavior of the underlying code. I don't think this is a major problem
or anything, just thought it fit in with the current topic. Maybe this
behavior is covered in the docs and I just missed it?

Cheers,
acechase

James Rosen

unread,
Sep 3, 2008, 4:06:38 PM9/3/08
to rubyonra...@googlegroups.com
acechase: I think your case is different, but extremely close to the
issue I'm getting at. Thank you for bringing it up.

Koz: are you prepared to argue that Ruby shouldn't have private
methods at all because programmers can get around them with __send__?
I want to make a method private so other programmers don't do stupid
stuff. I want it to be one more thing in the way of writing insecure,
unreliable code. There's no such thing as bulletproof code, but if I
can add more padding without adding conceptual weight, I'm all for it.

Also, I'd just like Rails Models to act like normal Ruby Objects.

-Gaius

Chris Eppstein

unread,
Sep 3, 2008, 6:05:08 PM9/3/08
to Ruby on Rails: Core
Gaius,

I applaud your desire to be safe with your user's information and
protect them from inadvertent programming errors. On my website, we
have a forum where sensitive matters are frequently discussed and so
we provide our users the option of posting anonymously. Of course, we
still need to know who posted it so that they can edit it later so I
had a similar conundrum that you're facing now. I didn't want to ever
let a developer accidentally reveal their identity when the anonymous
flag was set on their post. So I did little meta-programming to allow
me to make the user and user_id nil when the post was anonymous, but
still have access to the information through aliased methods if we
needed it.

http://gist.github.com/8654

Ruby on rails is great, but so is Ruby itself, and with a little
diligence, we can bend the world to our will to achieve a desired API.

-chris

P.S. For those not inclined to click through here's the rough sketch:
# == Schema Information
#
# Table name: posts
#
# id :integer(11) not null, primary key
# user_id :integer(11)
# anonymous :boolean(1)
# ...
class Post < ActiveRecord::Base
extend Anonymizer

belongs_to :user
anonymize_method :user
anonymize_attribute :user_id
end

>> p = Post.find(:first, :conditions => {:anonymous => true})
=> #<Post id: 31, ... >
>> p.user
=> nil
>> p.user_id
=> nil
>> p.user_id_without_anonymous
=> 1234
>> p.user_without_anonymous
=> #<User id: 1234, ... >

On Sep 3, 1:06 pm, "James Rosen" <james.a.ro...@gmail.com> wrote:
> acechase: I think your case is different, but extremely close to the
> issue I'm getting at.  Thank you for bringing it up.
>
> Koz: are you prepared to argue that Ruby shouldn't have private
> methods at all because programmers can get around them with __send__?
> I want to make a method private so other programmers don't do stupid
> stuff.  I want it to be one more thing in the way of writing insecure,
> unreliable code.  There's no such thing as bulletproof code, but if I
> can add more padding without adding conceptual weight, I'm all for it.
>
> Also, I'd just like Rails Models to act like normal Ruby Objects.
>
> -Gaius
>

Michael Koziarski

unread,
Sep 4, 2008, 4:08:29 AM9/4/08
to rubyonra...@googlegroups.com
> Koz: are you prepared to argue that Ruby shouldn't have private
> methods at all because programmers can get around them with __send__?
> I want to make a method private so other programmers don't do stupid
> stuff. I want it to be one more thing in the way of writing insecure,
> unreliable code. There's no such thing as bulletproof code, but if I
> can add more padding without adding conceptual weight, I'm all for it.

No, my point is simply that whatever safety you think you're receiving
by fixing this respond_to? block is no substitute for code review.
While this is definitely strange behaviour and a little counter
intuitive I can't imagine a situation where you'd actually have a
security issue caused by this code. I find it hard to think a
developer will be sitting in an irb session saying "does this
respond_to? :foo" rather than looking at the source to that model to
find the method they want :)

If you'd like to submit a patch for respond_to to check the private /
public status of methods defined by attributes, I'd be happy to apply
it.

--
Cheers

Koz

Michael Koziarski

unread,
Sep 4, 2008, 4:09:23 AM9/4/08
to rubyonra...@googlegroups.com
>>> User.first.secret_stuff
> => NoMethodError: private method `secret_stuff' called for #<User:
> 0x3d446dc> # as expected
>>> Purchase.first.user.secret_stuff
> => "foo" # d'oh!

Yeah, this is also a bug. If you want to whip up a patch for it, I'll
gladly apply it.

--
Cheers

Koz

John D. Hume

unread,
Sep 4, 2008, 8:50:17 PM9/4/08
to rubyonra...@googlegroups.com
> Also, I'd just like Rails Models to act like normal Ruby Objects.

It would be pretty weird to make attr_accessor methods private in a
normal Ruby object, since the writer method could never be called
except with a self.send. If some field was meant to be private, you'd
probably just use the instance variables directly and not have reader
or writer methods at all.

It seems like the closest analog in Active Record would be to not
define reader and writer methods for the attribute, only exposing it
via read_attribute and write_attribute (and [] and []=).

-hume.

James Rosen

unread,
Sep 4, 2008, 9:09:08 PM9/4/08
to rubyonra...@googlegroups.com
John, this is a _fantastic_ point. It's certainly true that there
would be no way to use the "setter" half of a private attr_accessor in
a regular Ruby object:

self.bar = 'baz' # violates private
bar = 'baz' # sets a local variable

But you can use the "getter" half:

return bar.dup # return a defensive copy of the instance variable

I consider Ruby's definition of "private" fairly crippled, but that's
not the fault of the Rails community, nor can they (we?) do much about
it.

Maybe my actual use case will help clear up why I want a private
attribute. I have a single-table-inheritance model:

table products:
integer version
string name

class Product < ActiveRecord::Base
attr_private :version # if such a method existed
end

class VersionedProduct < Foo
attr_accessor :version # reveal the method
end

I wouldn't normally ask this list for usage advice, but perhaps this
case will inform design.

Thanks,
Gaius

Jonathan Weiss

unread,
Sep 5, 2008, 2:22:48 AM9/5/08
to rubyonra...@googlegroups.com
On Fri, Sep 5, 2008 at 3:09 AM, James Rosen <james....@gmail.com> wrote:
>
> John, this is a _fantastic_ point. It's certainly true that there
> would be no way to use the "setter" half of a private attr_accessor in
> a regular Ruby object:
>
> self.bar = 'baz' # violates private
> bar = 'baz' # sets a local variable
>
> But you can use the "getter" half:
>
> return bar.dup # return a defensive copy of the instance variable
>
> I consider Ruby's definition of "private" fairly crippled, but that's
> not the fault of the Rails community, nor can they (we?) do much about
> it.

I still think you misunderstand and mix Ruby's private and Rails'
attr_protected/attr_accessible.

class A
private
def foo
"foo"
end
end

a = A.new
a.foo

>NoMethodError: private method `foo' called for #<A:0x6bae4>

So no crippling here. Note that you can still access foo if you really wan:

a.send(:foo)
=> "foo"

This is Ruby where we WANT to be able to do things like that.

Rails' attr_protected/attr_accessible are only there to protect you
from mass-assignment and have noting to do with private/protected.
Ruby's attr_accessor :foo will just create a getter and setter for you
if you are too lazy.

>
> Maybe my actual use case will help clear up why I want a private
> attribute. I have a single-table-inheritance model:
>
> table products:
> integer version
> string name
>
> class Product < ActiveRecord::Base
> attr_private :version # if such a method existed
> end
>
> class VersionedProduct < Foo
> attr_accessor :version # reveal the method
> end
>
> I wouldn't normally ask this list for usage advice, but perhaps this
> case will inform design.

The simple answer is: Just don't call version on products! It's your
code and you know where you use it.

If you really want to stop using version:

class Product < ActiveRecord::Base

def version
raise 'private'
end

end

class VersionedProduct < Product

def version
attributes[:version]
end

end

You could do the same for the setter or even write your attr_private
method that will generate the getters/setters.

But again, I see no point in doing so.

James Rosen

unread,
Sep 5, 2008, 3:55:48 PM9/5/08
to rubyonra...@googlegroups.com
Jonathan Weiss said,

<<<
The simple answer is: Just don't call version on products! It's your
code and you know where you use it.
>>>

It's my code right now, but not after I leave and some other coder
gets assigned to the project. I want to help future maintainers
prevent accidents. In Java, this sort of defensive programming is
very easy.

I want the protection offered by attr_protected (or the lack of
attr_accessible) AND the protection offered by restricted visibility.
I want the most protection I can possibly get without making the code
impossible to read.

All I'm saying is that Rails doesn't respect Ruby's native visibility.
John Hume brought up an even more important example of this.

-Gaius

Michael Koziarski

unread,
Sep 6, 2008, 11:46:38 AM9/6/08
to rubyonra...@googlegroups.com
> All I'm saying is that Rails doesn't respect Ruby's native visibility.
> John Hume brought up an even more important example of this.

As I mentioned earlier in the thread, these are both valid points and
you should send us a patch for them. While I may not agree with you
about the use of private for this purpose, you're right that there's
no valid reason to misbehave here, and there's no downside to doing
so.

--
Cheers

Koz

Adam

unread,
Sep 20, 2008, 10:54:05 PM9/20/08
to Ruby on Rails: Core
> John, this is a _fantastic_ point.  It's certainly true that there
> would be no way to use the "setter" half of a private attr_accessor in
> a regular Ruby object:
>
> self.bar = 'baz'  # violates private
> bar = 'baz'        # sets a local variable
Well, no, that would be silly. Private methods in Ruby are fairly
gimpy*, but mercifully not that gimpy. Ruby has a special case to
allow the self.bar = 'baz' syntax with a private accessor for bar.
Try it. However, this does cause problems with compound operators;
for instance, this won't work:

class Foo
def initialize
self.bar ||= "bar" # calls self.bar under the covers;
NoMethodError.
bar || = "bar" # sets a local variable.
end
private
attr_accessor :bar
end

In any case, I've always been a fan of judicious use of access
control, so I agree with Gaius that ActiveRecord attributes and
association proxies should act in a consistent manner in this regard.
I've submitted a patch for the issue brought up by acechase earlier in
this thread (http://rails.lighthouseapp.com/projects/8994-ruby-on-
rails/tickets/1083-calls-to-private-methods-via-association-proxies-
should-act-consistently-with-ruby-method-dispatch). I hope to submit
a second patch for the initial issue soon.

Interestingly, fixing the initial tests I wrote causes one unexpected
test failure, which is the reason for the line change in
has_one_association.rb. The method was calling #quote_value on the
instance (which is private) rather than #quote_value on the class
(which is public). All other calls to #quote_value in the nearby code
used the class method, which makes me think this was perhaps an
extremely minor bug; but, a bug exposed by proper access control.

*Incidentally, the fact that anyone can circumvent access control in
Ruby via the #send method isn't what makes me think it gimped. The
fact is, this still forces any consumers of my interface to make an
explicit decision to ignore my suggested usage. At this point they
know they've voided the warranty and proceed at their own peril. My
concerns with private methods in Ruby center more on how difficult
they are to use within the class that does have access to them;
disallowing an explicit sender, even when that sender is self, can
handicap readability in some cases, and leads to inconsistent usage
patterns, like the example I gave above with the ||= operator.

Adam

unread,
Sep 21, 2008, 4:29:31 AM9/21/08
to Ruby on Rails: Core
I've submitted a patch with the behavior requested way back at the
beginning of this thread:

http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1084-activerecord-attributes-should-respect-access-control
Reply all
Reply to author
Forward
0 new messages