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

Check that contextfunction, etc. is True #1145

Closed
davidism opened this issue Feb 3, 2020 · 2 comments · Fixed by #1146
Closed

Check that contextfunction, etc. is True #1145

davidism opened this issue Feb 3, 2020 · 2 comments · Fixed by #1146
Milestone

Comments

@davidism
Copy link
Member

@davidism davidism commented Feb 3, 2020

In order to determine if the context should be passed to a function or filter, Jinja has the contextfunction and contextfilter decorators which set corresponding attributes on the functions. However, they're checked with if getattr(f, "contextfunction", 0), which causes issues for objects that are callable and have permissive __getattr__ functions, such as Mock.

Changing everything to default to False and check is True, would be good for consistency.

The full list is contextfunction, evalcontextfunction, environmentfunction, contextfilter, evalcontextfilter, and environmentcontextfilter.

Extracted from #1139

@davidism davidism modified the milestones: 3.0.0, 2.11.2 Feb 3, 2020
tomaskrizek added a commit to tomaskrizek/jinja that referenced this issue Feb 3, 2020
Explicit checks for "is True" prevents unexpected behaviour with objects
that are callable and have permissive gettatr(), such as Mock.

Fixes pallets#1145
@mitsuhiko

This comment has been minimized.

Copy link
Member

@mitsuhiko mitsuhiko commented Feb 3, 2020

I'm honestly not sure if it's a good idea to pass mock objects to a template in the first place. FWIW the reason 0 is used is because this code was faster on 2.x where the constant optimization was not applied yet.

@davidism

This comment has been minimized.

Copy link
Member Author

@davidism davidism commented Feb 3, 2020

I agree it's not, but there was another example of it coming up outside testing and it's a straightforward fix. 0 was only used in one place, False was used everywhere else already.

davidism added a commit to tomaskrizek/jinja that referenced this issue Feb 4, 2020
Explicit checks for "is True" prevents unexpected behaviour with objects
that are callable and have permissive gettatr(), such as Mock.

Fixes pallets#1145
@davidism davidism closed this Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.