Opened 5 years ago
Closed 5 years ago
#31478 closed Bug (duplicate)
Template.get_exception_info() relies on internal state which isn't correct for sub-templates.
Reported by: | Keryn Knight | Owned by: | nobody |
---|---|---|---|
Component: | Template system | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently, the method is defined as:
def get_exception_info(self, exception, token):
and internal to it's scope, makes use of the values self.source
and self.origin.name
These 2 variables are not necessarily the data on which the operations should apply, at least in some scenarios. I have a demo project while I'll attach to hopefully demonstrate this.
The problem exists on 2.2, 3.0 & master (being 3.1 currently). Whilst I've not done an exhaustive sweep of the potential problems, a solution which seems to work in my limited test is thus:
Method signature becomes
def get_exception_info(self, exception, token, origin=None, source=None): if source is None: source = self.source if origin is None: origin = self.origin
Why support Nones? Because at the very least I think django-debug-toolbar makes use of the method, and it leaves it backwards compatible.
The callers I'm aware of are django.template.base.Template.compile_nodelist
, which would become:
e.template_debug = self.get_exception_info(e, e.token, self.origin, self.source)
though I think that given that is passing in the *same* data, it could actually remain unchanged, because the internal state is the same both ways.
... and django.template.base.Node.render_annotated
:
e.template_debug = context.render_context.template.get_exception_info(e, self.token, context.template.origin, context.template.source)
This one is what I presume is the crux of the problem, because the render context's template is not the same as the child template.
Of course, changing the origin and source might have other knock on effects that I've not encountered in my tests, and I haven't tried the proposed changes against the test suite etc.
Change History (3)
comment:1 by , 5 years ago
comment:3 by , 5 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Summary: | Template.get_exception_info relies on internal state which isn't correct for sub-templates. → Template.get_exception_info() relies on internal state which isn't correct for sub-templates. |
Thanks Keryn, I think we can mark this as a duplicate of #28935 (as Tim suggested). It's a regression in e643ba8bcf0b1da517cbab689ac157ee031202a3 and your patch restores the previous behavior. Unfortunately it also causes two failures:
====================================================================== FAIL: test_compile_tag_error_27584 (template_tests.tests.TemplateTests) ---------------------------------------------------------------------- ... AssertionError: '' != '{% badtag %}' + {% badtag %} ====================================================================== FAIL: test_compile_tag_error_27956 (template_tests.tests.TemplateTests) Errors in a child of {% extends %} are displayed correctly. ---------------------------------------------------------------------- ... AssertionError: 'nt.html" %}\n' != '{% badtag %}'
Example project is here: https://github.com/kezabelle/django-ticket-31478
The 2 dependencies are Django and the app with which I encountered the problem, which is itself a terrible set of monkeypatches upon the Django Template Language. There's probably cleaner ways to demonstrate the issue, but this is the quickest one I obviously have at hand. As far as I know it's not specifically the monkeypatch at fault, but I could be wrong.
Steps to hopefully reproduce:
pip install -r requirements.txt
)/fine
with DEBUG turned on, so the technical 500 renders. You should hopefully observe that the template's HTML is shown, and the error is highlighted correctly as{{ force_error }}
/problem
to see the error being described in the ticket proper. You should hopefully observe that the HTML shown is that of thebase.html
template rather than thesubtemplate.html
which extendsbase.html
- as a result, the HTML is wrong, there is no line to highlight, and it reports the error at line 0.With the proposed possible changes, the error is highlighted correctly on
/problem
, the line is reported as line 4 correctly, and the path to the template shown is the correct subtemplate rather than the base one.