Mark Sapiro pushed to branch master at GNU Mailman / Mailman Core
Commits:
00ea75ea by Mark Sapiro at 2019-01-12T18:52:31Z
Outgoing delivery closes the smtp connection after each message.
- - - - -
25e22287 by Mark Sapiro at 2019-01-12T20:09:15Z
Merge branch 'smtp_quit' into 'master'
Outgoing delivery closes the smtp connection after each message.
Closes #529
See merge request mailman/mailman!432
- - - - -
4 changed files:
- src/mailman/docs/NEWS.rst
- src/mailman/mta/deliver.py
- src/mailman/mta/tests/test_connection.py
- src/mailman/mta/tests/test_delivery.py
Changes:
=====================================
src/mailman/docs/NEWS.rst
=====================================
@@ -29,6 +29,8 @@ Bugs
(Closes #504)
* Message parts are now properly decoded when trying to remove an Approved:
header. (Closes #518)
+* Outgoing SMTP connections are now closed following message delivery
+ regardless of the max_sessions_per_connection setting. (Closes #529)
REST
----
=====================================
src/mailman/mta/deliver.py
=====================================
@@ -84,6 +84,15 @@ def deliver(mlist, msg, msgdata):
# for re-delivery later.
t0 = time.time()
refused = agent.deliver(mlist, msg, msgdata)
+ # At this point we have completed the initial SMTP for this message.
+ # We should close the SMTP connection regardless of the
+ # sessions_per_connection setting because otherwise if there are no more
+ # messages in the queue, the connection is left open until it times out
+ # which can cause problems in the MTA.
+ # XXX It would arguably be better to close only if the queue is empty, but
+ # this means examining the queue here or closing from the outgoing runner,
+ # either of which requires more information than should be available.
+ agent._connection.quit()
t1 = time.time()
# Log this posting.
size = getattr(msg, 'original_size', msgdata.get('original_size'))
=====================================
src/mailman/mta/tests/test_connection.py
=====================================
@@ -90,6 +90,14 @@ Subject: aardvarks
self.connection.quit()
self.assertEqual(SMTPLayer.smtpd.get_connection_count(), 2)
+ def test_count_2_no_quit(self):
+ self.connection.sendmail(
+ 'a...@example.com', ['b...@example.com'], self.msg_text)
+ self.connection.sendmail(
+ 'c...@example.com', ['d...@example.com'], self.msg_text)
+ self.connection.quit()
+ self.assertEqual(SMTPLayer.smtpd.get_connection_count(), 1)
+
def test_count_reset(self):
self.connection.sendmail(
'a...@example.com', ['b...@example.com'], self.msg_text)
=====================================
src/mailman/mta/tests/test_delivery.py
=====================================
@@ -30,7 +30,8 @@ from mailman.mta.bulk import BulkDelivery
from mailman.mta.deliver import Deliver
from mailman.testing.helpers import (
specialized_message_from_string as mfs, subscribe)
-from mailman.testing.layers import ConfigLayer
+from mailman.testing.layers import ConfigLayer, SMTPLayer
+from mailman.utilities.modules import find_name
from zope.component import getUtility
@@ -200,3 +201,61 @@ list: Test
Footer
""")
+
+
+class TestCloseAfterDelivery(unittest.TestCase):
+ """Test that connections close after delivery."""
+
+ layer = SMTPLayer
+
+ def setUp(self):
+ self._mlist = create_list('t...@example.com')
+ # Set personalized delivery.
+ self._mlist.personalize = Personalization.individual
+ # Make Anne a member of this mailing list.
+ self._anne = subscribe(self._mlist, 'Anne', email='a...@example.org')
+ self._msg = mfs("""\
+From: a...@example.org
+To: t...@example.com
+Subject: test
+
+""")
+ self._deliverer = find_name(config.mta.outgoing)
+ # Set the maximum transactions per connection.
+ config.push('maxtrans', """
+ [mta]
+ max_sessions_per_connection: 3
+ """)
+ self.addCleanup(config.pop, 'maxtrans')
+
+ def test_two_messages(self):
+ msgdata = dict(recipients=['a...@example.org'])
+ # Send a message.
+ self._deliverer(self._mlist, self._msg, msgdata)
+ # We should have made one SMTP connection.
+ self.assertEqual(SMTPLayer.smtpd.get_connection_count(), 1)
+ # Send a second message.
+ msgdata = dict(recipients=['b...@example.org'])
+ self._deliverer(self._mlist, self._msg, msgdata)
+ # This should result in a second connection.
+ self.assertEqual(SMTPLayer.smtpd.get_connection_count(), 2)
+
+ def test_one_message_two_recip(self):
+ msgdata = dict(recipients=['a...@example.org', 'b...@example.org'])
+ # Send the message.
+ self._deliverer(self._mlist, self._msg, msgdata)
+ # Sending to 2 recips sends 2 messages because of personalization.
+ # That's fewer than max_sessions_per_connection so there's only
+ # one connection.
+ self.assertEqual(SMTPLayer.smtpd.get_connection_count(), 1)
+
+ def test_one_message_four_recip(self):
+ msgdata = dict(recipients=['a...@example.org',
+ 'b...@example.org',
+ 'c...@example.com',
+ 'd...@example.com'])
+ # Send the message.
+ self._deliverer(self._mlist, self._msg, msgdata)
+ # Since max_sessions_per_connection is 3, sending 4 personalized
+ # messages creates 2 connections.
+ self.assertEqual(SMTPLayer.smtpd.get_connection_count(), 2)
View it on GitLab:
https://gitlab.com/mailman/mailman/compare/1e9597487a199f8b69bb9cf5de33e14ae7ab444e...25e2228723e878ab1fba751db349ef363d213e65
--
View it on GitLab:
https://gitlab.com/mailman/mailman/compare/1e9597487a199f8b69bb9cf5de33e14ae7ab444e...25e2228723e878ab1fba751db349ef363d213e65
You're receiving this email because of your account on gitlab.com.
_______________________________________________
Mailman-checkins mailing list
Mailman-checkins@python.org
Unsubscribe:
https://mail.python.org/mailman/options/mailman-checkins/archive%40jab.org
