diff --git a/helpdesk/lib.py b/helpdesk/lib.py index 028d6353..f08aa23d 100644 --- a/helpdesk/lib.py +++ b/helpdesk/lib.py @@ -9,6 +9,7 @@ lib.py - Common functions (eg multipart e-mail) from datetime import date, datetime, time from django.conf import settings +from django.core.exceptions import ValidationError from django.utils.encoding import smart_str from helpdesk.settings import CUSTOMFIELD_DATE_FORMAT, CUSTOMFIELD_DATETIME_FORMAT, CUSTOMFIELD_TIME_FORMAT import logging @@ -132,6 +133,7 @@ def process_attachments(followup, attached_files): max_email_attachment_size = getattr( settings, 'HELPDESK_MAX_EMAIL_ATTACHMENT_SIZE', 512000) attachments = [] + errors = set() for attached in attached_files: @@ -148,14 +150,21 @@ def process_attachments(followup, attached_files): 'application/octet-stream', size=attached.size, ) - att.full_clean() - att.save() + try: + att.full_clean() + except ValidationError as e: + errors.add(e) + else: + att.save() - if attached.size < max_email_attachment_size: - # Only files smaller than 512kb (or as defined in - # settings.HELPDESK_MAX_EMAIL_ATTACHMENT_SIZE) are sent via - # email. - attachments.append([filename, att.file]) + if attached.size < max_email_attachment_size: + # Only files smaller than 512kb (or as defined in + # settings.HELPDESK_MAX_EMAIL_ATTACHMENT_SIZE) are sent via + # email. + attachments.append([filename, att.file]) + + if errors: + raise ValidationError(list(errors)) return attachments diff --git a/helpdesk/tests/test_get_email.py b/helpdesk/tests/test_get_email.py index 0c298fa1..0fe945ff 100644 --- a/helpdesk/tests/test_get_email.py +++ b/helpdesk/tests/test_get_email.py @@ -186,13 +186,51 @@ class GetEmailCommonTests(TestCase): object_from_message(message.as_string(), self.queue_public, self.logger) self.assertIn( - "ERROR:helpdesk:{'file': ['Unsupported file extension: .jpg']}", + "ERROR:helpdesk:['Unsupported file extension: .jpg']", cm.output ) self.assertEqual(len(mail.outbox), 1) self.assertEqual(f'[test-1] {message.get("subject")} (Opened)', mail.outbox[0].subject) + def test_multiple_attachments(self): + """ + Tests the saving of multiple attachments + """ + message, _, _ = utils.generate_multipart_email(type_list=['plain', 'file', 'image']) + + self.assertEqual(len(mail.outbox), 0) + + object_from_message(message.as_string(), self.queue_public, self.logger) + + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(f'[test-1] {message.get("subject")} (Opened)', mail.outbox[0].subject) + + ticket = Ticket.objects.get() + followup = ticket.followup_set.get() + self.assertEqual(2, followup.followupattachment_set.count()) + + @override_settings(VALID_EXTENSIONS=['.txt']) + def test_multiple_attachments_with_wrong_extension(self): + """ + Tests that a wrong extension won't stop from saving other valid attachment + """ + message, _, _ = utils.generate_multipart_email(type_list=['plain', 'image', 'file', 'image']) + + self.assertEqual(len(mail.outbox), 0) + + with self.assertLogs(logger='helpdesk', level='ERROR') as cm: + object_from_message(message.as_string(), self.queue_public, self.logger) + + self.assertIn( + "ERROR:helpdesk:['Unsupported file extension: .jpg']", + cm.output + ) + + ticket = Ticket.objects.get() + followup = ticket.followup_set.get() + self.assertEqual(1, followup.followupattachment_set.count()) + class GetEmailParametricTemplate(object): """TestCase that checks basic email functionality across methods and socks configs.""" diff --git a/helpdesk/tests/utils.py b/helpdesk/tests/utils.py index 88f0f990..f2822732 100644 --- a/helpdesk/tests/utils.py +++ b/helpdesk/tests/utils.py @@ -205,16 +205,14 @@ def add_simple_email_headers(message: Message, locale: str="en_US", def generate_mime_part(locale: str="en_US", part_type: str="plain", - use_short_email: bool=False ) -> typing.Optional[Message]: """ Generates amime part of the sepecified type :param locale: change this to generate locale specific strings :param text_type: options are plain, html, image (attachment), file (attachment) - :param use_short_email: produces a "To" or "From" that is only the email address if True """ - if "plain" == part_type: + if "plain" == part_type: body = get_fake("text", locale=locale, min_length=1024) msg = MIMEText(body) elif "html" == part_type: @@ -241,7 +239,7 @@ def generate_multipart_email(locale: str="en_US", """ msg = MIMEMultipart() for part_type in type_list: - msg.attach(generate_mime_part(locale=locale, part_type=part_type, use_short_email=use_short_email)) + msg.attach(generate_mime_part(locale=locale, part_type=part_type)) from_meta, to_meta = add_simple_email_headers(msg, locale=locale, use_short_email=use_short_email) return msg, from_meta, to_meta