From 863cffeb36e7061bb78885f8b2797040103ea7b1 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Thu, 5 Feb 2015 16:04:02 +0000 Subject: [PATCH 1/4] Tests for StreamBlock --- wagtail/wagtailadmin/tests/test_blocks.py | 160 +++++++++++++++++++++- 1 file changed, 158 insertions(+), 2 deletions(-) diff --git a/wagtail/wagtailadmin/tests/test_blocks.py b/wagtail/wagtailadmin/tests/test_blocks.py index 78c94bea9..2f6acb5da 100644 --- a/wagtail/wagtailadmin/tests/test_blocks.py +++ b/wagtail/wagtailadmin/tests/test_blocks.py @@ -113,8 +113,8 @@ class TestStructBlock(unittest.TestCase): def test_render(self): class LinkBlock(blocks.StructBlock): - title = blocks.FieldBlock(forms.CharField(label="Title")) - link = blocks.FieldBlock(forms.URLField(label="Link")) + title = blocks.FieldBlock(forms.CharField(), label="Title") + link = blocks.FieldBlock(forms.URLField(), label="Link") block = LinkBlock() html = block.render({ @@ -213,3 +213,159 @@ class TestListBlock(unittest.TestCase): self.assertIn('', html) self.assertIn('', html) self.assertIn('', html) + + +class TestStreamBlock(unittest.TestCase): + def test_initialisation(self): + block = blocks.StreamBlock([ + ('heading', blocks.FieldBlock(forms.CharField())), + ('paragraph', blocks.FieldBlock(forms.CharField())), + ]) + + self.assertEqual(list(block.child_blocks.keys()), ['heading', 'paragraph']) + + def test_initialisation_from_subclass(self): + class ArticleBlock(blocks.StreamBlock): + heading = blocks.FieldBlock(forms.CharField()) + paragraph = blocks.FieldBlock(forms.CharField()) + + block = ArticleBlock() + + self.assertEqual(list(block.child_blocks.keys()), ['heading', 'paragraph']) + + def test_initialisation_from_subclass_with_extra(self): + class ArticleBlock(blocks.StreamBlock): + heading = blocks.FieldBlock(forms.CharField()) + paragraph = blocks.FieldBlock(forms.CharField()) + + block = ArticleBlock([ + ('intro', blocks.FieldBlock(forms.CharField())) + ]) + + self.assertEqual(list(block.child_blocks.keys()), ['heading', 'paragraph', 'intro']) + + def test_initialisation_with_multiple_subclassses(self): + class ArticleBlock(blocks.StreamBlock): + heading = blocks.FieldBlock(forms.CharField()) + paragraph = blocks.FieldBlock(forms.CharField()) + + class ArticleWithIntroBlock(ArticleBlock): + intro = blocks.FieldBlock(forms.CharField()) + + block = ArticleWithIntroBlock() + + self.assertEqual(list(block.child_blocks.keys()), ['heading', 'paragraph', 'intro']) + + @unittest.expectedFailure # Field order doesn't match inheritance order + def test_initialisation_with_mixins(self): + class ArticleBlock(blocks.StreamBlock): + heading = blocks.FieldBlock(forms.CharField()) + paragraph = blocks.FieldBlock(forms.CharField()) + + class IntroMixin(blocks.StreamBlock): + intro = blocks.FieldBlock(forms.CharField()) + + class ArticleWithIntroBlock(ArticleBlock, IntroMixin): + pass + + block = ArticleWithIntroBlock() + + self.assertEqual(list(block.child_blocks.keys()), ['heading', 'paragraph', 'intro']) + + def render_article(self, data): + class ArticleBlock(blocks.StreamBlock): + heading = blocks.FieldBlock(forms.CharField()) + paragraph = blocks.FieldBlock(forms.CharField()) + + block = ArticleBlock() + value = block.to_python(data) + + return block.render(value) + + def test_render(self): + html = self.render_article([ + { + 'type': 'heading', + 'value': "My title", + }, + { + 'type': 'paragraph', + 'value': 'My first paragraph', + }, + { + 'type': 'paragraph', + 'value': 'My second paragraph', + }, + ]) + + self.assertIn('
My title
', html) + self.assertIn('
My first paragraph
', html) + self.assertIn('
My second paragraph
', html) + + @unittest.expectedFailure + def test_render_unknown_type(self): + # This can happen if a developer removes a type from their StreamBlock + html = self.render_article([ + { + 'type': 'foo', + 'value': "Hello", + }, + ]) + + def render_form(self): + class ArticleBlock(blocks.StreamBlock): + heading = blocks.FieldBlock(forms.CharField()) + paragraph = blocks.FieldBlock(forms.CharField()) + + block = ArticleBlock() + value = block.to_python([ + { + 'type': 'heading', + 'value': "My title", + }, + { + 'type': 'paragraph', + 'value': 'My first paragraph', + }, + { + 'type': 'paragraph', + 'value': 'My second paragraph', + }, + ]) + return block.render_form(value, prefix='myarticle') + + def test_render_form_wrapper_class(self): + html = self.render_form() + + self.assertIn('
', html) + + def test_render_form_count_field(self): + html = self.render_form() + + self.assertIn('', html) + + def test_render_form_delete_field(self): + html = self.render_form() + + self.assertIn('', html) + + def test_render_form_order_fields(self): + html = self.render_form() + + self.assertIn('', html) + self.assertIn('', html) + self.assertIn('', html) + + def test_render_form_type_fields(self): + html = self.render_form() + + self.assertIn('', html) + self.assertIn('', html) + self.assertIn('', html) + + def test_render_form_value_fields(self): + html = self.render_form() + + self.assertIn('', html) + self.assertIn('', html) + self.assertIn('', html) From f3a66d34af9d591cc9c982ce1ddbef44a7810128 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 5 Feb 2015 16:23:31 +0000 Subject: [PATCH 2/4] implement RichTextBlock, and ensure that script tags embedded in html_declarations bodies are properly escaped --- wagtail/wagtailadmin/blocks.py | 11 +++++++++-- .../static/wagtailadmin/js/blocks/sequence.js | 14 ++++++++++---- .../wagtailadmin/templatetags/wagtailadmin_tags.py | 8 ++------ wagtail/wagtailcore/utils.py | 10 ++++++++++ 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/wagtail/wagtailadmin/blocks.py b/wagtail/wagtailadmin/blocks.py index ea008f1c4..c08533daf 100644 --- a/wagtail/wagtailadmin/blocks.py +++ b/wagtail/wagtailadmin/blocks.py @@ -19,6 +19,8 @@ from django.forms.utils import ErrorList import six +from wagtail.wagtailcore.utils import escape_script + # helpers for Javascript expression formatting def indent(string, depth=1): @@ -292,6 +294,11 @@ class CharBlock(FieldBlock): # TODO: some kwargs, such as max_length, and *possibly* things like help_text, should be passed to # the CharField constructor. Figure out a system for doing this +class RichTextBlock(FieldBlock): + def __init__(self, **kwargs): + from wagtail.wagtailcore.fields import RichTextArea + super(RichTextBlock, self).__init__(CharField(widget=RichTextArea), **kwargs) + # ======= # Chooser # ======= @@ -517,7 +524,7 @@ class ListBlock(Block): return format_html( '', - self.definition_prefix, list_member_html + self.definition_prefix, mark_safe(escape_script(list_member_html)) ) def js_initializer(self): @@ -641,7 +648,7 @@ class BaseStreamBlock(Block): ( self.definition_prefix, name, - self.render_list_member(name, child_block.default, '__PREFIX__', '') + mark_safe(escape_script(self.render_list_member(name, child_block.default, '__PREFIX__', ''))) ) for name, child_block in self.child_blocks.items() ] diff --git a/wagtail/wagtailadmin/static/wagtailadmin/js/blocks/sequence.js b/wagtail/wagtailadmin/static/wagtailadmin/js/blocks/sequence.js index 2ced19a9f..5e50b44c6 100644 --- a/wagtail/wagtailadmin/static/wagtailadmin/js/blocks/sequence.js +++ b/wagtail/wagtailadmin/static/wagtailadmin/js/blocks/sequence.js @@ -69,11 +69,17 @@ For example, they don't assume the presence of a 'delete' button - it's up to th newMember._markAdded(); } + function elementFromTemplate(template, newPrefix) { + /* generate a jquery object ready to be inserted into the list, based on the passed HTML template string. + '__PREFIX__' will be substituted with newPrefix, and script tags escaped as <-/script> will be un-escaped */ + return $(template.replace(/__PREFIX__/g, newPrefix).replace(/<-(-*)\/script>/g, '<$1/script>')); + } + self.insertMemberBefore = function(otherMember, template) { newMemberPrefix = getNewMemberPrefix(); /* Create the new list member element with the real prefix substituted in */ - var elem = $(template.replace(/__PREFIX__/g, newMemberPrefix)); + var elem = elementFromTemplate(template, newMemberPrefix); otherMember.container.before(elem); var newMember = SequenceMember(self, newMemberPrefix); var index = otherMember.getIndex(); @@ -94,7 +100,7 @@ For example, they don't assume the presence of a 'delete' button - it's up to th newMemberPrefix = getNewMemberPrefix(); /* Create the new list member element with the real prefix substituted in */ - var elem = $(template.replace(/__PREFIX__/g, newMemberPrefix)); + var elem = elementFromTemplate(template, newMemberPrefix); otherMember.container.after(elem); var newMember = SequenceMember(self, newMemberPrefix); var index = otherMember.getIndex() + 1; @@ -119,7 +125,7 @@ For example, they don't assume the presence of a 'delete' button - it's up to th newMemberPrefix = getNewMemberPrefix(); /* Create the new list member element with the real prefix substituted in */ - var elem = $(template.replace(/__PREFIX__/g, newMemberPrefix)); + var elem = elementFromTemplate(template, newMemberPrefix); list.prepend(elem); var newMember = SequenceMember(self, newMemberPrefix); @@ -139,7 +145,7 @@ For example, they don't assume the presence of a 'delete' button - it's up to th newMemberPrefix = getNewMemberPrefix(); /* Create the new list member element with the real prefix substituted in */ - var elem = $(template.replace(/__PREFIX__/g, newMemberPrefix)); + var elem = elementFromTemplate(template, newMemberPrefix); list.append(elem); var newMember = SequenceMember(self, newMemberPrefix); diff --git a/wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py b/wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py index cb049d0e8..650e49f35 100644 --- a/wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py +++ b/wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py @@ -1,14 +1,12 @@ from __future__ import unicode_literals -import re - from django.conf import settings from django import template from django.contrib.humanize.templatetags.humanize import intcomma from wagtail.wagtailcore import hooks from wagtail.wagtailcore.models import get_navigation_menu_items, UserPagePermissionsProxy, PageViewRestriction -from wagtail.wagtailcore.utils import camelcase_to_underscore +from wagtail.wagtailcore.utils import camelcase_to_underscore, escape_script from wagtail.wagtailadmin.menu import admin_menu @@ -136,7 +134,6 @@ def usage_count_enabled(): class EscapeScriptNode(template.Node): TAG_NAME = 'escapescript' - SCRIPT_RE = re.compile(r'<(-*)/script>') def __init__(self, nodelist): super(EscapeScriptNode, self).__init__() @@ -144,8 +141,7 @@ class EscapeScriptNode(template.Node): def render(self, context): out = self.nodelist.render(context) - escaped_out = self.SCRIPT_RE.sub(r'<-\1/script>', out) - return escaped_out + return escape_script(out) @classmethod def handle(cls, parser, token): diff --git a/wagtail/wagtailcore/utils.py b/wagtail/wagtailcore/utils.py index 1880684bc..52492e338 100644 --- a/wagtail/wagtailcore/utils.py +++ b/wagtail/wagtailcore/utils.py @@ -36,3 +36,13 @@ def resolve_model_string(model_string, default_app=None): else: raise LookupError("Can not resolve {0!r} into a model".format(model_string), model_string) + + +SCRIPT_RE = re.compile(r'<(-*)/script>') +def escape_script(text): + """ + Escape `` tags in 'text' so that it can be placed within a `