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

Dynamic choice variable #1768

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

liortct
Copy link
Contributor

@liortct liortct commented Aug 27, 2022

Addresses #1744 .

Fixed bug where dynamic non string variable rendered as string.
That's not allowed dynamic creation of choice variable.
Also, added a unit-test for recreate this scenario.

@ericof ericof added the bug This issue/PR relates to a bug. label Jun 13, 2023
return template.render(cookiecutter=cookiecutter_dict)
rendered_variable_str = template.render(cookiecutter=cookiecutter_dict)
try:
rendered_variable = ast.literal_eval(rendered_variable_str)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jensens Should we worry about this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a fan of even literal_eval in code which is meant to run on cookiecutters fetched from Git.

see https://docs.python.org/3/library/ast.html#ast.literal_eval

This function had been documented as “safe” in the past without defining what that meant. That was misleading. This is specifically designed not to execute Python code, unlike the more general eval(). There is no namespace, no name lookups, or ability to call out. But it is not free from attack: A relatively small input can lead to memory exhaustion or to C stack exhaustion, crashing the process. There is also the possibility for excessive CPU consumption denial of service on some inputs. Calling it on untrusted data is thus not recommended.

@ericof ericof added this to the 3.0.0 milestone Jun 13, 2023
@liortct
Copy link
Contributor Author

liortct commented Nov 27, 2023

@ericof . I can work on this PR again.
Do you want I will fix the conflicts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants