From f8a792ad33d1293e9c0255eed15c5319b284665f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= Date: Thu, 16 Jul 2015 11:49:00 +0200 Subject: [PATCH] general: escape substitution variables (#7860) In templates used for comments and editable texts the substitution is done on-the-fly by ezt. This is not enabled to the general template as most variables in there are supposed to be HTML and many sites created on that assumption. Therefore the substitution is done ahead and limited to context variables. --- tests/test_fields.py | 6 ++++ tests/test_texts.py | 4 +++ tests/test_workflows.py | 85 +++++++++++++++++++++++++++++++++-------------- wcs/fields.py | 2 +- wcs/qommon/admin/texts.py | 2 +- wcs/qommon/template.py | 6 +++- wcs/workflows.py | 13 +++++--- 7 files changed, 85 insertions(+), 33 deletions(-) diff --git a/tests/test_fields.py b/tests/test_fields.py index a9e1367..c7c6789 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -128,6 +128,12 @@ def test_comment(): field.add_to_form(form) assert '

Foobar

' in str(form.render()) + # test for proper escaping of substitution variables + field = fields.CommentField(label='[foo]') + form = Form() + field.add_to_form(form) + assert '

1 < 3

' in str(form.render()) + # test for html content field = fields.CommentField(label='

Foobar

') form = Form() diff --git a/tests/test_texts.py b/tests/test_texts.py index 8d69610..f708db3 100644 --- a/tests/test_texts.py +++ b/tests/test_texts.py @@ -48,3 +48,7 @@ def test_get_html_subst(): pub.cfg['texts'] = {'text-foo4': '[bar]'} pub.write_cfg() assert TextsDirectory.get_html_text('foo4') == '

Foobar

' + + pub.cfg['texts'] = {'text-foo4': '[foo]'} + pub.write_cfg() + assert TextsDirectory.get_html_text('foo4') == '

1 < 3

' diff --git a/tests/test_workflows.py b/tests/test_workflows.py index cc48eee..9007235 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -12,7 +12,8 @@ from wcs import sessions from wcs.fields import StringField, DateField from wcs.roles import Role from wcs.workflows import (Workflow, WorkflowStatusItem, - SendmailWorkflowStatusItem, SendSMSWorkflowStatusItem) + SendmailWorkflowStatusItem, SendSMSWorkflowStatusItem, + DisplayMessageWorkflowStatusItem) from wcs.wf.anonymise import AnonymiseWorkflowStatusItem from wcs.wf.dispatch import DispatchWorkflowStatusItem from wcs.wf.form import FormWorkflowStatusItem, WorkflowFormFieldsFormDef @@ -27,7 +28,16 @@ from utilities import (create_temporary_pub, MockSubstitutionVariables, emails, def setup_module(module): cleanup() - global pub, req + +def teardown_module(module): + clean_temporary_pub() + +def pytest_generate_tests(metafunc): + if 'two_pubs' in metafunc.fixturenames: + metafunc.parametrize('two_pubs', ['pickle', 'sql'], indirect=True) + +@pytest.fixture +def pub(request): pub = create_temporary_pub() pub.cfg['language'] = {'language': 'en'} pub.write_cfg() @@ -36,13 +46,7 @@ def setup_module(module): req.user = None pub._set_request(req) req.session = sessions.BasicSession(id=1) - -def teardown_module(module): - clean_temporary_pub() - -def pytest_generate_tests(metafunc): - if 'two_pubs' in metafunc.fixturenames: - metafunc.parametrize('two_pubs', ['pickle', 'sql'], indirect=True) + return pub @pytest.fixture def two_pubs(request): @@ -57,7 +61,7 @@ def two_pubs(request): return pub -def test_jump_nothing(): +def test_jump_nothing(pub): FormDef.wipe() formdef = FormDef() formdef.name = 'foobar' @@ -66,7 +70,7 @@ def test_jump_nothing(): item = JumpWorkflowStatusItem() assert item.must_jump(formdata) is True -def test_jump_datetime_condition(): +def test_jump_datetime_condition(pub): FormDef.wipe() formdef = FormDef() formdef.name = 'foobar' @@ -83,7 +87,7 @@ def test_jump_datetime_condition(): tomorrow.timetuple()[:3] assert item.must_jump(formdata) is False -def test_jump_count_condition(): +def test_jump_count_condition(pub): FormDef.wipe() formdef = FormDef() formdef.name = 'foobar' @@ -101,7 +105,7 @@ def test_jump_count_condition(): item.condition = 'form_objects.count < 2' assert item.must_jump(formdata) is False -def test_check_auth(): +def test_check_auth(pub): user = pub.user_class(name='foo') user.store() @@ -148,7 +152,7 @@ def test_check_auth(): formdata.workflow_roles = None assert status_item.check_auth(formdata, user) is True -def test_dispatch(): +def test_dispatch(pub): formdef = FormDef() formdef.name = 'baz' formdef.store() @@ -165,7 +169,7 @@ def test_dispatch(): item.perform(formdata) assert formdata.workflow_roles == {'_receiver': '1'} -def test_roles(): +def test_roles(pub): user = pub.user_class() user.store() @@ -206,7 +210,7 @@ def test_roles(): item.perform(formdata) assert pub.user_class.get(user.id).roles == ['2'] -def test_anonymise(): +def test_anonymise(pub): formdef = FormDef() formdef.name = 'baz' formdef.fields = [] @@ -222,7 +226,7 @@ def test_anonymise(): assert formdef.data_class().get(formdata.id).user_id is None assert formdef.data_class().get(formdata.id).anonymised -def test_remove(): +def test_remove(pub): formdef = FormDef() formdef.name = 'baz' formdef.store() @@ -239,6 +243,7 @@ def test_remove(): formdata.store() item = RemoveWorkflowStatusItem() + req = pub.get_request() req.response.filter['in_backoffice'] = True assert formdef.data_class().has_key(formdata.id) assert item.perform(formdata) == '..' @@ -246,7 +251,7 @@ def test_remove(): req.response.filter = {} assert req.session.message -def test_register_comment(): +def test_register_comment(pub): pub.substitutions.feed(MockSubstitutionVariables()) formdef = FormDef() @@ -286,7 +291,7 @@ def test_register_comment(): item.perform(formdata) assert formdata.evolution[-1].display_parts()[-1] == '
1 < 3
' -def test_email(): +def test_email(pub): pub.substitutions.feed(MockSubstitutionVariables()) formdef = FormDef() @@ -378,7 +383,7 @@ def test_email(): assert emails.count() == 1 assert emails.get('foobar')['email_rcpt'] == ['xyz@localhost'] -def test_webservice_call(): +def test_webservice_call(pub): pub.substitutions.feed(MockSubstitutionVariables()) formdef = FormDef() @@ -425,7 +430,7 @@ def test_webservice_call(): item.perform(formdata) assert 'signature=' in http_requests.get_last('url') -def test_timeout(): +def test_timeout(pub): workflow = Workflow(name='timeout') st1 = workflow.add_status('Status1', 'st1') st2 = workflow.add_status('Status2', 'st2') @@ -491,11 +496,11 @@ def test_timeout_then_remove(two_pubs): assert str(formdata_id) in [str(x) for x in formdef.data_class().keys()] time.sleep(0.2) - _apply_timeouts(pub) + _apply_timeouts(two_pubs) assert not str(formdata_id) in [str(x) for x in formdef.data_class().keys()] -def test_sms(): +def test_sms(pub): formdef = FormDef() formdef.name = 'baz' formdef.fields = [] @@ -572,9 +577,9 @@ def test_display_form(two_pubs): assert formdata.get_substitution_variables()['xxx_var_date_raw'] == \ time.strptime('2015-05-12', '%Y-%m-%d') - pub.cfg['language'] = {'language': 'en'} + two_pubs.cfg['language'] = {'language': 'en'} -def test_workflow_role_type_migration(): +def test_workflow_role_type_migration(pub): workflow = Workflow(name='role migration') st1 = workflow.add_status('Status1', 'st1') @@ -588,3 +593,33 @@ def test_workflow_role_type_migration(): reloaded_workflow = Workflow.get(workflow.id) assert reloaded_workflow.possible_status[0].items[0].by == ['1', '2'] + +def test_workflow_display_message(pub): + pub.substitutions.feed(MockSubstitutionVariables()) + + workflow = Workflow(name='display message') + st1 = workflow.add_status('Status1', 'st1') + + FormDef.wipe() + formdef = FormDef() + formdef.name = 'foobar' + formdef.fields = [] + formdef.store() + formdata = formdef.data_class()() + formdata.id = '1' + + display_message = DisplayMessageWorkflowStatusItem() + display_message.parent = st1 + + display_message.message = 'test' + assert display_message.get_message(formdata) == 'test' + + display_message.message = '[number]' + assert display_message.get_message(formdata) == str(formdata.id) + + display_message.message = '[bar]' + assert display_message.get_message(formdata) == 'Foobar' + + # makes sure the string is correctly escaped for HTML + display_message.message = '[foo]' + assert display_message.get_message(formdata) == '1 < 3' diff --git a/wcs/fields.py b/wcs/fields.py index 9bd3422..d862cc5 100644 --- a/wcs/fields.py +++ b/wcs/fields.py @@ -453,7 +453,7 @@ class CommentField(Field): label = self.label import wcs.workflows - label = wcs.workflows.template_on_string(label) + label = wcs.workflows.template_on_html_string(label) if '. +import cgi from cStringIO import StringIO import os import glob @@ -438,7 +439,10 @@ def decorate(body, response): breadcrumb = ' > '.join(s) vars = response.filter.copy() - vars.update(get_publisher().substitutions.get_context_variables()) + for var_key, var_value in get_publisher().substitutions.get_context_variables().items(): + if isinstance(var_value, basestring): + var_value = cgi.escape(var_value) + vars[var_key] = var_value vars.update(locals()) fd = StringIO() template.generate(fd, vars) diff --git a/wcs/workflows.py b/wcs/workflows.py index e6abf88..5aa4347 100644 --- a/wcs/workflows.py +++ b/wcs/workflows.py @@ -15,6 +15,7 @@ # along with this program; if not, see . from qommon import ezt +import cgi from cStringIO import StringIO import copy import xml.etree.ElementTree as ET @@ -1347,11 +1348,13 @@ class SendmailWorkflowStatusItem(WorkflowStatusItem): register_item_class(SendmailWorkflowStatusItem) -def template_on_string(template, process=None): - return template_on_formdata(None, template, process=process) +def template_on_html_string(template, process=None): + return template_on_formdata(None, template, process=process, + base_format=ezt.FORMAT_HTML) -def template_on_formdata(formdata=None, template=None, process=None): +def template_on_formdata(formdata=None, template=None, process=None, + base_format=ezt.FORMAT_RAW): assert template is not None if not '[' in template: return template @@ -1384,7 +1387,7 @@ def template_on_formdata(formdata=None, template=None, process=None): dict[k] = v.encode(charset, 'ignore') processor = ezt.Template(compress_whitespace=False) - processor.parse(template or '') + processor.parse(template or '', base_format=base_format) fd = StringIO() processor.generate(fd, dict) @@ -1459,7 +1462,7 @@ class DisplayMessageWorkflowStatusItem(WorkflowStatusItem): if not self.message: return '' tmpl = ezt.Template() - tmpl.parse(self.message) + tmpl.parse(self.message, base_format=ezt.FORMAT_HTML) dict = {} dict.update(get_publisher().substitutions.get_context_variables()) -- 2.1.4