Opened 9 years ago

Closed 9 years ago

#25469 closed New feature (fixed)

Add an auto escape setting to the Django template engine

Reported by: Aidan Lister Owned by: Jef Geskens
Component: Template system Version: 1.8
Severity: Normal Keywords: templates
Cc: aaronelliotross@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Passing Context(mydata, autoescape=True) is no longer effective when using the non-deprecated format:

    from django.template import engines
    engines['django'].from_string('hello {{person}}').render({'person': 'bob & co'})
    >>> 'hello bob & co'

vs

    from django.template import Template,Context
    Template('hello {{person}}').render(Context({'person': 'bob & co'}, autoescape=False))
    >>> 'hello bob & co'

This is a very useful feature when not generated HTML templates!

Attachments (1)

autoescape-setting.patch (5.2 KB ) - added by Aymeric Augustin 9 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 by Aymeric Augustin, 9 years ago

Resolution: invalid
Status: newclosed

Unless I missed something, this was a private API. You can use the public API which hasn't changed in Django 1.8: https://docs.djangoproject.com/en/1.7/ref/templates/builtins/#std:templatetag-autoescape

comment:2 by Aidan Lister, 9 years ago

So the background here is that I've got editable templates via a GUI for emails. The emails are plain text ... so forcing end-users to put {% autoescape off %} at the top of every template seems pretty gross. Can we reevaluate this?

Last edited 9 years ago by Tim Graham (previous) (diff)

comment:3 by Aymeric Augustin, 9 years ago

Resolution: invalid
Status: closednew

Can't you just keep using the same code? I don't think you should yet any deprecation warnings, do you?

Also it may be reasonable to add and autoescape option to the Django template library, similar to Jinja2.

comment:4 by Aidan Lister, 9 years ago

Isn't Context() deprecated in 1.10?

comment:5 by Aymeric Augustin, 9 years ago

Not when using django.template as a standalone template library.

comment:6 by Aymeric Augustin, 9 years ago

Summary: Autoescape is not possible with Django 1.8 template refactorAdd an auto escape setting to the Django template engine

comment:7 by Tim Graham, 9 years ago

Triage Stage: UnreviewedAccepted
Type: BugNew feature

comment:8 by Jef Geskens, 9 years ago

Owner: changed from nobody to Jef Geskens
Status: newassigned

comment:9 by Aaron Elliot Ross, 9 years ago

Jef, Are you at the sprints? I'd like to help with this! (I'm aaronelliotross on slack / irc.)

comment:10 by Jef Geskens, 9 years ago

Has patch: set
Needs documentation: set
Patch needs improvement: set

comment:11 by Carl Meyer, 9 years ago

I don't believe the PR quite matches what Aymeric had suggested above ("add an autoescape option to the Django template library, similar to Jinja2"). The Jinja2 backend doesn't accept an autoescape argument to its render method, it has a global autoescape option for the engine, which is configured in the settings dict.

I think that the signature of a template backend's render method should remain consistent between backends, and consistent with the documented signature, and not grow ad-hoc backend-specific additions. It's intended to be a generic lowest-common-denominator API; in the common case it is not called directly on the backend, but by the generic render() method. Backend configuration options, on the other hand, are intended to provide backend-specific configuration.

Presuming we did this as a configuration option (to parallel the Jinja2 backend), your use case might require you to configure two instances of the DTL in your settings file, one with autoescape turned on and one with autoescape turned off.

Aymeric, do you have any additional thoughts here? Do we need a way to pass per-render backend-specific options? Should the documented signature of a backend's render method include **kwargs, to be interpreted as desired by the backend?

comment:12 by Aymeric Augustin, 9 years ago

I confirm what Carl just said. This should be an option of the engine. The signature of render() should remain a lowest common denominator.

This is necessary for end-users to be able to override any template with their own version, written for their favorite engine.

comment:13 by Jef Geskens, 9 years ago

Ok, I understand the vision. I will create a new patch :-) Thanks for the review!

comment:14 by Aymeric Augustin, 9 years ago

I discussed this with aaronelliotross at the very end of the DutH sprints and quickly wrote a tentative implementation, which I'm attaching to this ticket. Aaron was planning to try it and write tests. You should attempt to coordinate in order not to duplicate efforts.

by Aymeric Augustin, 9 years ago

Attachment: autoescape-setting.patch added

comment:15 by Aaron Elliot Ross, 9 years ago

OK, pull request added (https://github.com/django/django/pull/5610).

Jef, want to take a look and see what you think? Sorry I didn't get in touch earlier, I will add myself to the CC here!

comment:16 by Aaron Elliot Ross, 9 years ago

Cc: aaronelliotross@… added

comment:17 by Jef Geskens, 9 years ago

Looks great, only two comments:

  1. I think there was some typo, I see "autocomplete" instead of "autoescape" on several places.
  1. Is it easy to "instantiate" engines ad-hoc? Then it would solve the problem of the reporter. We want to avoid having this as a "global" setting and ending up with a very insecure website.
Version 0, edited 9 years ago by Jef Geskens (next)

comment:18 by Carl Meyer, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:19 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In 19a5f6d:

Fixed #25469 -- Added autoescape option to DjangoTemplates backend.

Thanks Aymeric for the initial patch and Carl for review.

Note: See TracTickets for help on using tickets.
Back to Top