Django

Code

Ticket #2359 (closed: fixed)

Opened 2 years ago

Last modified 6 months ago

[patch] Auto-escaping template changes

Reported by: mtredinnick Assigned to: nobody
Component: Template system Version:
Keywords: Cc: mir@noris.de
Triage Stage: Design decision needed Has patch: 1
Needs documentation: 0 Needs tests: 0
Patch needs improvement: 1

Description

This is an initial patch implementing auto-escaping support. It follows the Simon Willison's AutoEscaping proposal fairly closely. Putting this up now to aid discussion about some of the uncertainties.

Still to come:

  • porting the contrib/ apps over to this
  • documentation patch.

Attachments

autoescape.diff (44.4 kB) - added by mtredinnick on 07/16/06 06:02:23.
Core changes and test suite
01-core-changes.diff (60.8 kB) - added by mtredinnick on 08/08/06 23:35:04.
Update core changes (supercedes previous patch)
02-misc-contrib-changes.diff (11.5 kB) - added by mtredinnick on 08/08/06 23:35:38.
Changes to most of the contrib/ applications (excludes admin)
03-admin-changes.diff (36.9 kB) - added by mtredinnick on 08/08/06 23:35:58.
Changes to contrib/admin/
01-core-changes.2.diff (69.7 kB) - added by Michael Radziej <mir@noris.de> on 02/07/07 01:52:20.
updated patch
02-misc-contrib-changes.2.diff (12.0 kB) - added by Michael Radziej <mir@noris.de> on 02/07/07 01:53:58.
updated patch
03-admin-changes.2.diff (30.9 kB) - added by Michael Radziej <mir@noris.de> on 02/07/07 01:54:31.
updated patch
auto.patch (2.0 kB) - added by smurf@smurf.noris.de on 02/10/07 01:41:28.
don't mark unprocessed markup as safe; fix a string initializer
01-core-changes.3.diff (70.9 kB) - added by Michael Radziej <mir@noris.de> on 03/02/07 05:56:53.
updated patch, includes part of smurf's patch, for rev. 4659
02-misc-contrib-changes.3.diff (11.5 kB) - added by Michael Radziej <mir@noris.de> on 03/02/07 05:58:11.
updated patch, includes part of smurf's patch, for rev. 4659
01-core-changes.4.diff (71.0 kB) - added by Michael Radziej <mir@noris.de> on 03/19/07 07:41:06.
updated patch, see comment
unicode-autoescape-1.diff (73.6 kB) - added by mir@noris.de on 05/25/07 12:39:48.
core changes ported for the unicode branch
unicode-autoescape-1.2.diff (71.9 kB) - added by mir@noris.de on 05/25/07 12:41:00.
core changes ported for the unicode branch (trac-readable)
unicode-autoescape-2.diff (11.2 kB) - added by mir@noris.de on 05/25/07 12:41:53.
contrib changes ported for the unicode branch
unicode-autoescape-1.3.diff (71.9 kB) - added by mir@noris.de on 05/25/07 12:42:36.
core changes ported for the unicode branch (for trac, 3rd attempt)
autoescape-1.diff (73.0 kB) - added by mir@noris.de on 07/18/07 07:10:15.
updated patch for svn release 5722
rev5722-01-core-changes.diff (71.2 kB) - added by mir@noris.de on 07/18/07 07:13:47.
updated patch for svn release 5722 (2nd attempt ...)
rev5722-01-core-changes.2.diff (69.7 kB) - added by mir@noris.de on 07/18/07 07:19:22.
updated patch for svn release 5722 (3rd attempt)
rev5722-01-core-changes.3.diff (73.0 kB) - added by mir@noris.de on 07/18/07 07:22:54.
updated patch for svn release 5722 (4th attempt)
rev5722-02-misc-contrib-changes.diff (11.9 kB) - added by mir@noris.de on 07/18/07 07:23:34.
updated patch for svn release 5722
rev5774-01-core-changes.diff (73.1 kB) - added by mir@noris.de on 08/01/07 09:12:50.
updated patch
rev5774-02-misc-contrib-changes.diff (11.9 kB) - added by mir@noris.de on 08/01/07 09:13:20.
updated patch
variable-node.diff (1.0 kB) - added by mir@noris.de on 08/09/07 09:26:28.
see comment 17

Change History

07/16/06 06:02:23 changed by mtredinnick

  • attachment autoescape.diff added.

Core changes and test suite

07/16/06 06:13:40 changed by mtredinnick

The mailing list thread for discussing this patch is here. Please use that instead of this ticket for the time being so that the conversation does not happen in two places at once.

07/17/06 05:27:59 changed by SmileyChris

Good job on the patch, Malcom :)

A couple of points: If a markup filter fails due to an import error, I don't think it should be marked as safe. From a skim read, I'm missing the purpose of having an .is_safe property on filters - can't you just check the outputted string to see if it's SafeData??

07/27/06 16:27:45 changed by mtredinnick

  • owner changed from adrian to mtredinnick.

08/08/06 23:35:04 changed by mtredinnick

  • attachment 01-core-changes.diff added.

Update core changes (supercedes previous patch)

08/08/06 23:35:38 changed by mtredinnick

  • attachment 02-misc-contrib-changes.diff added.

Changes to most of the contrib/ applications (excludes admin)

08/08/06 23:35:58 changed by mtredinnick

  • attachment 03-admin-changes.diff added.

Changes to contrib/admin/

08/08/06 23:41:59 changed by mtredinnick

  • owner changed from mtredinnick to adrian.

Adrian, this one's back in your court now if you want to give it a review. I think it's complete.

The patches are all independent of each other (each file is only changed by one patch), although 01-core-changes.diff should be applied first. It incorporates all our discussions from OSCON:

  1. off by default
  2. inherits through template extension (so you can mark the "content" in the base template with "autoescape" and be done with it).
  3. adds both "autoescape" and "noautoescape" as per Jacob's suggestion (so that the end tags tell you which tag you are closing explicitly).

The admin change is probably the most destabilising, since mistakes there will be noticed by everybody. It's also probably the hardest one to test in all its variations. I've used it a bit and tried to find problems, but ultimately we are just going to have to be responsive to feedback and fix things as they are noticed. Very few backwards-incompatible changes in the end. The "escape" filter only applying once is the only real one (and we have the new "force_escape" filter if multi-escaping is needed).

08/08/06 23:45:05 changed by mtredinnick

Hmm .. Trac's diff colorizer doesn't like new files being added in the diff. A couple of files appears as being patches to /dev/null. They are the addition of django/utils/safestring.py and tests/othertests/autoescape.py respectively, both in 01-core-changes.diff

The raw diff file has the correct information and can be applied with patch -p1 -i ....

10/02/06 22:34:58 changed by SmileyChris

Lest it fly under the Adrian radar, I just want to add that my alternative solution is nearly entirely self-contained, so would require no changes at all to the rest of django if it were to be implemented.

I've been using it for a project (including inside of the existing Admin site) with no hazardous side effects.

11/06/06 13:40:52 changed by adrian

#2984 was a duplicate.

01/17/07 17:04:15 changed by mir@noris.de

  • needs_better_patch set to 1.
  • stage changed from Unreviewed to Design decision needed.

Malcolm stated that this is still work in progress.

02/07/07 01:52:20 changed by Michael Radziej <mir@noris.de>

  • attachment 01-core-changes.2.diff added.

updated patch

02/07/07 01:53:58 changed by Michael Radziej <mir@noris.de>

  • attachment 02-misc-contrib-changes.2.diff added.

updated patch

02/07/07 01:54:31 changed by Michael Radziej <mir@noris.de>

  • attachment 03-admin-changes.2.diff added.

updated patch

02/07/07 01:59:58 changed by Michael Radziej <mir@noris.de>

I've brought the patches up to date. Bigger changes were:

  • Adapting the tests, this needed some changes in the templates tests. The autoescape tests subclass the templates tests, and this needed a few hooks (but should be ovious)
  • SafeString? needs a str() method, or else the one from the normal python string is used, and then it's not SafeData? anymore.

It still needs patches for newforms, but this is a fast moving target if I've ever seen one ...

02/10/07 01:41:28 changed by smurf@smurf.noris.de

  • attachment auto.patch added.

don't mark unprocessed markup as safe; fix a string initializer

03/02/07 05:56:53 changed by Michael Radziej <mir@noris.de>

  • attachment 01-core-changes.3.diff added.

updated patch, includes part of smurf's patch, for rev. 4659

03/02/07 05:58:11 changed by Michael Radziej <mir@noris.de>

  • attachment 02-misc-contrib-changes.3.diff added.

updated patch, includes part of smurf's patch, for rev. 4659

03/02/07 06:01:10 changed by Michael Radziej <mir@noris.de>

I've brought the first two patches up to date for revision 3659. They contain smurf's patch, and 01-core also adds empty init.py and models.py files who seemed to have slipped somehow.

It doesn't make sense for me to maintain 03-admin since I don't use the admin anywhere.

03/19/07 07:39:17 changed by Michael Radziej <mir@noris.de>

patch 01-core-changes: the template tag filter needs to be able to deal with functions decorated by stringfilter (since you cannot use escape or safe with filter).

03/19/07 07:41:06 changed by Michael Radziej <mir@noris.de>

  • attachment 01-core-changes.4.diff added.

updated patch, see comment

03/19/07 07:42:13 changed by Michael Radziej <mir@noris.de>

  • cc set to mir@noris.de.

04/18/07 18:44:45 changed by SmileyChris

In the current patch, the example in documentation of is_safe isn't actually safe - the value passed could be a "safe" string which contains a unicode HTML attribute '&#303;' so this filter will introduce a dangerous character ('&').

 	707	 2. If your filter is given a "safe" string, is it guaranteed to return a 
 	708	 "safe" string? If so, set the ``is_safe`` attribute on the function to be 
 	709	 ``True``. For example, a filter that replaced all numbers with the number 
 	710	 spelt out in words is going to be safe-string-preserving, since it cannot 
 	711	 introduce any of the five dangerous characters: <, >, ", ' or &. So we can 
 	712	 write:: 
 	713	 
 	714	    @register.filter 
 	715	    def convert_to_words(value): 
 	716	        # ... implementation here ... 
 	717	        return result 
 	718	 
 	719	    convert_to_words.is_safe = True 

04/26/07 07:54:26 changed by mir@noris.de

A small bug: Tracebacks for unexpected exceptions in template tags ("Template Error") show a part of the template where the bug hit. With autoescape on, the lines in the template display are escaped one time too often.

05/25/07 12:39:48 changed by mir@noris.de

  • attachment unicode-autoescape-1.diff added.

core changes ported for the unicode branch

05/25/07 12:41:00 changed by mir@noris.de

  • attachment unicode-autoescape-1.2.diff added.

core changes ported for the unicode branch (trac-readable)

05/25/07 12:41:53 changed by mir@noris.de

  • attachment unicode-autoescape-2.diff added.

contrib changes ported for the unicode branch

05/25/07 12:42:36 changed by mir@noris.de

  • attachment unicode-autoescape-1.3.diff added.

core changes ported for the unicode branch (for trac, 3rd attempt)

05/25/07 12:44:22 changed by mir@noris.de

I have attached two patches ported to the unicode branch; these positively need further review. Somehow trac refuses to display unicode-autoescape-1, I have no idea why. Sorry for spoiling the history ...

07/18/07 07:10:15 changed by mir@noris.de

  • attachment autoescape-1.diff added.

updated patch for svn release 5722

07/18/07 07:13:47 changed by mir@noris.de

  • attachment rev5722-01-core-changes.diff added.

updated patch for svn release 5722 (2nd attempt ...)

07/18/07 07:19:22 changed by mir@noris.de

  • attachment rev5722-01-core-changes.2.diff added.

updated patch for svn release 5722 (3rd attempt)

07/18/07 07:22:54 changed by mir@noris.de

  • attachment rev5722-01-core-changes.3.diff added.

updated patch for svn release 5722 (4th attempt)

07/18/07 07:23:34 changed by mir@noris.de

  • attachment rev5722-02-misc-contrib-changes.diff added.

updated patch for svn release 5722

07/18/07 07:25:40 changed by mir@noris.de

I brought the first two patches up to date to release 5722. I am aware that trac doesn't parse rev5722-01-core-changes.*.diff, but as much as I tried, I was unable to correct it. Please use rev5722-01-core-changes.3.diff and rev5722-02-misc-contrib-changes.diff

08/01/07 09:12:50 changed by mir@noris.de

  • attachment rev5774-01-core-changes.diff added.

updated patch

08/01/07 09:13:20 changed by mir@noris.de

  • attachment rev5774-02-misc-contrib-changes.diff added.

updated patch

08/09/07 09:22:07 changed by mir@noris.de

VariableNode.render() must call force_unicode earlier, so that it can see whether it needs to escape or not. I'll add a patch for this separately (variable-node.diff)

BTW: The patches do not (yet) deal with newforms.

08/09/07 09:26:28 changed by mir@noris.de

  • attachment variable-node.diff added.

see comment 17

11/14/07 06:58:54 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

(In [6671]) Implemented auto-escaping of variable output in templates. Fully controllable by template authors and it's possible to write filters and templates that simulataneously work in both auto-escaped and non-auto-escaped environments if you need to. Fixed #2359

See documentation in templates.txt and templates_python.txt for how everything works.

Backwards incompatible if you're inserting raw HTML output via template variables.

Based on an original design from Simon Willison and with debugging help from Michael Radziej.


Add/Change #2359 ([patch] Auto-escaping template changes)




Change Properties
Action