Apply correct HTML escaping to jinja2 include_block tag

wagtail · Jun 17, 2021 · 084a9c1 · 084a9c1
1 parent e24f10c
commit 084a9c1
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 11 deletions.
diff --git a/wagtail/core/jinja2tags.py b/wagtail/core/jinja2tags.py
@@ -31,30 +31,35 @@ def parse_include_block(self, parser):
 
         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
@@ -2,6 +2,7 @@
 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 @@ def test_include_block_tag_with_filtered_value(self):
             'language': 'fr',
         })
         self.assertIn('<body>999</body>', 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 <em>evil</em> HTML'))
+
+        result = render_to_string('tests/jinja2/include_block_test.html', {
+            'test_block': bound_block,
+        })
+        self.assertIn('<body>some &lt;em&gt;evil&lt;/em&gt; HTML</body>', result)
+
+        # {% autoescape off %} should be respected
+        result = render_to_string('tests/blocks/include_block_autoescape_off_test.html', {
+            'test_block': bound_block,
+        })
+        self.assertIn('<body>some <em>evil</em> HTML</body>', 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 <em>evil</em> HTML',
+        })
+        self.assertIn('<body>some &lt;em&gt;evil&lt;/em&gt; HTML</body>', result)
+
+        result = render_to_string('tests/jinja2/include_block_autoescape_off_test.html', {
+            'test_block': 'some <em>evil</em> HTML',
+        })
+        self.assertIn('<body>some <em>evil</em> HTML</body>', 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 <em>evil</em> HTML'))
+
+        result = render_to_string('tests/jinja2/include_block_test.html', {
+            'test_block': bound_block,
+        })
+        self.assertIn('<body>some <em>evil</em> HTML</body>', 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 <em>evil</em> HTML'),
+        })
+        self.assertIn('<body>some <em>evil</em> HTML</body>', 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
@@ -0,0 +1 @@
+<body>{% autoescape false %}{% include_block test_block %}{% endautoescape %}</body>