Opened 2 months ago

Last modified 2 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):  
    842842                        "not begin with underscores: '%s'" % var
    843843                    )
    844844                self.lookups = tuple(var.split(VARIABLE_ATTRIBUTE_SEPARATOR))
     845                if "" in self.lookups:
     846                    warnings.warn(...)
    845847
    846848    def resolve(self, context):
    847849        """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 Claude Paroz, 2 months ago

Triage Stage: UnreviewedAccepted

comment:2 by Sanjeev Holla S, 2 months ago

Owner: set to Sanjeev Holla S
Status: newassigned

Hello
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.

Last edited 2 months ago by Sanjeev Holla S (previous) (diff)

comment:3 by Sanjeev Holla S, 2 months ago

Cc: Sanjeev Holla S added

comment:4 by Adam Johnson, 2 months ago

in reply to:  4 comment:5 by Sanjeev Holla S, 2 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 Sanjeev Holla S, 2 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 Adam Johnson, 2 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.

See: https://docs.djangoproject.com/en/dev/internals/release-process/#internal-release-deprecation-policy

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 Sanjeev Holla S, 2 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 Sanjeev Holla S, 2 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?

comment:10 by Natalia Bidart, 2 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.

in reply to:  10 comment:11 by Sanjeev Holla S, 2 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

Note: See TracTickets for help on using tickets.
Back to Top