Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm wrote a main python module that need load a file parser to work, initially I was a only one text parser module, but I need add more parsers for different cases.

parser_class1.py

parser_class2.py

parser_class3.py

Only one is required for every running instance, then I'm thinking load it by command line:

mmain.py -p parser_class1

With this purpose I wrote this code in order to select the parser to load when the main module will be called:

#!/usr/bin/env python

import argparse
aparser = argparse.ArgumentParser()
aparser.add_argument('-p',
            action='store',
            dest='module',
            help='-i module to import')
results = aparser.parse_args()

if not results.module:
    aparser.error('Error! no module')
try:
    exec("import %s" %(results.module))
    print '%s imported done!'%(results.module)
except ImportError, e:
    print e

But, I was reading that this way is dangerous, maybe not standard.

Then is this approach ok?

or I must find another way to do it?

Why?

share|improve this question
9  
If you only know what library to import at runtime, you should use __import__ rather than 'exec'. This may be relevant to your case. You also have the importlib module as an option. –  Jaime Mar 25 '13 at 16:45
    
+1 on Jaime's comment. Even then, it's probably OK to import everything, even if you only use one module. –  Quentin Pradet May 3 '13 at 6:19
    
+1 again to Jaime comment, I think that deserves to be considered an answer! –  justhalf Sep 3 '13 at 14:02

1 Answer 1

Regarding the safety aspect of your question.

The reason why exec() can be dangerous is that is can allow a nefarious agent to execute code that you never intended.

Let's assume for example that somewhere in your program, you have sensitive data elements such as:

username = secret_username
password = never_share

And let's also assume that someone calls your program like this:

mmain.py -p 'parser_class1;print globals()'

Then your exec() statement would actually be:

exec("import %s" %('parser_class1;print globals()'))

And this would result in your program printing out all variables in your global space for them... including your username and password.

By making your program utilize the __import__ method as mentioned by @Jaime, you can at least prevent people from executing non-import statements in your code.

But, you should, whenever possible also examine the input from a user before using it to execute any dynamic code.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.