[Django] #27258: Context processors are not called when using RequestContext and Django templates
Django
templates
---------------------------------+--------------------
Reporter: Andi Albrecht | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+--------------------
When using RequestContext with a Django template, the context_processors
are not called to populate the context.
Here's an simple examle (see also attached test_request_context.py):
{{{
#!python
>>> from django.template.backends.django import DjangoTemplates
>>> from django.test import RequestFactory
>>> from django.template import Template, RequestContext
>>>
>>> # a simlpe context_processor:
>>> def test_processor_name(request):
... return {'name': 'Hello World'}
>>>
>>> # Create a RequestContext
>>> request = RequestFactory().get('/')
>>> context = RequestContext(request, {}, [test_processor_name])
>>>
>>> # Base template (everything's fine)
>>> template = Template('{{ name }}')
>>> template.render(context)
<<< u'Hello World'
>>>
>>> # Django template :(
>>> engine = DjangoTemplates({
... 'DIRS': [],
... 'APP_DIRS': False,
... 'NAME': 'django',
... 'OPTIONS': {
... 'context_processors': [test_processor_name],
... },
... })
>>> template = engine.from_string('{{ name }}')
>>> template.render(context)
<<< u''
}}}
The reason seems to be, that the `render()` method of a Django template
(as opposed to the base template) calls `make_context` which wraps
`RequestContext` in another `Context` instance. But when rendering the
template the `bind_template` context manager only of the outermost
`Context` instance is called, hence the context of a `RequestContext`
instance is never populated by context_processors.
--
Ticket URL: <https://code.djangoproject.com/ticket/27258>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
templates
Reporter: Andi Albrecht | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
Changes (by Andi Albrecht):
* Attachment "test_request_context.py" added.
Example functions to reproduce the bug
Django
templates
Reporter: Andi Albrecht | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
I didn't look into the details but the conversation in #25839 might be
useful.
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:1>
Django
templates
Reporter: Andi Albrecht | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Andi Albrecht):
Replying to [comment:1 Tim Graham]:
> I didn't look into the details but the conversation in #25839 might be
useful.
Both issues are related. To me #25839 describes the fact, that the
(global) `TEMPLATE_CONTEXT_PROCESSORS` couldn't be used. Here I describe a
scenario where no context_processor are called even when properly
configured or when explicitely given in the constructor of
`RequestContext`.
There's an example in the Django docs about using RequestContext:
https://docs.djangoproject.com/en/dev/ref/templates/api/#using-
requestcontext (the one with the `ip_address_processor`).
Just change `template = Template('{ title }}: {{ ip_address }}')` to
`template = loader.get_template('my_template.html')` and it doesn't work
anymore, since `django.template.backends.django.Template` behaves
different than `django.template.Template`.
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:2>
Django
templates
Reporter: Andi Albrecht | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Yes, that example was constructed knowing that template
`loader.get_template()` won't work (:ticket:25854#comment:11). I believe
it's expected behavior but I'd need to spend some time to understand and
explain why.
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:3>
Django
templates
Reporter: Andi Albrecht | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Andi Albrecht):
The more I think about it, the more I'm thinking that it's just a
documentation issue, too. `get_template` shouldn't be used together with
manually creating a `RequestContext`.
FWIW, this simple change in `django.template.context.make_context()`
solves the issue for me (and all existing tests still pass):
{{{
diff --git a/django/template/context.py b/django/template/context.py
index 1e1c391..aff6236 100644
--- a/django/template/context.py
+++ b/django/template/context.py
@@ -268,6 +268,8 @@ def make_context(context, request=None, **kwargs):
"""
Create a suitable Context from a plain dict and optionally an
HttpRequest.
"""
+ if isinstance(context, RequestContext):
+ return context
if request is None:
context = Context(context, **kwargs)
else:
}}}
IMO `make_context` has a strange behavior when called with a
`RequestContext` instance as `context`. It returns a `Context` instance
because it assumes that there's no request.
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:4>
Django
templates
Reporter: Andi Albrecht | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* cc: Aymeric Augustin (added)
Comment:
Maybe the patch you suggested would be reasonable. One possibility for
confusion would be if `template.backends.django.Template.render()`
receives both a `RequestContext` and a `request`. Perhaps an error would
need to be raised in that cause since `request` would be ignored.
Any thoughts, Aymeric?
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:5>
Django
templates
Reporter: Andi Albrecht | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin):
I tried to bury RequestContext as deep as I could and to turn it into a
mere implementation detail of the Django template engine... Unfortunately
it's been documented as a public API for a very long time and it's getting
cargo culted a lot, all the more since duck typing means it can replace at
dict transparently in some cases.
The docstring of `make_context` says `Create a suitable Context from a
plain dict`; clearly it isn't intented to receive a `RequestContext`.
Likewise `template.backends.django.Template.render` mustn't be called with
a `RequestContext` (I feel strongly about that one).
If you want to make a change similar to what Andi suggests, please convert
the `context` to a plain dict.
My preference would be to raise an exception if `RequestContext` is used
inappropriately instead of a dict. (Not sure what the best place for that
is. Perhaps `make_context`.) Or just to leave the code as it is and add
documentation.
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:6>
Django
template.backends.django.Template.render()
--------------------------------------+------------------------------------
Reporter: Andi Albrecht | Owner: nobody
Component: Template system | Version: 1.10
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
Raising an exception in `make_context()` seems like it could work since
`template.backends.django.Template.render()` is calling that method and
that's the only place that calls `make_context()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:7>
Django
template.backends.django.Template.render()
Reporter: Andi Albrecht | Owner: reficul31
Type: | Status: assigned
Cleanup/optimization |
Component: Template system | Version: 1.10
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by reficul31):
* status: new => assigned
* owner: nobody => reficul31
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:8>
Django
template.backends.django.Template.render()
-------------------------------------+-------------------------------------
Reporter: Andi Albrecht | Owner: reficul31
Type: | Status: assigned
Cleanup/optimization |
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/7675 PR] with some comments for
improvement.
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:9>
Django
template.backends.django.Template.render()
-------------------------------------+-------------------------------------
Reporter: Andi Albrecht | Owner: reficul31
Type: | Status: assigned
Cleanup/optimization |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
Comment:
I fixed up the [https://github.com/django/django/pull/7675 PR] according
to what I had in mind. The idea is that Jinja's `render()` doesn't work
with a non-dict, so to encourage code that works with multiple template
engines, Django shouldn't accept a non-dict there either.
I think making this change as a bug fix rather than having a deprecation
path is advantageous to avoid developers writing incorrect code for
another 2 releases of Django. Also, it's possible to fix existing code
(change `Context` to `dict` as done in the csrf view) without breaking
compatibility with older versions of Django so a deprecation doesn't
provide any benefit in that respect.
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:10>
Django
template.backends.django.Template.render()
-------------------------------------+-------------------------------------
Reporter: Andi Albrecht | Owner: reficul31
Cleanup/optimization |
Component: Template system | Version: 1.10
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"6a7495051304d75865add6ff96422018984e1663" 6a74950]:
{{{
#!CommitTicketReference repository=""
revision="6a7495051304d75865add6ff96422018984e1663"
Fixed #27258 -- Prohibited django.Template.render() with non-dict context.
Thanks Shivang Bharadwaj for the initial patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:11>
Django
template.backends.django.Template.render()
-------------------------------------+-------------------------------------
Reporter: Andi Albrecht | Owner: reficul31
Type: | Status: closed
Cleanup/optimization |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mark Jones):
This was never about a RequestContext (although I can see that being a
problem). It had to do with a Context being passed to make_context and
that causing a type error. The whole point of me passing a Context was to
specify turn off auto_escape
{{{
context = Context({"settings": settings,
"sys": sys, "os": os,
"options": options,
"username": getpass.getuser(),
"wsgi_path": wsgi_path,
"ssl": using_ssl,
},
autoescape=False)
}}}
I'm rendering a text file on the server in a management command where I'm
selecting the template based on:
{{{
servertemplate = loader.select_template(["deployment/%s" %
options['webserver'],
"deployment/default_%s" % options['webserver']])
}}}
Because Jinja is incapable of using a context, we've broken the ability to
pass a context. The fix I proposed was to let that context on thru,
instead the context is banned and the only way to get autoescape in now is
to build out a whole template Engine (where the autoescape flag can be
passed).
I've never understood people's fascination with jinja templates but
nothing made me use them. But now they are actively thwarting me, I begin
to dislike them.
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:12>
Django
template.backends.django.Template.render()
-------------------------------------+-------------------------------------
Reporter: Andi Albrecht | Owner: reficul31
Type: | Status: closed
Cleanup/optimization |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mark Jones):
And just to add insult to injury I decided I would quit griping and update
the code to build out it's own Engine instance like so:
{{{
from django.template.backends.django import Engine
from django.template import Context
engine = Engine(dirs=settings.TEMPLATES[0]['DIRS'],
app_dirs=True, debug=settings.DEBUG,
autoescape=False,
libraries={'deployment_tags':
'deployment.templatetags.deployment_tags'})
}}}
Then I still had to go back and use the Context object anyway
{{{
context = Context({"settings": settings,
"sys": sys, "os": os,
"options": options,
"username": getpass.getuser(),
"wsgi_path": wsgi_path,
"ssl": using_ssl,
}, autoescape=False)
}}}
Instead of just passing a dict.
There is a serious impedance mismatch in the interface around templates
now.
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:13>
Django
template.backends.django.Template.render()
-------------------------------------+-------------------------------------
Reporter: Andi Albrecht | Owner: reficul31
Type: | Status: closed
Cleanup/optimization |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Comments 12 and 13 stem from #28491. I replied with a solution there.
--
Ticket URL: <https://code.djangoproject.com/ticket/27258#comment:14>