From 67f70fd2b7197c8136df3dd6d4267f601a275c1c Mon Sep 17 00:00:00 2001 From: "Pablo S. Blum de Aguiar" Date: Sun, 29 Jul 2018 15:37:31 +0100 Subject: [PATCH 1/7] bpo-34246: Use tuple as default arg for mail_options Some methods of the SMTP class use mutable default arguments. Specially `send_message` is affected in which it mutates one of the args by appending items to it, whic has side effects on further calls. --- Lib/smtplib.py | 4 ++-- Lib/test/test_smtplib.py | 20 +++++++++++++++++++ Misc/ACKS | 1 + .../2018-07-29-15-25-15.bpo-34246.xiKq-Q.rst | 2 ++ 4 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-07-29-15-25-15.bpo-34246.xiKq-Q.rst diff --git a/Lib/smtplib.py b/Lib/smtplib.py index b679875fd2c539..5a7f005b06bfd8 100755 --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -890,7 +890,7 @@ def sendmail(self, from_addr, to_addrs, msg, mail_options=[], 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 @@ def send_message(self, msg, from_addr=None, to_addrs=None, 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 495764f9aca902..71ae17b112594c 100644 --- a/Lib/test/test_smtplib.py +++ b/Lib/test/test_smtplib.py @@ -20,6 +20,7 @@ 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,25 @@ def testNonnumericPort(self): "localhost:bogus") +class DefaultArgumentsTests(unittest.TestCase): + + def setUp(self): + self.smtp = smtplib.SMTP() + self.smtp.ehlo = Mock(return_value=(200, 'OK')) + self.smtp.has_extn, self.smtp.sendmail = Mock(), Mock() + + def testSendMessage(self): + msg = EmailMessage() + msg['From'] = "Páolo " + expected_mail_options = ('SMTPUTF8', 'BODY=8BITMIME') + self.smtp.send_message(msg) + self.smtp.send_message(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) + + # test response of client to a non-successful HELO message class BadHELOServerTests(unittest.TestCase): diff --git a/Misc/ACKS b/Misc/ACKS index 0cb73a00308d09..b1be9a8e174b21 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 00000000000000..6f63226b216c6e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-07-29-15-25-15.bpo-34246.xiKq-Q.rst @@ -0,0 +1,2 @@ +Use an empty tuple as the default argument for mail_options and avoid it from +being mutated. From e2e4e5f7ce08ba3716983fdb24c3bc359f223bbe Mon Sep 17 00:00:00 2001 From: "Pablo S. Blum de Aguiar" Date: Mon, 6 Aug 2018 13:41:06 +0200 Subject: [PATCH 2/7] Change NEWS file content as suggested --- .../next/Library/2018-07-29-15-25-15.bpo-34246.xiKq-Q.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index 6f63226b216c6e..50c91ece07ef41 100644 --- 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 @@ -1,2 +1,2 @@ -Use an empty tuple as the default argument for mail_options and avoid it from -being mutated. +:meth:`smtplib.SMTP.send_message` no longer modifies the content of the +*mail_options* argument. Patch by Pablo S. Blum de Aguiar. From 72b15ce4fe652ff92e96f4952f0e019b95ddd93d Mon Sep 17 00:00:00 2001 From: "Pablo S. Blum de Aguiar" Date: Mon, 6 Aug 2018 13:41:30 +0200 Subject: [PATCH 3/7] Add one more test case Just to ensure `smtplib.SMTP.send_message` no longer modifies the content of the *mail_options* argument --- Lib/test/test_smtplib.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py index 71ae17b112594c..8a29e98a4f303a 100644 --- a/Lib/test/test_smtplib.py +++ b/Lib/test/test_smtplib.py @@ -582,21 +582,29 @@ def testNonnumericPort(self): 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): - msg = EmailMessage() - msg['From'] = "Páolo " expected_mail_options = ('SMTPUTF8', 'BODY=8BITMIME') - self.smtp.send_message(msg) - self.smtp.send_message(msg) + 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): From ca6176ebac5a887ec6bd0cbfd915a2e7f261096e Mon Sep 17 00:00:00 2001 From: "Pablo S. Blum de Aguiar" Date: Mon, 20 Aug 2018 13:09:31 +0200 Subject: [PATCH 4/7] Make rcpt_options compatible with what sendmail expects --- Lib/smtplib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/smtplib.py b/Lib/smtplib.py index 5a7f005b06bfd8..c0a885cbf01179 100755 --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -890,7 +890,7 @@ def sendmail(self, from_addr, to_addrs, msg, mail_options=[], 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 From cdf9fd3b92f361e7750c2f36c6deca954ee7c20d Mon Sep 17 00:00:00 2001 From: "Pablo S. Blum de Aguiar" Date: Mon, 20 Aug 2018 13:10:18 +0200 Subject: [PATCH 5/7] Update method signature in Doc/library/smtplib.rst --- Doc/library/smtplib.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/library/smtplib.rst b/Doc/library/smtplib.rst index 86e769e6a1f8dc..4606dc101cafb8 100644 --- a/Doc/library/smtplib.rst +++ b/Doc/library/smtplib.rst @@ -484,7 +484,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 From 0324dbb1a5632b5830999b0f5dc519d6252d45a8 Mon Sep 17 00:00:00 2001 From: "Pablo S. Blum de Aguiar" Date: Mon, 20 Aug 2018 14:21:47 +0200 Subject: [PATCH 6/7] Make empty tuple as values for options arguments --- Doc/library/smtplib.rst | 4 ++-- Lib/smtplib.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Doc/library/smtplib.rst b/Doc/library/smtplib.rst index 4606dc101cafb8..d46e6cd57e184f 100644 --- a/Doc/library/smtplib.rst +++ b/Doc/library/smtplib.rst @@ -412,7 +412,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 @@ -484,7 +484,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 c0a885cbf01179..b4c5dfcfbf9cb1 100755 --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -534,7 +534,7 @@ def mail(self, sender, options=[]): 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 @@ def starttls(self, keyfile=None, certfile=None, context=None): 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 @@ def sendmail(self, from_addr, to_addrs, msg, mail_options=[], 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 From 1d4621f71bd48b17022e4c3cca93dd5b5da3502a Mon Sep 17 00:00:00 2001 From: "Pablo S. Blum de Aguiar" Date: Mon, 20 Aug 2018 15:11:48 +0200 Subject: [PATCH 7/7] Use empty tuple as default value for `options` in `mail` --- Lib/smtplib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/smtplib.py b/Lib/smtplib.py index b4c5dfcfbf9cb1..048c6bfb0671b1 100755 --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -513,7 +513,7 @@ def noop(self): """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: