Specify and apply numpy commands in config files #92
Conversation
|
I guess you didn't see my comment above? |
| expressionString = self.get(section, option) | ||
| result = ast.literal_eval(expressionString) | ||
| if usenumpy: | ||
| result = eval(expressionString) |
There was a problem hiding this comment.
@pwolfram, I was worried when I saw your examples in the tutorial that you were using eval for this. When I implemented the getExpression method, I looked into various ways of evaluating the list. I ended up with ast.literal_eval because I found numerous warnings that eval is very dangerous. This is for something like the same reasons that subprocess.call should not be called with shell=True unless absolutely necessary. There is simply too much danger that unintended or malicious code gets put into whatever string is being evaluated. Here is an example of a page discussing why eval is dangerous. Taking a string from a config file seems to me to be exactly the kind of case that this page is warning about.
http://nedbatchelder.com/blog/201206/eval_really_is_dangerous.html
I'm willing to continue my review of this code but only after we've had a discussion about how eval can be made safe or another method can be found for evaluating strings with numpy commands.
There was a problem hiding this comment.
By the way, I would really love to have this functionality in a safe way, so I hope we can find a good solution.
|
Ah, I don't think I understood how a review works. Do you not see my comments until I post the review? |
|
@xylar, I think so. Presumably the functionality is so that you could edit comments made at the beginning of the review following your completion of the review to presumably handle the case where there was a question early on in the review that was resolved by reading code later in the review. |
|
Yes, but if you don't even see my comments until I post the review, you obviously can't respond to them. That's what I hadn't understood. Did my first comment about |
|
Yes, how long ago did you do the review? Presumably longer than an hour ago... |
|
Here's more on why |
|
I think this is the solution: http://stackoverflow.com/questions/10076300/python-using-sympy-sympify-to-perform-a-safe-eval-on-mathematical-functions. We basically pass a restricted grammar into eval corresponding to acceptable numpy functions ... |
pwolfram
force-pushed
the
config_eval_numpy
branch
3 times, most recently
from
d29ebd6 to
2a01e1e
Compare
Previously numpy commands were not valid to specify config file settings. Now, numpy commands can be used as the values in config key-value pairs. This is beneficial because long arrays are now easily specified, e.g., np.arange(0,1,100).
pwolfram
force-pushed
the
config_eval_numpy
branch
from
2a01e1e to
47e84ab
Compare
|
Okay, great, I'm good with this safer version. I imagine there are crazy hacks like in this post that might be able to get around your restrictions but let's be reasonable. |
|
@xylar, based on that post it seems like we should exclude strings of the form |
|
@pwolfram, If you want to do a further patch, that's fine. I'm not too worried about it now that we've done our best to keep it from being abused. |
2 participants
This merge allows numpy commands to be used as the values
in config key-value pairs. This is beneficial because long
arrays are now easily specified, e.g.,
np.arange(0,1,100).