Forum: Ruby best way to protect class instance variables

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
A70410682bd132d5edf76b42189ebb45?d=identicon&s=25 Barun Singh (barunio)
on 2009-02-08 20:12
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?
E088bb5c80fd3c4fd02c2020cdacbaf0?d=identicon&s=25 Jesús Gabriel y Galán (Guest)
on 2009-02-08 20:33
(Received via mailing list)
On Sun, Feb 8, 2009 at 8:11 PM, Barun Singh <barunio@gmail.com> wrote:
> 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.
A246f7c0ce5f2909483d358bd9e83e4e?d=identicon&s=25 Mike Gold (mikegold)
on 2009-02-08 21:24
Jesús Gabriel y Galán wrote:
>
> 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]
1bc63d01bd3fcccc36fb030a62039352?d=identicon&s=25 David Masover (Guest)
on 2009-02-08 21:49
(Received via mailing list)
Mike Gold wrote:
> 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.
F53b05cdbdf561cfe141f69b421244f3?d=identicon&s=25 David A. Black (Guest)
on 2009-02-08 21:59
(Received via mailing list)
On Mon, 9 Feb 2009, David Masover wrote:

>>
> class << MyClass
> define_method :blah { blah }
> end

The blah in { blah } is going to be undefined there, because the class
definition block starts a new local scope. However, you can certainly
get that effect:

   def MyClass.blah
     # some wrong value
   end

and of course:

   MyClass.blah << 2

(unless it's frozen). I agree with your point about not programming on
the assumption that the other programmers who use your code are going
to sabotage your classes.


David

--
David A. Black / Ruby Power and Light, LLC
Ruby/Rails consulting & training: http://www.rubypal.com
Coming in 2009: The Well-Grounded Rubyist (http://manning.com/black2)

http://www.wishsight.com => Independent, social wishlist management!
703fbc991fd63e0e1db54dca9ea31b53?d=identicon&s=25 Robert Dober (Guest)
on 2009-02-08 22:00
(Received via mailing list)
On Sun, Feb 8, 2009 at 9:48 PM, David Masover <ninja@slaphack.com>
wrote:
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
A70410682bd132d5edf76b42189ebb45?d=identicon&s=25 Barun Singh (barunio)
on 2009-02-08 22:31
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..
1bc63d01bd3fcccc36fb030a62039352?d=identicon&s=25 David Masover (Guest)
on 2009-02-08 22:43
(Received via mailing list)
Robert Dober wrote:
> On Sun, Feb 8, 2009 at 9:48 PM, David Masover <ninja@slaphack.com> wrote:
> 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[i] }
    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.
703fbc991fd63e0e1db54dca9ea31b53?d=identicon&s=25 Robert Dober (Guest)
on 2009-02-09 00:28
(Received via mailing list)
On Sun, Feb 8, 2009 at 10:42 PM, David Masover <ninja@slaphack.com>
wrote:
> Robert Dober wrote:
>>
>> On Sun, Feb 8, 2009 at 9:48 PM, David Masover <ninja@slaphack.com> wrote:
>> 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 --
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
A246f7c0ce5f2909483d358bd9e83e4e?d=identicon&s=25 Mike Gold (mikegold)
on 2009-02-09 05:57
David Masover wrote:
> Mike Gold wrote:
>> 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.

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.
This topic is locked and can not be replied to.