#27722 closed Bug (fixed)
if a template context is an instance of get_template(), it will raise "TypeError: context must be a dict rather than RequestContext"
Reported by: | Sayid Munawar | Owned by: | nobody |
---|---|---|---|
Component: | Template system | Version: | 1.11 |
Severity: | Release blocker | Keywords: | |
Cc: | FunkyBob, Aymeric Augustin | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
This simple view:
from django.views.generic import TemplateView from django.template.loader import get_template class Ngopi(TemplateView): template_name = 'base.html' def get_context_data(self, **kwargs): context = super(Ngopi, self).get_context_data(**kwargs) context.update({ 'menu': get_template('menu.html') }) return context
with base.html
:
<html> <body> <h1>Ngeteh</h1> {% include menu %} </body> </html>
and menu.html
:
<ul> <li>Ngeteh</li> <li>Ngopi</li> </ul>
This situation will raise this exception:
Traceback (most recent call last): File "/Users/ayik/Repo/Django/django/django/core/handlers/exception.py", line 38, in inner response = get_response(request) File "/Users/ayik/Repo/Django/django/django/core/handlers/base.py", line 217, in _get_response response = self.process_exception_by_middleware(e, request) File "/Users/ayik/Repo/Django/django/django/core/handlers/base.py", line 215, in _get_response response = response.render() File "/Users/ayik/Repo/Django/django/django/template/response.py", line 107, in render self.content = self.rendered_content File "/Users/ayik/Repo/Django/django/django/template/response.py", line 84, in rendered_content content = template.render(context, self._request) File "/Users/ayik/Repo/Django/django/django/template/backends/django.py", line 66, in render return self.template.render(context) File "/Users/ayik/Repo/Django/django/django/template/base.py", line 207, in render return self._render(context) File "/Users/ayik/Repo/Django/django/django/template/base.py", line 199, in _render return self.nodelist.render(context) File "/Users/ayik/Repo/Django/django/django/template/base.py", line 990, in render bit = node.render_annotated(context) File "/Users/ayik/Repo/Django/django/django/template/base.py", line 957, in render_annotated return self.render(context) File "/Users/ayik/Repo/Django/django/django/template/loader_tags.py", line 212, in render return template.render(context) File "/Users/ayik/Repo/Django/django/django/template/backends/django.py", line 64, in render context = make_context(context, request, autoescape=self.backend.engine.autoescape) File "/Users/ayik/Repo/Django/django/django/template/context.py", line 285, in make_context raise TypeError('context must be a dict rather than %s.' % context.__class__.__name__) TypeError: context must be a dict rather than RequestContext.
This is fine with django 1.10, but not master (mine is currently 1.11.dev20170109230310)
a quick workaround is to edit django/template/backends/django.py Line 63 to be like this:
def render(self, context=None, request=None): if isinstance(context, dict): # <-- my temporary workaround context = make_context(context, request, autoescape=self.backend.engine.autoescape) try: return self.template.render(context) except TemplateDoesNotExist as exc: reraise(exc, self.backend)
Change History (18)
comment:2 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
sorry Tim, my proposed change didn't mean to really fix the problem, just a workaround so that i can continue to code.
using context.update({'menu': 'menu.html'})
solved my problem. it just weird since it was working fine until 1.10
comment:3 by , 8 years ago
Resolution: | fixed → wontfix |
---|
It's allowed to call {% include %}
with a django.template.base.Template
but not django.template.backends.django.Template
(added in 5cdacbda034af928f5033c9afc7b50ee0b13f75c).
comment:4 by , 8 years ago
Cc: | added |
---|
Curtis, as the author of 5cdacbda034af928f5033c9afc7b50ee0b13f75c, I wonder if you have any input here? I'm not happy that I broke backwards compatibility (due to #27258) with such a cryptic error message here. At least the {% include %}
docs that say, "The variable may also be any object with a render()
method that accepts a context." might need some clarification.
The use case is described in the django-material issue.
comment:5 by , 8 years ago
Description: | modified (diff) |
---|
comment:6 by , 8 years ago
Hi Tim, sorry to continue to discuss about this, from your last comment:
It's allowed to call {% include %} with a
django.template.base.Template
but notdjango.template.backends.django.Template
but get_template()
will return django.template.backends.django.Template
.
further investigation using ipdb, i found that make_context
will raise type_error since the context
is already an instance of RequestContext
in the 2nd iteration (see below):
> /Users/ayik/Repo/Django/django/django/template/backends/django.py(65)render() 63 def render(self, context=None, request=None): 64 import ipdb; ipdb.set_trace() ---> 65 context = make_context(context, request, autoescape=self.backend.engine.autoescape) 66 try: 67 return self.template.render(context) ipdb> type(context) <class 'dict'> ipdb> c > /Users/ayik/Repo/Django/django/django/template/backends/django.py(65)render() 63 def render(self, context=None, request=None): 64 import ipdb; ipdb.set_trace() ---> 65 context = make_context(context, request, autoescape=self.backend.engine.autoescape) 66 try: 67 return self.template.render(context) ipdb> type(context) <class 'django.template.context.RequestContext'> ipdb>
comment:7 by , 8 years ago
The question in my mind is whether or not to keep the fix for #27258 (maybe it's too restrictive -- in your use case, I'm not sure if django.Template.render()
with RequestContext
will give any surprising behaviors) or maybe some change could be made to {% include %}
to either raise a more descriptive error message or to allow the use case to keep working.
comment:8 by , 8 years ago
Resolution: | wontfix |
---|---|
Severity: | Normal → Release blocker |
Status: | closed → new |
Triage Stage: | Unreviewed → Accepted |
Version: | master → 1.11 |
At least documentation must be clarified.
comment:9 by , 8 years ago
Quoting Mikhail Podgurskiy from the duplicated ticket:
As stated in the django docs, the
{% include %}
tag could accept a template instance to render - https://docs.djangoproject.com/en/1.10/ref/templates/builtins/#include
include
Loads a template and renders it with the current context. This is a way of “including” other templates within a template.
...
The variable may also be any object with a
render()
method that accepts a context. This allows you to reference a compiled Template in your context.
This leads to RequestContext from the main template been passed to the included template.render method and this became prohibited within https://code.djangoproject.com/ticket/27258 changeset
comment:10 by , 8 years ago
Cc: | added |
---|
Aymeric, do you have any advice on this one? It seems the technique described in this ticket doesn't have the issue of no context processors running (#27258) because the RequestContext
that the django.Template()
is called with already has the context processors run for the parent template. If backwards-compatibility must be kept for this case, I'm inclined to revert 6a7495051304d75865add6ff96422018984e1663 rather than try to devise a more complicated solution.
comment:11 by , 8 years ago
I didn't know this was possible. I'm not seeing obvious use cases -- why would you pass a template instance rather than a template identifier and let the template loader do its job? But indeed it's documented.
Anyway, {% include compiled_template %}
path wraps a Context in another Context, while {% include 'template_to_compile.html' %}
doesn't. I don't see why these two code paths handle context differently when compiled_template = get_template('template_to_compile.html')
. This is likely a bug or a potential source of bugs, and the root cause of the problem described here.
Of course, if no one wants to look into that, reverting 6a7495051304d75865add6ff96422018984e1663 is a reasonable way to avoid a backwards incompatibility. It would still be worth creating another ticket for resolving the inconsistency in context handling.
follow-up: 13 comment:12 by , 8 years ago
A use case for calling {% include %}
with a compiled template is described in #17356. The use case was for {% include %}
with a django.template.base.Template
, however, it had the side effect of also allowing django.template.backends.django.Template
since the check is if the template
argument has a render() method.
I don't think the issue here has to do with how contexts are handled but rather that {% include "string" %}
uses engine.get_template() to load the template which returns base.Template
. The use case of this ticket instead uses loader.get_template()
which returns a django.Template
.
In ticket:27258#comment:6 you said, "template.backends.django.Template.render
mustn't be called with a RequestContext
(I feel strongly about that one)." but that's what happening in the latter case (apparently without any ill effects?).
comment:13 by , 8 years ago
Replying to Tim Graham:
A use case for calling
{% include %}
with a compiled template is described in #17356. The use case was for{% include %}
with adjango.template.base.Template
, however, it had the side effect of also allowingdjango.template.backends.django.Template
since the check is if thetemplate
argument has a render() method.
Perhaps {% include %}
could unwrap the django.template.base.Template
from the django.template.backends.django.Template
when this happens, after checking that both templates were loaded with the same engine? If you load them with different engines, you may get hilarious results, especially if you find XSS vulnerabilities funny and one engine has autoescape turned off.
Alternatively it could reject django.template.base.Template
instances, but that would only result in another ticket asking to support it.
In ticket:27258#comment:6 you said, "
template.backends.django.Template.render
mustn't be called with aRequestContext
(I feel strongly about that one)." but that's what happening in the latter case (apparently without any ill effects?).
You could say that mostly me being obsessed with technical correctness and never having really come to terms with duck-typing and you wouldn't be totally wrong but not totally true either.
The underlying motivation was to discourage confusion between:
template.backends.django.Template
which is the backend-agnostic, compatible API that expects adict
and optionally a requestdjango.template.base.Template
which is the Django-templates-only API that expects aContext
orRequestContext
The root cause of the bug we're discussing here is one type being used where the other is expected. So that motivation isn't without merit.
comment:14 by , 8 years ago
comment:15 by , 8 years ago
Has patch: | set |
---|
I attached a start of #24577 to that ticket, but I don't think that needs to be completed now or backported to 1.11 as the fix for this ticket does. Do you see the situation differently, Aymeric?
comment:16 by , 8 years ago
I think you can go ahead with this and leave engine restrictions to #24577.
Does
get_template()
create some behavior difference as opposed tocontext.update({'menu': 'menu.html'})
? See #27258 for the reason this error was added. Your proposed change effectively removes that fix.