#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: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.Templatebut 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.Templatesince the check is if thetemplateargument 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.rendermustn'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 right either.
The underlying motivation was to discourage confusion between:
template.backends.django.Templatewhich is the backend-agnostic, compatible API that expects adictand optionally a requestdjango.template.base.Templatewhich is the Django-templates-only API that expects aContextorRequestContext
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.