[Django] #35493: Allow `./` and `../` in paths when recursively including templates
32 views
Skip to first unread message
Django
Jun 2, 2024, 11:17:26 AMJun 2
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------------+--------------------------
Reporter: gnpivo | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 5.0
Severity: Normal | Keywords: template
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------------+--------------------------
Hi. Currently, when trying to recursively include a Django template within
itself using the `include` tag with a path that contains `./` or `../`,
Django raises a `TemplateSyntaxError`. However, using a path that does not
contain `./` or `../` does not raise the error. When the error is raised,
the debug toolbar describes it like this:
> = TemplateSyntaxError at /
>
> The relative path ‘“./ul.html”’ was translated to template name
‘app/ul.html’, the same template in which the tag appears.
Here is an example of a template in a Django app called `app` with the
path `app/templates/app/ul.html` that would produce the error given above:
{{{
<ul>
{% for section in sections %}
<li>
<p>{{ section.name }}</p>
{% if section.sections|length != 0 %}
{% include "./ul.html" with sections=section.sections %}
{% endif %}
</li>
{% endfor %}
</ul>
}}}
However, replacing the directory `./ul.html` with the equivalent
`app/ul.html` makes the error go away (assuming that the project's
`settings.py` specifies `APP_DIRS = True` and the views and URLs are
configured correctly). The actual paths are translated identically in both
cases, and the behavior of the `include` tag should not depend simply on
whether or not the path string uses `./` or `../` (or if it should, this
is not documented in the Django documentation). Therefore, it seems that
this is a bug. The expected behavior is that an error is only raised when
recursively using the `extends` template, not when recursively using the
`include` template.
Contrapositively, it appears that recursively extending a template using
the `extends` tag with a path that does ''not'' contain `./` or `../`
raises a `TemplateDoesNotExist` exception, which is semantically
inaccurate since the template is referencing itself and therefore
necessarily exists.
One possible fix is to modify the `django/template/loader_tags.py` file
(https://github.com/django/django/blob/main/django/template/loader_tags.py)
such that the error is raised when a template attempts to extend itself
(not when a template attempts to include itself, which would otherwise be
valid). The error handling logic in question starts on line 267 of that
file within the `construct_relative_path` function; perhaps it should live
somewhere in the `do_extends` function instead.
Here is a relevant discussion in the Django forums:
https://forum.djangoproject.com/t/template-recursion-why-does-django-not-
allow-and/31689
--
Ticket URL: <https://code.djangoproject.com/ticket/35493>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
-------------------------------------------+--------------------------
Reporter: gnpivo | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 5.0
Severity: Normal | Keywords: template
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------------+--------------------------
Hi. Currently, when trying to recursively include a Django template within
itself using the `include` tag with a path that contains `./` or `../`,
Django raises a `TemplateSyntaxError`. However, using a path that does not
contain `./` or `../` does not raise the error. When the error is raised,
the debug toolbar describes it like this:
> = TemplateSyntaxError at /
>
> The relative path ‘“./ul.html”’ was translated to template name
‘app/ul.html’, the same template in which the tag appears.
Here is an example of a template in a Django app called `app` with the
path `app/templates/app/ul.html` that would produce the error given above:
{{{
<ul>
{% for section in sections %}
<li>
<p>{{ section.name }}</p>
{% if section.sections|length != 0 %}
{% include "./ul.html" with sections=section.sections %}
{% endif %}
</li>
{% endfor %}
</ul>
}}}
However, replacing the directory `./ul.html` with the equivalent
`app/ul.html` makes the error go away (assuming that the project's
`settings.py` specifies `APP_DIRS = True` and the views and URLs are
configured correctly). The actual paths are translated identically in both
cases, and the behavior of the `include` tag should not depend simply on
whether or not the path string uses `./` or `../` (or if it should, this
is not documented in the Django documentation). Therefore, it seems that
this is a bug. The expected behavior is that an error is only raised when
recursively using the `extends` template, not when recursively using the
`include` template.
Contrapositively, it appears that recursively extending a template using
the `extends` tag with a path that does ''not'' contain `./` or `../`
raises a `TemplateDoesNotExist` exception, which is semantically
inaccurate since the template is referencing itself and therefore
necessarily exists.
One possible fix is to modify the `django/template/loader_tags.py` file
(https://github.com/django/django/blob/main/django/template/loader_tags.py)
such that the error is raised when a template attempts to extend itself
(not when a template attempts to include itself, which would otherwise be
valid). The error handling logic in question starts on line 267 of that
file within the `construct_relative_path` function; perhaps it should live
somewhere in the `do_extends` function instead.
Here is a relevant discussion in the Django forums:
https://forum.djangoproject.com/t/template-recursion-why-does-django-not-
allow-and/31689
--
Ticket URL: <https://code.djangoproject.com/ticket/35493>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Jun 4, 2024, 2:45:57 PMJun 4
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: nobody
Pivovarov |
Type: Bug | Status: new
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Component: Template system | Version: 5.0
Keywords: template | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Description changed by Gabriel Nick Pivovarov:
Old description:
One possible fix is to modify the `django/template/loader_tags.py` file
(https://github.com/django/django/blob/main/django/template/loader_tags.py)
such that the error is raised when a template attempts to extend itself
(not when a template attempts to include itself, which would otherwise be
valid). The error handling logic in question starts on line 267 of that
file within the `construct_relative_path` function; perhaps it should live
somewhere in the `do_extends` function instead.
Here is a relevant discussion in the Django forums:
https://forum.djangoproject.com/t/template-recursion-why-does-django-not-
allow-and/31689
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:1>
Django
Jun 4, 2024, 9:53:05 PMJun 4
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Gabriel
-------------------------------------+-------------------------------------
Pivovarov | Nick Pivovarov
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Gabriel Nick Pivovarov):
Severity: Normal | Resolution:
Keywords: template | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Gabriel Nick Pivovarov
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:2>
Django
Jun 4, 2024, 10:42:55 PMJun 4
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Gabriel
Pivovarov | Nick Pivovarov
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Gabriel
Pivovarov | Nick Pivovarov
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Gabriel Nick Pivovarov):
* has_patch: 0 => 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Gabriel Nick Pivovarov):
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:3>
Django
Jun 4, 2024, 10:44:19 PMJun 4
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Gabriel
Pivovarov | Nick Pivovarov
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Gabriel
Pivovarov | Nick Pivovarov
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Gabriel Nick Pivovarov):
* needs_tests: 0 => 1
-------------------------------------+-------------------------------------
Changes (by Gabriel Nick Pivovarov):
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:4>
Django
Jun 4, 2024, 11:39:28 PMJun 4
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Gabriel
Pivovarov | Nick Pivovarov
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Gabriel
Pivovarov | Nick Pivovarov
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
be used when called from the `do_extends` function.
Here is a relevant discussion in the Django forums:
https://forum.djangoproject.com/t/template-recursion-why-does-django-not-
allow-and/31689
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:5>
Django
Jun 12, 2024, 10:45:41 AMJun 12
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Gabriel
Pivovarov | Nick Pivovarov
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Gabriel
Pivovarov | Nick Pivovarov
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
Relative path support was added in #26402 and recursive include support
was added in #3544.
I think this was missed when working on #26402, replicated. Here is my
test:
{{{#!diff
diff --git a/tests/template_tests/syntax_tests/test_include.py
b/tests/template_tests/syntax_tests/test_include.py
index 3ee99b3798..6dafba5040 100644
--- a/tests/template_tests/syntax_tests/test_include.py
+++ b/tests/template_tests/syntax_tests/test_include.py
@@ -339,6 +339,14 @@ class IncludeTests(SimpleTestCase):
.replace("\n", " ")
.strip(),
)
+ t = engine.get_template("recursive_relative_include.html")
+ self.assertEqual(
+ "Recursion! A1 Recursion! B1 B2 B3 Recursion! C1",
+ t.render(Context({"comments": comments}))
+ .replace(" ", "")
+ .replace("\n", " ")
+ .strip(),
+ )
def test_include_cache(self):
"""
diff --git
a/tests/template_tests/templates/recursive_relative_include.html
b/tests/template_tests/templates/recursive_relative_include.html
index e69de29bb2..ae49cc0a43 100644
--- a/tests/template_tests/templates/recursive_relative_include.html
+++ b/tests/template_tests/templates/recursive_relative_include.html
@@ -0,0 +1,7 @@
+Recursion!
+{% for comment in comments %}
+ {{ comment.comment }}
+ {% if comment.children %}
+ {% include "./recursive_relative_include.html" with
comments=comment.children %}
+ {% endif %}
+{% endfor %}
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:6>
Django
Aug 10, 2024, 9:16:51 AMAug 10
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner:
Pivovarov | YashRaj1506
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner:
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by YashRaj1506):
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Gabriel Nick Pivovarov => YashRaj1506
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:7>
Django
Nov 25, 2024, 1:35:10 AMNov 25
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: (none)
-------------------------------------+-------------------------------------
Pivovarov |
Type: Bug | Status: new
Type: Bug | Status: new
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by YashRaj1506):
* owner: YashRaj1506 => (none)
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by YashRaj1506):
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:8>
Django
Nov 25, 2024, 9:52:08 AMNov 25
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Brock
-------------------------------------+-------------------------------------
Pivovarov | Smickley
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by BSmick6):
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: (none) => Brock Smickley
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:9>
Django
Nov 26, 2024, 9:30:20 AMNov 26
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Brock
Pivovarov | Smickley
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Brock
Pivovarov | Smickley
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Brock Smickley):
-------------------------------------+-------------------------------------
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:10>
Django
Nov 26, 2024, 9:49:26 AMNov 26
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Brock
Pivovarov | Smickley
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Brock Smickley):
* owner: Brock Smickley => Brock Smickley
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Brock
Pivovarov | Smickley
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Brock Smickley):
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:11>
Django
Nov 26, 2024, 3:01:13 PMNov 26
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Brock
Pivovarov | Smickley
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Brock Smickley):
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Brock
Pivovarov | Smickley
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
https://github.com/django/django/pull/18850
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:12>
Django
Dec 5, 2024, 10:36:05 AMDec 5
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Brock
Pivovarov | Smickley
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
Keywords: template | Triage Stage: Ready for
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Brock
Pivovarov | Smickley
Type: Bug | Status: assigned
Component: Template system | Version: 5.0
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:13>
Django
Dec 6, 2024, 4:44:20 AMDec 6
to django-...@googlegroups.com
#35493: Allow `./` and `../` in paths when recursively including templates
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Brock
Pivovarov | Smickley
Type: Bug | Status: closed
-------------------------------------+-------------------------------------
Reporter: Gabriel Nick | Owner: Brock
Pivovarov | Smickley
Component: Template system | Version: 5.0
Severity: Normal | Resolution: fixed
Keywords: template | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce <42296566+sarahboyce@…>):
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* resolution: => fixed
* status: assigned => closed
Comment:
In [changeset:"55855bc6d0d2a270eb34952c578ee352389367ad" 55855bc]:
{{{#!CommitTicketReference repository=""
revision="55855bc6d0d2a270eb34952c578ee352389367ad"
Fixed #35493 -- Allowed template self-inclusion with relative paths.
Co-authored-by: Brock <bsmi...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35493#comment:14>
Reply all
Reply to author
Forward
0 new messages