Merge pull request #1253 from django-helpdesk/fix_error_when_no_email_body

Fix error when email has attachment mime part and content mime part missing or empty
This commit is contained in:
Christopher Broderick 2025-04-03 23:03:42 +01:00 committed by GitHub
commit ca66e0aa1d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 60 additions and 7 deletions

View File

@ -850,6 +850,9 @@ def extract_email_message_content(
replies must be extracted replies must be extracted
""" """
message_part: MIMEPart = part.get_body() message_part: MIMEPart = part.get_body()
# handle the case where there is no content, just an attachment
if not message_part:
return None, None
parent_part: MIMEPart = part parent_part: MIMEPart = part
content_type = message_part.get_content_type() content_type = message_part.get_content_type()
# Handle the possibility of a related part formatted email # Handle the possibility of a related part formatted email
@ -902,7 +905,9 @@ def extract_email_message_content(
# Is either text/plain or some random content-type so just decode the part content and store as is # Is either text/plain or some random content-type so just decode the part content and store as is
mime_content = mime_content_to_string(message_part) mime_content = mime_content_to_string(message_part)
# We should now have the mime content # We should now have the mime content
filtered_body = parse_email_content(mime_content, include_chained_msgs) filtered_body = (
parse_email_content(mime_content, include_chained_msgs) if mime_content else ""
)
if not filtered_body or "" == filtered_body.strip(): if not filtered_body or "" == filtered_body.strip():
# A unit test that has a different HTML content to plain text which seems an invalid case as email # A unit test that has a different HTML content to plain text which seems an invalid case as email
# tools should retain the HTML to be consistent with the plain text but manage this as a special case # tools should retain the HTML to be consistent with the plain text but manage this as a special case
@ -1088,7 +1093,8 @@ def extract_email_metadata(
counter, content_parts_excluded = extract_attachments( counter, content_parts_excluded = extract_attachments(
message_obj, files, logger message_obj, files, logger
) )
if not content_parts_excluded: # Check if there is expected to be a content part
if not content_parts_excluded and (filtered_body or full_body):
# Unexpected situation and may mean there is a hole in the email processing logic # Unexpected situation and may mean there is a hole in the email processing logic
logger.warning( logger.warning(
"Failed to exclude email content when parsing all MIME parts in the multipart.\ "Failed to exclude email content when parsing all MIME parts in the multipart.\

View File

@ -514,6 +514,45 @@ class GetEmailCommonTests(TestCase):
) )
self.assertIsNone(ticket, f"Ticket was created when it should not be: {ticket}") self.assertIsNone(ticket, f"Ticket was created when it should not be: {ticket}")
def test_with_attachment_but_no_body_part(self):
"""
Test an email that has no body content but does have an attachment
"""
message, _, _ = utils.generate_multipart_email(
type_list=["image"],
)
# Now send the part to the email workflow
extract_email_metadata(message.as_string(), self.queue_public, self.logger)
self.assertEqual(len(mail.outbox), 1) # @UndefinedVariable
ticket = Ticket.objects.get()
followup = ticket.followup_set.get()
# Check attachment is stored as attached file
email_attachment_found = followup.followupattachment_set.exists()
self.assertTrue(
email_attachment_found,
"Email attachment file not found in ticket attachment for empty body.",
)
def test_with_attachment_but_empty_body(self):
"""
Test an email that has an empty body but does have an attachment
"""
message, _, _ = utils.generate_multipart_email(
type_list=["plain", "image"],
body="",
)
# Now send the part to the email workflow
extract_email_metadata(message.as_string(), self.queue_public, self.logger)
self.assertEqual(len(mail.outbox), 1) # @UndefinedVariable
ticket = Ticket.objects.get()
followup = ticket.followup_set.get()
# Check attachment is stored as attached file
email_attachment_found = followup.followupattachment_set.exists()
self.assertTrue(
email_attachment_found,
"Email attachment file not found in ticket attachment for empty body.",
)
class EmailTaskTests(TestCase): class EmailTaskTests(TestCase):
def setUp(self): def setUp(self):

View File

@ -288,15 +288,18 @@ def add_simple_email_headers(
def generate_mime_part( def generate_mime_part(
locale: str = "en_US", locale: str = "en_US",
part_type: str = "plain", part_type: str = "plain",
body: str = None,
) -> typing.Optional[Message]: ) -> typing.Optional[Message]:
""" """
Generates amime part of the sepecified type Generates a mime part of the specified type
:param locale: change this to generate locale specific strings :param locale: change this to generate locale specific strings
:param text_type: options are plain, html, image (attachment), file (attachment) :param text_type: options are plain, html, image (attachment), file (attachment)
:param body: if provided then will be added to the plain mime part only
""" """
if "plain" == part_type: if "plain" == part_type:
body = get_fake("text", locale=locale, min_length=1024) if body is None:
body = get_fake("text", locale=locale, min_length=1024)
msg = MIMEText(body, part_type) msg = MIMEText(body, part_type)
elif "html" == part_type: elif "html" == part_type:
body = get_fake_html(locale=locale, wrap_in_body_tag=True) body = get_fake_html(locale=locale, wrap_in_body_tag=True)
@ -317,6 +320,7 @@ def generate_multipart_email(
type_list: typing.List[str] = ["plain", "html", "image"], type_list: typing.List[str] = ["plain", "html", "image"],
sub_type: str = None, sub_type: str = None,
use_short_email: bool = False, use_short_email: bool = False,
body: str = None,
) -> typing.Tuple[Message, typing.Tuple[str, str], typing.Tuple[str, str]]: ) -> typing.Tuple[Message, typing.Tuple[str, str], typing.Tuple[str, str]]:
""" """
Generates an email including headers with the defined multiparts Generates an email including headers with the defined multiparts
@ -325,10 +329,11 @@ def generate_multipart_email(
:param type_list: options are plain, html, image (attachment), file (attachment), and executable (.exe attachment) :param type_list: options are plain, html, image (attachment), file (attachment), and executable (.exe attachment)
:param sub_type: multipart sub type that defaults to "mixed" if not specified :param sub_type: multipart sub type that defaults to "mixed" if not specified
:param use_short_email: produces a "To" or "From" that is only the email address if True :param use_short_email: produces a "To" or "From" that is only the email address if True
:param body: if provided then will be added to the plain mime part only
""" """
msg = MIMEMultipart(sub_type) if sub_type else MIMEMultipart() msg = MIMEMultipart(sub_type) if sub_type else MIMEMultipart()
for part_type in type_list: for part_type in type_list:
msg.attach(generate_mime_part(locale=locale, part_type=part_type)) msg.attach(generate_mime_part(locale=locale, part_type=part_type, body=body))
from_meta, to_meta = add_simple_email_headers( from_meta, to_meta = add_simple_email_headers(
msg, locale=locale, use_short_email=use_short_email msg, locale=locale, use_short_email=use_short_email
) )
@ -336,12 +341,15 @@ def generate_multipart_email(
def generate_text_email( def generate_text_email(
locale: str = "en_US", use_short_email: bool = False locale: str = "en_US",
use_short_email: bool = False,
body: str = None,
) -> typing.Tuple[Message, typing.Tuple[str, str], typing.Tuple[str, str]]: ) -> typing.Tuple[Message, typing.Tuple[str, str], typing.Tuple[str, str]]:
""" """
Generates an email including headers Generates an email including headers
""" """
body = get_fake("text", locale=locale, min_length=1024) if body is None:
body = get_fake("text", locale=locale, min_length=1024)
msg = MIMEText(body) msg = MIMEText(body)
from_meta, to_meta = add_simple_email_headers( from_meta, to_meta = add_simple_email_headers(
msg, locale=locale, use_short_email=use_short_email msg, locale=locale, use_short_email=use_short_email