* Fixed Unicode errors in the digest runner and when sending messages to the

   site owner as a fallback.  Given by Aurélien Bompard.  (LP: #1130957).

Also:

 * Convert some uses of the unicode() built-in to bytes.decode() in
   preparation for Python 3 and to eliminate some pyflakes errors.

 * Added LogFileMark.read() as a convenience method.

6 files changed, 158 insertions, 5 deletions

diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index 6223dc9d0..5afc6a1e0 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -12,6 +12,11 @@ Here is a history of user visible changes to Mailman.
 ====================================
 (2014-XX-XX)
 
+Bugs
+----
+ * Fixed Unicode errors in the digest runner and when sending messages to the
+   site owner as a fallback.  Given by Aurélien Bompard.  (LP: #1130957).
+
 Commands
 --------
  * The `mailman conf` command no longer takes the `-t/--sort` option; the
diff --git a/src/mailman/handlers/owner_recipients.py b/src/mailman/handlers/owner_recipients.py
index de6d575e1..5a1d0bd2e 100644
--- a/src/mailman/handlers/owner_recipients.py
+++ b/src/mailman/handlers/owner_recipients.py
@@ -55,7 +55,12 @@ class OwnerRecipients:
         # To prevent -owner messages from going into a black hole, if there
         # are no administrators available, the message goes to the site owner.
         if len(recipients) == 0:
-            msgdata['recipients'] = set((config.mailman.site_owner,))
+            # Ensure that the site owner address is a unicode.
+            # See LP: #1130957
+            site_owner = config.mailman.site_owner
+            if isinstance(site_owner, bytes):
+                site_owner = site_owner.decode('utf-8')
+            msgdata['recipients'] = set((site_owner,))
         else:
             msgdata['recipients'] = recipients
         # Don't decorate these messages with the header/footers.  Eventually
diff --git a/src/mailman/handlers/tests/test_recipients.py b/src/mailman/handlers/tests/test_recipients.py
index 04789c281..afe533a7e 100644
--- a/src/mailman/handlers/tests/test_recipients.py
+++ b/src/mailman/handlers/tests/test_recipients.py
@@ -198,3 +198,24 @@ To: test-owner@example.com
         msgdata = {}
         self._process(self._mlist, self._msg, msgdata)
         self.assertEqual(msgdata['recipients'], set(('noreply@example.com',)))
+
+    def test_site_admin_unicode(self):
+        # Since the config file is read as bytes, the site_owner is also a
+        # bytes and must be converted to unicode when used as a fallback.
+        self._cris.unsubscribe()
+        self._dave.unsubscribe()
+        self.assertEqual(self._mlist.administrators.member_count, 0)
+        msgdata = {}
+        # In order to properly mimic the testing environment, use
+        # config.push()/config.pop() directly instead of using the
+        # configuration() context manager.
+        config.push('test_site_admin_unicode', b"""\
+[mailman]
+site_owner: siteadmin@example.com
+""")
+        try:
+            self._process(self._mlist, self._msg, msgdata)
+        finally:
+            config.pop('test_site_admin_unicode')
+        self.assertEqual(len(msgdata['recipients']), 1)
+        self.assertIsInstance(list(msgdata['recipients'])[0], unicode)
diff --git a/src/mailman/runners/digest.py b/src/mailman/runners/digest.py
index f2c888275..98fe82f39 100644
--- a/src/mailman/runners/digest.py
+++ b/src/mailman/runners/digest.py
@@ -148,7 +148,7 @@ class Digester:
             for value in keepers[header]:
                 msg[header] = value
         # Add some useful extra stuff.
-        msg['Message'] = unicode(count)
+        msg['Message'] = count.decode('utf-8')
 
 
 
@@ -260,13 +260,18 @@ class RFC1153Digester(Digester):
         # Add the payload.  If the decoded payload is empty, this may be a
         # multipart message.  In that case, just stringify it.
         payload = msg.get_payload(decode=True)
-        payload = (payload if payload else msg.as_string().split('\n\n', 1)[1])
+        if not payload:
+            # Split using bytes so as not to turn the payload into unicode
+            # strings due to unicode_literals above.
+            payload = msg.as_string().split(b'\n\n', 1)[1]
         try:
+            # Do the decoding inside the try/except so that if the charset
+            # conversion fails, we'll just drop back to ascii.
             charset = msg.get_content_charset('us-ascii')
-            payload = unicode(payload, charset, 'replace')
+            payload = payload.decode(charset, 'replace')
         except (LookupError, TypeError):
             # Unknown or empty charset.
-            payload = unicode(payload, 'us-ascii', 'replace')
+            payload = payload.decode('us-ascii', 'replace')
         print(payload, file=self._text)
         if not payload.endswith('\n'):
             print(file=self._text)
diff --git a/src/mailman/runners/tests/test_digest.py b/src/mailman/runners/tests/test_digest.py
new file mode 100644
index 000000000..80cf253bc
--- /dev/null
+++ b/src/mailman/runners/tests/test_digest.py
@@ -0,0 +1,112 @@
+# Copyright (C) 2014 by the Free Software Foundation, Inc.
+#
+# This file is part of GNU Mailman.
+#
+# GNU Mailman is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option)
+# any later version.
+#
+# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test the digest runner."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'TestDigest',
+    ]
+
+
+import os
+import unittest
+
+from email.mime.text import MIMEText
+from mailman.app.lifecycle import create_list
+from mailman.config import config
+from mailman.email.message import Message
+from mailman.runners.digest import DigestRunner
+from mailman.testing.helpers import (
+    LogFileMark, digest_mbox, get_queue_messages, make_testable_runner,
+    specialized_message_from_string as mfs)
+from mailman.testing.layers import ConfigLayer
+
+
+
+class TestDigest(unittest.TestCase):
+    """Test the digest runner."""
+
+    layer = ConfigLayer
+
+    def setUp(self):
+        self._mlist = create_list('test@example.com')
+        self._mlist.digest_size_threshold = 1
+        self._digestq = config.switchboards['digest']
+        self._shuntq = config.switchboards['shunt']
+        self._virginq = config.switchboards['virgin']
+        self._runner = make_testable_runner(DigestRunner, 'digest')
+        self._process = config.handlers['to-digest'].process
+
+    def test_simple_message(self):
+        msg = mfs("""\
+From: anne@example.org
+To: test@example.com
+
+message triggering a digest
+""")
+        mbox_path = os.path.join(self._mlist.data_path, 'digest.mmdf')
+        self._process(self._mlist, msg, {})
+        self._digestq.enqueue(
+            msg,
+            listname=self._mlist.fqdn_listname,
+            digest_path=mbox_path,
+            volume=1, digest_number=1)
+        self._runner.run()
+        # There are two messages in the virgin queue: the digest as plain-text
+        # and as multipart.
+        messages = get_queue_messages('virgin')
+        self.assertEqual(len(messages), 2)
+        self.assertEqual(
+            sorted(item.msg.get_content_type() for item in messages),
+            ['multipart/mixed', 'text/plain'])
+        for item in messages:
+            self.assertEqual(item.msg['subject'],
+                             'Test Digest, Vol 1, Issue 1')
+
+    def test_non_ascii_message(self):
+        msg = Message()
+        msg['From'] = 'anne@example.org'
+        msg['To'] = 'test@example.com'
+        msg['Content-Type'] = 'multipart/mixed'
+        msg.attach(MIMEText('message with non-ascii chars: \xc3\xa9',
+                            'plain', 'utf-8'))
+        mbox = digest_mbox(self._mlist)
+        mbox_path = os.path.join(self._mlist.data_path, 'digest.mmdf')
+        mbox.add(msg.as_string())
+        self._digestq.enqueue(
+            msg,
+            listname=self._mlist.fqdn_listname,
+            digest_path=mbox_path,
+            volume=1, digest_number=1)
+        # Use any error logs as the error message if the test fails.
+        error_log = LogFileMark('mailman.error')
+        self._runner.run()
+        # The runner will send the file to the shunt queue on exception.
+        self.assertEqual(len(self._shuntq.files), 0, error_log.read())
+        # There are two messages in the virgin queue: the digest as plain-text
+        # and as multipart.
+        messages = get_queue_messages('virgin')
+        self.assertEqual(len(messages), 2)
+        self.assertEqual(
+            sorted(item.msg.get_content_type() for item in messages),
+            ['multipart/mixed', 'text/plain'])
+        for item in messages:
+            self.assertEqual(item.msg['subject'],
+                             'Test Digest, Vol 1, Issue 1')
diff --git a/src/mailman/testing/helpers.py b/src/mailman/testing/helpers.py
index aab23ab75..b0fe14a0d 100644
--- a/src/mailman/testing/helpers.py
+++ b/src/mailman/testing/helpers.py
@@ -524,3 +524,8 @@ class LogFileMark:
         with open(self._filename) as fp:
             fp.seek(self._filepos)
             return fp.readline()
+
+    def read(self):
+        with open(self._filename) as fp:
+            fp.seek(self._filepos)
+            return fp.read()