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.
...
> 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.
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 apologise if adding the tests has made it harder to improve the behaviour of the tag.
--
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+unsubscribe@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.
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.
--
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+unsubscribe@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.
+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 %}
--
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+unsubscribe@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.
+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 <L.Pla...@cantab.net> 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.
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 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-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/8b260b00-7921-4977-9521-f61f073e717b%40googlegroups.com.
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.
On Sat, Feb 25, 2017 at 2:10 PM, Tim Graham <timog...@gmail.com> 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."
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-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/b9d7b557-5826-4c7e-a064-5f8b59312faa%40googlegroups.com.
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 %}
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.
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
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.
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.