Re: Template handling of undefined variables

[Tim Martin]

On Wednesday, 1 March 2017 19:52:22 UTC, Luke Plant wrote:
>
>
> This is a really big backwards incompatibility, as far as I can see. It 
> means that any filter may now get passed `UndefinedVariable` instead of 
> being passed `None` or string_if_invalid. So for example, the following 
> template behaves oppositely with your patch, if variable 'missing' is 
> undefined:
>
> {% if missing|yesno == "maybe" %}true{% else %}false{% endif %}
>
> There are likely many other filters that will behave differently e.g.:
>
> {{ missing|add:"x" }}
>
> Without the patch, if 'missing' is undefined this returns "x", but with 
> the patch it returns "". It's not just builtin filters I'm thinking about, 
> it's everyone else's that would also have to be made aware of this new 
> false-y value that it will start receiving. This is a really big backwards 
> incompatible change, I don't see how we could justify this. If we were 
> starting from scratch it would be another matter.
>

Yes, I can see the issue and I'm open to the idea of abandoning this patch. 
If we're not willing to tolerate substantial changes to the behaviour then 
I don't think there's a way forward with this patch - changing the 
semantics of undefined variables is pretty much the whole point of it.

In fairness, it's possible to change all the built-in filters to preserve 
existing behaviour (I've already done it for everything that's documented 
or covered by unit tests, though I guess I missed 'yesno', which should 
clearly return maybe for an undefined variable) and any third-party filters 
that are relying on the semantics of undefined variables (outside of 'if', 
'for' and 'regroup') are based on undefined behaviour. Imposing changes on 
third-party filters in a major release of Django doesn't seem too 
unreasonable. But I don't know that there's a strong enough positive case 
for making the change. I don't care much about it, I only picked up this 
bug because it looked like an easy way to learn the template system.

For the record, I don't like the design of treating undefined values as 
None, and I really don't like the design of them having different values in 
different contexts. But I guess that's the design we're stuck with.

Tim

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/c5b64d00-40c4-404e-9960-c7ac21251061%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Luke Plant]

On 01/03/17 01:53, Tim Martin wrote:


On Tuesday, 28 February 2017 13:39:21 UTC, Luke Plant wrote:


On 28/02/17 15:24, Marten Kenbeek wrote:

What about adding a filter |definedthat returns True if a
variable is defined, False otherwise? It may not solve any
problems when it's left implicit, but at least it allows users
the explicitly define the behaviour for non-existing template
variables, and it makes the intention of the template code
crystal clear. And of course it's fully backwards compatible. The
drawback is perhaps the resulting verbosity of if statements:

{% if user.role|defined and roles.staff|defined and user.role
== roles.staff %}
[sensitive information here]
{% endif %}



The only way to do it would be to hard code this filter into the
template engine, because otherwise the 'defined' filter runs too
late to know whether the variable is defined.  This is really
hacky and magic.



Is that true? In my patch, the undefined variable gets expanded into 
an UndefinedVariable sentinel object, which can be detected by the 
defined modifier, and can compare == to None (or not, if you wish). 
Other than the result of the 'is' operator, I think you can make this 
fully backward compatible if you choose. Whether or not this is 
desirable is another question, of course.


I was talking about with the current behaviour of template engine i.e. 
without changing how undefined variables are handled. For this to work 
as you proposed, you would have to be passing the UndefinedVariable 
object into these filters. Having checked out your branch, I can see 
that is what you are doing.


This is a really big backwards incompatibility, as far as I can see. It 
means that any filter may now get passed `UndefinedVariable` instead of 
being passed `None` or string_if_invalid. So for example, the following 
template behaves oppositely with your patch, if variable 'missing' is 
undefined:


{% if missing|yesno == "maybe" %}true{% else %}false{% endif %}

There are likely many other filters that will behave differently e.g.:

{{ missing|add:"x" }}

Without the patch, if 'missing' is undefined this returns "x", but with 
the patch it returns "". It's not just builtin filters I'm thinking 
about, it's everyone else's that would also have to be made aware of 
this new false-y value that it will start receiving. This is a really 
big backwards incompatible change, I don't see how we could justify 
this. If we were starting from scratch it would be another matter.


Luke

--
You received this message because you are subscribed to the Google Groups "Django 
developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/fb53a533-9e83-476c-a20d-6bc7953b928c%40cantab.net.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Luke Plant]

In the context of template variable interpolation i.e. `{{ }}`
syntax, it is defined that `x` in this case will get converted to
the empty string (or your 'string_if_invalid' setting) -

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


- unlike the case for `if` tags.

When this value (the empty string) is passed to `default_if_none`,
the value is not changed, since it is not None. See

https://docs.djangoproject.com/en/1.10/ref/templates/builtins/#default-if-none




The missing piece of information for me (and even reading the docs 
it's far from obvious) is that undefined variables mean two different 
things in the different contexts.


The value *is* changed in the second case, because it *is* None. 
Undefined values are '' in {{ }} context but are None in {% %} 
context. Maybe that's obvious to you, but it's not to me. In fact, 
after a few minutes searching I still can't find where this is 
mentioned in the documentation. Only the vague statement that invalid 
variables are "generally" replaced with string_if_invalid.


I meant above that in the {{ }} context 'default_if_none' doesn't return 
the default value, but just returns the empty string input value.


I also had to read the docs to understand the behaviour - it is 
mentioned in the 3rd paragraph down in the docs linked above, "How 
invalid variables are handled" - the `if`, `for` and `regroup` tags are 
specifically mentioned.  I think the docs could be clearly about how 
things work with variables passed to template tags etc. "In general" as 
you say is rather vague. Definitely room for improvement here!



Luke

--
You received this message because you are subscribed to the Google Groups "Django 
developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/49c674a7-ab69-cfd6-0fdc-9ba444fd1ffa%40cantab.net.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Tim Martin]

On Monday, 27 February 2017 10:43:02 UTC, Luke Plant wrote:
>
>
>
> On 05/01/17 02:39, Tim Martin wrote:
>
> 2) There appears to be an inconsistency in the default_if_none
>modifier. If I have a template with
>
>x|default_if_none:y = {{x|default_if_none:y}}
>{% if x|default_if_none:y %}
>x|default_if_none:y is apparently True
>{% endif %}
>
>with y defined to True and x undefined, then it produces:
>
>x|default_if_none:y = 
>x|default_if_none:y is apparently True
>
>IOW it appears that the default_if_none doesn't cause y's value to
>be used in the case where the variable is being printed, even
>though it causes the expression to have y's value when evaluated in
>an if. I think this is something to do with the fact that the two
>ways of handling failures (ignore_failures and string_if_invalid)
>do different things.
>
>I don't see a way to make backward compatible code here.
>
>I think this is just a bug, does anyone think otherwise?
>
>
> This seems to be working exactly as it should.
>
> In the context of template variable interpolation i.e. `{{ }}` syntax, it 
> is defined that `x` in this case will get converted to the empty string (or 
> your 'string_if_invalid' setting) - 
> https://docs.djangoproject.com/en/dev/ref/templates/api/#how-invalid-variables-are-handled
>  
> - unlike the case for `if` tags.
>
> When this value (the empty string) is passed to `default_if_none`, the 
> value is not changed, since it is not None. See 
> https://docs.djangoproject.com/en/1.10/ref/templates/builtins/#default-if-none
>

The missing piece of information for me (and even reading the docs it's far 
from obvious) is that undefined variables mean two different things in the 
different contexts.

The value *is* changed in the second case, because it *is* None. Undefined 
values are '' in {{ }} context but are None in {% %} context. Maybe that's 
obvious to you, but it's not to me. In fact, after a few minutes searching 
I still can't find where this is mentioned in the documentation. Only the 
vague statement that invalid variables are "generally" replaced with 
string_if_invalid.

Tim

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/9f9a2f60-ab5d-4010-b81e-90a47625a5e2%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Tim Martin]

On Tuesday, 28 February 2017 13:39:21 UTC, Luke Plant wrote:
>
>
>
> On 28/02/17 15:24, Marten Kenbeek wrote:
>
> What about adding a filter |defined that returns True if a variable is 
> defined, False otherwise? It may not solve any problems when it's left 
> implicit, but at least it allows users the explicitly define the behaviour 
> for non-existing template variables, and it makes the intention of the 
> template code crystal clear. And of course it's fully backwards compatible. 
> The drawback is perhaps the resulting verbosity of if statements: 
>
> {% if user.role|defined and roles.staff|defined and user.role == 
> roles.staff %}
> [sensitive information here]
> {% endif %}
>
>
>
> The only way to do it would be to hard code this filter into the template 
> engine, because otherwise the 'defined' filter runs too late to know 
> whether the variable is defined.  This is really hacky and magic.
>


Is that true? In my patch, the undefined variable gets expanded into an 
UndefinedVariable sentinel object, which can be detected by the defined 
modifier, and can compare == to None (or not, if you wish). Other than the 
result of the 'is' operator, I think you can make this fully backward 
compatible if you choose. Whether or not this is desirable is another 
question, of course.

Tim

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/5e9949b2-11ae-466e-86ae-03f41754955f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Luke Plant]

On 28/02/17 15:24, Marten Kenbeek wrote:
What about adding a filter |definedthat returns True if a variable is 
defined, False otherwise? It may not solve any problems when it's left 
implicit, but at least it allows users the explicitly define the 
behaviour for non-existing template variables, and it makes the 
intention of the template code crystal clear. And of course it's fully 
backwards compatible. The drawback is perhaps the resulting verbosity 
of if statements:


{% if user.role|defined and roles.staff|defined and user.role == 
roles.staff %}

[sensitive information here]
{% endif %}



The only way to do it would be to hard code this filter into the 
template engine, because otherwise the 'defined' filter runs too late to 
know whether the variable is defined.  This is really hacky and magic.


The alternative would be for this filter to take a string e.g.:


{% if "user.role"|defined and "roles.staff"|defined and user.role 
== roles.staff %}

[sensitive information here]
{% endif %}

As an analogy, in Python you can't write:

if hasattr(foo.bar)

you have to write:

if hasattr(foo, 'bar')

Another alternative would be to have some special syntax that could 
check for defined variables e.g.


   {% if user.role? and roles.staff? and user.role == roles.staff %}

My own preference, if I had a time machine, would be to have missing 
data/attributes just raise NameError/KeyError/AttributeError, and then 
have special syntax for replacing missing values with None - like 
https://msdn.microsoft.com/en-us/library/dn986595(v=vs.140).aspx .


However, I think that any option involving special syntax here has 
already got way too complex for the design philosophy of Django 
templates - to be simpler than normal Python code, and more designer 
friendly - this would take us in the opposite direction.


Luke

--
You received this message because you are subscribed to the Google Groups "Django 
developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/c1bada04-9852-e49a-e651-b12c68dcb284%40cantab.net.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Marten Kenbeek]

What about adding a filter |defined that returns True if a variable is 
defined, False otherwise? It may not solve any problems when it's left 
implicit, but at least it allows users the explicitly define the behaviour 
for non-existing template variables, and it makes the intention of the 
template code crystal clear. And of course it's fully backwards compatible. 
The drawback is perhaps the resulting verbosity of if statements:

{% if user.role|defined and roles.staff|defined and user.role == 
roles.staff %}
[sensitive information here]
{% endif %}

Marten

On Monday, February 27, 2017 at 11:36:09 AM UTC+1, Luke Plant wrote:
>
>
> On 26/02/17 00:44, Karen Tracey wrote:
>
> On Sat, Feb 25, 2017 at 2:10 PM, Tim Graham  > wrote:
>
>> I think any use of undefined template variables should raise an 
>> exception. In the long run, keeping a setting to allow some other behavior 
>> seems confusing and, considering the case of templates that might be reused 
>> in different projects with different settings, even dangerous.
>>
>
> I think I'm confused...Django templates have allowed use of undefined 
> variables and documented their use as evaluating to the empty string for as 
> long as I recall. Wouldn't a change to instead raise exceptions be a major 
> backwards-incompatibility?
>
> https://docs.djangoproject.com/en/1.7/topics/templates/#variables said 
> "If you use a variable that doesn’t exist, the template system will insert 
> the value of the TEMPLATE_STRING_IF_INVALID setting, which is set to '' 
> (the empty string) by default."
>
>
> This behaviour applies only to what happens when the variable is rendered. 
> In the context of `if` tags, undefined variables get converted to `None`. 
> This behaviour is documented - 
> https://docs.djangoproject.com/en/dev/ref/templates/api/#how-invalid-variables-are-handled
>  
> but perhaps not in as much detail as necessary.
>
> The question is about changing this, especially due to templates like this:
>
> {% if user.role == roles.staff %}
> [sensitive information here]
> {% endif %}
>
> If:
> 1) you forget to provide both user and role to template context or
> 2) 'role' or 'staff' are invalid attributes, or
> 3) 'role' returns None but 'staff' is an invalid attribute, for example,
> 4) various combinations of these
>
> then this results in the sensitive info being shown, because `None == 
> None`. The proposal is to introduce a value that doesn't not compare equal 
> to itself and avoid this kind of issue.
>
> Having thought about it more, I've realised that this really isn't going 
> to work at all.
>
> First attempt:
>
> class Undefined1(object):
> def __eq__(self, other):
> return False
> 
> If we use this, then consider the following template code:
>
> {% if user.role != roles.staff %}
> This info is private, sorry
> {% else %}
> [sensitive information here]
> {% endif %}
>
>
> The default implementation of != means we will again get the sensitive 
> data shown for some of the error situations given above (e.g. 1 and 2). 
> Second attempt:
>
> class Undefined2(object):
> def __eq__(self, other):
> return False
> def __new__(self, other):
> return False
>
> This object is neither equals nor not-equals to itself or anything else. 
> But consider this template:
>
>
> {% if user.role == roles.customer %}
> This info is private, sorry
> {% else %}
> [sensitive information here]
> {% endif %}
>
> With `Undefined2` being used for invalid attributes, we again get the 
> sensitive information being shown in the case of developer error and 
> missing attributes. Plus, we now have the situation that `{% if a != b %}` 
> is not the same as `{% if not a == b %}`, which is very confusing behaviour 
> for most people.
>
> In other words, we are getting into the territory of needing a value that 
> behaves like NULL in SQL. Even if there were no implementation issues (and 
> there will be lots, because operators like 'or' 'and' etc. would need to 
> cope with this, not to mention 3rd party code), and there were no backwards 
> compatibility issues (there are lots), this would be extremely dubious to 
> me, because ternary logic is extremely tricky.
>
> The only sensible alternative to current behaviour is to raise exceptions, 
> but huge backwards incompatibility issues I think rule this out. If I were 
> to start again, I think I would make the template system not silence any 
> errors you would normally get in Python, but just have some special syntax 
> for converting non-existent data or attributes into None. But I don't have 
> that time machine.
>
> Regards,
>
> Luke
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 

Re: Template handling of undefined variables

[Shai Berger]

On Sunday 26 February 2017 11:16:54 Florian Apolloner wrote:
> As much as I'd like variables to raise an error if they cannot resolve, I
> am with Karen here -- the backwards compatibility considerations should
> take priority here. At least we should have a possibility in the templates
> to check if variables are undefined before we start raising exceptions.
> 

I concur.

Adam's {% load undefined_vars from future %} sounds like a good idea -- with 
two caveats:

1) We shouldn't make it part of a deprecation plan (at least, not yet)

2) We should consider how to enable it to affect many templates from one place. 
My first thought: If included in a template, it should apply (recursively) to 
any template which {% extends %} it, and to any {% include %}'d template.

Then it's not "modify each template" to use, and it isn't all-or-nothing 
unless you want it to be.

My 2 cents,
Shai

Re: Template handling of undefined variables

[Luke Plant]

On 05/01/17 02:39, Tim Martin wrote:

2) There appears to be an inconsistency in the default_if_none
   modifier. If I have a template with

   x|default_if_none:y = {{x|default_if_none:y}}
   {% if x|default_if_none:y %}
   x|default_if_none:y is apparently True
   {% endif %}

   with y defined to True and x undefined, then it produces:

   x|default_if_none:y =
   x|default_if_none:y is apparently True

   IOW it appears that the default_if_none doesn't cause y's value to
   be used in the case where the variable is being printed, even
   though it causes the expression to have y's value when evaluated in
   an if. I think this is something to do with the fact that the two
   ways of handling failures (ignore_failures and string_if_invalid)
   do different things.

   I don't see a way to make backward compatible code here.

   I think this is just a bug, does anyone think otherwise?


This seems to be working exactly as it should.

In the context of template variable interpolation i.e. `{{ }}` syntax, 
it is defined that `x` in this case will get converted to the empty 
string (or your 'string_if_invalid' setting) - 
https://docs.djangoproject.com/en/dev/ref/templates/api/#how-invalid-variables-are-handled 
- unlike the case for `if` tags.


When this value (the empty string) is passed to `default_if_none`, the 
value is not changed, since it is not None. See 
https://docs.djangoproject.com/en/1.10/ref/templates/builtins/#default-if-none


The behaviour here follows 100% logically from the documented behaviour 
of the two components.


Regards,

Luke




Sorry for the lengthy email. Thanks for any input.

Tim
--
You received this message because you are subscribed to the Google 
Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send 
an email to django-developers+unsubscr...@googlegroups.com 
.
To post to this group, send email to 
django-developers@googlegroups.com 
.

Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/b9d7b557-5826-4c7e-a064-5f8b59312faa%40googlegroups.com 
.

For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "Django 
developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/a7dc3b9e-195f-e4e4-2692-4b07de68c0cd%40cantab.net.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Luke Plant]

On 26/02/17 00:44, Karen Tracey wrote:
On Sat, Feb 25, 2017 at 2:10 PM, Tim Graham > wrote:


I think any use of undefined template variables should raise an
exception. In the long run, keeping a setting to allow some other
behavior seems confusing and, considering the case of templates
that might be reused in different projects with different
settings, even dangerous.


I think I'm confused...Django templates have allowed use of undefined 
variables and documented their use as evaluating to the empty string 
for as long as I recall. Wouldn't a change to instead raise exceptions 
be a major backwards-incompatibility?


https://docs.djangoproject.com/en/1.7/topics/templates/#variables said 
"If you use a variable that doesn’t exist, the template system will 
insert the value of the TEMPLATE_STRING_IF_INVALID setting, which is 
set to '' (the empty string) by default."


This behaviour applies only to what happens when the variable is 
rendered. In the context of `if` tags, undefined variables get converted 
to `None`. This behaviour is documented - 
https://docs.djangoproject.com/en/dev/ref/templates/api/#how-invalid-variables-are-handled 
but perhaps not in as much detail as necessary.


The question is about changing this, especially due to templates like this:

{% if user.role == roles.staff %}
[sensitive information here]
{% endif %}

If:
1) you forget to provide both user and role to template context or
2) 'role' or 'staff' are invalid attributes, or
3) 'role' returns None but 'staff' is an invalid attribute, for example,
4) various combinations of these

then this results in the sensitive info being shown, because `None == 
None`. The proposal is to introduce a value that doesn't not compare 
equal to itself and avoid this kind of issue.


Having thought about it more, I've realised that this really isn't going 
to work at all.


First attempt:

class Undefined1(object):
def __eq__(self, other):
return False

If we use this, then consider the following template code:

{% if user.role != roles.staff %}
This info is private, sorry
{% else %}
[sensitive information here]
{% endif %}


The default implementation of != means we will again get the sensitive 
data shown for some of the error situations given above (e.g. 1 and 2). 
Second attempt:


class Undefined2(object):
def __eq__(self, other):
return False
def __new__(self, other):
return False

This object is neither equals nor not-equals to itself or anything else. 
But consider this template:



{% if user.role == roles.customer %}
This info is private, sorry
{% else %}
[sensitive information here]
{% endif %}

With `Undefined2` being used for invalid attributes, we again get the 
sensitive information being shown in the case of developer error and 
missing attributes. Plus, we now have the situation that `{% if a != b 
%}` is not the same as `{% if not a == b %}`, which is very confusing 
behaviour for most people.


In other words, we are getting into the territory of needing a value 
that behaves like NULL in SQL. Even if there were no implementation 
issues (and there will be lots, because operators like 'or' 'and' etc. 
would need to cope with this, not to mention 3rd party code), and there 
were no backwards compatibility issues (there are lots), this would be 
extremely dubious to me, because ternary logic is extremely tricky.


The only sensible alternative to current behaviour is to raise 
exceptions, but huge backwards incompatibility issues I think rule this 
out. If I were to start again, I think I would make the template system 
not silence any errors you would normally get in Python, but just have 
some special syntax for converting non-existent data or attributes into 
None. But I don't have that time machine.


Regards,

Luke

--
You received this message because you are subscribed to the Google Groups "Django 
developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/4b0a8ec9-c588-de8d-76ba-0d3b55b8fe20%40cantab.net.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Florian Apolloner]

As much as I'd like variables to raise an error if they cannot resolve, I 
am with Karen here -- the backwards compatibility considerations should 
take priority here. At least we should have a possibility in the templates 
to check if variables are undefined before we start raising exceptions.

Cheers,
Florian

On Sunday, February 26, 2017 at 3:37:20 AM UTC+1, Karen Tracey wrote:
>
> On Sat, Feb 25, 2017 at 8:21 PM, Fred Stluka  > wrote:
>
>> I agree that use of undefined variables should raise an exception.
>> The incompatibility with previous versions will mostly catch errors
>> that have been going undetected.
>>
>
> I disagree, unless you are limiting this change specifically to arguments 
> to non-builtin template tags. (Which I thought already raised exceptions, 
> so I'm still confused here as to what change is being proposed.)
>
> Given the documented behavior of evaluating undefined variables to empty 
> strings its been common practice to use:
>
> {% if var_that_may_not_exist %}
> ...stuff that should only show when var exists...
> {% endif %}
>
> or to include {{ var_that_may_not_exist }} somewhere in a template and 
> rely on it evaluating to an empty string. (admin itself was documented as 
> not working correctly if you set the setting to evaluate undefined to 
> something other than empty string).
>
> Changing either of these now to raise exceptions would be a huge backwards 
> incompatibility going against previously documented behavior.  Please don't 
> do that.
>
> It may well be friendlier to developers (I've never been a fan of the 
> "templates shouldn't raise exceptions" philosophy) but the fact is for many 
> years now it's been perfectly acceptable to use what might be undefined 
> variables in templates and the behavior has been documented as to how it 
> will work. Changing that now to start raising exceptions would be 
> incredibly unfriendly to existing code that has been written to rely on it. 
>
> Karen
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/d996f168-3a80-4c64-ab93-9b0dd4e94c70%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Karen Tracey]

On Sat, Feb 25, 2017 at 8:21 PM, Fred Stluka  wrote:

> I agree that use of undefined variables should raise an exception.
> The incompatibility with previous versions will mostly catch errors
> that have been going undetected.
>

I disagree, unless you are limiting this change specifically to arguments
to non-builtin template tags. (Which I thought already raised exceptions,
so I'm still confused here as to what change is being proposed.)

Given the documented behavior of evaluating undefined variables to empty
strings its been common practice to use:

{% if var_that_may_not_exist %}
...stuff that should only show when var exists...
{% endif %}

or to include {{ var_that_may_not_exist }} somewhere in a template and rely
on it evaluating to an empty string. (admin itself was documented as not
working correctly if you set the setting to evaluate undefined to something
other than empty string).

Changing either of these now to raise exceptions would be a huge backwards
incompatibility going against previously documented behavior.  Please don't
do that.

It may well be friendlier to developers (I've never been a fan of the
"templates shouldn't raise exceptions" philosophy) but the fact is for many
years now it's been perfectly acceptable to use what might be undefined
variables in templates and the behavior has been documented as to how it
will work. Changing that now to start raising exceptions would be
incredibly unfriendly to existing code that has been written to rely on it.

Karen

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CACS9raeFmaZQqtH3wyT6abRsXSHFjL2%2B-VfGnjgG2-18_M%2BRsg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Fred Stluka]

Tim and others,

+1 for raising an exception.

Specifically...

I agree that use of undefined variables should raise an exception.
The incompatibility with previous versions will mostly catch errors
that have been going undetected.

Personally, I find it very hard to detect such bugs when reviewing
code written by myself or other team members, especially since we
make heavy use of template inheritance, and we reuse the same
templates from multiple views.  So, it's not easy to spot a case
where we forgot to pass a value to a template.

I think Django should at least offer this as an option.

--Fred

Fred Stluka -- mailto:f...@bristle.com -- http://bristle.com/~fred/
Bristle Software, Inc -- http://bristle.com -- Glad to be of service!
Open Source: Without walls and fences, we need no Windows or Gates.


On 2/25/17 7:28 PM, Tim Graham wrote:
My proposal was only for the use of undefined variables in template 
tags. I didn't realize that the behavior of undefined variables in 
some tags resolving to None is documented, but I still think it's a 
useful change to raise an exception instead. The philosophy that 
template tags shouldn't raise exceptions is more unhelpful than 
helpful in my experience. I think the change would be consistent with 
the deprecation that starts in Django 1.11 to change {% include %} not 
to silencing exceptions and render an empty string, for example.


On Saturday, February 25, 2017 at 4:44:30 PM UTC-5, Karen Tracey wrote:

On Sat, Feb 25, 2017 at 2:10 PM, Tim Graham  wrote:

I think any use of undefined template variables should raise
an exception. In the long run, keeping a setting to allow some
other behavior seems confusing and, considering the case of
templates that might be reused in different projects with
different settings, even dangerous.


I think I'm confused...Django templates have allowed use of
undefined variables and documented their use as evaluating to the
empty string for as long as I recall. Wouldn't a change to instead
raise exceptions be a major backwards-incompatibility?

https://docs.djangoproject.com/en/1.7/topics/templates/#variables

said "If you use a variable that doesn’t exist, the template
system will insert the value of the TEMPLATE_STRING_IF_INVALID
setting, which is set to '' (the empty string) by default."


https://docs.djangoproject.com/en/dev/ref/templates/api/#invalid-template-variables


has refined that doc to note that the behavior is slightly
different in some tags.

Are we really considering changing this behavior to now raise
exceptions?

--
You received this message because you are subscribed to the Google 
Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send 
an email to django-developers+unsubscr...@googlegroups.com 
.
To post to this group, send email to 
django-developers@googlegroups.com 
.

Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/8b260b00-7921-4977-9521-f61f073e717b%40googlegroups.com 
.

For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "Django 
developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/8ff7366a-b8ba-5ac3-566d-6e6258e4ed81%40bristle.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Melvyn Sopacua]

On Saturday 25 February 2017 16:28:17 Tim Graham wrote:
> My proposal was only for the use of undefined variables in template
> tags. I didn't realize that the behavior of undefined variables in
> some tags resolving to None is documented, but I still think it's a
> useful change to raise an exception instead. The philosophy that
> template tags shouldn't raise exceptions is more unhelpful than
> helpful in my experience.

I sincerely hope that the change only affects DEBUG mode (including the 
include change). But once the templates are validated and working as they 
should, they should not generate exceptions unless Django provides a way 
to catch them. For example, if a service is unavailable that makes up a 
small part of a page, I do not want the whole page to crash or information to 
leak.

-- 
Melvyn Sopacua

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/2409576.D1JWrnJBHJ%40devstation.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Tim Graham]

My proposal was only for the use of undefined variables in template tags. I 
didn't realize that the behavior of undefined variables in some tags 
resolving to None is documented, but I still think it's a useful change to 
raise an exception instead. The philosophy that template tags shouldn't 
raise exceptions is more unhelpful than helpful in my experience. I think 
the change would be consistent with the deprecation that starts in Django 
1.11 to change {% include %} not to silencing exceptions and render an 
empty string, for example.

On Saturday, February 25, 2017 at 4:44:30 PM UTC-5, Karen Tracey wrote:
>
> On Sat, Feb 25, 2017 at 2:10 PM, Tim Graham  > wrote:
>
>> I think any use of undefined template variables should raise an 
>> exception. In the long run, keeping a setting to allow some other behavior 
>> seems confusing and, considering the case of templates that might be reused 
>> in different projects with different settings, even dangerous.
>>
>
> I think I'm confused...Django templates have allowed use of undefined 
> variables and documented their use as evaluating to the empty string for as 
> long as I recall. Wouldn't a change to instead raise exceptions be a major 
> backwards-incompatibility?
>
> https://docs.djangoproject.com/en/1.7/topics/templates/#variables said 
> "If you use a variable that doesn’t exist, the template system will insert 
> the value of the TEMPLATE_STRING_IF_INVALID setting, which is set to '' 
> (the empty string) by default."
>
>
> https://docs.djangoproject.com/en/dev/ref/templates/api/#invalid-template-variables
>  
> has refined that doc to note that the behavior is slightly different in 
> some tags. 
>
> Are we really considering changing this behavior to now raise exceptions?
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/8b260b00-7921-4977-9521-f61f073e717b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Karen Tracey]

On Sat, Feb 25, 2017 at 2:10 PM, Tim Graham  wrote:

> I think any use of undefined template variables should raise an exception.
> In the long run, keeping a setting to allow some other behavior seems
> confusing and, considering the case of templates that might be reused in
> different projects with different settings, even dangerous.
>

I think I'm confused...Django templates have allowed use of undefined
variables and documented their use as evaluating to the empty string for as
long as I recall. Wouldn't a change to instead raise exceptions be a major
backwards-incompatibility?

https://docs.djangoproject.com/en/1.7/topics/templates/#variables said "If
you use a variable that doesn’t exist, the template system will insert the
value of the TEMPLATE_STRING_IF_INVALID setting, which is set to '' (the
empty string) by default."

https://docs.djangoproject.com/en/dev/ref/templates/api/#invalid-template-variables
has refined that doc to note that the behavior is slightly different in
some tags.

Are we really considering changing this behavior to now raise exceptions?

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CACS9rad-TtehoMxLCruRv5M8yT_GHH-nZxvBETcVaRrLfHVkUw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Tim Graham]

I think any use of undefined template variables should raise an exception. 
In the long run, keeping a setting to allow some other behavior seems 
confusing and, considering the case of templates that might be reused in 
different projects with different settings, even dangerous.

On Saturday, February 25, 2017 at 9:54:10 AM UTC-5, Tim Martin wrote:
>
> Actually, I can imagine that the option might be worth keeping 
> permanently. I think both the "exception on use of undefined" and "treat 
> undefined as different from all other objects" would both be valid modes. 
> Treating undefined as None is probably only justifiable for backward 
> compatibility, though. I'll rework the patch to support a setting unless 
> anyone comes up with a better idea.
>
> I'm not sure I like the proposal of throwing an exception on `if` but not 
> in other cases. It seems more consistent to just raise an exception on any 
> use of an undefined variable.
>
> Tim
>
> On Monday, 20 February 2017 11:54:37 UTC, Marc Tamlyn wrote:
>>
>> +1 to not having to add (and then remove later) a {% load %} tag to every 
>> template - that was seriously dull with the URL change.
>>
>> Marc
>>
>> On 20 February 2017 at 11:42, Luke Plant  wrote:
>>
>>> On 19/02/17 12:55, Adam Johnson wrote:
>>>
>>> +1 for more obvious errors, silently changing the behaviour could indeed 
>>> lead to unconsidered security holes like 
>>>
>>> {% if user is None %}
>>> non-sensitive information
>>> {% else %}
>>> sensitive information
>>> {% endif %}
>>>
>>> ...which doesn't seem like an unrealistic template snippet. We all know 
>>> variables can go missing in refactorings.
>>>
>>> Another option, perhaps not feasible to implement, would be deprecating 
>>> the old behaviour, similar to the previous change in url with something 
>>> like:
>>>
>>> {% load undefined_vars from future %}
>>>
>>>
>>> I agree there are a lot of potential security/correctness issues with 
>>> this, it is potentially quite a big change (though very helpful IMO).
>>>
>>> A different approach to introducing it might be a setting, possibly an 
>>> option to the Django template engine instead - 
>>> https://docs.djangoproject.com/en/1.10/ref/settings/#std:setting-TEMPLATES-OPTIONS
>>>  
>>> . I think this would be more appropriate for something that is more of a 
>>> global behaviour issue, more practical than having to add hundreds of 'load 
>>> from future' tags, plus it would then parallel other similar settings like 
>>> 'string_if_invalid'. In the next version of Django the option would default 
>>> to False (i.e. old behaviour), but raise a deprecation warning, in future 
>>> versions it would simply be True, and raise an error if someone tries to 
>>> pass False (but allow True, for the sake of apps that are spanning multiple 
>>> Django versions).
>>>
>>> This would allow people to test their site with the new mechanism and 
>>> have time to fix issues, which can be especially important for 3rd party 
>>> Django apps.
>>>
>>> Ideally there would be some way to instrument code and see if the output 
>>> would be different with the new behaviour, but I can't think of an easy way 
>>> to do this.
>>>
>>> Luke
>>>
>>> -- 
>>> You received this message because you are subscribed to the Google 
>>> Groups "Django developers (Contributions to Django itself)" group.
>>> To unsubscribe from this group and stop receiving emails from it, send 
>>> an email to django-develop...@googlegroups.com.
>>> To post to this group, send email to django-d...@googlegroups.com.
>>> Visit this group at https://groups.google.com/group/django-developers.
>>> To view this discussion on the web visit 
>>> https://groups.google.com/d/msgid/django-developers/496fa4a8-b902-657a-92c2-9766bfff8a26%40cantab.net
>>>  
>>> 
>>> .
>>>
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>
>>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/e32a0ad9-50bf-4b8c-8ac5-1cd9ce2b0eda%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Tim Martin]

Actually, I can imagine that the option might be worth keeping permanently. 
I think both the "exception on use of undefined" and "treat undefined as 
different from all other objects" would both be valid modes. Treating 
undefined as None is probably only justifiable for backward compatibility, 
though. I'll rework the patch to support a setting unless anyone comes up 
with a better idea.

I'm not sure I like the proposal of throwing an exception on `if` but not 
in other cases. It seems more consistent to just raise an exception on any 
use of an undefined variable.

Tim

On Monday, 20 February 2017 11:54:37 UTC, Marc Tamlyn wrote:
>
> +1 to not having to add (and then remove later) a {% load %} tag to every 
> template - that was seriously dull with the URL change.
>
> Marc
>
> On 20 February 2017 at 11:42, Luke Plant  > wrote:
>
>> On 19/02/17 12:55, Adam Johnson wrote:
>>
>> +1 for more obvious errors, silently changing the behaviour could indeed 
>> lead to unconsidered security holes like 
>>
>> {% if user is None %}
>> non-sensitive information
>> {% else %}
>> sensitive information
>> {% endif %}
>>
>> ...which doesn't seem like an unrealistic template snippet. We all know 
>> variables can go missing in refactorings.
>>
>> Another option, perhaps not feasible to implement, would be deprecating 
>> the old behaviour, similar to the previous change in url with something 
>> like:
>>
>> {% load undefined_vars from future %}
>>
>>
>> I agree there are a lot of potential security/correctness issues with 
>> this, it is potentially quite a big change (though very helpful IMO).
>>
>> A different approach to introducing it might be a setting, possibly an 
>> option to the Django template engine instead - 
>> https://docs.djangoproject.com/en/1.10/ref/settings/#std:setting-TEMPLATES-OPTIONS
>>  
>> . I think this would be more appropriate for something that is more of a 
>> global behaviour issue, more practical than having to add hundreds of 'load 
>> from future' tags, plus it would then parallel other similar settings like 
>> 'string_if_invalid'. In the next version of Django the option would default 
>> to False (i.e. old behaviour), but raise a deprecation warning, in future 
>> versions it would simply be True, and raise an error if someone tries to 
>> pass False (but allow True, for the sake of apps that are spanning multiple 
>> Django versions).
>>
>> This would allow people to test their site with the new mechanism and 
>> have time to fix issues, which can be especially important for 3rd party 
>> Django apps.
>>
>> Ideally there would be some way to instrument code and see if the output 
>> would be different with the new behaviour, but I can't think of an easy way 
>> to do this.
>>
>> Luke
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to django-develop...@googlegroups.com .
>> To post to this group, send email to django-d...@googlegroups.com 
>> .
>> Visit this group at https://groups.google.com/group/django-developers.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/496fa4a8-b902-657a-92c2-9766bfff8a26%40cantab.net
>>  
>> 
>> .
>>
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/b627d2ab-da52-4cbb-b32d-e54df5fd37b4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Marc Tamlyn]

+1 to not having to add (and then remove later) a {% load %} tag to every
template - that was seriously dull with the URL change.

Marc

On 20 February 2017 at 11:42, Luke Plant  wrote:

> On 19/02/17 12:55, Adam Johnson wrote:
>
> +1 for more obvious errors, silently changing the behaviour could indeed
> lead to unconsidered security holes like
>
> {% if user is None %}
> non-sensitive information
> {% else %}
> sensitive information
> {% endif %}
>
> ...which doesn't seem like an unrealistic template snippet. We all know
> variables can go missing in refactorings.
>
> Another option, perhaps not feasible to implement, would be deprecating
> the old behaviour, similar to the previous change in url with something
> like:
>
> {% load undefined_vars from future %}
>
>
> I agree there are a lot of potential security/correctness issues with
> this, it is potentially quite a big change (though very helpful IMO).
>
> A different approach to introducing it might be a setting, possibly an
> option to the Django template engine instead - https://docs.djangoproject.
> com/en/1.10/ref/settings/#std:setting-TEMPLATES-OPTIONS . I think this
> would be more appropriate for something that is more of a global behaviour
> issue, more practical than having to add hundreds of 'load from future'
> tags, plus it would then parallel other similar settings like
> 'string_if_invalid'. In the next version of Django the option would default
> to False (i.e. old behaviour), but raise a deprecation warning, in future
> versions it would simply be True, and raise an error if someone tries to
> pass False (but allow True, for the sake of apps that are spanning multiple
> Django versions).
>
> This would allow people to test their site with the new mechanism and have
> time to fix issues, which can be especially important for 3rd party Django
> apps.
>
> Ideally there would be some way to instrument code and see if the output
> would be different with the new behaviour, but I can't think of an easy way
> to do this.
>
> Luke
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-developers.
> To view this discussion on the web visit https://groups.google.com/d/
> msgid/django-developers/496fa4a8-b902-657a-92c2-9766bfff8a26%40cantab.net
> 
> .
>
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMwjO1GmpDk8ByyEW2tjpstV9OpfPNokfGKNgufEmDWFhi27hQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Luke Plant]

On 19/02/17 12:55, Adam Johnson wrote:
+1 for more obvious errors, silently changing the behaviour could 
indeed lead to unconsidered security holes like


{% if user is None %}
non-sensitive information
{% else %}
sensitive information
{% endif %}

...which doesn't seem like an unrealistic template snippet. We all 
know variables can go missing in refactorings.


Another option, perhaps not feasible to implement, would be 
deprecating the old behaviour, similar to the previous change in 
url with something like:


{% load undefined_vars from future %}


I agree there are a lot of potential security/correctness issues with 
this, it is potentially quite a big change (though very helpful IMO).


A different approach to introducing it might be a setting, possibly an 
option to the Django template engine instead - 
https://docs.djangoproject.com/en/1.10/ref/settings/#std:setting-TEMPLATES-OPTIONS 
. I think this would be more appropriate for something that is more of a 
global behaviour issue, more practical than having to add hundreds of 
'load from future' tags, plus it would then parallel other similar 
settings like 'string_if_invalid'. In the next version of Django the 
option would default to False (i.e. old behaviour), but raise a 
deprecation warning, in future versions it would simply be True, and 
raise an error if someone tries to pass False (but allow True, for the 
sake of apps that are spanning multiple Django versions).


This would allow people to test their site with the new mechanism and 
have time to fix issues, which can be especially important for 3rd party 
Django apps.


Ideally there would be some way to instrument code and see if the output 
would be different with the new behaviour, but I can't think of an easy 
way to do this.


Luke

--
You received this message because you are subscribed to the Google Groups "Django 
developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/496fa4a8-b902-657a-92c2-9766bfff8a26%40cantab.net.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Tim Graham]

After reviewing the pull request, I wonder if it would be better to raise 
exceptions when comparing nonexistent variables in {% if %} rather than 
altering the behavior. For existing projects, this would prevent possible 
inadvertent information leakage if some {% if %} starts evaluating 
differently than it did before. While the current behavior may not be 
documented, it seems risky to silently change it.

There are examples of that behavior change on the PR: 
https://github.com/django/django/pull/7901

On Saturday, January 7, 2017 at 12:21:33 PM UTC-5, Tim Martin wrote:
>
> On Friday, 6 January 2017 10:15:22 UTC, Alasdair Nicol wrote:
>>
>> Hi,
>>
>> On Thursday, 5 January 2017 17:51:39 UTC, Tim Martin wrote:
>>>
>>>
>>>
>>> On Thursday, 5 January 2017 04:15:31 UTC, Carl Meyer wrote:

 Hi Tim, 

 On 01/04/2017 03:39 PM, Tim Martin wrote: 
  
 > 1) There are test cases where we have templates that should treat "x 
 >is y" as True where x and y are both undefined. 

 Really? Is this an accidental side effect of some historical change, or 
 really the intent of the test? That behavior seems buggy to me; more 
 likely to be a bug-magnet than useful. 

>>>
>>> There are a couple of test cases in test_if.py: 
>>> test_if_is_both_variables_missing and 
>>> test_if_is_not_both_variables_missing. These are verifying this specific 
>>> case, so they haven't been introduced by accident. I haven't got as far as 
>>> figuring out whether it was a fully thought through decision or whether 
>>> someone just created these cases for completeness.
>>>
>>
>> I added the tests in ticket 26479 (see comment 5 onwards) [1]. I believe 
>> I added them for consistency with == and != operators, which have similar 
>> tests [2], [3] (trickier to spot because they are numbered). I apologise if 
>> adding the tests has made it harder to improve the behaviour of the tag. 
>>
>
> Thanks for the background info Alasdair, that saved me lots of time 
> hunting around.
>
> I agree you were right to add the tests, which were useful since without 
> them I wouldn't have realised the potential behaviour change I was 
> introducing. As I see it, these tests don't in themselves form a reason not 
> to change the behaviour (since AIUI you originally added them for 
> completeness, rather than because this specific behaviour was desired).
>
> Does anyone think changing the behaviour of {% if a is None %} is the 
> wrong thing to do? I realise there can potentially be template code out 
> there relying on this, but after a quick scout through the documentation I 
> can't see anywhere that the behaviour on undefined variables is specified 
> officially.
>
> For what it's worth, the same issue doesn't come up with ==, because 
> although the existing template behaviour has the same pattern, it's 
> possible to override == and != so that the Undefined object gives the same 
> behaviour as None did before. It's only for 'is' that this can't be 
> achieved. I don't know how for to go with this: preserving the existing == 
> behaviour but changing it for 'is' leaves the two operations superficially 
> inconsistent (though there's nothing fundamentally wrong with two things 
> that satisfy equality but not 'is'). I can see a couple of alternatives (in 
> all cases all variables are undefined):
>
> * "x is y" is false, but "x == y" is true and "x != y" is false (minimal 
> difference from the current behaviour)
> * "x is y" and "x == y" are both false, "x != y" is true (probably the 
> most mutually consistent?)
> * "x is y", "x == y" and "x != y" are all false (the SQL NULL alternative 
> - I'm not sure if I like this, but it has the merit that wrongly skipping 
> sections of a template is usually less bad than wrongly including parts of 
> a template)
>
> Tim
>  
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/76adf765-bdd8-4ecc-b12c-6766d8363751%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Tim Martin]

On Friday, 6 January 2017 10:15:22 UTC, Alasdair Nicol wrote:
>
> Hi,
>
> On Thursday, 5 January 2017 17:51:39 UTC, Tim Martin wrote:
>>
>>
>>
>> On Thursday, 5 January 2017 04:15:31 UTC, Carl Meyer wrote:
>>>
>>> Hi Tim, 
>>>
>>> On 01/04/2017 03:39 PM, Tim Martin wrote: 
>>>  
>>> > 1) There are test cases where we have templates that should treat "x 
>>> >is y" as True where x and y are both undefined. 
>>>
>>> Really? Is this an accidental side effect of some historical change, or 
>>> really the intent of the test? That behavior seems buggy to me; more 
>>> likely to be a bug-magnet than useful. 
>>>
>>
>> There are a couple of test cases in test_if.py: 
>> test_if_is_both_variables_missing and 
>> test_if_is_not_both_variables_missing. These are verifying this specific 
>> case, so they haven't been introduced by accident. I haven't got as far as 
>> figuring out whether it was a fully thought through decision or whether 
>> someone just created these cases for completeness.
>>
>
> I added the tests in ticket 26479 (see comment 5 onwards) [1]. I believe I 
> added them for consistency with == and != operators, which have similar 
> tests [2], [3] (trickier to spot because they are numbered). I apologise if 
> adding the tests has made it harder to improve the behaviour of the tag. 
>

Thanks for the background info Alasdair, that saved me lots of time hunting 
around.

I agree you were right to add the tests, which were useful since without 
them I wouldn't have realised the potential behaviour change I was 
introducing. As I see it, these tests don't in themselves form a reason not 
to change the behaviour (since AIUI you originally added them for 
completeness, rather than because this specific behaviour was desired).

Does anyone think changing the behaviour of {% if a is None %} is the wrong 
thing to do? I realise there can potentially be template code out there 
relying on this, but after a quick scout through the documentation I can't 
see anywhere that the behaviour on undefined variables is specified 
officially.

For what it's worth, the same issue doesn't come up with ==, because 
although the existing template behaviour has the same pattern, it's 
possible to override == and != so that the Undefined object gives the same 
behaviour as None did before. It's only for 'is' that this can't be 
achieved. I don't know how for to go with this: preserving the existing == 
behaviour but changing it for 'is' leaves the two operations superficially 
inconsistent (though there's nothing fundamentally wrong with two things 
that satisfy equality but not 'is'). I can see a couple of alternatives (in 
all cases all variables are undefined):

* "x is y" is false, but "x == y" is true and "x != y" is false (minimal 
difference from the current behaviour)
* "x is y" and "x == y" are both false, "x != y" is true (probably the most 
mutually consistent?)
* "x is y", "x == y" and "x != y" are all false (the SQL NULL alternative - 
I'm not sure if I like this, but it has the merit that wrongly skipping 
sections of a template is usually less bad than wrongly including parts of 
a template)

Tim
 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/d1b0879b-5514-4fc7-8abf-a38710e00efe%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Adam Johnson]

>
> I apologise if adding the tests has made it harder to improve the
> behaviour of the tag.


I don't think you have anything to apologise for! More tests is always
better. This has clarified the current behaviour 

On 6 January 2017 at 10:15, Alasdair Nicol  wrote:

> Hi,
>
> On Thursday, 5 January 2017 17:51:39 UTC, Tim Martin wrote:
>>
>>
>>
>> On Thursday, 5 January 2017 04:15:31 UTC, Carl Meyer wrote:
>>>
>>> Hi Tim,
>>>
>>> On 01/04/2017 03:39 PM, Tim Martin wrote:
>>>
>>> > 1) There are test cases where we have templates that should treat "x
>>> >is y" as True where x and y are both undefined.
>>>
>>> Really? Is this an accidental side effect of some historical change, or
>>> really the intent of the test? That behavior seems buggy to me; more
>>> likely to be a bug-magnet than useful.
>>>
>>
>> There are a couple of test cases in test_if.py:
>> test_if_is_both_variables_missing and test_if_is_not_both_variables_missing.
>> These are verifying this specific case, so they haven't been introduced by
>> accident. I haven't got as far as figuring out whether it was a fully
>> thought through decision or whether someone just created these cases for
>> completeness.
>>
>
> I added the tests in ticket 26479 (see comment 5 onwards) [1]. I believe I
> added them for consistency with == and != operators, which have similar
> tests [2], [3] (trickier to spot because they are numbered). I apologise if
> adding the tests has made it harder to improve the behaviour of the tag.
>
> cheers,
> Alasdair
>
> [1]: https://code.djangoproject.com/ticket/26479#comment:5
> [2]: https://github.com/django/django/blob/master/tests/
> template_tests/syntax_tests/test_if.py#L85
> [3]: https://github.com/django/django/blob/master/tests/
> template_tests/syntax_tests/test_if.py#L112
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-developers.
> To view this discussion on the web visit https://groups.google.com/d/
> msgid/django-developers/96f764fa-25c6-4104-9b6b-
> 8226e3b6bd8f%40googlegroups.com
> 
> .
>
> For more options, visit https://groups.google.com/d/optout.
>



-- 
Adam

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM1_EWy8PTCbvQF1qU4vPAWtALpCJzFm7-dfQtU2JuR_Fw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Alasdair Nicol]

Hi,

On Thursday, 5 January 2017 17:51:39 UTC, Tim Martin wrote:
>
>
>
> On Thursday, 5 January 2017 04:15:31 UTC, Carl Meyer wrote:
>>
>> Hi Tim, 
>>
>> On 01/04/2017 03:39 PM, Tim Martin wrote: 
>>  
>> > 1) There are test cases where we have templates that should treat "x 
>> >is y" as True where x and y are both undefined. 
>>
>> Really? Is this an accidental side effect of some historical change, or 
>> really the intent of the test? That behavior seems buggy to me; more 
>> likely to be a bug-magnet than useful. 
>>
>
> There are a couple of test cases in test_if.py: 
> test_if_is_both_variables_missing and 
> test_if_is_not_both_variables_missing. These are verifying this specific 
> case, so they haven't been introduced by accident. I haven't got as far as 
> figuring out whether it was a fully thought through decision or whether 
> someone just created these cases for completeness.
>

I added the tests in ticket 26479 (see comment 5 onwards) [1]. I believe I 
added them for consistency with == and != operators, which have similar 
tests [2], [3] (trickier to spot because they are numbered). I apologise if 
adding the tests has made it harder to improve the behaviour of the tag. 

cheers,
Alasdair

[1]: https://code.djangoproject.com/ticket/26479#comment:5
[2]: 
https://github.com/django/django/blob/master/tests/template_tests/syntax_tests/test_if.py#L85
[3]: 
https://github.com/django/django/blob/master/tests/template_tests/syntax_tests/test_if.py#L112

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/96f764fa-25c6-4104-9b6b-8226e3b6bd8f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Tim Martin]

On Thursday, 5 January 2017 04:15:31 UTC, Carl Meyer wrote:
>
> Hi Tim, 
>
> On 01/04/2017 03:39 PM, Tim Martin wrote: 
>  
> > 1) There are test cases where we have templates that should treat "x 
> >is y" as True where x and y are both undefined. 
>
> Really? Is this an accidental side effect of some historical change, or 
> really the intent of the test? That behavior seems buggy to me; more 
> likely to be a bug-magnet than useful. 
>

There are a couple of test cases in test_if.py: 
test_if_is_both_variables_missing and 
test_if_is_not_both_variables_missing. These are verifying this specific 
case, so they haven't been introduced by accident. I haven't got as far as 
figuring out whether it was a fully thought through decision or whether 
someone just created these cases for completeness.
 

> ... 
> >I don't see any way to satisfy both these requirements. Is it 
> >reasonable to relax the requirement that "x is y" should be treated 
> >as True when both variables are undefined? 
>
> Seems reasonable to me. 
>
> > 2) There appears to be an inconsistency in the default_if_none 
> >modifier.
>
[...] 

> >I think this is just a bug, does anyone think otherwise? 
>
> I agree. 
>

OK. I think I'll open this as a separate bug and solve that first.

Tim 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/4f1a359c-909b-48a7-b854-273b77066b19%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Carl Meyer]

Hi Tim,

On 01/04/2017 03:39 PM, Tim Martin wrote:
> I've been looking at bug #24977 (Template variables with a value of
> None are considered to be == to non-existent properties).
...
> The suggestion on the ticket is to use an instance of a special
> `Undefined` class to indicate an undefined variable, which could never
> compare equal to anything else.
> 
> This sounds sensible, but in implementing it I got to thinking about
> whether this can generalise and simplify. It seems like it might be
> possible to combine both Engine.string_if_invalid and the
> ignore_failures parameter to FilterExpression.resolve:
> 
> * The Undefined object can have a __str__() that returns the
>   string_if_invalid value, so it acts like a string for the purposes
>   of rendering.
> 
> * Returning the Undefined value is also sufficient to provide the
>   behaviour we need in the ignore_failures case: the calling code can
>   check for Undefined in the same case where it currently checks for
>   None.
> 
> Overall the code is simpler, because a bunch of code paths are
> eliminated.

Sounds great!

> However, there are 2 issues:
>
> 1) There are test cases where we have templates that should treat "x
>is y" as True where x and y are both undefined.

Really? Is this an accidental side effect of some historical change, or
really the intent of the test? That behavior seems buggy to me; more
likely to be a bug-magnet than useful.

...
>I don't see any way to satisfy both these requirements. Is it
>reasonable to relax the requirement that "x is y" should be treated
>as True when both variables are undefined?

Seems reasonable to me.

> 2) There appears to be an inconsistency in the default_if_none
>modifier. If I have a template with
> 
>x|default_if_none:y = {{x|default_if_none:y}}
>{% if x|default_if_none:y %}
>x|default_if_none:y is apparently True
>{% endif %}
> 
>with y defined to True and x undefined, then it produces:
> 
>x|default_if_none:y =
>x|default_if_none:y is apparently True
>   
>IOW it appears that the default_if_none doesn't cause y's value to
>be used in the case where the variable is being printed, even
>though it causes the expression to have y's value when evaluated in
>an if. I think this is something to do with the fact that the two
>ways of handling failures (ignore_failures and string_if_invalid)
>do different things.
> 
>I don't see a way to make backward compatible code here.
> 
>I think this is just a bug, does anyone think otherwise?

I agree.

Carl

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/14c76486-2649-c68f-a463-b5acf41b9556%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature

Template handling of undefined variables

[Tim Martin]

Hi all,

I've been looking at bug #24977 (Template variables with a value of
None are considered to be == to non-existent properties). The problem
is that in a template:

{% if user.pk == some_object.invalid_property %}
... this gets rendered when the user is logged out
{% endif %}

This is because undefined variables get silently expanded to None,
which can lead to a silent application bug, potentially a security
hole, if an object is refactored so the attribute name in the
comparison is no longer valid.

The problem is that None has a perfectly valid meaning to the
application in some circumstances (such as when it indicates a
logged-out user's PK), so its meaning gets overloaded by using it to
mean an invalid variable (which IMO should never compare equal to
anything else).

The suggestion on the ticket is to use an instance of a special
`Undefined` class to indicate an undefined variable, which could never
compare equal to anything else.

This sounds sensible, but in implementing it I got to thinking about
whether this can generalise and simplify. It seems like it might be
possible to combine both Engine.string_if_invalid and the
ignore_failures parameter to FilterExpression.resolve:

* The Undefined object can have a __str__() that returns the
  string_if_invalid value, so it acts like a string for the purposes
  of rendering.

* Returning the Undefined value is also sufficient to provide the
  behaviour we need in the ignore_failures case: the calling code can
  check for Undefined in the same case where it currently checks for
  None.

Overall the code is simpler, because a bunch of code paths are
eliminated. However, there are 2 issues:

1) There are test cases where we have templates that should treat "x
   is y" as True where x and y are both undefined. This means that
   (unless we want to change the behaviour) we have to return multiple
   copies of a single Undefined instance, rather than a fresh instance
   each time. And this doesn't work in the case where
   string_if_invalid contains a %s and the string value should have
   the variable name expanded into it.

   I don't see any way to satisfy both these requirements. Is it
   reasonable to relax the requirement that "x is y" should be treated
   as True when both variables are undefined?

2) There appears to be an inconsistency in the default_if_none
   modifier. If I have a template with

   x|default_if_none:y = {{x|default_if_none:y}}
   {% if x|default_if_none:y %}
   x|default_if_none:y is apparently True
   {% endif %}

   with y defined to True and x undefined, then it produces:

   x|default_if_none:y = 
   x|default_if_none:y is apparently True
   
   IOW it appears that the default_if_none doesn't cause y's value to
   be used in the case where the variable is being printed, even
   though it causes the expression to have y's value when evaluated in
   an if. I think this is something to do with the fact that the two
   ways of handling failures (ignore_failures and string_if_invalid)
   do different things.

   I don't see a way to make backward compatible code here.

   I think this is just a bug, does anyone think otherwise?

Sorry for the lengthy email. Thanks for any input.

Tim

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/b9d7b557-5826-4c7e-a064-5f8b59312faa%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.