Merge lp:~abompard/mailman/bug-1130957 into lp:mailman

Proposed by Aurélien Bompard
Status: Merged
Merged at revision: 7268
Proposed branch: lp:~abompard/mailman/bug-1130957
Merge into: lp:mailman
Diff against target: 151 lines (+115/-2)
4 files modified
src/mailman/handlers/owner_recipients.py (+1/-1)
src/mailman/handlers/tests/test_recipients.py (+17/-0)
src/mailman/runners/digest.py (+1/-1)
src/mailman/runners/tests/test_digest.py (+96/-0)
To merge this branch: bzr merge lp:~abompard/mailman/bug-1130957
Reviewer Review Type Date Requested Status
Barry Warsaw Approve
Review via email: mp+243060@code.launchpad.net

Description of the change

Fixes the unicode issues reported in bug #1130957 (unit tests included! ;-))

To post a comment you must log in.
Revision history for this message
Barry Warsaw (barry) wrote :

Looks great, thanks! I cleaned some things up and changed a few things stylistically. All tests pass so I committed fixes.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/mailman/handlers/owner_recipients.py'
2--- src/mailman/handlers/owner_recipients.py 2014-01-01 14:59:42 +0000
3+++ src/mailman/handlers/owner_recipients.py 2014-11-27 16:04:23 +0000
4@@ -55,7 +55,7 @@
5 # To prevent -owner messages from going into a black hole, if there
6 # are no administrators available, the message goes to the site owner.
7 if len(recipients) == 0:
8- msgdata['recipients'] = set((config.mailman.site_owner,))
9+ msgdata['recipients'] = set((unicode(config.mailman.site_owner),))
10 else:
11 msgdata['recipients'] = recipients
12 # Don't decorate these messages with the header/footers. Eventually
13
14=== modified file 'src/mailman/handlers/tests/test_recipients.py'
15--- src/mailman/handlers/tests/test_recipients.py 2014-01-01 14:59:42 +0000
16+++ src/mailman/handlers/tests/test_recipients.py 2014-11-27 16:04:23 +0000
17@@ -198,3 +198,20 @@
18 msgdata = {}
19 self._process(self._mlist, self._msg, msgdata)
20 self.assertEqual(msgdata['recipients'], set(('noreply@example.com',)))
21+
22+ def test_site_admin_unicode(self):
23+ # Since the conf file is read as a bytestring, the site_owner is also a
24+ # bytestring and must be converted to unicode when used as a fallback.
25+ self._cris.unsubscribe()
26+ self._dave.unsubscribe()
27+ self.assertEqual(self._mlist.administrators.member_count, 0)
28+ msgdata = {}
29+ strconf = b"""
30+ [mailman]
31+ site_owner: siteadmin@example.com
32+ """
33+ config.push("test_site_admin_unicode", strconf)
34+ self._process(self._mlist, self._msg, msgdata)
35+ config.pop("test_site_admin_unicode")
36+ self.assertEqual(len(msgdata['recipients']), 1)
37+ self.assertTrue(isinstance(list(msgdata['recipients'])[0], unicode))
38
39=== modified file 'src/mailman/runners/digest.py'
40--- src/mailman/runners/digest.py 2014-04-28 15:23:35 +0000
41+++ src/mailman/runners/digest.py 2014-11-27 16:04:23 +0000
42@@ -260,7 +260,7 @@
43 # Add the payload. If the decoded payload is empty, this may be a
44 # multipart message. In that case, just stringify it.
45 payload = msg.get_payload(decode=True)
46- payload = (payload if payload else msg.as_string().split('\n\n', 1)[1])
47+ payload = (payload if payload else msg.as_string().split(b'\n\n', 1)[1])
48 try:
49 charset = msg.get_content_charset('us-ascii')
50 payload = unicode(payload, charset, 'replace')
51
52=== added file 'src/mailman/runners/tests/test_digest.py'
53--- src/mailman/runners/tests/test_digest.py 1970-01-01 00:00:00 +0000
54+++ src/mailman/runners/tests/test_digest.py 2014-11-27 16:04:23 +0000
55@@ -0,0 +1,96 @@
56+# Copyright (C) 2012-2014 by the Free Software Foundation, Inc.
57+#
58+# This file is part of GNU Mailman.
59+#
60+# GNU Mailman is free software: you can redistribute it and/or modify it under
61+# the terms of the GNU General Public License as published by the Free
62+# Software Foundation, either version 3 of the License, or (at your option)
63+# any later version.
64+#
65+# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT
66+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
67+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
68+# more details.
69+#
70+# You should have received a copy of the GNU General Public License along with
71+# GNU Mailman. If not, see <http://www.gnu.org/licenses/>.
72+
73+"""Test the digest runner."""
74+
75+from __future__ import absolute_import, print_function, unicode_literals
76+
77+__metaclass__ = type
78+__all__ = [
79+ 'TestDigest',
80+ ]
81+
82+import os
83+import unittest
84+from email.mime.text import MIMEText
85+
86+from mailman.app.lifecycle import create_list
87+from mailman.config import config
88+from mailman.email.message import Message
89+from mailman.runners.digest import DigestRunner
90+from mailman.testing.helpers import (
91+ digest_mbox, make_testable_runner, reset_the_world,
92+ specialized_message_from_string as mfs)
93+from mailman.testing.layers import ConfigLayer
94+
95+
96+
97
98+class TestDigest(unittest.TestCase):
99+ """Test the digest runner."""
100+
101+ layer = ConfigLayer
102+
103+ def setUp(self):
104+ self._mlist = create_list('test@example.com')
105+ self._mlist.digest_size_threshold = 1
106+ #self._mlist.volume = 1
107+ #self._mlist.next_digest_number = 1
108+ self._digestq = config.switchboards['digest']
109+ self._shuntq = config.switchboards['shunt']
110+ self._virginq = config.switchboards['virgin']
111+ self._runner = make_testable_runner(DigestRunner, 'digest')
112+ self._process = config.handlers['to-digest'].process
113+
114+ def tearDown(self):
115+ reset_the_world()
116+
117+ def test_simple_message(self):
118+ msg = mfs("""\
119+From: anne@example.org
120+To: test@example.com
121+
122+message triggering a digest
123+""")
124+ mbox = digest_mbox(self._mlist)
125+ mbox_path = os.path.join(self._mlist.data_path, 'digest.mmdf')
126+ self._process(self._mlist, msg, {})
127+ self._digestq.enqueue(msg,
128+ listname=self._mlist.fqdn_listname,
129+ digest_path=mbox_path,
130+ volume=1, digest_number=1)
131+ self._runner.run()
132+
133+ def test_non_ascii_message(self):
134+ msg = Message()
135+ msg["From"] = "anne@example.org"
136+ msg["To"] = "test@example.com"
137+ msg["Content-Type"] = "multipart/mixed"
138+ msg.attach(MIMEText("message with non-ascii chars: \xc3\xa9",
139+ "plain", "utf-8"))
140+ mbox = digest_mbox(self._mlist)
141+ mbox_path = os.path.join(self._mlist.data_path, 'digest.mmdf')
142+ mbox.add(msg.as_string())
143+ self._digestq.enqueue(msg,
144+ listname=self._mlist.fqdn_listname,
145+ digest_path=mbox_path,
146+ volume=1, digest_number=1)
147+ self._runner.run()
148+ errorlog = open(os.path.join(config.LOG_DIR, "error")).read()
149+ # The runner will send the file to the shunt queue on exception
150+ self.assertEqual(len(self._shuntq.files), 0, errorlog)
151+ # 2 messages in the virgin queue: the digest as plain-text and as multipart
152+ self.assertEqual(len(self._virginq.files), 2, errorlog)