By Nikwin


2009-12-02 13:34:07 8 Comments

I am using the following class to easily store data of my songs.

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:
            exec 'self.%s=None'%(att.lower()) in locals()
    def setDetail(self, key, val):
        if key in self.attsToStore:
            exec 'self.%s=val'%(key.lower()) in locals()

I feel that this is just much more extensible than writing out an if/else block. However, eval seems to be considered a bad practice and unsafe to use. If so, can anyone explain to me why and show me a better way of defining the above class?

8 comments

@Lokeshwar Tailor 2018-10-29 07:26:51

In addition to @Nadia Alramli answer, since I am new to Python and was eager to check how using eval will affect the timings, I tried a small program and below were the observations:

#Difference while using print() with eval() and w/o eval() to print an int = 0.528969s per 100000 evals()

from datetime import datetime
def strOfNos():
    s = []
    for x in range(100000):
        s.append(str(x))
    return s

strOfNos()
print(datetime.now())
for x in strOfNos():
    print(x) #print(eval(x))
print(datetime.now())

#when using eval(int)
#2018-10-29 12:36:08.206022
#2018-10-29 12:36:10.407911
#diff = 2.201889 s

#when using int only
#2018-10-29 12:37:50.022753
#2018-10-29 12:37:51.090045
#diff = 1.67292

@moooeeeep 2018-05-29 09:47:23

When eval() is used to process user-provided input, you enable the user to Drop-to-REPL providing something like this:

"__import__('code').InteractiveConsole(locals=globals()).interact()"

You may get away with it, but normally you don't want vectors for arbitrary code execution in your applications.

@Jim Fasarakis Hilliard 2016-11-27 17:20:00

Other users pointed out how your code can be changed as to not depend on eval; I'll offer a legitimate use-case for using eval, one that is found even in CPython: testing.

Here's one example I found in test_unary.py where a test on whether (+|-|~)b'a' raises a TypeError:

def test_bad_types(self):
    for op in '+', '-', '~':
        self.assertRaises(TypeError, eval, op + "b'a'")
        self.assertRaises(TypeError, eval, op + "'a'")

The usage is clearly not bad practice here; you define the input and merely observe behavior. eval is handy for testing.

Take a look at this search for eval, performed on the CPython git repository; testing with eval is heavily used.

@Hackaholic 2016-05-06 20:41:56

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]"

@Nadia Alramli 2009-12-02 13:37:08

Yes, using eval is a bad practice. Just to name a few reasons:

  1. There is almost always a better way to do it
  2. Very dangerous and insecure
  3. Makes debugging difficult
  4. Slow

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)

EDIT:

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.

EDIT 2:

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.

EDIT 3: Reordered point 1 and 4

@extraneon 2009-12-02 13:39:08

I like this better than __dic__ since it's more explicit on what you intend to do.

@codeape 2009-12-02 13:41:39

In this case, eval is a bad practice. But eval is not a universal bad practice. There are cases where using eval/exec/execfile is great.

@Nikwin 2009-12-02 13:50:43

Thank you, I just changed my code to use setattr.

@S.Lott 2009-12-02 15:41:00

-1: "Very dangerous and insecure" is false. The other three are outstandingly clear. Please reorder them so that 2 and 4 are the first two. It's only insecure if you are surrounded by evil sociopaths who are looking for ways to subvert your application.

@Nadia Alramli 2009-12-02 16:20:55

@S.Lott, Insecurity is a very important reason to avoid eval/exec in general. Many applications like websites should take extra care. Take the OP example in a website that expects users to enter the song name. It is bound to be exploited sooner or later. Even an innocent input like: Let's have fun. will cause a syntax error and expose the vulnerability.

@riza 2009-12-02 16:25:08

If eval is evil, why do it exists, in the first place?

@Nadia Alramli 2009-12-02 16:32:59

@Selinap, eval has practical use cases but they are rare and special. In the OP case it is very very clear eval is a bad and insecure choice. If something exists that doesn't mean we should blindly use it.

@Denis Otkidach 2009-12-02 17:06:39

I agree with S.Lott: there is no user data in executed statement, so there is no reason to consider it insecure.

@Nadia Alramli 2009-12-02 17:08:56

@Denis, you are right the OP case doesn't have user data. But still the question was general: "Is Using eval In Python A Bad Practice?"

@S.Lott 2009-12-02 17:38:45

@Nadia Alramli: User input and eval have nothing to do with each other. An application that's fundamentally mis-designed is fundamentally mis-designed. eval is no more the root cause of bad design than division by zero or attempting to import a module which is known not to exist. eval isn't insecure. Applications are insecure.

@Jeffrey Jose 2010-01-29 20:03:24

S.Lott's point is really valid. eval fundamentally isnt bad/evil. Its the design which is almost always to blame. If your design doesnt guarantee you against evil user-input, eval might not be what you should be worried about.

@L̲̳o̲̳̳n̲̳̳g̲̳̳p̲̳o̲̳̳k̲̳̳e̲̳̳ 2010-07-04 17:23:40

@jeffjose: Actually, it is fundamentally bad/evil because it's treating unparamaterized data as code (this is why XSS, SQL injection, and stack smashes exist). @S.Lott: "It's only insecure if you are surrounded by evil sociopaths who are looking for ways to subvert your application." Cool, so say you make a program calc, and to add numbers it executes print(eval("{} + {}".format(n1, n2))) and exits. Now you distribute this program with some OS. Then someone makes a bash script that takes some numbers from a stock site and adds them using calc. boom?

@L̲̳o̲̳̳n̲̳̳g̲̳̳p̲̳o̲̳̳k̲̳̳e̲̳̳ 2010-07-04 17:27:12

@S.Lott: It's probably better to say that eval is sane if and only if the method using eval doesn't execute insane code as a side effect... Say I have a JSON decoder in JavaScript... I want to pass anything to it, I don't want to waste my time writing sanitization code every time I call it (which would be the case if the function just returned eval(input).

@martineau 2010-10-15 16:55:08

Look, everyone using Python isn't writing a web script or application where security is a major concern. So, while I agree that there's often a better way, say using setattr(), and that one should make the effort to find them for a better design, I definitely don't agree that eval/exec are inherently 'very dangerous and insecure'. There's good reasons they're available and having the abilities they provide is one of the coolest and most powerful features of interpreted languages like Python.

@Bradley Kreider 2011-04-06 17:03:15

@martineau: in comparison to the alternatives it is very inherently dangerous and insecure. If/when you have no alternative, there is nothing to compare it against so it might be ok to use.

@Owen S. 2011-08-19 16:52:08

I'm not sure why Nadia's assertion is so contentious. It seems simple to me: eval is a vector for code injection, and is dangerous in a way that most other Python functions are not. That doesn't mean you shouldn't use it at all, but I think you should use it judiciously.

@jpmc26 2013-11-01 02:06:40

I'm with Owen S. Saying that eval is "not dangerous or insecure" is kind of like saying "hand built SQL isn't dangerous or insecure." Sure, there might be rare use cases where it's necessary or times when you simply don't care because security is not an issue for your application, but neither of those negates the fact that eval has the ability to very easily make your application vulnerable to outside attack. "Very easily making my application vulnerable to outside attack" sounds like a reasonable qualification for deeming something "very dangerous and insecure."

@GoingMyWay 2018-06-02 09:08:30

Hi, thank you for your answer. If it is slow and why?

@Robert Rossney 2009-12-02 18:19:56

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.

@Josh Lee 2009-12-02 18:41:30

Another way that's arguably more (or less) Pythonic: Instead of using the object's __dict__ directly, give the object an actual dictionary object, either through inheritance or as an attribute.

@bruno desthuilliers 2018-05-29 12:27:19

"A less obvious approach is updating the object's dict object directly" => Note that this will bypass any descriptor (property or other) or __setattr__ override, which might lead to unexpected results. setattr() doesn't have this problem.

@S.Lott 2009-12-02 18:08:39

Using eval is weak, not a clearly bad practice.

  1. 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.

  2. 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.

  3. 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.

@Owen S. 2011-08-19 16:55:19

I'm curious what you think a sociopath is going to do with eval? And why that would be a use case that a more innocent programmer might not stumble across as well?

@S.Lott 2011-08-19 21:34:55

@Owen S. The point is this. Folks will tell you that eval is some kind of "security vulnerability". As if Python -- itself -- was not just a bunch of interpreted source that anyone could modify. When confronted with the "eval is a security hole", you can only assume that it's a security hole in the hands of sociopaths. Ordinary programmers merely modify the existing Python source and cause their problems directly. Not indirectly through eval magic.

@Owen S. 2012-01-06 18:42:47

Well, I can tell you exactly why I would say eval is a security vulnerability, and it has to do with the trustworthiness of the string it's given as input. If that string comes, in whole or in part, from the outside world, there's a possibility of a scripting attack on your program if you're not careful. But that's thge derangement of an outside attacker, not of the user or administrator.

@S.Lott 2012-01-06 19:08:18

@OwenS.: "If that string comes, in whole or in part, from the outside world" Often false. This isn't a "careful" thing. It's black and white. If the text comes from a user, it can never be trusted. Care isn't really part of it, it's absolutely untrustable. Otherwise, the text comes from a developer, installer or admin, and can be trusted.

@Owen S. 2012-01-06 22:48:55

Untrusted strings can often be transformed into trusted strings through proper escaping. That's what I meant by "careful". We don't disagree, much as you seem to wish we would. :-)

@S.Lott 2012-01-06 22:54:34

@OwenS.: There's no possible escaping for a string of untrusted Python code that would make it trustable. I agree with most of what you're saying except for the "careful" part. It's a very crisp distinction. Code from the outside world is untrustable. AFAIK, no amount of escaping or filtering can clean it up. If you have some kind of escaping function that would make code acceptable, please share. I didn't think such a thing was possible. For example while True: pass would be hard to clean up with some kind of escaping.

@Owen S. 2012-01-08 00:23:28

Say what you were accepting from the outside world was intended as a string, not arbitrary code. In that case being careful means not splicing the string directly into the code you want to eval – or making sure quotation marks in the string are escaped properly if you do incorporate them into code passed to eval. Or if you're going to take an expression to eval, making sure that what is given to you is of the limited subset of syntax you're willing to accept and have Python work with.

@S.Lott 2012-01-09 00:01:25

@OwenS.: "intended as a string, not arbitrary code". That's unrelated. That's just a string value, which you would never pass through eval(), since it's a string. Code from the "outside world" cannot be sanitized. Strings from the outside world are just strings. I'm unclear on what you're talking about. Perhaps you should provide a more complete blog post and link to it here.

@Owen S. 2012-01-11 02:25:11

In conclusion, they can and they aren't, depending on what you're using eval for. I am not going to spend more time discussing it.

@S.Lott 2012-01-11 11:18:48

@OwenS.: "depending on what you're using eval for."? What? Your comments started out talking about eval of data coming from "the outside world" -- an untrusted source -- and being used as scripting attack. Apparently, you're no longer talking about that? If so, then I'm unable to figure out what you don't like about eval. If you're saying eval is not wholly evil, then it appears we agree.

@Kasapo 2012-04-17 19:57:33

@S.Lott Do you realize that you CAN pass strings to eval? eval accepts code objects or strings, which are then parsed as python. If you accept any user input and any of that input goes into the string that is evaled, there is a huge potential for arbitrary code injection. Running code that is composed from user input might sound like a bad idea, and it should, but the point is that not everyone thinks about that. Some who would use eval ARE using user input, and they may even be sanitizing (or THINK they're sanitizing) that input, but may neglect some escape sequence which could inject code.

@Josh Lee 2009-12-02 13:38:31

In this case, yes. Instead of

exec 'self.Foo=val'

you should use the builtin function setattr:

setattr(self, 'Foo', val)

Related Questions

Sponsored Content

10 Answered Questions

[SOLVED] What does Python's eval() do?

  • 2012-02-21 19:19:16
  • Billjk
  • 363265 View
  • 244 Score
  • 10 Answer
  • Tags:   python eval

10 Answered Questions

9 Answered Questions

16 Answered Questions

3 Answered Questions

[SOLVED] What's the difference between eval, exec, and compile?

9 Answered Questions

[SOLVED] Python join: why is it string.join(list) instead of list.join(string)?

  • 2009-01-29 22:45:13
  • Evan Fosmark
  • 1181978 View
  • 1557 Score
  • 9 Answer
  • Tags:   python string list join

26 Answered Questions

[SOLVED] Why are Python lambdas useful?

25 Answered Questions

[SOLVED] Why is using the JavaScript eval function a bad idea?

2 Answered Questions

[SOLVED] Python - Bad practice to store instance vars in local vars to avoid "self"?

  • 2015-11-05 17:37:55
  • Just some guy
  • 121 View
  • 4 Score
  • 2 Answer
  • Tags:   python

1 Answered Questions

[SOLVED] Python Error Init of Class

  • 2013-10-01 15:29:31
  • user2417731
  • 282 View
  • -1 Score
  • 1 Answer
  • Tags:   python class init

Sponsored Content