diff --git a/.travis.yml b/.travis.yml index 62879e4d..37e94b6b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ python: - "2.7" - 3.4.4 - "3.5" + - "3.6" env: - DJANGO=1.8.17 @@ -18,7 +19,7 @@ install: before_script: - "pep8 --exclude=migrations --ignore=E501 helpdesk" -script: +script: - coverage run --source='.' quicktest.py helpdesk after_success: diff --git a/helpdesk/management/commands/get_email.py b/helpdesk/management/commands/get_email.py index 97b5b8cc..68e74ecb 100644 --- a/helpdesk/management/commands/get_email.py +++ b/helpdesk/management/commands/get_email.py @@ -10,6 +10,7 @@ scripts/get_email.py - Designed to be run from cron, this script checks the helpdesk, creating tickets from the new messages (or adding to existing tickets if needed) """ +from __future__ import unicode_literals from datetime import timedelta import email @@ -158,7 +159,7 @@ def process_queue(q, logger): msgNum = msg.split(" ")[0] logger.info("Processing message %s" % msgNum) - full_message = "\n".join(server.retr(msgNum)[1]) + full_message = encoding.force_text("\n".join(server.retr(msgNum)[1]), errors='ignore') ticket = ticket_from_message(message=full_message, queue=q, logger=logger) if ticket: @@ -198,7 +199,8 @@ def process_queue(q, logger): for num in msgnums: logger.info("Processing message %s" % num) status, data = server.fetch(num, '(RFC822)') - ticket = ticket_from_message(message=encoding.smart_text(data[0][1], errors='replace'), queue=q, logger=logger) + full_message = encoding.force_text(data[0][1], errors='ignore') + ticket = ticket_from_message(message=full_message, queue=q, logger=logger) if ticket: server.store(num, '+FLAGS', '\\Deleted') logger.info("Successfully processed message %s, deleted from IMAP server" % num) @@ -218,7 +220,8 @@ def process_queue(q, logger): for i, m in enumerate(mail, 1): logger.info("Processing message %d" % i) with open(m, 'r') as f: - ticket = ticket_from_message(message=f.read(), queue=q, logger=logger) + full_message = encoding.force_text(f.read(), errors='ignore') + ticket = ticket_from_message(message=full_message, queue=q, logger=logger) if ticket: logger.info("Successfully processed message %d, ticket/comment created." % i) try: @@ -243,10 +246,10 @@ def decodeUnknown(charset, string): if type(string) is not str: if not charset: try: - return str(string, encoding='utf-8', errors='replace') + return str(string, encoding='utf-8', errors='ignore') except: - return str(string, encoding='iso8859-1', errors='replace') - return str(string, encoding=charset, errors='replace') + return str(string, encoding='iso8859-1', errors='ignore') + return str(string, encoding=charset, errors='ignore') return string @@ -255,7 +258,7 @@ def decode_mail_headers(string): if six.PY2: return u' '.join([unicode(msg, charset or 'utf-8') for msg, charset in decoded]) elif six.PY3: - return u' '.join([str(msg, encoding=charset, errors='replace') if charset else str(msg) for msg, charset in decoded]) + return u' '.join([str(msg, encoding=charset, errors='ignore') if charset else str(msg) for msg, charset in decoded]) def ticket_from_message(message, queue, logger): @@ -302,9 +305,9 @@ def ticket_from_message(message, queue, logger): if part.get_content_maintype() == 'text' and name is None: if part.get_content_subtype() == 'plain': - body = EmailReplyParser.parse_reply( - decodeUnknown(part.get_content_charset(), part.get_payload(decode=True)) - ) + body = encoding.force_text(EmailReplyParser.parse_reply( + decodeUnknown(part.get_content_charset(), encoding.force_text(part.get_payload(decode=True))) + )) logger.debug("Discovered plain text MIME part") else: files.append( diff --git a/helpdesk/tests/test_get_email.py b/helpdesk/tests/test_get_email.py index 3a296514..87eddffd 100644 --- a/helpdesk/tests/test_get_email.py +++ b/helpdesk/tests/test_get_email.py @@ -1,3 +1,5 @@ +from __future__ import unicode_literals + from helpdesk.models import Queue, Ticket from django.test import TestCase from django.core.management import call_command @@ -40,7 +42,7 @@ class GetEmailCommonTests(TestCase): class GetEmailParametricTemplate(object): - """TestCase that checks email functionality accross methods and socks configs.""" + """TestCase that checks email functionality across methods and socks configs.""" def setUp(self): @@ -52,7 +54,8 @@ class GetEmailParametricTemplate(object): "allow_email_submission": True, "email_box_type": self.method, "logging_dir": self.temp_logdir, - "logging_type": 'none'} + "logging_type": 'none' + } if self.method == 'local': kwargs["email_box_local_dir"] = '/var/lib/mail/helpdesk/' @@ -74,10 +77,13 @@ class GetEmailParametricTemplate(object): def test_read_email(self): """Tests reading emails from a queue and creating tickets. For each email source supported, we mock the backend to provide - authenticly formatted responses containing our test data.""" + authentically formatted responses containing our test data.""" test_email = "To: update.public@example.com\nFrom: comment@example.com\nSubject: Some Comment\n\nThis is the helpdesk comment via email." test_mail_len = len(test_email) + test_unicode_email = "To: update.public@example.com\nFrom: comment@example.com\nSubject: Some Unicode Comment\n\nThis is the helpdesk comment via email with unicode chars \u2013 inserted for testing purposes." + test_unicode_email_len = len(test_unicode_email) + if self.socks: from socks import ProxyConnectionError with self.assertRaisesRegexp(ProxyConnectionError, '%s:%s' % (unrouted_socks_server, unused_port)): @@ -98,13 +104,25 @@ class GetEmailParametricTemplate(object): mocked_isfile.assert_any_call('/var/lib/mail/helpdesk/filename1') mocked_isfile.assert_any_call('/var/lib/mail/helpdesk/filename2') + with mock.patch('helpdesk.management.commands.get_email.listdir') as mocked_listdir, \ + mock.patch('helpdesk.management.commands.get_email.isfile') as mocked_isfile, \ + mock.patch('builtins.open' if six.PY3 else '__builtin__.open', mock.mock_open(read_data=test_unicode_email)): + mocked_isfile.return_value = True + mocked_listdir.return_value = ['filename3'] + + call_command('get_email') + + mocked_listdir.assert_called_with('/var/lib/mail/helpdesk/') + mocked_isfile.assert_any_call('/var/lib/mail/helpdesk/filename3') + elif self.method == 'pop3': # mock poplib.POP3's list and retr methods to provide responses as per RFC 1939 pop3_emails = { '1': ("+OK", test_email.split('\n')), '2': ("+OK", test_email.split('\n')), + '3': ("+OK", test_unicode_email.split('\n')), } - pop3_mail_list = ("+OK 2 messages", ("1 %d" % test_mail_len, "2 %d" % test_mail_len)) + pop3_mail_list = ("+OK 3 messages", ("1 %d" % test_mail_len, "2 %d" % test_mail_len, "3 %d" % test_unicode_email_len)) mocked_poplib_server = mock.Mock() mocked_poplib_server.list = mock.Mock(return_value=pop3_mail_list) mocked_poplib_server.retr = mock.Mock(side_effect=lambda x: pop3_emails[x]) @@ -117,8 +135,9 @@ class GetEmailParametricTemplate(object): imap_emails = { "1": ("OK", (("1", test_email),)), "2": ("OK", (("2", test_email),)), + "3": ("OK", (("3", test_unicode_email),)), } - imap_mail_list = ("OK", ("1 2",)) + imap_mail_list = ("OK", ("1 2 3",)) mocked_imaplib_server = mock.Mock() mocked_imaplib_server.search = mock.Mock(return_value=imap_mail_list) @@ -136,6 +155,10 @@ class GetEmailParametricTemplate(object): self.assertEqual(ticket2.ticket_for_url, "QQ-%s" % ticket2.id) self.assertEqual(ticket2.description, "This is the helpdesk comment via email.") + ticket3 = get_object_or_404(Ticket, pk=3) + self.assertEqual(ticket3.ticket_for_url, "QQ-%s" % ticket3.id) + self.assertEqual(ticket3.description, "This is the helpdesk comment via email with unicode chars \u2013 inserted for testing purposes.") + # build matrix of test cases case_methods = [c[0] for c in Queue._meta.get_field('email_box_type').choices]