From dd6ebd7a48ddf59815d5d8fbd9ee647a14589fd4 Mon Sep 17 00:00:00 2001 From: Mira Date: Mon, 10 Jun 2024 16:41:52 +0200 Subject: [PATCH] Improve validation of email templates (#4184) * Improve validation of email templates * simplify SafeFormatter (skip attribute access code path altogether instead of blocklisting characters) * let SafeFormatter optionally raise on missing key * simplify placeholder validation * rename parameter * Remove unused import --------- Co-authored-by: Raphael Michel --- src/pretix/base/forms/validators.py | 31 ++++++++++++++--------------- src/pretix/control/views/event.py | 29 ++++++++++++++++----------- src/pretix/helpers/format.py | 14 ++++++------- 3 files changed, 38 insertions(+), 36 deletions(-) diff --git a/src/pretix/base/forms/validators.py b/src/pretix/base/forms/validators.py index bc9d69f265..35f7323ba0 100644 --- a/src/pretix/base/forms/validators.py +++ b/src/pretix/base/forms/validators.py @@ -32,13 +32,13 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations under the License. -import re - from django.core.exceptions import ValidationError from django.core.validators import BaseValidator from django.utils.translation import gettext_lazy as _ from i18nfield.strings import LazyI18nString +from pretix.helpers.format import format_map + class PlaceholderValidator(BaseValidator): """ @@ -47,6 +47,12 @@ class PlaceholderValidator(BaseValidator): which are not presented in taken list. """ + error_message = _( + 'There is an error with your placeholder syntax. Please check that the opening "{" and closing "}" curly ' + 'brackets on your placeholders match up. ' + 'Please note: to use literal "{" or "}", you need to double them as "{{" and "}}".' + ) + def __init__(self, limit_value): super().__init__(limit_value) self.limit_value = limit_value @@ -57,22 +63,15 @@ class PlaceholderValidator(BaseValidator): self.__call__(v) return - if value.count('{') != value.count('}'): + try: + format_map(value, {key.strip('{}'): "" for key in self.limit_value}, raise_on_missing=True) + except ValueError: + raise ValidationError(self.error_message, code='invalid_placeholder_syntax') + except KeyError as e: raise ValidationError( - _('Invalid placeholder syntax: You used a different number of "{" than of "}".'), - code='invalid_placeholder_syntax', - ) - - data_placeholders = list(re.findall(r'({[^}]*})', value, re.X)) - invalid_placeholders = [] - for placeholder in data_placeholders: - if placeholder not in self.limit_value: - invalid_placeholders.append(placeholder) - if invalid_placeholders: - raise ValidationError( - _('Invalid placeholder(s): %(value)s'), + _('Invalid placeholder: {%(value)s}'), code='invalid_placeholders', - params={'value': ", ".join(invalid_placeholders,)}) + params={'value': e.args[0]}) def clean(self, x): return x diff --git a/src/pretix/control/views/event.py b/src/pretix/control/views/event.py index cb4838dd67..15f5f68018 100644 --- a/src/pretix/control/views/event.py +++ b/src/pretix/control/views/event.py @@ -73,6 +73,7 @@ from i18nfield.utils import I18nJSONEncoder from pretix.base.channels import get_all_sales_channels from pretix.base.email import get_available_placeholders +from pretix.base.forms import PlaceholderValidator from pretix.base.models import Event, LogEntry, Order, TaxRule, Voucher from pretix.base.models.event import EventMetaValue from pretix.base.services import tickets @@ -713,11 +714,6 @@ class MailSettingsSetup(EventPermissionRequiredMixin, MailSettingsSetupView): class MailSettingsPreview(EventPermissionRequiredMixin, View): permission = 'can_change_event_settings' - # return the origin text if key is missing in dict - class SafeDict(dict): - def __missing__(self, key): - return '{' + key + '}' - # create index-language mapping @cached_property def supported_locale(self): @@ -742,7 +738,7 @@ class MailSettingsPreview(EventPermissionRequiredMixin, View): _('This value will be replaced based on dynamic parameters.'), s ) - return self.SafeDict(ctx) + return ctx def post(self, request, *args, **kwargs): preview_item = request.POST.get('item', '') @@ -758,12 +754,21 @@ class MailSettingsPreview(EventPermissionRequiredMixin, View): idx = matched.group('idx') if idx in self.supported_locale: with language(self.supported_locale[idx], self.request.event.settings.region): - if k.startswith('mail_subject_'): - msgs[self.supported_locale[idx]] = format_map(bleach.clean(v), self.placeholders(preview_item)) - else: - msgs[self.supported_locale[idx]] = markdown_compile_email( - format_map(v, self.placeholders(preview_item)) - ) + try: + if k.startswith('mail_subject_'): + msgs[self.supported_locale[idx]] = format_map( + bleach.clean(v), self.placeholders(preview_item), raise_on_missing=True + ) + else: + msgs[self.supported_locale[idx]] = markdown_compile_email( + format_map(v, self.placeholders(preview_item), raise_on_missing=True) + ) + except ValueError: + msgs[self.supported_locale[idx]] = '
{}
'.format( + PlaceholderValidator.error_message) + except KeyError as e: + msgs[self.supported_locale[idx]] = '
{}
'.format( + _('Invalid placeholder: {%(value)s}') % {'value': e.args[0]}) return JsonResponse({ 'item': preview_item, diff --git a/src/pretix/helpers/format.py b/src/pretix/helpers/format.py index 6fa115ed48..7a31775d08 100644 --- a/src/pretix/helpers/format.py +++ b/src/pretix/helpers/format.py @@ -30,17 +30,15 @@ class SafeFormatter(Formatter): Customized version of ``str.format`` that (a) behaves just like ``str.format_map`` and (b) does not allow any unwanted shenanigans like attribute access or format specifiers. """ - def __init__(self, context): + def __init__(self, context, raise_on_missing=False): self.context = context + self.raise_on_missing = raise_on_missing def get_field(self, field_name, args, kwargs): - if '.' in field_name or '[' in field_name: - logger.warning(f'Ignored invalid field name "{field_name}"') - return ('{' + str(field_name) + '}', field_name) - return super().get_field(field_name, args, kwargs) + return self.get_value(field_name, args, kwargs), field_name def get_value(self, key, args, kwargs): - if key not in self.context: + if not self.raise_on_missing and key not in self.context: return '{' + str(key) + '}' return self.context[key] @@ -49,7 +47,7 @@ class SafeFormatter(Formatter): return super().format_field(value, '') -def format_map(template, context): +def format_map(template, context, raise_on_missing=False): if not isinstance(template, str): template = str(template) - return SafeFormatter(context).format(template) + return SafeFormatter(context, raise_on_missing).format(template)