From d5fbe9b1a3d65ceeb9159c5ba999ee966a945f76 Mon Sep 17 00:00:00 2001 From: Pablo Aguiar Date: Sat, 8 Sep 2018 00:04:48 +0200 Subject: bpo-34246: Use no mutable default args in smtplib (GH-8554) Some methods of the SMTP class use mutable default arguments. Specially `send_message` is affected as it mutates one of the args by appending items to it, which has side effects on further calls. --- Doc/library/smtplib.rst | 4 ++-- Lib/smtplib.py | 12 +++++----- Lib/test/test_smtplib.py | 28 ++++++++++++++++++++++ Misc/ACKS | 1 + .../2018-07-29-15-25-15.bpo-34246.xiKq-Q.rst | 2 ++ 5 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-07-29-15-25-15.bpo-34246.xiKq-Q.rst diff --git a/Doc/library/smtplib.rst b/Doc/library/smtplib.rst index e5effd0..8052172 100644 --- a/Doc/library/smtplib.rst +++ b/Doc/library/smtplib.rst @@ -419,7 +419,7 @@ An :class:`SMTP` instance has the following methods: :exc:`SMTPException`. -.. method:: SMTP.sendmail(from_addr, to_addrs, msg, mail_options=[], rcpt_options=[]) +.. method:: SMTP.sendmail(from_addr, to_addrs, msg, mail_options=(), rcpt_options=()) Send mail. The required arguments are an :rfc:`822` from-address string, a list of :rfc:`822` to-address strings (a bare string will be treated as a list with 1 @@ -491,7 +491,7 @@ An :class:`SMTP` instance has the following methods: .. method:: SMTP.send_message(msg, from_addr=None, to_addrs=None, \ - mail_options=[], rcpt_options=[]) + mail_options=(), rcpt_options=()) This is a convenience method for calling :meth:`sendmail` with the message represented by an :class:`email.message.Message` object. The arguments have diff --git a/Lib/smtplib.py b/Lib/smtplib.py index b679875..048c6bf 100755 --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -513,7 +513,7 @@ class SMTP: """SMTP 'noop' command -- doesn't do anything :>""" return self.docmd("noop") - def mail(self, sender, options=[]): + def mail(self, sender, options=()): """SMTP 'mail' command -- begins mail xfer session. This method may raise the following exceptions: @@ -534,7 +534,7 @@ class SMTP: self.putcmd("mail", "FROM:%s%s" % (quoteaddr(sender), optionlist)) return self.getreply() - def rcpt(self, recip, options=[]): + def rcpt(self, recip, options=()): """SMTP 'rcpt' command -- indicates 1 recipient for this mail.""" optionlist = '' if options and self.does_esmtp: @@ -785,8 +785,8 @@ class SMTP: raise SMTPResponseException(resp, reply) return (resp, reply) - def sendmail(self, from_addr, to_addrs, msg, mail_options=[], - rcpt_options=[]): + def sendmail(self, from_addr, to_addrs, msg, mail_options=(), + rcpt_options=()): """This command performs an entire mail transaction. The arguments are: @@ -890,7 +890,7 @@ class SMTP: return senderrs def send_message(self, msg, from_addr=None, to_addrs=None, - mail_options=[], rcpt_options={}): + mail_options=(), rcpt_options=()): """Converts message to a bytestring and passes it to sendmail. The arguments are as for sendmail, except that msg is an @@ -958,7 +958,7 @@ class SMTP: if international: g = email.generator.BytesGenerator( bytesmsg, policy=msg.policy.clone(utf8=True)) - mail_options += ['SMTPUTF8', 'BODY=8BITMIME'] + mail_options = (*mail_options, 'SMTPUTF8', 'BODY=8BITMIME') else: g = email.generator.BytesGenerator(bytesmsg) g.flatten(msg_copy, linesep='\r\n') diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py index 495764f..8a29e98 100644 --- a/Lib/test/test_smtplib.py +++ b/Lib/test/test_smtplib.py @@ -20,6 +20,7 @@ import threading import unittest from test import support, mock_socket from test.support import HOST, HOSTv4, HOSTv6 +from unittest.mock import Mock if sys.platform == 'darwin': @@ -578,6 +579,33 @@ class NonConnectingTests(unittest.TestCase): "localhost:bogus") +class DefaultArgumentsTests(unittest.TestCase): + + def setUp(self): + self.msg = EmailMessage() + self.msg['From'] = 'Páolo ' + self.smtp = smtplib.SMTP() + self.smtp.ehlo = Mock(return_value=(200, 'OK')) + self.smtp.has_extn, self.smtp.sendmail = Mock(), Mock() + + def testSendMessage(self): + expected_mail_options = ('SMTPUTF8', 'BODY=8BITMIME') + self.smtp.send_message(self.msg) + self.smtp.send_message(self.msg) + self.assertEqual(self.smtp.sendmail.call_args_list[0][0][3], + expected_mail_options) + self.assertEqual(self.smtp.sendmail.call_args_list[1][0][3], + expected_mail_options) + + def testSendMessageWithMailOptions(self): + mail_options = ['STARTTLS'] + expected_mail_options = ('STARTTLS', 'SMTPUTF8', 'BODY=8BITMIME') + self.smtp.send_message(self.msg, None, None, mail_options) + self.assertEqual(mail_options, ['STARTTLS']) + self.assertEqual(self.smtp.sendmail.call_args_list[0][0][3], + expected_mail_options) + + # test response of client to a non-successful HELO message class BadHELOServerTests(unittest.TestCase): diff --git a/Misc/ACKS b/Misc/ACKS index 82fbc92..75047d8 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -22,6 +22,7 @@ Eitan Adler Anton Afanasyev Ali Afshar Nitika Agarwal +Pablo S. Blum de Aguiar Jim Ahlstrom Farhan Ahmad Matthew Ahrens diff --git a/Misc/NEWS.d/next/Library/2018-07-29-15-25-15.bpo-34246.xiKq-Q.rst b/Misc/NEWS.d/next/Library/2018-07-29-15-25-15.bpo-34246.xiKq-Q.rst new file mode 100644 index 0000000..50c91ec --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-07-29-15-25-15.bpo-34246.xiKq-Q.rst @@ -0,0 +1,2 @@ +:meth:`smtplib.SMTP.send_message` no longer modifies the content of the +*mail_options* argument. Patch by Pablo S. Blum de Aguiar. -- cgit v0.12