Quantcast

Template handling of undefined variables

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
28 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Carl Meyer-4
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 [hidden email].
To post to this group, send email to [hidden email].
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 (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Adam Johnson-2
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 <[hidden email]> 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

--
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 [hidden email].
To post to this group, send email to [hidden email].
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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Tim Martin
In reply to this post by Alasdair Nicol
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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Tim Graham-2
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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Adam Johnson-2
+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 %}

On 17 February 2017 at 23:34, Tim Graham <[hidden email]> wrote:
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 [hidden email].
To post to this group, send email to [hidden email].
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.



--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAMyDDM1a0MJP8DTRbY4BEQmDMT31-rpZT9kOSc_7%2BTyaYvpc%2Bw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Luke Plant-2
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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 <[hidden email]> 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 [hidden email].
To post to this group, send email to [hidden email].
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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="PJwETnd9CAAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">L.Pla...@...> 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 - <a href="https://docs.djangoproject.com/en/1.10/ref/settings/#std:setting-TEMPLATES-OPTIONS" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fdocs.djangoproject.com%2Fen%2F1.10%2Fref%2Fsettings%2F%23std%3Asetting-TEMPLATES-OPTIONS\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEGntbTIFcWpOCm472hDVFaN1k2Rw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fdocs.djangoproject.com%2Fen%2F1.10%2Fref%2Fsettings%2F%23std%3Asetting-TEMPLATES-OPTIONS\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEGntbTIFcWpOCm472hDVFaN1k2Rw&#39;;return true;">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 <a href="javascript:" target="_blank" gdf-obfuscated-mailto="PJwETnd9CAAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">django-develop...@googlegroups.com.
To post to this group, send email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="PJwETnd9CAAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">django-d...@googlegroups.com.
Visit this group at <a href="https://groups.google.com/group/django-developers" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/group/django-developers&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/django-developers&#39;;return true;">https://groups.google.com/group/django-developers.
To view this discussion on the web visit <a href="https://groups.google.com/d/msgid/django-developers/496fa4a8-b902-657a-92c2-9766bfff8a26%40cantab.net?utm_medium=email&amp;utm_source=footer" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/msgid/django-developers/496fa4a8-b902-657a-92c2-9766bfff8a26%40cantab.net?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/msgid/django-developers/496fa4a8-b902-657a-92c2-9766bfff8a26%40cantab.net?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;">https://groups.google.com/d/msgid/django-developers/496fa4a8-b902-657a-92c2-9766bfff8a26%40cantab.net.

For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Tim Graham-2
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 <[hidden email]> 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 - <a href="https://docs.djangoproject.com/en/1.10/ref/settings/#std:setting-TEMPLATES-OPTIONS" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fdocs.djangoproject.com%2Fen%2F1.10%2Fref%2Fsettings%2F%23std%3Asetting-TEMPLATES-OPTIONS\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEGntbTIFcWpOCm472hDVFaN1k2Rw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fdocs.djangoproject.com%2Fen%2F1.10%2Fref%2Fsettings%2F%23std%3Asetting-TEMPLATES-OPTIONS\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEGntbTIFcWpOCm472hDVFaN1k2Rw&#39;;return true;">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 [hidden email].
Visit this group at <a href="https://groups.google.com/group/django-developers" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/group/django-developers&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/django-developers&#39;;return true;">https://groups.google.com/group/django-developers.
To view this discussion on the web visit <a href="https://groups.google.com/d/msgid/django-developers/496fa4a8-b902-657a-92c2-9766bfff8a26%40cantab.net?utm_medium=email&amp;utm_source=footer" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/msgid/django-developers/496fa4a8-b902-657a-92c2-9766bfff8a26%40cantab.net?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/msgid/django-developers/496fa4a8-b902-657a-92c2-9766bfff8a26%40cantab.net?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;">https://groups.google.com/d/msgid/django-developers/496fa4a8-b902-657a-92c2-9766bfff8a26%40cantab.net.

For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Karen Tracey-2
On Sat, Feb 25, 2017 at 2:10 PM, Tim Graham <[hidden email]> 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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Tim Graham-2
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 <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="UgO2CEVKBwAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">timog...@...> 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?

<a href="https://docs.djangoproject.com/en/1.7/topics/templates/#variables" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fdocs.djangoproject.com%2Fen%2F1.7%2Ftopics%2Ftemplates%2F%23variables\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGyhe_cT6nrXWBw80fLqhE2W5VQqw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fdocs.djangoproject.com%2Fen%2F1.7%2Ftopics%2Ftemplates%2F%23variables\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGyhe_cT6nrXWBw80fLqhE2W5VQqw&#39;;return true;">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."

<a href="https://docs.djangoproject.com/en/dev/ref/templates/api/#invalid-template-variables" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fdocs.djangoproject.com%2Fen%2Fdev%2Fref%2Ftemplates%2Fapi%2F%23invalid-template-variables\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFoVt95kKCIqC9McP_FtL6YhPxafw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fdocs.djangoproject.com%2Fen%2Fdev%2Fref%2Ftemplates%2Fapi%2F%23invalid-template-variables\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFoVt95kKCIqC9McP_FtL6YhPxafw&#39;;return true;">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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Fred Stluka
In reply to this post by Tim Graham-2
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 -- [hidden email] -- 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 <<a moz-do-not-send="true" href="javascript:" target="_blank" gdf-obfuscated-mailto="UgO2CEVKBwAJ" rel="nofollow" onmousedown="this.href='javascript:';return true;" onclick="this.href='javascript:';return true;">timog...@...> 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?

<a moz-do-not-send="true" href="https://docs.djangoproject.com/en/1.7/topics/templates/#variables" target="_blank" rel="nofollow" onmousedown="this.href='https://www.google.com/url?q\x3dhttps%3A%2F%2Fdocs.djangoproject.com%2Fen%2F1.7%2Ftopics%2Ftemplates%2F%23variables\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGyhe_cT6nrXWBw80fLqhE2W5VQqw';return true;" onclick="this.href='https://www.google.com/url?q\x3dhttps%3A%2F%2Fdocs.djangoproject.com%2Fen%2F1.7%2Ftopics%2Ftemplates%2F%23variables\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGyhe_cT6nrXWBw80fLqhE2W5VQqw';return true;">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."

<a moz-do-not-send="true" href="https://docs.djangoproject.com/en/dev/ref/templates/api/#invalid-template-variables" target="_blank" rel="nofollow" onmousedown="this.href='https://www.google.com/url?q\x3dhttps%3A%2F%2Fdocs.djangoproject.com%2Fen%2Fdev%2Fref%2Ftemplates%2Fapi%2F%23invalid-template-variables\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFoVt95kKCIqC9McP_FtL6YhPxafw';return true;" onclick="this.href='https://www.google.com/url?q\x3dhttps%3A%2F%2Fdocs.djangoproject.com%2Fen%2Fdev%2Fref%2Ftemplates%2Fapi%2F%23invalid-template-variables\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFoVt95kKCIqC9McP_FtL6YhPxafw';return true;">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 [hidden email].
To post to this group, send email to [hidden email].
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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Karen Tracey-2
On Sat, Feb 25, 2017 at 8:21 PM, Fred Stluka <[hidden email]> 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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="UEEI9z9aBwAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">fr...@...> 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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Luke Plant-2
In reply to this post by Karen Tracey-2

On 26/02/17 00:44, Karen Tracey wrote:
On Sat, Feb 25, 2017 at 2:10 PM, Tim Graham <[hidden email]> 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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Template handling of undefined variables

Luke Plant-2
In reply to this post by Tim Martin



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 [hidden email].
To post to this group, send email to [hidden email].
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 [hidden email].
To post to this group, send email to [hidden email].
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.
12
Loading...