GNU Mailman

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

  1. bug-1158721
  2. Merge into 3.0
Proposed by Aurélien Bompard
Status: Merged
Approved by: Barry Warsaw
Approved revision: 7270
Merged at revision: 7270
Proposed branch: lp:~abompard/mailman/bug-1158721
Merge into: lp:mailman
Diff against target: 54 lines (+35/-1)
2 files modified
src/mailman/rules/approved.py (+4/-1)
src/mailman/rules/tests/test_approved.py (+31/-0)
To merge this branch: bzr merge lp:~abompard/mailman/bug-1158721
Reviewer Review Type Date Requested Status
Barry Warsaw Approve
Review via email: mp+243229@code.launchpad.net

Description of the change

Emails without a text/plain part crash the Approved rule, this branch contains the fix for bug #1158721.

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

I can't help but tweak a little :) but LGTM, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/mailman/rules/approved.py'
2--- src/mailman/rules/approved.py 2014-01-01 14:59:42 +0000
3+++ src/mailman/rules/approved.py 2014-11-30 10:31:24 +0000
4@@ -73,7 +73,10 @@
5 stripped = False
6 for part in typed_subpart_iterator(msg, 'text', 'plain'):
7 break
8- payload = part.get_payload(decode=True)
9+ if part is None:
10+ payload = None
11+ else:
12+ payload = part.get_payload(decode=True)
13 if payload is not None:
14 charset = part.get_content_charset('us-ascii')
15 payload = payload.decode(charset, 'replace')
16
17=== modified file 'src/mailman/rules/tests/test_approved.py'
18--- src/mailman/rules/tests/test_approved.py 2014-01-01 14:59:42 +0000
19+++ src/mailman/rules/tests/test_approved.py 2014-11-30 10:31:24 +0000
20@@ -491,3 +491,34 @@
21 self.assertFalse(result)
22 self.assertEqual(self._mlist.moderator_password,
23 b'{plaintext}super secret')
24+
25+
26+class TestApprovedNoPlainText(unittest.TestCase):
27+ """Test the approved handler with HTML-only messages."""
28+
29+ layer = ConfigLayer
30+
31+ def setUp(self):
32+ self._mlist = create_list('test@example.com')
33+ self._rule = approved.Approved()
34+
35+ def test_noplaintext(self):
36+ # When the message body only contains HTML, the rule should not throw
37+ # AttributeError: 'NoneType' object has no attribute 'get_payload'
38+ # LP: #1158721
39+ msg = mfs("""\
40+From: anne@example.com
41+To: test@example.com
42+Subject: HTML only email
43+Message-ID: <ant>
44+MIME-Version: 1.0
45+Content-Type: text/html; charset="Windows-1251"
46+Content-Transfer-Encoding: 7bit
47+
48+<HTML>
49+<BODY>
50+<P>This message contains only HTML, no plain/text part</P>
51+</BODY></HTML>
52+""")
53+ result = self._rule.check(self._mlist, msg, {})
54+ self.assertFalse(result)