Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not allow assignment operator #65

Open
PPakalns opened this issue Oct 29, 2019 · 8 comments
Open

Do not allow assignment operator #65

PPakalns opened this issue Oct 29, 2019 · 8 comments
Labels
Milestone

Comments

@PPakalns
Copy link

The following statement successfully executes.

simpleeval.simple_eval("a = 2")

And returns 2.
I suggest that assignment operator is entirely disallowed as input to simple eval.

Because "a == 2" can be easily mistyped as "a = 2" and existence of variable "a" is not even checked in evaluation of this input.

@daveisfera
Copy link
Contributor

We currently use this functionality and prefer that it continue to be allowed ( see #25 (comment) )

@PPakalns
Copy link
Author

PPakalns commented Jan 2, 2020

We currently use this functionality and prefer that it continue to be allowed ( see #25 (comment) )

(Is the provided comment the correct one?)

We can add this validation as optional. Anyway, this assignment thing should be handled in better way because as it is now the assignment target can be anything (variable name is not used anywhere) and can easily introduce errors if second equality sign is missed.

@daveisfera
Copy link
Contributor

Yes, that's the comment that I intended to link to but it doesn't tell the whole story. We actually use simpleeval in a wrapper (called ComplexEval as a sort of joke) that allows it to run multi-line Python expressions so it's a sort of Python sandbox. So assignments are a crucial part of that and make it so that the sandbox can actually "do something". There's probably some protections that could be put in around that (i.e. only allow assignment to exist variables in some cases), but for us the current setup works very well.
I'd be willing to contribute that wrapper upstream, if there's interest in adding it to this module.

@danthedeckie
Copy link
Owner

danthedeckie commented Jan 7, 2020

@daveisfera Would https://github.com/danthedeckie/simpleeval/tree/warn_on_assign work for you? As in, not mess up your workflow. It could actually raise an exception which you then over-ride, but I don't want to introduce changes to current behaviour unless it really makes sense to.

I've realised that I've really not done a great job up til now of maintaining this.

I want to improve that in the future - but it will mean some breaking changes, so my ideal plan at the moment is to lock it down to a 1.0 "stable" release, and then do some refactoring & sanifying (if that's a word) as a 2.0 branch

@PPakalns
Copy link
Author

PPakalns commented Jan 7, 2020

@daveisfera Would https://github.com/danthedeckie/simpleeval/tree/warn_on_assign work for you? As in, not mess up your workflow. It could actually raise an exception which you then over-ride, but I don't want to introduce changes to current behaviour unless it really makes sense to.

I've realised that I've really not done a great job up til now of maintaining this.

I want to improve that in the future - but it will mean some breaking changes, so my ideal plan at the moment is to lock it down to a 1.0 "stable" release, and then do some refactoring & sanifying (if that's a word) as a 2.0 branch

I believe that 2.0 version is good idea because a lot of changes looks like breaking ones.
It would be good to identify the main needed features for the new release in a separate issue.
At least I would like to provide some input for it :)

@danthedeckie
Copy link
Owner

@PPakalns that sounds great! Thanks for wanting to be part of this 😄

@daveisfera
Copy link
Contributor

Yes, that would be fine because I should be able to override with functions that do the assignment in my "complex wrapper"

@jeffreyscottgraham
Copy link

I use the assignment. Please dont remove. Optional is fine with me.
How do I turn off the warning about assignment?

@danthedeckie danthedeckie added this to the 2.0-alpha milestone Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants