Fixed #17664 -- Deprecated silencing exceptions in the {% if %} template tag.

[jdufresne]

Thanks @raiderrobert for the orignal PR #9713.

https://code.djangoproject.com/ticket/17664

[adamchainz]

No docs change on the if tag?

[jdufresne]

I added a .. deprecated:: 3.1 note to the if template tag docs. I read through the rest of them and they do not mention how exceptions are handled, so I didn't change the docs beyond that. However, if you see something you think should be changed, please let me know and I'll give it is a look.

[jdufresne]

Rebased for Django 3.2

[carltongibson]

Erm... so... I may have missed the point but, this looks like a much more significant proposal that just, "raise exceptions". Requiring if var and... checks everywhere is a serious regression, I'm inclined to think.

Thoughts?

[carltongibson]

This isn't nice, especially not if we generalize over the entire set of old templates out there that are relying on the soft-fail here: it's old as the arc that you can define an if block that simply won't show if the variable is missing.

I'm not sure that requiring if var and var.attr everywhere is a win. (In fact, stronger than that...)

Are there alternatives? I see that if var.attr raises (like an actual exception) we want that to raise, but can we maintain the historic/super behaviour of allowing var to not be defined?

[jdufresne]

These checks exist to facilitate testing d522b51. In hindsight, I think it would be simpler to always pass the request object to Django admin templates and drop the requirement on the context processor. Doing so would allow me to drop the request checks in this template. I've done this in the latest revision, which simplifies the changes to this template.

[smithdc1]

To try and understand the implications of this I ran this against crispy-forms and get 5 failures.

The error / warning messages don't give you an indication on where to start looking or how to go about solving it, which is an issue for me. So if we accepted this change we're asking a lot of people to do a lot of work to 'fix' their templates. Given how much feedback the url() change generated, I can only imagine the comments we'd get on this...

Here is the impact it has (once you've managed to work out which template it is in). Take this line here

{% if 'col' in div.css_class %} becomes {% if div.css_class and 'col' in div.css_class %}. I'm unclear on what the benefit of the new format is?

A harder example is this template. There are 36 ifs, try working out which are wrong.

[smithdc1]

https://docs.djangoproject.com/en/3.0/ref/templates/api/#how-invalid-variables-are-handled

Note: The current behaviour is explained here in the docs.

[jdufresne]

Requiring if var and... checks everywhere is a serious regression, I'm inclined to think.

No, they are not required "everywhere", that is an exaggeration. These checks are only in {% if %} template tags that raise an exception. If a template knows a variable exists or knows a variable's type, they are certainly not required. They also do not affect variable evaluation outside of {% if %} template tags.

There is also a typical deprecation period. Per normal procedure, this will not break existing code though the deprecation period, users will see a warning and be nudged to update their code to be future proof for after the deprecation period ends.

The overall goal here is to make the Django template more robust and helpful to developers to avoid bugs. IMO, the silencing of exceptions allows bugs to go unnoticed during development and testing and only be noticed later in production -- perhaps by confused users.

This brings template error handling closer to Python. For example,

def foo(bar=None):
    if bar is None and bar.baz():
        return bar.bizzle()
    return bazzle()

One might say that checking if bar is None is annoying, can't the if evaluate as False in that case when .baz() is called on None? While perhaps some parallel universe Python could do that, doing so eventually results in unwanted bugs that are harder to control for.

This becomes especially important for templates that expose private data. For example:

{% if thing.is_not_pirvate %}
   <p>Public stuff.</p>
{% else %}
   <p>Super secret stuff</p>
{% endif %}

Notice the typo in the word "private". In Python, this would result in an AttributeError, but in Django templates, it evaluates the if as False, dropping the user into the "super secret" block. In this scenario, a typo may go uncaught during development and testing and will leak private data during production.

Are there alternatives? I see that if var.attr raises (like an actual exception) we want that to raise, but can we maintain the historic/super behaviour of allowing var to not be defined?

Can you elaborate on what you're suggesting?

In the example above if thing is not defined, which block should evaluate? The True case or the False case? What if the boolean expression was reversed? I think whatever you choose private data could be mistakenly leaked by a programming typo.

---

Without these checks I consider the silencing of exceptions in {% if %} blocks a serious misfeature that makes Django templates insecure by design.

[jdufresne]

Note: The current behaviour is explained here in the docs.

Those docs explain how undefined variables are rendered. This PR is about how undefined variables are handled in {% if %} template tags -- especially when chained with other operations. The behavior you linked to still exists after this work.

[adamchainz]

When removing a check, we should preserve a record of the code somewhere so it doesn't get reused in the future (as it could already be used in projects' SILENCED_SYSTEM_CHECKS).

[jdufresne]

That makes sense, thanks. Do you know if there is an established approach to this? Should I just leave the line as is? Move it somewhere else? Or modify it some some marker?

[jdufresne]

I added *This check appeared in Django 3.1*. which I saw done for other system checks.

[adamchainz]

Yeah it looks like that's the mechanism right now.

[carltongibson]

Closing as per ticket.