Eval going boom

Object X is going to be an exact clone of Object Y, but it’s a member
of a different class.

This does not work:

x.attributes.each {|attr, value| eval(“y.#{attr}= #{value}”)}

irb says:

(eval):1: warning: parenthesize argument(s) for future version
SyntaxError: (eval):1:in `irb_binding’: compile error
(eval):1: parse error, unexpected tINTEGER, expecting $
wv.updated_at= Mon May 07 11:13:27 -0700 2007

The second two lines make perfect sense to me; the first two seem pretty
weird.

I suppose I should just create an AbstractZ class, from which both Y
and X inherit.


Giles B.

Blog: http://gilesbowkett.blogspot.com
Portfolio: http://www.gilesgoatboy.org

The problem, basically, is that the attributes come back from
#inspect. So a Date becomes a String (in the example of the error
reported in my previous mail).

This works perfectly:

x.attributes.each {|attr, value| eval(“y.#{attr}= x.#{attr}”)}

w00t w00t
00ntz 00ntz 00ntz


Giles B.

Blog: http://gilesbowkett.blogspot.com
Portfolio: http://www.gilesgoatboy.org

On Jun 20, 2007, at 17:19 , Giles B. wrote:

Object X is going to be an exact clone of Object Y, but it’s a member
of a different class.

This does not work:

x.attributes.each {|attr, value| eval(“y.#{attr}= #{value}”)}

p x.class
=> CommandsIShouldNeverEval

p x.attributes
=> { :z => “rm -rf /” }

x.attributes.each {|attr, value| eval(“y.#{attr}= #{value}”)}

Don’t use eval. There is no need in this example.

CHALLENGE: rewrite the above code to not use eval at all. For that
matter, don’t even use #send.

Hi,

Am Donnerstag, 21. Jun 2007, 09:19:47 +0900 schrieb Giles B.:

This does not work:

x.attributes.each {|attr, value| eval(“y.#{attr}= #{value}”)}

Maybe #{value.inspect} is better or even

y.send “{#attr}=”, value

Untested.

Bertram

CHALLENGE: rewrite the above code to not use eval at all. For that
matter, don’t even use #send.

original_obj.attributes.each{|attr, value|
new_obj.update_attribute(attr.to_sym, value)}


Giles B.

Blog: http://gilesbowkett.blogspot.com
Portfolio: http://www.gilesgoatboy.org

Don’t use eval. There is no need in this example.

Why’s everyone so scared of eval? Yes it can destroy your system
completely and forever, but life’s short. Might as well enjoy it.
Drive without your seat belt. Go to wild parties. Use eval().

Nobody on their deathbed will ever say “I wish I’d never used eval().”

CHALLENGE: rewrite the above code to not use eval at all. For that
matter, don’t even use #send.

Aargh! Must rise to challenge!

I tried with write_attribute and failed. I can do it with
instance_eval but that’s just a loophole.

On my blog Marcel Molina Jr. pointed out you can use

x.attributes = y.attributes

That’s almost an answer to the challenge, but Pat M. pointed out
it won’t cover protected attributes.

(e.g., attr_protected :some_attribute)

Pat also wrote up the send version. Haven’t tested it but it looks
solid.

(http://gilesbowkett.blogspot.com/2007/06/clone-activerecord-models.html)

Anyway, so far, this challenge has kicked my butt. But I’m not giving up
yet!


Giles B.

Blog: http://gilesbowkett.blogspot.com
Portfolio: http://www.gilesgoatboy.org

CHALLENGE: rewrite the above code to not use eval at all. For that
matter, don’t even use #send.

original_obj.attributes.each{|attr, value|
new_obj.update_attribute(attr.to_sym, value)}

prettier:

original_obj.attributes.each do |attr, value|
new_obj.update_attribute(attr.to_sym, value)
end


Giles B.

Blog: http://gilesbowkett.blogspot.com
Portfolio: http://www.gilesgoatboy.org

original_obj.attributes.each do |attr, value|
new_obj.update_attribute(attr.to_sym, value)
end

You don’t even need #to_sym :-). Note that rewrite has the side-
effect of actually saving those attributes of new_object in the
database, whereas the original assignments didn’t. Perhaps that’s OK
though.

Well, if you’ve got validation callbacks (e.g.
validates_uniqueness_of) this version could blow up on that. That’s
assuming you’re copying objects of the same class as one another - I
originally came up with this to copy things of different classes
(near-identical subclasses of an as-yet-unconstructed abstract
superclass).

Also - I didn’t realize these methods came from ActiveSupport. Doh.
The original ran in pure Ruby without Rails - it’d probably be ideal
to implement the solution in pure Ruby.


Giles B.

Blog: http://gilesbowkett.blogspot.com
Portfolio: http://www.gilesgoatboy.org

On Jun 22, 2007, at 10:30 PM, Giles B. wrote:

end
You don’t even need #to_sym :-). Note that rewrite has the side-
effect of actually saving those attributes of new_object in the
database, whereas the original assignments didn’t. Perhaps that’s OK
though.

– fxn

On Jun 23, 2007, at 12:06 AM, Giles B. wrote:

validates_uniqueness_of) this version could blow up on that.
Not really, update_attribute() bypasses validations by default, it’s
written like this:

send(name.to_s + ‘=’, value)
save(false)

Also - I didn’t realize these methods came from ActiveSupport. Doh.
The original ran in pure Ruby without Rails - it’d probably be ideal
to implement the solution in pure Ruby.

Yeah, except for the call to attributes(), which is a Railism. In
fact I thought this thread belonged to the Rails mailing list because
of that.

– fxn

Giles B. wrote:

Don’t use eval. There is no need in this example.

Why’s everyone so scared of eval? Yes it can destroy your system
completely and forever, but life’s short. Might as well enjoy it.
Drive without your seat belt. Go to wild parties. Use eval().

Do you tell that your customers too?

Besides security issues:
-eval is factors slower
-eval is less concise (remember, you read code far more often than you
write it)
-eval is more annoying to debug

I’m sure there are more reasons. You see, only “security issue” (which
is huge, mind you), none of them is based on “fear”.

Regards
Stefan

Yeah, except for the call to attributes(), which is a Railism. In
fact I thought this thread belonged to the Rails mailing list because
of that.

sorry.


Giles B.

Blog: http://gilesbowkett.blogspot.com
Portfolio: http://www.gilesgoatboy.org

On 6/23/07, Stefan R. [email protected] wrote:

Giles B. wrote:

Don’t use eval. There is no need in this example.

Why’s everyone so scared of eval? Yes it can destroy your system
completely and forever, but life’s short. Might as well enjoy it.
Drive without your seat belt. Go to wild parties. Use eval().

Do you tell that your customers too?

Of course not. That’s a rude, ridiculous question.

Besides security issues:
-eval is factors slower
-eval is less concise (remember, you read code far more often than you
write it)
-eval is more annoying to debug

I’m sure there are more reasons. You see, only “security issue” (which
is huge, mind you), none of them is based on “fear”.

I wouldn’t recommend eval() in production code, without bulletproof
ways of keeping it clear of user input, but I find it a lot more
concise and a lot easier to debug. Certainly the code examples in
this thread which use eval are slimmer than the non-eval versions. (To
my eyes, they’re also clearer.) To each his own, I guess.


Giles B.

Blog: http://gilesbowkett.blogspot.com
Portfolio: http://www.gilesgoatboy.org

On 6/23/07, Stefan R. [email protected] wrote:

Of course not. That’s a rude, ridiculous question.

I don’t think it was rude. Assuming your application is in some way
responsible for sensitive data then creating a security hole with eval
is incompetence at best and willful endangerment at worst. Explaining
the reason of the loss of sensitive data to an affected customer without
lying could be a bit difficult.

I thought Giles was joking…

is incompetence at best and willful endangerment at worst. Explaining
the reason of the loss of sensitive data to an affected customer without
lying could be a bit difficult.
Also, IANAL, but I think in my country you could (theoretically -
practically it’s probably a bit too difficult to prove) even be held
responsible for sensitive data lost, compromised or stolen through such
a security hole. So I’m quite serious about that.
But my apologies if it offended you.

My bad, I totally took it wrong. I thought you meant the part about
going to wild parties and living on the edge. Telling customers to
play fast and loose for the hell of it, no, I generally don’t tell
them to do that. I actually totally do appreciate the security
concerns. The code the thread started with is really just for playing
around and for using in irb.

(I did look into packaging up that code for distribution, but mostly
just for the exercise of it - I didn’t actually anticipate anyone
finding it remarkably useful.)

I thought Giles was joking…

No, I was dead serious. I went home and cried. Then I retained a
lawyer. We’re suing for mental anguish.


Giles B.

Blog: http://gilesbowkett.blogspot.com
Portfolio: http://www.gilesgoatboy.org

Giles B. wrote:

On 6/23/07, Stefan R. [email protected] wrote:

Giles B. wrote:

Don’t use eval. There is no need in this example.

Why’s everyone so scared of eval? Yes it can destroy your system
completely and forever, but life’s short. Might as well enjoy it.
Drive without your seat belt. Go to wild parties. Use eval().

Do you tell that your customers too?

Of course not. That’s a rude, ridiculous question.

I don’t think it was rude. Assuming your application is in some way
responsible for sensitive data then creating a security hole with eval
is incompetence at best and willful endangerment at worst. Explaining
the reason of the loss of sensitive data to an affected customer without
lying could be a bit difficult.
Also, IANAL, but I think in my country you could (theoretically -
practically it’s probably a bit too difficult to prove) even be held
responsible for sensitive data lost, compromised or stolen through such
a security hole. So I’m quite serious about that.
But my apologies if it offended you.

I wouldn’t recommend eval() in production code, without bulletproof
ways of keeping it clear of user input, but I find it a lot more
concise and a lot easier to debug. Certainly the code examples in
this thread which use eval are slimmer than the non-eval versions. (To
my eyes, they’re also clearer.) To each his own, I guess.

Ok. Different opinions obviously, no issue with that :slight_smile:

Regards
Stefan