From 084a9c10cd206b79fa44d922d81beecf660cf494 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 10 Jun 2021 16:07:53 +0100 Subject: [PATCH] Apply correct HTML escaping to jinja2 include_block tag --- wagtail/core/jinja2tags.py | 27 +++++----- wagtail/core/tests/test_jinja2.py | 49 +++++++++++++++++++ .../include_block_autoescape_off_test.html | 1 + 3 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 wagtail/tests/testapp/jinja2_templates/tests/jinja2/include_block_autoescape_off_test.html diff --git a/wagtail/core/jinja2tags.py b/wagtail/core/jinja2tags.py index bb59eff0da..f6cd9d2c99 100644 --- a/wagtail/core/jinja2tags.py +++ b/wagtail/core/jinja2tags.py @@ -31,30 +31,35 @@ class WagtailCoreExtension(Extension): args = [parser.parse_expression()] - with_context = True + # always pass context to _include_block - even if we're not passing it on to render_as_block, + # we need it to see if autoescaping is enabled + args.append(jinja2.nodes.ContextReference()) + + use_context = True if parser.stream.current.test_any('name:with', 'name:without') and parser.stream.look().test('name:context'): - with_context = next(parser.stream).value == 'with' + use_context = next(parser.stream).value == 'with' parser.stream.skip() - if with_context: - args.append(jinja2.nodes.ContextReference()) - else: - # Actually we can just skip else branch because context arg default to None - args.append(jinja2.nodes.Const(None)) + args.append(jinja2.nodes.Const(use_context)) node = self.call_method('_include_block', args, lineno=lineno) return jinja2.nodes.Output([node], lineno=lineno) - def _include_block(self, value, context=None): + def _include_block(self, value, context, use_context): if hasattr(value, 'render_as_block'): - if context: + if use_context: new_context = context.get_all() else: new_context = {} - return jinja2.Markup(value.render_as_block(context=new_context)) + result = value.render_as_block(context=new_context) + else: + result = value - return jinja2.Markup(value) + if context.eval_ctx.autoescape: + return jinja2.escape(result) + else: + return jinja2.Markup(result) # Nicer import names diff --git a/wagtail/core/tests/test_jinja2.py b/wagtail/core/tests/test_jinja2.py index 7685a89f68..cfd2233328 100644 --- a/wagtail/core/tests/test_jinja2.py +++ b/wagtail/core/tests/test_jinja2.py @@ -2,6 +2,7 @@ from django.http import HttpRequest from django.template import engines from django.template.loader import render_to_string from django.test import TestCase +from django.utils.safestring import mark_safe from wagtail import __version__ from wagtail.core import blocks @@ -182,3 +183,51 @@ class TestIncludeBlockTag(TestCase): 'language': 'fr', }) self.assertIn('999', result) + + def test_include_block_html_escaping(self): + """ + Output of include_block should be escaped as per Django autoescaping rules + """ + block = blocks.CharBlock() + bound_block = block.bind(block.to_python('some evil HTML')) + + result = render_to_string('tests/jinja2/include_block_test.html', { + 'test_block': bound_block, + }) + self.assertIn('some <em>evil</em> HTML', result) + + # {% autoescape off %} should be respected + result = render_to_string('tests/blocks/include_block_autoescape_off_test.html', { + 'test_block': bound_block, + }) + self.assertIn('some evil HTML', result) + + # The same escaping should be applied when passed a plain value rather than a BoundBlock - + # a typical situation where this would occur would be rendering an item of a StructBlock, + # e.g. {% include_block person_block.first_name %} as opposed to + # {% include_block person_block.bound_blocks.first_name %} + result = render_to_string('tests/jinja2/include_block_test.html', { + 'test_block': 'some evil HTML', + }) + self.assertIn('some <em>evil</em> HTML', result) + + result = render_to_string('tests/jinja2/include_block_autoescape_off_test.html', { + 'test_block': 'some evil HTML', + }) + self.assertIn('some evil HTML', result) + + # Blocks that explicitly return 'safe HTML'-marked values (such as RawHTMLBlock) should + # continue to produce unescaped output + block = blocks.RawHTMLBlock() + bound_block = block.bind(block.to_python('some evil HTML')) + + result = render_to_string('tests/jinja2/include_block_test.html', { + 'test_block': bound_block, + }) + self.assertIn('some evil HTML', result) + + # likewise when applied to a plain 'safe HTML' value rather than a BoundBlock + result = render_to_string('tests/jinja2/include_block_test.html', { + 'test_block': mark_safe('some evil HTML'), + }) + self.assertIn('some evil HTML', result) diff --git a/wagtail/tests/testapp/jinja2_templates/tests/jinja2/include_block_autoescape_off_test.html b/wagtail/tests/testapp/jinja2_templates/tests/jinja2/include_block_autoescape_off_test.html new file mode 100644 index 0000000000..5e102f25ba --- /dev/null +++ b/wagtail/tests/testapp/jinja2_templates/tests/jinja2/include_block_autoescape_off_test.html @@ -0,0 +1 @@ +{% autoescape false %}{% include_block test_block %}{% endautoescape %}