[Django] #24977: Template variables with a value of None are considered to be == to non-existent properties
Django
existent properties
---------------------------------+-------------------------------------
Reporter: danielquinn | Owner: nobody
Type: Uncategorized | Status: new
Component: Template system | Version: 1.7
Severity: Normal | Keywords: string, if, equivalence
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-------------------------------------
This bit me today and I thought I'd point it out since I would consider
this a bug
{{{
{% if user.pk == some_undefined_value %}
This is rendered if user is not logged in
{% endif %}
{% if user.pk == some_object.some_invalid_property %}
This is also rendered if user is not logged in
{% endif %}
}}}
It's understood that the template shouldn't flip out with an exception in
the event that we're trying to access an undefined value, but when testing
against these in an `{% if %}` block, some very scary stuff can happen.
In my case in particular, I had something like this:
{{{
{% if user.pk == product.product_owner_id %}
This is private data
{% endif %}
}}}
Changing the attribute name `product_owner_id` to something like
`owner_id`, now accidentally leaks private data to unauthenticated users
because the templating engine considers `None` equal to what is
effectively an `AttributeError`.
What's worse, if you try to render these two values, you get `None` and
`""`, so they're not even equivalent when cast as a string.
--
Ticket URL: <https://code.djangoproject.com/ticket/24977>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
existent properties
Reporter: danielquinn | Owner: nobody
Type: Uncategorized | Status: new
Component: Template system | Version: 1.7
Keywords: string, if, | Triage Stage:
equivalence | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bmispelon):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Hi,
I think I was bitten by this in the past as well.
However, I'm not quite sure if this is intended to be a feature or if it's
an actual bug.
And if it's a bug, fixing it in a backwards-compatible way will be tricky.
I spent some time digging around the implementation of `{% if %}` and I
think the key to this issue is this line:
https://github.com/django/django/blob/master/django/template/defaulttags.py#L945
If you remove `ignore_failures=True`, then `{% if %}` behaves the way
you're suggesting it should.
But that change also brings several failures in the test suite, which is
not a good sign :(
We might have to resolve to just document this, unless someone has a
better idea...
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:1>
Django
existent properties
Reporter: danielquinn | Owner: nobody
Type: Uncategorized | Status: new
Component: Template system | Version: 1.7
Keywords: string, if, | Triage Stage:
equivalence | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by danielquinn):
I'm not familiar with the inner workings of the template engine, but I
would think that this could be done in a backward-friendly way by defining
failed lookups as something other than None:
{{{
class NotAThing(object):
def __str__(self):
return ""
}}}
That way you could test if something evaluating to `None` is equal to an
instance of `NotAThing` and hopefully it'd pan out as not-equal, right?
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:2>
Django
existent properties
Reporter: danielquinn | Owner: nobody
Type: Uncategorized | Status: new
Component: Template system | Version: 1.7
Keywords: string, if, | Triage Stage:
equivalence | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MoritzS):
That's actually how jinja2 does it.
All undefined variables are an instance of `jinja2.Undefined` and `{% if
foo.undefined_attr is none %}` evaluates to false.
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:3>
Django
existent properties
Reporter: danielquinn | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Template system | Version: 1.7
Keywords: string, if, | Triage Stage: Accepted
equivalence |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:4>
Django
existent properties
Reporter: Daniel Quinn | Owner: Tim
Type: | Martin
Cleanup/optimization | Status: assigned
Component: Template system | Version: 1.7
Keywords: string, if, | Triage Stage: Accepted
equivalence |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Martin):
* status: new => assigned
* owner: nobody => Tim Martin
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:5>
Django
existent properties
Reporter: Daniel Quinn | Owner: Tim
Type: | Martin
Cleanup/optimization | Status: assigned
Keywords: string, if, | Triage Stage: Accepted
equivalence |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Martin):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/7901 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:6>
Django
existent properties
Reporter: Daniel Quinn | Owner: Tim
Type: | Martin
Cleanup/optimization | Status: assigned
Keywords: string, if, | Triage Stage: Accepted
equivalence |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Martin):
I've addressed the comments on the pull request aside from the question of
whether we need a deprecation path for the `ignore_failures` parameter.
I'm assuming it's OK to change an internal API without a deprecation path;
if this is controversial then it's probably best to discuss in the Django
developers list.
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:7>
Django
existent properties
Reporter: Daniel Quinn | Owner: Tim
Type: | Martin
Cleanup/optimization | Status: assigned
Keywords: string, if, | Triage Stage: Accepted
equivalence |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Martin):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:8>
Django
existent properties
Reporter: Daniel Quinn | Owner: Tim
Type: | Martin
Cleanup/optimization | Status: assigned
Keywords: string, if, | Triage Stage: Accepted
equivalence |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Martin):
* needs_better_patch: 1 => 0
Comment:
I've fixed up the UT and flakes failures.
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:9>
Django
existent properties
Reporter: Daniel Quinn | Owner: Tim
Type: | Martin
Cleanup/optimization | Status: assigned
Keywords: string, if, | Triage Stage: Accepted
equivalence |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:10>
Django
existent properties
Reporter: Daniel Quinn | Owner: Tim
Type: | Martin
Cleanup/optimization | Status: assigned
Keywords: string, if, | Triage Stage: Accepted
equivalence |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Martin):
* needs_better_patch: 1 => 0
Comment:
Thanks for the feedback, I've improved the pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:11>
Django
existent properties
Reporter: Daniel Quinn | Owner: Tim
Type: | Martin
Cleanup/optimization | Status: assigned
Keywords: string, if, | Triage Stage: Accepted
equivalence |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:12>
Django
existent properties
Reporter: Daniel Quinn | Owner: Tim
Type: | Martin
Component: Template system | Version: 1.7
Keywords: string, if, | Triage Stage: Accepted
equivalence |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Tim Graham):
* status: assigned => closed
* resolution: => wontfix
Comment:
Per [https://groups.google.com/d/topic/django-
developers/LT5ESP0w0gQ/discussion the discussion on django-developers], it
seems we can't make the change due to backwards compatibility (see the
[https://docs.djangoproject.com/en/dev/ref/templates/api/#how-invalid-
variables-are-handled documentation of the current behavior]).
--
Ticket URL: <https://code.djangoproject.com/ticket/24977#comment:13>