Work in progress on unicode chars

.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:

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='iso8859-1', errors='ignore')
-            return str(string, encoding=charset, errors='replace')
+            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(
+                body = encoding.force_text(EmailReplyParser.parse_reply(
-                    decodeUnknown(part.get_content_charset(), part.get_payload(decode=True))
+                    decodeUnknown(part.get_content_charset(), encoding.force_text(part.get_payload(decode=True)))
-                )
+                ))
                 logger.debug("Discovered plain text MIME part")
             else:
                 files.append(

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]