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
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
respond_to? is a Ruby core object method, it's not implemented by Rails.
All I'm saying is that the override devined in
ActiveRecord::AttributeMethods is inconsistent with the core Ruby
contract.
-Gaius
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
$ 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
Well, we are talking about attr_protected, right?
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
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
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
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
Yeah, this is also a bug. If you want to whip up a patch for it, I'll
gladly apply it.
--
Cheers
Koz
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.
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
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.
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
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