From 05ff9904010a488cc640637ac8255cae41b270dd Mon Sep 17 00:00:00 2001 From: R David Murray Date: Fri, 17 Jun 2011 12:54:56 -0400 Subject: [PATCH] #11767: use context manager to close file in __getitem__ to prevent FD leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All of the other methods in mailbox that create message objects take care to close the file descriptors they use, so it seems to make sense to have __getitem__ do so as well. Patch by Filip GruszczyƄski. --- Lib/mailbox.py | 4 +++- Lib/test/test_mailbox.py | 33 ++++++++++++++++++++++++++++++++- Misc/NEWS | 2 ++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/Lib/mailbox.py b/Lib/mailbox.py index 0e4f99bcaf5..b96b270d687 100644 --- a/Lib/mailbox.py +++ b/Lib/mailbox.py @@ -20,6 +20,7 @@ import email import email.message import email.generator import io +import contextlib try: if sys.platform == 'os2emx': # OS/2 EMX fcntl() not adequate @@ -76,7 +77,8 @@ class Mailbox: if not self._factory: return self.get_message(key) else: - return self._factory(self.get_file(key)) + with contextlib.closing(self.get_file(key)) as file: + return self._factory(file) def get_message(self, key): """Return a Message representation or raise a KeyError.""" diff --git a/Lib/test/test_mailbox.py b/Lib/test/test_mailbox.py index 8dc73262dd6..10317c3bb91 100644 --- a/Lib/test/test_mailbox.py +++ b/Lib/test/test_mailbox.py @@ -1201,6 +1201,37 @@ class TestBabyl(TestMailbox): self.assertEqual(set(self._box.get_labels()), set(['blah'])) +class FakeFileLikeObject: + + def __init__(self): + self.closed = False + + def close(self): + self.closed = True + + +class FakeMailBox(mailbox.Mailbox): + + def __init__(self): + mailbox.Mailbox.__init__(self, '', lambda file: None) + self.files = [FakeFileLikeObject() for i in range(10)] + + def get_file(self, key): + return self.files[key] + + +class TestFakeMailBox(unittest.TestCase): + + def test_closing_fd(self): + box = FakeMailBox() + for i in range(10): + self.assertFalse(box.files[i].closed) + for i in range(10): + box[i] + for i in range(10): + self.assertTrue(box.files[i].closed) + + class TestMessage(TestBase): _factory = mailbox.Message # Overridden by subclasses to reuse tests @@ -2113,7 +2144,7 @@ def test_main(): TestBabyl, TestMessage, TestMaildirMessage, TestMboxMessage, TestMHMessage, TestBabylMessage, TestMMDFMessage, TestMessageConversion, TestProxyFile, TestPartialFile, - MaildirTestCase) + MaildirTestCase, TestFakeMailBox) support.run_unittest(*tests) support.reap_children() diff --git a/Misc/NEWS b/Misc/NEWS index 6cc0f03637d..ec8b72ec32b 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -25,6 +25,8 @@ Core and Builtins Library ------- +- Issue #11767: Correct file descriptor leak in mailbox's __getitem__ method. + - Issue #12133: AbstractHTTPHandler.do_open() of urllib.request closes the HTTP connection if its getresponse() method fails with a socket error. Patch written by Ezio Melotti.