From c3d38eeac1e5858660dd85f48759c59ab6b890b6 Mon Sep 17 00:00:00 2001 From: Thomas NOEL Date: Sat, 7 Oct 2017 18:55:17 +0200 Subject: [PATCH] utils: accept Django syntax in templated_url (#19261) --- combo/data/models.py | 12 ++++--- combo/utils.py | 25 +++++++++++--- tests/test_cells.py | 93 ++++++++++++++++++++++++++++++++++++++++++++++++---- tests/test_utils.py | 45 ++++++++++++++++++++++--- 4 files changed, 156 insertions(+), 19 deletions(-) diff --git a/combo/data/models.py b/combo/data/models.py index 0d73c94..179f9ec 100644 --- a/combo/data/models.py +++ b/combo/data/models.py @@ -871,11 +871,15 @@ class JsonCellBase(CellBase): log_errors = data_url_dict.get('log_errors', self.log_errors) try: url = utils.get_templated_url(data_url_dict['url'], context) - except utils.UnknownTemplateVariableError: + except utils.TemplateError as e: logger = logging.getLogger(__name__) - logger.warning('unknown variable in template URL (%s)', self.url) + logger.warning('error in templated URL (%s): %s', data_url_dict['url'], e) continue extra_context[data_key + '_url'] = url + if not url: + logger = logging.getLogger(__name__) + logger.warning('templated URL (%s) is empty', data_url_dict['url']) + continue try: json_response = utils.requests.get(url, headers={'Accept': 'application/json'}, @@ -960,8 +964,8 @@ class JsonCellBase(CellBase): try: url = utils.get_templated_url(self.actions[action]['url'], context) - except utils.UnknownTemplateVariableError: - logger.warning('unknown variable in URL (%s)', self.actions[action]['url']) + except utils.TemplateError as e: + logger.warning('error in templated URL (%s): %s', self.actions[action]['url'], e) raise PostException(error_message) json_response = utils.requests.post(url, diff --git a/combo/utils.py b/combo/utils.py index b95dee4..ba16990 100644 --- a/combo/utils.py +++ b/combo/utils.py @@ -35,7 +35,7 @@ from requests import Response, Session as RequestsSession from django.conf import settings from django.core.cache import cache -from django.template import Context +from django.template import Context, Template, TemplateSyntaxError, VariableDoesNotExist from django.utils.html import strip_tags from django.utils.http import urlencode, quote @@ -174,10 +174,19 @@ class Requests(RequestsSession): requests = Requests() -class UnknownTemplateVariableError(KeyError): - pass + +class TemplateError(Exception): + def __init__(self, msg, params=()): + self.msg = msg + self.params = params + + def __str__(self): + return self.msg % self.params + def get_templated_url(url, context=None): + if '{{' not in url and '{%' not in url and '[' not in url: + return url template_vars = Context() if context: template_vars.update(context) @@ -189,12 +198,20 @@ def get_templated_url(url, context=None): if hasattr(user, 'saml_identifiers') and user.saml_identifiers.exists(): template_vars['user_nameid'] = quote(user.saml_identifiers.first().name_id) template_vars.update(settings.TEMPLATE_VARS) + if '{{' in url or '{%' in url: # Django template + try: + return Template(url).render(template_vars) + except VariableDoesNotExist as e: + raise TemplateError(e.msg, e.params) + except TemplateSyntaxError: + raise TemplateError('syntax error') + # ezt-like template def repl(matchobj): varname = matchobj.group(0)[1:-1] if varname == '[': return '[' if varname not in template_vars: - raise UnknownTemplateVariableError(varname) + raise TemplateError('unknown variable %s', varname) return unicode(template_vars[varname]) return re.sub(r'(\[.+?\])', repl, url) diff --git a/tests/test_cells.py b/tests/test_cells.py index e2c278a..57aef4c 100644 --- a/tests/test_cells.py +++ b/tests/test_cells.py @@ -134,6 +134,7 @@ def test_json_cell(): cell = JsonCell() cell.page = page + cell.url = 'http://example.net/' cell.title = 'Example Site' cell.order = 0 cell.save() @@ -158,23 +159,22 @@ def test_json_cell(): requests_get.return_value = mock.Mock(content=json.dumps(data), status_code=200) context = cell.get_cell_extra_context({}) assert context['json'] == data - assert context['json_url'] == '' + assert context['json_url'] == 'http://example.net/' assert context['json_status'] == 200 assert 'json_error' not in context requests_get.return_value = mock.Mock(status_code=204) # 204 : No Content context = cell.get_cell_extra_context({}) assert context['json'] is None - assert context['json_url'] == '' + assert context['json_url'] == 'http://example.net/' assert context['json_status'] == 204 assert 'json_error' not in context - cell.url = 'http://test2' requests_get.return_value = mock.Mock(content='not found', status_code=404, headers={}) context = cell.get_cell_extra_context({}) assert context['json'] is None - assert context['json_url'] == 'http://test2' + assert context['json_url'] == 'http://example.net/' assert context['json_status'] == 404 assert 'json_error' not in context @@ -182,7 +182,7 @@ def test_json_cell(): headers={'content-type': 'application/json'}) context = cell.get_cell_extra_context({}) assert context['json'] is None - assert context['json_url'] == 'http://test2' + assert context['json_url'] == 'http://example.net/' assert context['json_status'] == 404 assert context['json_error'] == data @@ -192,10 +192,18 @@ def test_json_cell(): context = cell.get_cell_extra_context({}) assert context['json'] is None assert context['json_status'] == -1 - assert context['json_url'] == 'http://test2' + assert context['json_url'] == 'http://example.net/' assert context['json_error'] == 'boom' assert isinstance(context['json_exception'], requests.ConnectionError) + cell.url = '' # no URL -> no request, no data, no status + requests_get.return_value = mock.Mock(content=json.dumps(data), status_code=200) + context = cell.get_cell_extra_context({}) + assert context['json'] is None + assert context['json_url'] == '' + assert 'json_status' not in context + assert 'json_error' not in context + with pytest.raises(NothingInCacheException): cell.url = 'http://test3' cell.render({}) @@ -494,3 +502,76 @@ def test_config_json_cell_additional_url(app): assert context['plop_url'] == 'http://bar' assert context['plop_status'] == 200 assert 'plop_error' not in context + + # additional-data url depends on others results, with Django-syntax URL + with override_settings(JSON_CELL_TYPES={ + 'test-config-json-cell-2': { + 'name': 'Foobar', + 'url': 'http://foo', + 'additional-data': [ + {'key': 'plop', 'url': 'http://{{json.data}}', 'log_errors': False, 'timeout': 42}, + {'key': 'plop2', 'url': '{% if plop %}http://{{json.data}}/{{plop.data}}{% endif %}', 'log_errors': False, + 'timeout': 10}, + ] + }}, + TEMPLATE_DIRS=['%s/templates-1' % os.path.abspath(os.path.dirname(__file__))]): + cell = ConfigJsonCell() + cell.key = 'test-config-json-cell-2' + cell.page = page + cell.title = 'Example Site' + cell.order = 0 + cell.save() + + data = {'data': 'bar'} + + with mock.patch('combo.utils.requests.get') as requests_get: + requests_get.return_value = mock.Mock(content=json.dumps(data), status_code=200) + url = reverse('combo-public-ajax-page-cell', + kwargs={'page_pk': page.id, 'cell_reference': cell.get_reference()}) + resp = app.get(url) + assert resp.body.strip() == '/var1=bar/var2=bar/' + assert len(requests_get.mock_calls) == 3 + assert requests_get.mock_calls[0][1][0] == 'http://foo' + assert requests_get.mock_calls[0][-1]['log_errors'] == True + assert requests_get.mock_calls[0][-1]['timeout'] == 28 + assert requests_get.mock_calls[1][1][0] == 'http://bar' + assert requests_get.mock_calls[1][-1]['log_errors'] == False + assert requests_get.mock_calls[1][-1]['timeout'] == 42 + assert requests_get.mock_calls[2][1][0] == 'http://bar/bar' + assert requests_get.mock_calls[2][-1]['log_errors'] == False + assert requests_get.mock_calls[2][-1]['timeout'] == 10 + context = cell.get_cell_extra_context({}) + assert context['json'] == data + assert context['json_url'] == 'http://foo' + assert context['json_status'] == 200 + assert context['plop'] == data + assert context['plop_url'] == 'http://bar' + assert context['plop_status'] == 200 + assert context['plop2'] == data + assert context['plop2_url'] == 'http://bar/bar' + assert context['plop2_status'] == 200 + + with mock.patch('combo.utils.requests.get') as requests_get: + requests_get.return_value = mock.Mock(content=json.dumps(data), status_code=404, + headers={'content-type': 'application/json'}) + url = reverse('combo-public-ajax-page-cell', + kwargs={'page_pk': page.id, 'cell_reference': cell.get_reference()}) + resp = app.get(url) + assert resp.body.strip() == '/var1=/var2=/' + # can not create plop and plop2 url: only one request for "json" + assert len(requests_get.mock_calls) == 2 + assert requests_get.mock_calls[0][1][0] == 'http://foo' + context = cell.get_cell_extra_context({}) + assert context['json'] == None + assert context['json_url'] == 'http://foo' + assert context['json_status'] == 404 + assert context['json_error'] == data + assert context['plop'] == None + assert context['plop_url'] == 'http://' + assert context['plop_status'] == 404 + assert context['plop_error'] == data + # plop2 url is empty, no request: None value, no status + assert context['plop2'] == None + assert context['plop2_url'] == '' + assert 'plop2_status' not in context + assert 'plop2_error' not in context diff --git a/tests/test_utils.py b/tests/test_utils.py index 376b335..3701168 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,7 +1,7 @@ import pytest from combo.utils import (aes_hex_decrypt, aes_hex_encrypt, get_templated_url, - UnknownTemplateVariableError) + TemplateError) from django.conf import settings from django.test import override_settings from django.template import Context @@ -44,10 +44,14 @@ def test_templated_url(): assert get_templated_url('foobar[[]]') == 'foobar[]' assert get_templated_url('foobar[[]test]') == 'foobar[test]' - with pytest.raises(UnknownTemplateVariableError): + assert get_templated_url('{{ test_url }}') == '' + with pytest.raises(TemplateError, match='unknown variable test_url'): get_templated_url('[test_url]') + with override_settings(TEMPLATE_VARS={'test_url': 'http://www.example.net'}): + assert get_templated_url('{{ test_url }}') == 'http://www.example.net' assert get_templated_url('[test_url]') == 'http://www.example.net' + assert get_templated_url('{{ test_url }}/hello') == 'http://www.example.net/hello' assert get_templated_url('[test_url]/hello') == 'http://www.example.net/hello' # contexts without users @@ -55,27 +59,34 @@ def test_templated_url(): request.user = None for context in (None, Context({}), Context({'request': None}), Context({'request': request})): + assert get_templated_url('NameID={{ user_nameid }}', context=context) == 'NameID=' + assert get_templated_url('email={{ user_email }}', context=context) == 'email=' if context is None: - with pytest.raises(UnknownTemplateVariableError) as e: + with pytest.raises(TemplateError, match='unknown variable user_nameid'): get_templated_url('NameID=[user_nameid]', context=context) - with pytest.raises(UnknownTemplateVariableError) as e: + with pytest.raises(TemplateError, match='unknown variable user_email'): get_templated_url('email=[user_email]', context=context) else: assert get_templated_url('NameID=[user_nameid]', context=context) == 'NameID=' assert get_templated_url('email=[user_email]', context=context) == 'email=' - with pytest.raises(UnknownTemplateVariableError) as e: + with pytest.raises(TemplateError, match='unknown variable bar'): get_templated_url('foo=[bar]', context=context) if context: context['foobar'] = 'barfoo' + assert get_templated_url('{{foobar}}', context=context) == 'barfoo' assert get_templated_url('[foobar]', context=context) == 'barfoo' # contexts with users request = RequestFactory().get('/') request.user = MockUser(samlized=False) context = Context({'request': request}) + assert get_templated_url('email={{ user_email }}', context=context) == \ + 'email=foo%3D3%40example.net' assert get_templated_url('email=[user_email]', context=context) == \ 'email=foo%3D3%40example.net' request.user = MockUser(samlized=True) + assert get_templated_url('email={{user_email}}&NameID={{user_nameid}}', context=context) == \ + 'email=foo%3D3%40example.net&NameID=r2%26d2' assert get_templated_url('email=[user_email]&NameID=[user_nameid]', context=context) == \ 'email=foo%3D3%40example.net&NameID=r2%26d2' @@ -84,12 +95,17 @@ def test_templated_url(): request.user = MockUser(samlized=True) context = Context({'request': request}) context['foobar'] = 'barfoo' + assert get_templated_url('{{ foobar }}/email={{ user_email }}&NameID={{ user_nameid }}', + context=context) == 'barfoo/email=foo%3D3%40example.net&NameID=r2%26d2' assert get_templated_url('[foobar]/email=[user_email]&NameID=[user_nameid]', context=context) == 'barfoo/email=foo%3D3%40example.net&NameID=r2%26d2' with override_settings(TEMPLATE_VARS={'test_url': 'http://www.example.net'}): request = RequestFactory().get('/') request.user = MockUser(samlized=True) context = Context({'foobar': 'barfoo', 'request': request}) + assert get_templated_url('{{test_url}}/{{foobar}}/?NameID={{user_nameid}}&email={{user_email}}', + context=context) == \ + 'http://www.example.net/barfoo/?NameID=r2%26d2&email=foo%3D3%40example.net' assert get_templated_url('[test_url]/[foobar]/?NameID=[user_nameid]&email=[user_email]', context=context) == \ 'http://www.example.net/barfoo/?NameID=r2%26d2&email=foo%3D3%40example.net' @@ -101,4 +117,23 @@ def test_templated_url(): context.update({'foo': 'bar'}) ctx = Context() ctx.update(context) + assert get_templated_url('{{ foo }}', context=ctx) == 'bar' assert get_templated_url('[foo]', context=ctx) == 'bar' + + # accept django syntax + assert get_templated_url('{% if "foo" %}ok{% endif %}') == 'ok' + assert get_templated_url('{% if foo %}{{ foo }}{% endif %}') == '' + assert get_templated_url('{% if foo %}{{ foo }}{% endif %}', context={'foo': 'ok'}) == 'ok' + assert get_templated_url('{{ bar|default:"ok" }}') == 'ok' + assert get_templated_url('{{ foo|default:"ko" }}', context={'foo': 'ok'}) == 'ok' + assert get_templated_url('{nodjango}') == '{nodjango}' + assert get_templated_url('{{') == '{{' + assert get_templated_url('{%{{ ok }}{%') == '{%{%' + assert get_templated_url('{{ foo.bar }}', context={'foo': {'bar': 'ok'}}) == 'ok' + assert get_templated_url('{{ foo.bar }}') == '' + assert get_templated_url('{{ foo.0 }}{{ foo.1 }}{{ foo.2 }}', context={'foo': ['ok', 'doc']}) == 'okdoc' + assert get_templated_url('{{ foo.0.bar }}{{ foo.0.zoo }}', context={'foo': [{'bar': 'ok'}, 'ko']}) == 'ok' + # catch django syntax errors in TemplateError + for template in ('{% foobar %}', '{% if "coucou" %}', '{{}}', '{{ if "x" }}', '{{ _private }}'): + with pytest.raises(TemplateError, match='syntax error'): + assert get_templated_url(template, context=ctx) == 'bar' -- 2.14.2