Skip to content

Commit 310ea57

Browse files
jennifer-richardsjennifer-richards
committed
Accept/replace invalid Unicode bytes when processing ipr response emails. Fixes ietf-tools#3489. Commit ready for merge.
 - Legacy-Id: 19766
1 parent fd0df6f commit 310ea57

File tree

4 files changed

+64
-34
lines changed

4 files changed

+64
-34
lines changed

‎ietf/ipr/mail.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import re
1212

1313
from django.template.loader import render_to_string
14-
from django.utils.encoding import force_text, force_str
14+
from django.utils.encoding import force_text, force_bytes
1515

1616
import debug # pyflakes:ignore
1717

@@ -174,7 +174,7 @@ def process_response_email(msg):
174174
a matching value in the reply_to field, associated to an IPR disclosure through
175175
IprEvent. Create a Message object for the incoming message and associate it to
176176
the original message via new IprEvent"""
177-
message = email.message_from_string(force_str(msg))
177+
message = email.message_from_bytes(force_bytes(msg))
178178
to = message.get('To', '')
179179

180180
# exit if this isn't a response we're interested in (with plus addressing)

‎ietf/ipr/management/commands/process_email.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,8 @@ def handle(self, *args, **options):
2525
email = options.get('email', None)
2626
binary_input = io.open(email, 'rb') if email else sys.stdin.buffer
2727
self.msg_bytes = binary_input.read()
28-
msg = self.msg_bytes.decode()
29-
3028
try:
31-
process_response_email(msg)
29+
process_response_email(self.msg_bytes)
3230
except ValueError as e:
3331
raise CommandError(e)
3432

‎ietf/ipr/management/tests.py‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ def test_process_email(self, process_mock):
1818
with name_of_file_containing('contents') as filename:
1919
call_command('process_email', email_file=filename)
2020
self.assertEqual(process_mock.call_count, 1, 'process_response_email should be called once')
21+
(msg,) = process_mock.call_args.args
2122
self.assertEqual(
22-
process_mock.call_args.args,
23-
('contents',),
23+
msg.decode(),
24+
'contents',
2425
'process_response_email should receive the correct contents'
2526
)
2627

@@ -52,16 +53,15 @@ def test_send_error_to_admin(self, process_mock, send_smtp_mock):
5253
@mock.patch('ietf.utils.management.base.send_smtp')
5354
@mock.patch('ietf.ipr.management.commands.process_email.process_response_email')
5455
def test_invalid_character_encodings(self, process_mock, send_smtp_mock):
55-
"""The process_email command should attach messages with invalid encoding when using a file input"""
56+
"""The process_email command should accept messages with invalid encoding when using a file input"""
5657
invalid_characters = b'\xfe\xff'
5758
with name_of_file_containing(invalid_characters, mode='wb') as filename:
5859
call_command('process_email', email_file=filename)
5960

60-
self.assertFalse(process_mock.called) # should not even try to process illegally encoded messages
61-
self.assertTrue(send_smtp_mock.called)
62-
(msg,) = send_smtp_mock.call_args.args
63-
parts = msg.get_payload()
64-
self.assertEqual(len(parts), 3, 'Error email should contain message, traceback, and original message')
61+
self.assertFalse(send_smtp_mock.called) # should not send an error email
62+
self.assertTrue(process_mock.called)
63+
(msg,) = process_mock.call_args.args
64+
self.assertEqual(msg, invalid_characters, 'Invalid unicode should be passed to process_email()')
6565

6666
@mock.patch.object(sys.stdin.buffer, 'read')
6767
@mock.patch('ietf.utils.management.base.send_smtp')
@@ -72,8 +72,7 @@ def test_invalid_character_encodings_via_stdin(self, process_mock, send_smtp_moc
7272
stdin_read_mock.return_value = invalid_characters
7373
call_command('process_email')
7474

75-
self.assertFalse(process_mock.called) # should not even try to process illegally encoded messages
76-
self.assertTrue(send_smtp_mock.called)
77-
(msg,) = send_smtp_mock.call_args.args
78-
parts = msg.get_payload()
79-
self.assertEqual(len(parts), 3, 'Error email should contain message, traceback, and original message')
75+
self.assertFalse(send_smtp_mock.called) # should not send an error email
76+
self.assertTrue(process_mock.called)
77+
(msg,) = process_mock.call_args.args
78+
self.assertEqual(msg, invalid_characters, 'Invalid unicode should be passed to process_email()')

‎ietf/ipr/tests.py‎

Lines changed: 49 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -592,8 +592,7 @@ def test_notify_generic(self):
592592
self.assertEqual(len(outbox),2)
593593
self.assertIn('Secretariat on '+ipr.get_latest_event_submitted().time.strftime("%Y-%m-%d"), get_payload_text(outbox[1]).replace('\n',' '))
594594

595-
def test_process_response_email(self):
596-
# first send a mail
595+
def send_ipr_email_helper(self):
597596
ipr = HolderIprDisclosureFactory()
598597
url = urlreverse('ietf.ipr.views.email',kwargs={ "id": ipr.id })
599598
self.client.login(username="secretary", password="secretary+password")
@@ -607,27 +606,32 @@ def test_process_response_email(self):
607606
response_due=yesterday.isoformat())
608607
empty_outbox()
609608
r = self.client.post(url,data,follow=True)
610-
#print r.content
611609
self.assertEqual(r.status_code,200)
612610
q = Message.objects.filter(reply_to=data['reply_to'])
613611
self.assertEqual(q.count(),1)
614612
event = q[0].msgevents.first()
615613
self.assertTrue(event.response_past_due())
616614
self.assertEqual(len(outbox), 1)
617615
self.assertTrue('joe@test.com' in outbox[0]['To'])
618-
616+
return data['reply_to'], event
617+
618+
uninteresting_ipr_message_strings = [
619+
("To: {to}\nCc: {cc}\nFrom: joe@test.com\nDate: {date}\nSubject: test\n"),
620+
("Cc: {cc}\nFrom: joe@test.com\nDate: {date}\nSubject: test\n"), # no To
621+
("To: {to}\nFrom: joe@test.com\nDate: {date}\nSubject: test\n"), # no Cc
622+
("From: joe@test.com\nDate: {date}\nSubject: test\n"), # no To or Cc
623+
("Cc: {cc}\nDate: {date}\nSubject: test\n"), # no To
624+
("To: {to}\nDate: {date}\nSubject: test\n"), # no Cc
625+
("Date: {date}\nSubject: test\n"), # no To or Cc
626+
]
627+
628+
def test_process_response_email(self):
629+
# first send a mail
630+
reply_to, event = self.send_ipr_email_helper()
631+
619632
# test process response uninteresting messages
620633
addrs = gather_address_lists('ipr_disclosure_submitted').as_strings()
621-
uninteresting_message_strings = [
622-
("To: {to}\nCc: {cc}\nFrom: joe@test.com\nDate: {date}\nSubject: test\n"),
623-
("Cc: {cc}\nFrom: joe@test.com\nDate: {date}\nSubject: test\n"), # no To
624-
("To: {to}\nFrom: joe@test.com\nDate: {date}\nSubject: test\n"), # no Cc
625-
("From: joe@test.com\nDate: {date}\nSubject: test\n"), # no To or Cc
626-
("Cc: {cc}\nDate: {date}\nSubject: test\n"), # no To
627-
("To: {to}\nDate: {date}\nSubject: test\n"), # no Cc
628-
("Date: {date}\nSubject: test\n"), # no To or Cc
629-
]
630-
for message_string in uninteresting_message_strings:
634+
for message_string in self.uninteresting_ipr_message_strings:
631635
result = process_response_email(
632636
message_string.format(
633637
to=addrs.to,
@@ -642,12 +646,41 @@ def test_process_response_email(self):
642646
From: joe@test.com
643647
Date: {}
644648
Subject: test
645-
""".format(data['reply_to'],datetime.datetime.now().ctime())
649+
""".format(reply_to, datetime.datetime.now().ctime())
646650
result = process_response_email(message_string)
647651

648-
self.assertIsInstance(result,Message)
652+
self.assertIsInstance(result, Message)
649653
self.assertFalse(event.response_past_due())
650654

655+
def test_process_response_email_with_invalid_encoding(self):
656+
"""Interesting emails with invalid encoding should be handled"""
657+
reply_to, _ = self.send_ipr_email_helper()
658+
# test process response
659+
message_string = """To: {}
660+
From: joe@test.com
661+
Date: {}
662+
Subject: test
663+
""".format(reply_to, datetime.datetime.now().ctime())
664+
message_bytes = message_string.encode('utf8') + b'\nInvalid stuff: \xfe\xff\n'
665+
result = process_response_email(message_bytes)
666+
self.assertIsInstance(result, Message)
667+
# \ufffd is a rhombus character with an inverse ?, used to replace invalid characters
668+
self.assertEqual(result.body, 'Invalid stuff: \ufffd\ufffd\n\n', # not sure where the extra \n is from
669+
'Invalid characters should be replaced with \ufffd characters')
670+
671+
def test_process_response_email_uninteresting_with_invalid_encoding(self):
672+
"""Uninteresting emails with invalid encoding should be quietly dropped"""
673+
self.send_ipr_email_helper()
674+
addrs = gather_address_lists('ipr_disclosure_submitted').as_strings()
675+
for message_string in self.uninteresting_ipr_message_strings:
676+
message_bytes = message_string.format(
677+
to=addrs.to,
678+
cc=addrs.cc,
679+
date=datetime.datetime.now().ctime(),
680+
).encode('utf8') + b'\nInvalid stuff: \xfe\xff\n'
681+
result = process_response_email(message_bytes)
682+
self.assertIsNone(result)
683+
651684
def test_ajax_search(self):
652685
url = urlreverse('ietf.ipr.views.ajax_search')
653686
response=self.client.get(url+'?q=disclosure')

0 commit comments

Comments
 (0)