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

Fix bug involving calling set on a template parameter within all branches of an if block #1665

Merged
merged 1 commit into from
Dec 21, 2024

Conversation

kevin-brown
Copy link
Member

@kevin-brown kevin-brown commented May 2, 2022

This fixes a bug where calling {% set %} in all 3 branches of an {% if %} block could result in the parameter being undefined sometimes. If the {% set %} call was not done in all branches or it was done outside of an {% if %} block then the bug would not appear. This was determined to be a bad assumption being made by the ID tracking for variables within the template loader where if the {% set %} occurred in all branches then it was not going to be a previously defined variable.

Fixes #1253

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@kevin-brown kevin-brown marked this pull request as ready for review May 2, 2022 22:15
@kevin-brown kevin-brown requested a review from davidism May 2, 2022 22:15
src/jinja2/idtracking.py Show resolved Hide resolved
tests/test_regression.py Outdated Show resolved Hide resolved
src/jinja2/idtracking.py Show resolved Hide resolved
src/jinja2/idtracking.py Show resolved Hide resolved
src/jinja2/idtracking.py Show resolved Hide resolved
@davidism davidism added this to the 3.1.5 milestone Dec 20, 2024
@davidism
Copy link
Member

Targeting this for the 3.1.5 release. Needs to be rebased onto latest stable branch (git rebase --onto origin/stable origin/main). I'll step through it in more detail tomorrow, it looks like we only need to add some clarifying comments to the code.

@davidism davidism changed the base branch from main to stable December 20, 2024 22:57
@davidism davidism force-pushed the GH-1253-set-within-if branch from 81426ca to 0a8bbdf Compare December 20, 2024 22:57
There was a bug that came as the result of an early optimization done
within ID tracking that caused loading parameters to fail in a very
specific and rare edge case. That edge case only occurred when the
parameter was being set within all 3 standard branches of an if block,
since the optimization would assume that the parameter was never being
referenced and was only ever being set. This would cause the variable to
be set to undefined.

The fix for this was to remove the optimization and still continue to
load in the parameter even if it is set in all 3 branches.
@davidism davidism force-pushed the GH-1253-set-within-if branch from 0a8bbdf to 66587ce Compare December 21, 2024 17:40
@davidism davidism merged commit c8fdce1 into stable Dec 21, 2024
12 checks passed
@davidism davidism deleted the GH-1253-set-within-if branch December 21, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What's wrong with this Jinja template when using {% elif %}
2 participants