diff --git a/ietf/ipr/management/commands/process_email.py b/ietf/ipr/management/commands/process_email.py index 64ae61df8..b8ea0a0b5 100644 --- a/ietf/ipr/management/commands/process_email.py +++ b/ietf/ipr/management/commands/process_email.py @@ -23,12 +23,9 @@ class Command(EmailOnFailureCommand): def handle(self, *args, **options): email = options.get('email', None) - if not email: - msg = sys.stdin.read() - self.msg_bytes = msg.encode() - else: - self.msg_bytes = io.open(email, "rb").read() - msg = self.msg_bytes.decode() + binary_input = io.open(email, 'rb') if email else sys.stdin.buffer + self.msg_bytes = binary_input.read() + msg = self.msg_bytes.decode() try: process_response_email(msg) diff --git a/ietf/ipr/management/tests.py b/ietf/ipr/management/tests.py index d3b836848..de7e09874 100644 --- a/ietf/ipr/management/tests.py +++ b/ietf/ipr/management/tests.py @@ -2,6 +2,7 @@ # -*- coding: utf-8 -*- """Tests of ipr management commands""" import mock +import sys from django.core.management import call_command from django.test.utils import override_settings @@ -26,7 +27,7 @@ class ProcessEmailTests(TestCase): @mock.patch('ietf.utils.management.base.send_smtp') @mock.patch('ietf.ipr.management.commands.process_email.process_response_email') def test_send_error_to_admin(self, process_mock, send_smtp_mock): - """The process_email command should email the admins on error""" + """The process_email command should email the admins on error in process_response_email""" # arrange an mock error during processing process_mock.side_effect = RuntimeError('mock error') @@ -47,3 +48,32 @@ class ProcessEmailTests(TestCase): self.assertIn('mock.py', content, 'File where error occurred should be included in error email') self.assertIn('traceback', traceback.lower(), 'Traceback should be attached to error email') self.assertEqual(original, 'contents', 'Original message should be attached to error email') + + @mock.patch('ietf.utils.management.base.send_smtp') + @mock.patch('ietf.ipr.management.commands.process_email.process_response_email') + def test_invalid_character_encodings(self, process_mock, send_smtp_mock): + """The process_email command should attach messages with invalid encoding when using a file input""" + invalid_characters = b'\xfe\xff' + with name_of_file_containing(invalid_characters, mode='wb') as filename: + call_command('process_email', email_file=filename) + + self.assertFalse(process_mock.called) # should not even try to process illegally encoded messages + self.assertTrue(send_smtp_mock.called) + (msg,) = send_smtp_mock.call_args.args + parts = msg.get_payload() + self.assertEqual(len(parts), 3, 'Error email should contain message, traceback, and original message') + + @mock.patch.object(sys.stdin.buffer, 'read') + @mock.patch('ietf.utils.management.base.send_smtp') + @mock.patch('ietf.ipr.management.commands.process_email.process_response_email') + def test_invalid_character_encodings_via_stdin(self, process_mock, send_smtp_mock, stdin_read_mock): + """The process_email command should attach messages with invalid encoding when using stdin""" + invalid_characters = b'\xfe\xff' + stdin_read_mock.return_value = invalid_characters + call_command('process_email') + + self.assertFalse(process_mock.called) # should not even try to process illegally encoded messages + self.assertTrue(send_smtp_mock.called) + (msg,) = send_smtp_mock.call_args.args + parts = msg.get_payload() + self.assertEqual(len(parts), 3, 'Error email should contain message, traceback, and original message') diff --git a/ietf/nomcom/management/commands/feedback_email.py b/ietf/nomcom/management/commands/feedback_email.py index ef2958adf..32e9c9aa2 100644 --- a/ietf/nomcom/management/commands/feedback_email.py +++ b/ietf/nomcom/management/commands/feedback_email.py @@ -42,10 +42,8 @@ class Command(EmailOnFailureCommand): except NomCom.DoesNotExist: raise CommandError("NomCom %s does not exist or it isn't active" % year) - if not email: - self.msg = io.open(sys.stdin.fileno(), 'rb').read() - else: - self.msg = io.open(email, "rb").read() + binary_input = io.open(email, 'rb') if email else sys.stdin.buffer + self.msg = binary_input.read() try: feedback = create_feedback_email(self.nomcom, self.msg) diff --git a/ietf/nomcom/management/tests.py b/ietf/nomcom/management/tests.py index 03739d043..fd178786d 100644 --- a/ietf/nomcom/management/tests.py +++ b/ietf/nomcom/management/tests.py @@ -2,6 +2,7 @@ # -*- coding: utf-8 -*- """Tests of nomcom management commands""" import mock +import sys from collections import namedtuple @@ -83,3 +84,30 @@ class FeedbackEmailTests(TestCase): (self.nomcom, b'feedback message'), 'feedback_email should process the correct email for the correct nomcom' ) + + @mock.patch('ietf.utils.management.base.send_smtp') + def test_invalid_character_encodings(self, send_smtp_mock): + """The feedback_email command should send a message when file input has invalid encoding""" + # mock an exception in create_feedback_email() + invalid_characters = b'\xfe\xff' + with name_of_file_containing(invalid_characters, mode='wb') as filename: + call_command('feedback_email', nomcom_year=self.year, email_file=filename) + + self.assertTrue(send_smtp_mock.called) + (msg,) = send_smtp_mock.call_args.args # get the message to be sent + parts = msg.get_payload() + self.assertEqual(len(parts), 2, 'Error email should contain message and original message') + + @mock.patch.object(sys.stdin.buffer, 'read') + @mock.patch('ietf.utils.management.base.send_smtp') + def test_invalid_character_encodings_via_stdin(self, send_smtp_mock, stdin_read_mock): + """The feedback_email command should send a message when stdin input has invalid encoding""" + # mock an exception in create_feedback_email() + invalid_characters = b'\xfe\xff' + stdin_read_mock.return_value = invalid_characters + call_command('feedback_email', nomcom_year=self.year) + + self.assertTrue(send_smtp_mock.called) + (msg,) = send_smtp_mock.call_args.args # get the message to be sent + parts = msg.get_payload() + self.assertEqual(len(parts), 2, 'Error email should contain message and original message') diff --git a/ietf/utils/test_utils.py b/ietf/utils/test_utils.py index add7bedfd..1098a0acb 100644 --- a/ietf/utils/test_utils.py +++ b/ietf/utils/test_utils.py @@ -104,9 +104,9 @@ def reload_db_objects(*objects): return t @contextmanager -def name_of_file_containing(contents): +def name_of_file_containing(contents, mode='w'): """Get a context with the name of an email file""" - f = NamedTemporaryFile('w', delete=False) + f = NamedTemporaryFile(mode, delete=False) f.write(contents) f.close() yield f.name # hand the filename to the context