Suggested commit message:
Issue #1825952 by Fabianx, joelpittet, bdragon, heddn, chx, xjm, pwolanin, mikey_p, ti2m, bfr, dags, cilefen, scor: Turn on twig autoescape by default
Note: The approach has changed several times in this issue, so anything before comment #139 may not be relevant.
Work on this patch is done in a sandbox:
git clone --branch autoescape2--xjm http://git.drupal.org/sandbox/chx/1857558.git d8_autoescape
Contact chx for access.
No one can write XSS-safe code. Not core contributors, nor contrib developers, no one.
- Turn on Twig autoescaping by default.
- Add a new SafeMarkup class. Sanitized markup strings can be flagged (with caution) as safe with SafeMarkup::set() and thoroughly documented (see ). These will be rendered as-is by Twig; everything else will be escaped automatically.
|Task||Novice task?||Contributor instructions||Complete?|
|Add unit tests for SafeMarkup||Instructions||Done (#205)|
|Add detailed documentation for SafeMarkup|
|Draft a change record for the API changes||Instructions||Done (needs review)|
User interface changes
If we do this right, then none. If we don't then you will see a new kind of bug: double escaping.
New SafeMarkup class and SafeMarkup::implode() helper.
You are not allowed to put unsafe user data in #attached. This can be relaxed in a followup but it truly gets gnarly. You are advised to not use
#type => html_tag if at all possible or at least not with unsafe user data. This is not something I want to waste an effort on making it work.
Original report by @catch
Part of meta issue
Twig as it stands introduces a fair bit of overhead into the theme system. Fabianx indicated that a lot of this is from marking $variables as secure so they're not double escaped later.
Ideally, if Twig autoescape is going to be enabled, then we should just pass raw variables to it and let it do the work. This way, if a template doesn't print the date, or a link, or whatever might currently be check_plain()ed in preprocess, we're not spending all this time creating it for it to be never used. In general, we should be able to remove a large chunk of preprocess work, and just let Twig sort out variables on demand.
Doing this means that a PHPTemplate engine in contrib is going to have to add back a way to securely format variables, but I don't see a way around this if we don't want a serious performance regression.
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 74,363 pass(es). View
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 74,634 pass(es). View