Malicious code : invoking eval can crash a computer. For example: if you use eval server-side and a mischievous user decides to use an infinite loop as their username. Terribly slow : the JavaScript language is designed to use the full gamut of JavaScript types (numbers, functions, objects, etc)… Not just strings!
Since the eval() function will evaluate any Python expressions, the hacker can easily get a list of files and folders on the server. To be honest, you probably will be fired if the above string is really evaluated by the eval() function.
Your server could be compromised and the data source could be tampered with.
An alternative to eval is Function() . Just like eval() , Function() takes some expression as a string for execution, except, rather than outputting the result directly, it returns an anonymous function to you that you can call. `Function() is a faster and more secure alternative to eval().
Yes, using eval
is a bad practice. Just to name a few reasons:
In your case you can use setattr instead:
class Song:
"""The class to store the details of each song"""
attsToStore=('Name', 'Artist', 'Album', 'Genre', 'Location')
def __init__(self):
for att in self.attsToStore:
setattr(self, att.lower(), None)
def setDetail(self, key, val):
if key in self.attsToStore:
setattr(self, key.lower(), val)
There are some cases where you have to use eval
or exec
. But they are rare. Using eval
in your case is a bad practice for sure. I'm emphasizing on bad practice because eval
and exec
are frequently used in the wrong place.
Replying to the comments:
It looks like some disagree that eval
is 'very dangerous and insecure' in the OP case. That might be true for this specific case but not in general. The question was general and the reasons I listed are true for the general case as well.
Using eval
is weak, not a clearly bad practice.
It violates the "Fundamental Principle of Software". Your source is not the sum total of what's executable. In addition to your source, there are the arguments to eval
, which must be clearly understood. For this reason, it's the tool of last resort.
It's usually a sign of thoughtless design. There's rarely a good reason for dynamic source code, built on-the-fly. Almost anything can be done with delegation and other OO design techniques.
It leads to relatively slow on-the-fly compilation of small pieces of code. An overhead which can be avoided by using better design patterns.
As a footnote, in the hands of deranged sociopaths, it may not work out well. However, when confronted with deranged sociopathic users or administrators, it's best to not give them interpreted Python in the first place. In the hands of the truly evil, Python can a liability; eval
doesn't increase the risk at all.
In this case, yes. Instead of
exec 'self.Foo=val'
you should use the builtin function setattr
:
setattr(self, 'Foo', val)
Yes, it is:
Hack using Python:
>>> eval(input())
"__import__('os').listdir('.')"
...........
........... #dir listing
...........
The below code will list all tasks running on a Windows machine.
>>> eval(input())
"__import__('subprocess').Popen(['tasklist'],stdout=__import__('subprocess').PIPE).communicate()[0]"
In Linux:
>>> eval(input())
"__import__('subprocess').Popen(['ps', 'aux'],stdout=__import__('subprocess').PIPE).communicate()[0]"
It's worth noting that for the specific problem in question, there are several alternatives to using eval
:
The simplest, as noted, is using setattr
:
def __init__(self):
for name in attsToStore:
setattr(self, name, None)
A less obvious approach is updating the object's __dict__
object directly. If all you want to do is initialize the attributes to None
, then this is less straightforward than the above. But consider this:
def __init__(self, **kwargs):
for name in self.attsToStore:
self.__dict__[name] = kwargs.get(name, None)
This allows you to pass keyword arguments to the constructor, e.g.:
s = Song(name='History', artist='The Verve')
It also allows you to make your use of locals()
more explicit, e.g.:
s = Song(**locals())
...and, if you really want to assign None
to the attributes whose names are found in locals()
:
s = Song(**dict([(k, None) for k in locals().keys()]))
Another approach to providing an object with default values for a list of attributes is to define the class's __getattr__
method:
def __getattr__(self, name):
if name in self.attsToStore:
return None
raise NameError, name
This method gets called when the named attribute isn't found in the normal way. This approach somewhat less straightforward than simply setting the attributes in the constructor or updating the __dict__
, but it has the merit of not actually creating the attribute unless it exists, which can pretty substantially reduce the class's memory usage.
The point of all this: There are lots of reasons, in general, to avoid eval
- the security problem of executing code that you don't control, the practical problem of code you can't debug, etc. But an even more important reason is that generally, you don't need to use it. Python exposes so much of its internal mechanisms to the programmer that you rarely really need to write code that writes code.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With