From 7e35eadfe888431ae00f025f2ac6e189c4b0ea98 Mon Sep 17 00:00:00 2001 From: Nis Martensen Date: Sat, 10 Feb 2024 20:10:56 +0100 Subject: [PATCH 1/7] smtplib: store the server name in ._host in .connect() Original patch by gigaplastik, extended with a few more tests. Addresses: https://github.com/python/cpython/issues/70039 BPO 25852 --- Lib/smtplib.py | 2 +- Lib/test/test_smtplib.py | 26 ++++++++++++++++++++ Lib/test/test_smtpnet.py | 53 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/Lib/smtplib.py b/Lib/smtplib.py index b3cc68a789a7d8..bc542d076a62fd 100755 --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -244,7 +244,6 @@ def __init__(self, host='', port=0, local_hostname=None, will be used. """ - self._host = host self.timeout = timeout self.esmtp_features = {} self.command_encoding = 'ascii' @@ -335,6 +334,7 @@ def connect(self, host='localhost', port=0, source_address=None): port = int(port) except ValueError: raise OSError("nonnumeric port") + self._host = host if not port: port = self.default_port sys.audit("smtplib.connect", self, host, port) diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py index 4c9fc14bd43f54..bea3592b7444f8 100644 --- a/Lib/test/test_smtplib.py +++ b/Lib/test/test_smtplib.py @@ -158,6 +158,32 @@ def test_debuglevel_2(self): re.MULTILINE) self.assertRegex(stderr.getvalue(), expected) + def test_host_port_host(self): + mock_socket.reply_with(b"220 Hello world") + # create instance without arguments + smtp = smtplib.SMTP(f"{HOST}:{self.port}") + # .connect must set ._host since it is used by .starttls + self.assertEqual(smtp._host, HOST) + smtp.close() + + def test_explicit_connect(self): + mock_socket.reply_with(b"220 Hello world") + # create instance without arguments + smtp = smtplib.SMTP() + # .connect must set ._host since it is used by .starttls + smtp.connect(HOST, self.port) + self.assertEqual(smtp._host, HOST) + smtp.close() + + def test_explicit_connect_host_port(self): + mock_socket.reply_with(b"220 Hello world") + # create instance without arguments + smtp = smtplib.SMTP() + # .connect must set ._host since it is used by .starttls + smtp.connect(f"{HOST}:{self.port}") + self.assertEqual(smtp._host, HOST) + smtp.close() + class SMTPGeneralTests(GeneralTests, unittest.TestCase): diff --git a/Lib/test/test_smtpnet.py b/Lib/test/test_smtpnet.py index 2e0dc1aa276f35..e29385f4f602ec 100644 --- a/Lib/test/test_smtpnet.py +++ b/Lib/test/test_smtpnet.py @@ -42,6 +42,59 @@ def test_connect_starttls(self): server.ehlo() server.quit() + def test_connect2_starttls(self): + support.get_attribute(smtplib, 'SMTP_SSL') + context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) + context.check_hostname = False + context.verify_mode = ssl.CERT_NONE + with socket_helper.transient_internet(self.testServer): + server = smtplib.SMTP(f'{self.testServer}:{self.remotePort}') + try: + server.starttls(context=context) + except smtplib.SMTPException as e: + if e.args[0] == 'STARTTLS extension not supported by server.': + unittest.skip(e.args[0]) + else: + raise + server.ehlo() + server.quit() + + def test_connect3_starttls(self): + support.get_attribute(smtplib, 'SMTP_SSL') + context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) + context.check_hostname = False + context.verify_mode = ssl.CERT_NONE + with socket_helper.transient_internet(self.testServer): + server = smtplib.SMTP() + server.connect(self.testServer, self.remotePort) + try: + server.starttls(context=context) + except smtplib.SMTPException as e: + if e.args[0] == 'STARTTLS extension not supported by server.': + unittest.skip(e.args[0]) + else: + raise + server.ehlo() + server.quit() + + def test_connect4_starttls(self): + support.get_attribute(smtplib, 'SMTP_SSL') + context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) + context.check_hostname = False + context.verify_mode = ssl.CERT_NONE + with socket_helper.transient_internet(self.testServer): + server = smtplib.SMTP() + server.connect(f'{self.testServer}:{self.remotePort}') + try: + server.starttls(context=context) + except smtplib.SMTPException as e: + if e.args[0] == 'STARTTLS extension not supported by server.': + unittest.skip(e.args[0]) + else: + raise + server.ehlo() + server.quit() + class SmtpSSLTest(unittest.TestCase): testServer = 'smtp.gmail.com' From 699384fe4d029c057400bbce5f0775e4052e21c8 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sat, 10 Feb 2024 21:25:23 +0000 Subject: [PATCH 2/7] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst diff --git a/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst b/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst new file mode 100644 index 00000000000000..c5aa5655a0c4bf --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst @@ -0,0 +1 @@ +In smtplib.SMTP, set ._host in .connect() and not immediately during instantiation. This allows .starttls() to work if the instance was created without arguments and host and port were only provided via an explicit .connect() call. If will also set ._host to the correct host value if the host and port were specified using the 'host:port' notation, which according to the documentation is supported by .connect(). From 5ea81fce9ee1095b25311eef460848688afce5fc Mon Sep 17 00:00:00 2001 From: Nis Martensen Date: Mon, 19 Feb 2024 21:50:51 +0100 Subject: [PATCH 3/7] move and rename tests --- Lib/test/test_smtplib.py | 54 +++++++++++++++++++++------------------- Lib/test/test_smtpnet.py | 6 ++--- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py index bea3592b7444f8..3219f725223292 100644 --- a/Lib/test/test_smtplib.py +++ b/Lib/test/test_smtplib.py @@ -158,32 +158,6 @@ def test_debuglevel_2(self): re.MULTILINE) self.assertRegex(stderr.getvalue(), expected) - def test_host_port_host(self): - mock_socket.reply_with(b"220 Hello world") - # create instance without arguments - smtp = smtplib.SMTP(f"{HOST}:{self.port}") - # .connect must set ._host since it is used by .starttls - self.assertEqual(smtp._host, HOST) - smtp.close() - - def test_explicit_connect(self): - mock_socket.reply_with(b"220 Hello world") - # create instance without arguments - smtp = smtplib.SMTP() - # .connect must set ._host since it is used by .starttls - smtp.connect(HOST, self.port) - self.assertEqual(smtp._host, HOST) - smtp.close() - - def test_explicit_connect_host_port(self): - mock_socket.reply_with(b"220 Hello world") - # create instance without arguments - smtp = smtplib.SMTP() - # .connect must set ._host since it is used by .starttls - smtp.connect(f"{HOST}:{self.port}") - self.assertEqual(smtp._host, HOST) - smtp.close() - class SMTPGeneralTests(GeneralTests, unittest.TestCase): @@ -308,6 +282,34 @@ def testBasic(self): # connect smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=support.LOOPBACK_TIMEOUT) + # implicit .connect must set ._host since it is used by .starttls + self.assertEqual(smtp._host, HOST) + smtp.quit() + + def test_host_port_host(self): + # create instance with host:port notation + smtp = smtplib.SMTP(f"{HOST}:{self.port}", local_hostname='localhost', + timeout=support.LOOPBACK_TIMEOUT) + # implicit .connect must set ._host since it is used by .starttls + self.assertEqual(smtp._host, HOST) + smtp.quit() + + def test_explicit_connect(self): + # create instance without arguments + smtp = smtplib.SMTP(local_hostname='localhost', + timeout=support.LOOPBACK_TIMEOUT) + # explicit .connect must set ._host since it is used by .starttls + smtp.connect(HOST, self.port) + self.assertEqual(smtp._host, HOST) + smtp.quit() + + def test_explicit_connect_host_port(self): + # create instance without arguments + smtp = smtplib.SMTP(local_hostname='localhost', + timeout=support.LOOPBACK_TIMEOUT) + # explicit .connect with host:port notation must set ._host, too + smtp.connect(f"{HOST}:{self.port}") + self.assertEqual(smtp._host, HOST) smtp.quit() def testSourceAddress(self): diff --git a/Lib/test/test_smtpnet.py b/Lib/test/test_smtpnet.py index e29385f4f602ec..86c19fa38c7d83 100644 --- a/Lib/test/test_smtpnet.py +++ b/Lib/test/test_smtpnet.py @@ -42,7 +42,7 @@ def test_connect_starttls(self): server.ehlo() server.quit() - def test_connect2_starttls(self): + def test_connect_host_port_starttls(self): support.get_attribute(smtplib, 'SMTP_SSL') context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) context.check_hostname = False @@ -59,7 +59,7 @@ def test_connect2_starttls(self): server.ehlo() server.quit() - def test_connect3_starttls(self): + def test_explicit_connect_starttls(self): support.get_attribute(smtplib, 'SMTP_SSL') context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) context.check_hostname = False @@ -77,7 +77,7 @@ def test_connect3_starttls(self): server.ehlo() server.quit() - def test_connect4_starttls(self): + def test_explicit_connect_host_port_starttls(self): support.get_attribute(smtplib, 'SMTP_SSL') context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) context.check_hostname = False From 75f4265c1e0c037fd7c66e60107ca3c9f45f713e Mon Sep 17 00:00:00 2001 From: Nis Martensen Date: Tue, 23 Dec 2025 18:47:23 +0100 Subject: [PATCH 4/7] Drop test_smtplib tests again There are already other tests for the functionality, therefore there is no need to test implementation details. --- Lib/test/test_smtplib.py | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py index 3219f725223292..4c9fc14bd43f54 100644 --- a/Lib/test/test_smtplib.py +++ b/Lib/test/test_smtplib.py @@ -282,34 +282,6 @@ def testBasic(self): # connect smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=support.LOOPBACK_TIMEOUT) - # implicit .connect must set ._host since it is used by .starttls - self.assertEqual(smtp._host, HOST) - smtp.quit() - - def test_host_port_host(self): - # create instance with host:port notation - smtp = smtplib.SMTP(f"{HOST}:{self.port}", local_hostname='localhost', - timeout=support.LOOPBACK_TIMEOUT) - # implicit .connect must set ._host since it is used by .starttls - self.assertEqual(smtp._host, HOST) - smtp.quit() - - def test_explicit_connect(self): - # create instance without arguments - smtp = smtplib.SMTP(local_hostname='localhost', - timeout=support.LOOPBACK_TIMEOUT) - # explicit .connect must set ._host since it is used by .starttls - smtp.connect(HOST, self.port) - self.assertEqual(smtp._host, HOST) - smtp.quit() - - def test_explicit_connect_host_port(self): - # create instance without arguments - smtp = smtplib.SMTP(local_hostname='localhost', - timeout=support.LOOPBACK_TIMEOUT) - # explicit .connect with host:port notation must set ._host, too - smtp.connect(f"{HOST}:{self.port}") - self.assertEqual(smtp._host, HOST) smtp.quit() def testSourceAddress(self): From 8810ffbd49cdbe9d609673be388dc4a5030ce4e8 Mon Sep 17 00:00:00 2001 From: Nis Martensen Date: Tue, 23 Dec 2025 19:02:02 +0100 Subject: [PATCH 5/7] Revised NEWS entry --- .../next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst b/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst index c5aa5655a0c4bf..206e144b8b7773 100644 --- a/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst +++ b/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst @@ -1 +1 @@ -In smtplib.SMTP, set ._host in .connect() and not immediately during instantiation. This allows .starttls() to work if the instance was created without arguments and host and port were only provided via an explicit .connect() call. If will also set ._host to the correct host value if the host and port were specified using the 'host:port' notation, which according to the documentation is supported by .connect(). +Fixed bug where :meth:SMTP.starttls could fail if :meth:SMTP.connect is called explicitly rather than implicitly. From eb2b5204d3a3ff02943734371c4788081a94c618 Mon Sep 17 00:00:00 2001 From: Nis Martensen Date: Tue, 23 Dec 2025 19:54:50 +0100 Subject: [PATCH 6/7] add missing backticks to NEWS message --- .../next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst b/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst index 206e144b8b7773..72bb327bd45aa9 100644 --- a/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst +++ b/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst @@ -1 +1 @@ -Fixed bug where :meth:SMTP.starttls could fail if :meth:SMTP.connect is called explicitly rather than implicitly. +Fixed bug where :meth:`SMTP.starttls` could fail if :meth:`SMTP.connect` is called explicitly rather than implicitly. From 00355850a65c90be786e2f56aca6e8134194f7b5 Mon Sep 17 00:00:00 2001 From: Nis Martensen Date: Tue, 23 Dec 2025 22:48:02 +0100 Subject: [PATCH 7/7] NEWS entry: add missing smtplib context --- .../next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst b/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst index 72bb327bd45aa9..8bb2cd188e89fa 100644 --- a/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst +++ b/Misc/NEWS.d/next/Library/2024-02-10-21-25-22.gh-issue-70039.6wvcAP.rst @@ -1 +1 @@ -Fixed bug where :meth:`SMTP.starttls` could fail if :meth:`SMTP.connect` is called explicitly rather than implicitly. +Fixed bug where :meth:`smtplib.SMTP.starttls` could fail if :meth:`smtplib.SMTP.connect` is called explicitly rather than implicitly.