best way to protect class instance variables

B

Barun Singh

Suppose I generate a class instance variable and create an accessor
function for it as follows:

class MyClass
@blah = [0,1]
def self.blah
@blah
end
end

I would like to prevent any other piece of code from having any way of
changing the value of the class variable. While the code above prevents
me from saying "MyClass.blah = 'hello'", it does allow me to say
"MyClass.blah[0] = 'hello'", which changes the value of the class
instance variable (to ['hello',1]) any time I try to access it in the
future.

I know that one way to fix this problem is to simple return a copy of
the instance variable instead of returning the instance variable itself,
by changing the accessor to:

def self.blah
@blah.dup
end

This way, another piece of code is still able to say "MyClass.blah[0] =
'hello'", but it won't affect the results returned by "MyClass.blah" in
the future.

So this works for me, but I'm just wondering -- is there a better way to
do this?
 
J

Jesús Gabriel y Galán

Suppose I generate a class instance variable and create an accessor
function for it as follows:

class MyClass
@blah = [0,1]
def self.blah
@blah
end
end

I would like to prevent any other piece of code from having any way of
changing the value of the class variable. While the code above prevents
me from saying "MyClass.blah = 'hello'", it does allow me to say
"MyClass.blah[0] = 'hello'", which changes the value of the class
instance variable (to ['hello',1]) any time I try to access it in the
future.

You could freeze it:


irb(main):009:0> class Blah
irb(main):010:1> @blah = [0,1].freeze
irb(main):011:1> def self.blah
irb(main):012:2> @blah
irb(main):013:2> end
irb(main):014:1> end
=> nil
irb(main):015:0> Blah.blah
=> [0, 1]
irb(main):016:0> Blah.blah[0] = "hello"
TypeError: can't modify frozen array
from (irb):16:in `[]='
from (irb):16


This could save you from accidental changes, but somebody could always
do this and trash your frozen array:

irb(main):017:0> class Blah
irb(main):018:1> @blah = %w{nothing is really safe}
irb(main):019:1> end
=> ["nothing", "is", "really", "safe"]
irb(main):020:0> Blah.blah
=> ["nothing", "is", "really", "safe"]

or this:

irb(main):021:0> Blah.send :)instance_variable_set, "@blah", [1,2,3])
(irb):21: warning: don't put space before argument parentheses
=> [1, 2, 3]
irb(main):022:0> Blah.blah
=> [1, 2, 3]

or probably many other things to achieve that. So it's not really safe.

Jesus.
 
M

Mike Gold

Jesús Gabriel y Galán said:
This could save you from accidental changes, but somebody could always
do this and trash your frozen array:

irb(main):017:0> class Blah
irb(main):018:1> @blah = %w{nothing is really safe}
irb(main):019:1> end

Data from a closure cannot be overwritten in this way, and could qualify
as being "really safe".

class MyClass
class << self
blah = [0,1]
define_method:)blah) { blah }
end
end

p MyClass.blah #=> [0, 1]
 
D

David Masover

Mike said:
Data from a closure cannot be overwritten in this way, and could qualify
as being "really safe".
But, you've already provided a method for breaking that anyway --
define_method.

So, someone could always do:

blah = MyClass.blah.dup
blah[0] = 5
class << MyClass
define_method :blah { blah }
end


To the original poster, you really want to provide a convention, instead
-- using .dup, .freeze, or simply documenting that you shouldn't change
that array should be enough, unless people start doing metaprogramming.
Once they start metaprogramming, there's really nothing you can do short
of running all your code with SAFE.

The question you need to ask is, are you trying to protect against truly
malicious code? If so, stop using eval, or run it with a high SAFE level
and research Ruby sandboxing. Or are you trying to protect against
programmer error? If so, provide clear documentation, and make it hard
to do by accident -- no one's going to call instance_variable_set on
your class by accident.
 
R

Robert Dober

What about fine graining the control to the closure?

Not tested:
class MyBlah
blah = [ 0, 1 ]
extend( Module::new do
define_method :[] do |idx| blah[ idx ] end
define_method :blah do blah.dup.freeze end
define_method :[]= do |idx, value| do_all_my_checks
blah[ idx ] = some_fancy_function( value )
end
end
)
end

HTH
Robert
 
B

Barun Singh

Thanks for all of the helpful suggestions. My concern wasn't related to
malicious code, it was more just trying to find the best way to prevent
someone from accidentally overwriting the value of the class instance
variable without realizing it (for example, if someone accidentally
types "x = MyClass.blah" instead of "x == MyClass.blah". I didn't like
my original way of doing it only because it required copying all of the
variable's attributes every time the accessor method was called, which
seemed wasteful. So for my purposes, the "freeze" method works great..
 
D

David Masover

Robert said:
What about fine graining the control to the closure?
Doesn't really matter, so long as it can be read and duplicated
satisfactorily to fool the rest of the module. And your method doesn't
quite work -- I assume you were wanting blah to return something with []
methods that restrict access to the scope'd blah?

class MyBlah
blah = [0,1]
@blah = Object.new
@blah.extend(Module.new do
define_method :[] { |i| blah }
define_method :[]= do |idx, value|
do_checks
blah[idx] = value
end
end)
class << self
attr_reader :blah
end
end

Of course, this prevents the internal array from being modified, so I
see the point -- if there are other methods sharing the same scope. Even
then, it's still possible to obliterate the whole thing, it's just not
as easy to change what a method does by changing the array out from
under it.

Interesting exercise -- still going to recommend that you don't do this
without good reason. It's not sufficient to prevent evil things if you
eval untrusted code, and it's complete overkill versus just having good
documentation and conventions.
 
R

Robert Dober

Doesn't really matter, so long as it can be read and duplicated
satisfactorily to fool the rest of the module. And your method doesn't quite
work --
How can it? I did not test it! I guess I have hidden the local
variable with my method :blah :)
The important thing to retain is IMHO.
Closures are the only way to hide your data in Ruby. I make this
statement without applying any judgment if one shall or shall not :).
I assume you were wanting blah to return something with [] methods
that restrict access to the scope'd blah?
No not quite, that is why I dupfroze it.
As I have to keep blah mutable, for the purpose of the general
exercise I cannot expose it directly and I have to intercept all
mutable method calls.
I will try to be clearer this time.

class MyBlah
_blah = %w{ the human brain is a wonderful thing. it starts working
before you are born and continues to work
up to the moment you post to ruby talk }
extend( Module::new do

define_method :blah do _blah.dup.freeze end # We will not care
about errors in this toy example

# the [] forwarder method was an unnecessary

# complication, omitted
define_method :an_access_example do |*args, &blk| # Ruby1.9
# And here goes all our data encapsulation logic before
actually changing _blah
end
end )
end
Of course, this prevents the internal array from being modified, so I see
the point -- if there are other methods sharing the same scope. Even then,
it's still possible to obliterate the whole thing,
I am not sure to understand? What do you mean by scope? Of course one
can access _blah by editing the source ;). But that is the *only* way.
Reopening the class will not give access to _blah.

Cheers
Robert
 
M

Mike Gold

David said:
But, you've already provided a method for breaking that anyway --
define_method.

define_method will give a warning if the method already exists. That's
a heckofa lot better than a silent bug from a variable overwrite.

Preventing name collisions is something I take seriously for large
projects. It's hard to show the utility in small examples; indeed for
small cases there is none.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,755
Messages
2,569,536
Members
45,009
Latest member
GidgetGamb

Latest Threads

Top