Merge lp:~macobo/mailman/macobo-rcpt-stage-reject into lp:mailman

Proposed by Karl-Aksel Puulmann on 2013-04-06
Status: Needs review
Proposed branch: lp:~macobo/mailman/macobo-rcpt-stage-reject
Merge into: lp:mailman
Diff against target: 82 lines (+48/-0)
2 files modified
src/mailman/runners/lmtp.py (+18/-0)
src/mailman/runners/tests/test_lmtp.py (+30/-0)
To merge this branch: bzr merge lp:~macobo/mailman/macobo-rcpt-stage-reject
Reviewer Review Type Date Requested Status
Mailman Coders 2013-04-06 Pending
Review via email: mp+157516@code.launchpad.net

Description of the change

It probably needs a little more work before merging.

Open questions:
1) Is the test naming okay?
2) Should the tests be done in a different way? I didn't find any other tests that used the mock library.
3) Should the mocking be done within the setUp stage?
4) If that's okay, should get_mocked_lmtp_channel be moved to tests/helpers.py?

To post a comment you must log in.
7213. By Karl-Aksel Puulmann on 2013-04-06

Remove white-space added by mistake

Unmerged revisions

7213. By Karl-Aksel Puulmann on 2013-04-06

Remove white-space added by mistake

7212. By Karl-Aksel Puulmann on 2013-04-06

Moved get_mocked_lmtp_channel to top of file

7211. By Karl-Aksel Puulmann on 2013-04-06

Fixed a LMTP bug, causing it not to validate email list addresses until DATA stage. (LP: 982555)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/mailman/runners/lmtp.py'
2--- src/mailman/runners/lmtp.py 2013-01-20 21:23:09 +0000
3+++ src/mailman/runners/lmtp.py 2013-04-06 14:55:22 +0000
4@@ -147,6 +147,24 @@
5 """HELO is not a valid LMTP command."""
6 self.push(ERR_502)
7
8+ def smtp_RCPT(self, arg):
9+ if not self._SMTPChannel__mailfrom:
10+ self.push(b'503 Error: need MAIL command')
11+ return
12+ address = self._SMTPChannel__getaddr('TO:', arg) if arg else None
13+ if not address:
14+ self.push(b'501 Syntax: RCPT TO: <address>')
15+ return
16+ # Refresh the list of list names every time we process a message
17+ # since the set of mailing lists could have changed.
18+ listnames = set(getUtility(IListManager).names)
19+ listname, subaddress, domain = split_recipient(address)
20+ listname = listname.lower() + '@' + domain
21+ if listname not in listnames:
22+ self.push(ERR_550)
23+ return
24+ self._SMTPChannel__rcpttos.append(address)
25+ self.push(b'250 Ok')
26
27
28
29 class LMTPRunner(Runner, smtpd.SMTPServer):
30
31=== modified file 'src/mailman/runners/tests/test_lmtp.py'
32--- src/mailman/runners/tests/test_lmtp.py 2013-03-06 21:59:15 +0000
33+++ src/mailman/runners/tests/test_lmtp.py 2013-04-06 14:55:22 +0000
34@@ -29,8 +29,10 @@
35 import smtplib
36 import unittest
37
38+from mock import Mock
39 from datetime import datetime
40
41+from mailman.runners.lmtp import Channel
42 from mailman.config import config
43 from mailman.app.lifecycle import create_list
44 from mailman.database.transaction import transaction
45@@ -38,6 +40,14 @@
46 from mailman.testing.layers import LMTPLayer
47
48
49+def get_mocked_lmtp_channel():
50+ connection = Mock()
51+ connection.getpeername = Mock(return_value='FakePeer')
52+ channel = Channel(None, connection, None)
53+ channel.push = Mock()
54+ return channel
55+
56+
57
58
59 class TestLMTP(unittest.TestCase):
60 """Test various aspects of the LMTP server."""
61@@ -144,3 +154,23 @@
62 self.assertEqual(len(messages), 1)
63 self.assertEqual(messages[0].msgdata['listname'],
64 'my-list@example.com')
65+
66+ def test_lp982555_RCPT_before_mail(self):
67+ channel = get_mocked_lmtp_channel()
68+ channel.smtp_RCPT('TO: <thislist@doesnotexist.com>')
69+ channel.push.assert_called_with('503 Error: need MAIL command')
70+
71+ def test_lp982555_RCPT_existing_list(self):
72+ with transaction():
73+ create_list('my-list@example.com')
74+ channel = get_mocked_lmtp_channel()
75+ channel.smtp_MAIL('FROM: anne@example.com')
76+ channel.smtp_RCPT('TO: <my-list@example.com>')
77+ channel.push.assert_called_with('250 Ok')
78+
79+ def test_lp982555_RCPT_nonexisting_list(self):
80+ channel = get_mocked_lmtp_channel()
81+ channel.smtp_MAIL('FROM: anne@example.com')
82+ channel.smtp_RCPT('TO: <thislist@doesnotexist.com>')
83+ channel.push.assert_called_with(
84+ '550 Requested action not taken: mailbox unavailable')