Skip to content

bpo-34246: Use no mutable default args in smtplib #8554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pablogsal merged 7 commits into from
Sep 7, 2018
Merged

bpo-34246: Use no mutable default args in smtplib #8554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
67f70fd
bpo-34246: Use tuple as default arg for mail_options
scorphus Jul 29, 2018
e2e4e5f
Change NEWS file content as suggested
scorphus Aug 6, 2018
72b15ce
Add one more test case
scorphus Aug 6, 2018
ca6176e
Make rcpt_options compatible with what sendmail expects
scorphus Aug 20, 2018
cdf9fd3
Update method signature in Doc/library/smtplib.rst
scorphus Aug 20, 2018
0324dbb
Make empty tuple as values for options arguments
scorphus Aug 20, 2018
1d4621f
Use empty tuple as default value for `options` in `mail`
scorphus Aug 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Doc/library/smtplib.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions Lib/smtplib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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')
Copy link
Copy Markdown
Member

@methane methane Aug 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this change. += seems more common/readable, and it's valid for immutable objects
like tuple and str. (You need to change [] to () in this case)

I'm sorry, I was wrong. When caller passed list, list += tuple will raise TypeError.

else:
g = email.generator.BytesGenerator(bytesmsg)
g.flatten(msg_copy, linesep='\r\n')
Expand Down
28 changes: 28 additions & 0 deletions Lib/test/test_smtplib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down Expand Up @@ -578,6 +579,33 @@ def testNonnumericPort(self):
"localhost:bogus")


class DefaultArgumentsTests(unittest.TestCase):

def setUp(self):
self.msg = EmailMessage()
self.msg['From'] = 'Páolo <főo@bar.com>'
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):

Expand Down
1 change: 1 addition & 0 deletions Misc/ACKS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Eitan Adler
Anton Afanasyev
Ali Afshar
Nitika Agarwal
Pablo S. Blum de Aguiar
Jim Ahlstrom
Farhan Ahmad
Matthew Ahrens
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2018-07-29-15-25-15.bpo-34246.xiKq-Q.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.