Raise an exception if RequestContext is used with template.backends.django.Template.render()

[Andi Albrecht]

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):

>>> 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.

[Andi Albrecht]

Example functions to reproduce the bug

[Tim Graham]

I didn't look into the details but the conversation in #25839 might be useful.

[Andi Albrecht]

Replying to 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.

[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.

[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.

[Tim Graham]

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?

[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.

[Tim Graham]

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().

[Tim Graham]

​PR with some comments for improvement.

[Tim Graham]

I fixed up the ​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.

[Tim Graham <timograham@…>]

In 6a74950:

Fixed #27258 -- Prohibited django.Template.render() with non-dict context.

Thanks Shivang Bharadwaj for the initial patch.

[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.

[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.

[Tim Graham]

Comments 12 and 13 stem from #28491. I replied with a solution there.