https://github.com/python/cpython/commit/74125a60b7a477451ff2b8385bfbce3fdae... commit: 74125a60b7a477451ff2b8385bfbce3fdaee8dbc branch: master author: Victor Stinner <vstinner@redhat.com> committer: GitHub <noreply@github.com> date: 2019-04-15T18:23:20+02:00 summary: bpo-36348: IMAP4.logout() doesn't ignore exc (GH-12411) The imap.IMAP4.logout() method no longer ignores silently arbitrary exceptions. Changes: * The IMAP4.logout() method now expects a "BYE" untagged response, rather than relying on _check_bye() which raises a self.abort() exception. * IMAP4.__exit__() now does nothing if the client already logged out. * Add more debug info if test_logout() tests fail. files: A Misc/NEWS.d/next/Library/2019-03-18-16-16-55.bpo-36348.E0w_US.rst M Doc/library/imaplib.rst M Doc/whatsnew/3.8.rst M Lib/imaplib.py M Lib/test/test_imaplib.py diff --git a/Doc/library/imaplib.rst b/Doc/library/imaplib.rst index d0709f8b678e..f027f82ddebe 100644 --- a/Doc/library/imaplib.rst +++ b/Doc/library/imaplib.rst @@ -327,6 +327,9 @@ An :class:`IMAP4` instance has the following methods: Shutdown connection to server. Returns server ``BYE`` response. + .. versionchanged:: 3.8 + The method no longer ignores silently arbitrary exceptions. + .. method:: IMAP4.lsub(directory='""', pattern='*') diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 39a0da5e61e9..f866f9ccb8c1 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -709,6 +709,9 @@ Changes in Python behavior Changes in the Python API ------------------------- +* The :meth:`imap.IMAP4.logout` method no longer ignores silently arbitrary + exceptions. + * The function :func:`platform.popen` has been removed, it was deprecated since Python 3.3: use :func:`os.popen` instead. diff --git a/Lib/imaplib.py b/Lib/imaplib.py index dd237f7704ac..341ee25ae965 100644 --- a/Lib/imaplib.py +++ b/Lib/imaplib.py @@ -272,6 +272,9 @@ def __enter__(self): return self def __exit__(self, *args): + if self.state == "LOGOUT": + return + try: self.logout() except OSError: @@ -625,11 +628,8 @@ def logout(self): Returns server 'BYE' response. """ self.state = 'LOGOUT' - try: typ, dat = self._simple_command('LOGOUT') - except: typ, dat = 'NO', ['%s: %s' % sys.exc_info()[:2]] + typ, dat = self._simple_command('LOGOUT') self.shutdown() - if 'BYE' in self.untagged_responses: - return 'BYE', self.untagged_responses['BYE'] return typ, dat @@ -1012,16 +1012,17 @@ def _command(self, name, *args): def _command_complete(self, name, tag): + logout = (name == 'LOGOUT') # BYE is expected after LOGOUT - if name != 'LOGOUT': + if not logout: self._check_bye() try: - typ, data = self._get_tagged_response(tag) + typ, data = self._get_tagged_response(tag, expect_bye=logout) except self.abort as val: raise self.abort('command: %s => %s' % (name, val)) except self.error as val: raise self.error('command: %s => %s' % (name, val)) - if name != 'LOGOUT': + if not logout: self._check_bye() if typ == 'BAD': raise self.error('%s command error: %s %s' % (name, typ, data)) @@ -1117,7 +1118,7 @@ def _get_response(self): return resp - def _get_tagged_response(self, tag): + def _get_tagged_response(self, tag, expect_bye=False): while 1: result = self.tagged_commands[tag] @@ -1125,9 +1126,15 @@ def _get_tagged_response(self, tag): del self.tagged_commands[tag] return result + if expect_bye: + typ = 'BYE' + bye = self.untagged_responses.pop(typ, None) + if bye is not None: + # Server replies to the "LOGOUT" command with "BYE" + return (typ, bye) + # If we've seen a BYE at this point, the socket will be # closed, so report the BYE now. - self._check_bye() # Some have reported "unexpected response" exceptions. diff --git a/Lib/test/test_imaplib.py b/Lib/test/test_imaplib.py index aec36af6c525..9305e47ee993 100644 --- a/Lib/test/test_imaplib.py +++ b/Lib/test/test_imaplib.py @@ -470,8 +470,8 @@ def test_logout(self): self.assertEqual(typ, 'OK') self.assertEqual(data[0], b'LOGIN completed') typ, data = client.logout() - self.assertEqual(typ, 'BYE') - self.assertEqual(data[0], b'IMAP4ref1 Server logging out') + self.assertEqual(typ, 'BYE', (typ, data)) + self.assertEqual(data[0], b'IMAP4ref1 Server logging out', (typ, data)) self.assertEqual(client.state, 'LOGOUT') def test_lsub(self): @@ -937,7 +937,7 @@ def test_logout(self): with transient_internet(self.host): rs = self.server.logout() self.server = None - self.assertEqual(rs[0], 'BYE') + self.assertEqual(rs[0], 'BYE', rs) @unittest.skipUnless(ssl, "SSL not available") @@ -995,7 +995,7 @@ def test_logout(self): with transient_internet(self.host): _server = self.imap_class(self.host, self.port) rs = _server.logout() - self.assertEqual(rs[0], 'BYE') + self.assertEqual(rs[0], 'BYE', rs) def test_ssl_context_certfile_exclusive(self): with transient_internet(self.host): diff --git a/Misc/NEWS.d/next/Library/2019-03-18-16-16-55.bpo-36348.E0w_US.rst b/Misc/NEWS.d/next/Library/2019-03-18-16-16-55.bpo-36348.E0w_US.rst new file mode 100644 index 000000000000..2320b4c05b53 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-03-18-16-16-55.bpo-36348.E0w_US.rst @@ -0,0 +1,2 @@ +The :meth:`imap.IMAP4.logout` method no longer ignores silently arbitrary +exceptions.