Opened 3 months ago
Last modified 3 months ago
#35738 assigned Cleanup/optimization
Deprecate double-dot variable lookups
Reported by: | Adam Johnson | Owned by: | Sanjeev Holla S |
---|---|---|---|
Component: | Template system | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Sanjeev Holla S | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently, a double-dot variable lookup, like {{ book..title }}
, is allowed in templates. It maps to a lookup of the empty string before the next lookup of the named attribute.
Supporting double-dot lookups means that when written accidentally, the user gets a silent failure, as for a missing variable, rather than a syntax error. The empty string lookup is unlikely to be used intentionally.
This behaviour is also inconsistent with Python and Jinja, which both raise a syntax error.
Let’s deprecated ..
in Django templates, eventually turning it into a syntax error. We should be able to start by adding the warning in Variable.__init__
:
-
django/template/base.py
def __init__(self, var): 842 842 "not begin with underscores: '%s'" % var 843 843 ) 844 844 self.lookups = tuple(var.split(VARIABLE_ATTRIBUTE_SEPARATOR)) 845 if "" in self.lookups: 846 warnings.warn(...) 845 847 846 848 def resolve(self, context): 847 849 """Resolve this variable against a given context."""
The warning message should include the template file and line.
This ticket follows this forum discussion, where Carlton +1'd me on the concept.
Change History (11)
comment:1 by , 3 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 3 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:3 by , 3 months ago
Cc: | added |
---|
follow-up: 5 comment:4 by , 3 months ago
Sanjeev, can you open a pull request? See the guide at: https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/working-with-git/
comment:5 by , 3 months ago
Replying to Adam Johnson:
Sanjeev, can you open a pull request? See the guide at: https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/working-with-git/
Yes I'll create the pull request, just a small doubt
When a double-dot variable lookup is used, a warning with RemovedInDjango60Warning
should be raised?
comment:6 by , 3 months ago
Also I noticed one more thing, that is, the warning message can be seen in the console only once because Django caches templates after they are compiled, so warnings are triggered only during the initial compilation. So is there any work around for this or is this fine? (Anyways, In the settings.py file, the caching can be disabled which make sures that the warning messages are shown whenever the page with double-dot variable lookup is used)
comment:7 by , 3 months ago
When a double-dot variable lookup is used, a warning with RemovedInDjango60Warning should be raised?
No, target 6.1 for removal, using RemovedInDjango61Warning
.
Also I noticed one more thing, that is, the warning message can be seen in the console only once because Django caches templates after they are compiled, so warnings are triggered only during the initial compilation. So is there any work around for this or is this fine?
This is fine, and I think desirable. We don’t want to spam production logs where a dot-dot has been deployed.
comment:8 by , 3 months ago
I have created a pull request.
PR - https://github.com/django/django/pull/18551
This would require many more improvements and optimizations in the code but I would want to know whether the approach that I have used to get the template name and the line number in the warning is fine or not?
So provide your feedback
comment:9 by , 3 months ago
Btw, is there any standards on how we display the warning messages that include the file name and the specific line that triggered the warning?
follow-up: 11 comment:10 by , 3 months ago
Hello Sanjeev!
The process of how to deprecate a feature are described in this link: https://docs.djangoproject.com/en/5.1/internals/contributing/writing-code/submitting-patches/#deprecating-a-feature
In there you'll find details on how the process should be done, what code change should be there and what tests we require.
comment:11 by , 3 months ago
Replying to Natalia Bidart:
Hello Sanjeev!
The process of how to deprecate a feature are described in this link: https://docs.djangoproject.com/en/5.1/internals/contributing/writing-code/submitting-patches/#deprecating-a-feature
In there you'll find details on how the process should be done, what code change should be there and what tests we require.
Hey!
Sure I'll look into it. Actually I have added the warning similar to how its mentioned in the doc but since it was suggested to show the template
name and line number which triggered the warning in the warning message, I have added that functionality as well, but not sure if that would be fine to go ahead with. So please check my PR once and confirm. Meanwhile I'll add the tests for these in the PR
I understood the issue and made the changes as suggested. Also I am able to show the template file and the line in the warning. I will create a PR for this.