fix: close open things (ietf-tools#5593)

* fix: close open things

* fix: clean up test created files

* fix: remove one close too many

Author: rjsparks

ietf/doc/tests_bofreq.py

@@ -320,22 +320,26 @@ def test_submit(self):
             file = NamedTemporaryFile(delete=False,mode="w+",encoding='utf-8')
             file.write(f'# {username}')
             file.close()
-            for postdict in [
-                        {'bofreq_submission':'enter','bofreq_content':f'# {username}'},
-                        {'bofreq_submission':'upload','bofreq_file':open(file.name,'rb')},
-                     ]:
-                docevent_count = doc.docevent_set.count()
-                empty_outbox()
-                r = self.client.post(url, postdict)
-                self.assertEqual(r.status_code, 302)
-                doc = reload_db_objects(doc)
-                self.assertEqual('%02d'%(int(rev)+1) ,doc.rev)
-                self.assertEqual(f'# {username}', doc.text())
-                self.assertEqual(docevent_count+1, doc.docevent_set.count())
-                self.assertEqual(1, len(outbox))
-                rev = doc.rev
+            try:
+                with open(file.name, 'rb') as bofreq_fd:
+                    for postdict in [
+                                {'bofreq_submission':'enter','bofreq_content':f'# {username}'},
+                                {'bofreq_submission':'upload','bofreq_file':bofreq_fd},
+                            ]:
+                        docevent_count = doc.docevent_set.count()
+                        empty_outbox()
+                        r = self.client.post(url, postdict)
+                        self.assertEqual(r.status_code, 302)
+                        doc = reload_db_objects(doc)
+                        self.assertEqual('%02d'%(int(rev)+1) ,doc.rev)
+                        self.assertEqual(f'# {username}', doc.text())
+                        self.assertEqual(docevent_count+1, doc.docevent_set.count())
+                        self.assertEqual(1, len(outbox))
+                        rev = doc.rev
+            finally:
+                os.unlink(file.name)
+
             self.client.logout()
-            os.unlink(file.name)
 
     def test_start_new_bofreq(self):
         url = urlreverse('ietf.doc.views_bofreq.new_bof_request')
@@ -350,25 +354,28 @@ def test_start_new_bofreq(self):
         file = NamedTemporaryFile(delete=False,mode="w+",encoding='utf-8')
         file.write('some stuff')
         file.close()
-        for postdict in [
-                            dict(title='title one', bofreq_submission='enter', bofreq_content='some stuff'),
-                            dict(title='title two', bofreq_submission='upload', bofreq_file=open(file.name,'rb')),
-                        ]:
-            empty_outbox()
-            r = self.client.post(url, postdict)
-            self.assertEqual(r.status_code,302)
-            name = f"bofreq-{xslugify(nobody.last_name())[:64]}-{postdict['title']}".replace(' ','-')
-            bofreq = Document.objects.filter(name=name,type_id='bofreq').first()
-            self.assertIsNotNone(bofreq)
-            self.assertIsNotNone(DocAlias.objects.filter(name=name).first())
-            self.assertEqual(bofreq.title, postdict['title'])
-            self.assertEqual(bofreq.rev, '00')
-            self.assertEqual(bofreq.get_state_slug(), 'proposed')
-            self.assertEqual(list(bofreq_editors(bofreq)), [nobody])
-            self.assertEqual(bofreq.latest_event(NewRevisionDocEvent).rev, '00')
-            self.assertEqual(bofreq.text_or_error(), 'some stuff')
-            self.assertEqual(len(outbox),1)
-        os.unlink(file.name)
+        try:
+            with open(file.name,'rb') as bofreq_fd:
+                for postdict in [
+                                    dict(title='title one', bofreq_submission='enter', bofreq_content='some stuff'),
+                                    dict(title='title two', bofreq_submission='upload', bofreq_file=bofreq_fd),
+                                ]:
+                    empty_outbox()
+                    r = self.client.post(url, postdict)
+                    self.assertEqual(r.status_code,302)
+                    name = f"bofreq-{xslugify(nobody.last_name())[:64]}-{postdict['title']}".replace(' ','-')
+                    bofreq = Document.objects.filter(name=name,type_id='bofreq').first()
+                    self.assertIsNotNone(bofreq)
+                    self.assertIsNotNone(DocAlias.objects.filter(name=name).first())
+                    self.assertEqual(bofreq.title, postdict['title'])
+                    self.assertEqual(bofreq.rev, '00')
+                    self.assertEqual(bofreq.get_state_slug(), 'proposed')
+                    self.assertEqual(list(bofreq_editors(bofreq)), [nobody])
+                    self.assertEqual(bofreq.latest_event(NewRevisionDocEvent).rev, '00')
+                    self.assertEqual(bofreq.text_or_error(), 'some stuff')
+                    self.assertEqual(len(outbox),1)
+        finally:
+            os.unlink(file.name)
         existing_bofreq = BofreqFactory(requester_lastname=nobody.last_name())
         for postdict in [
                             dict(title='', bofreq_submission='enter', bofreq_content='some stuff'),

ietf/doc/tests_review.py

@@ -499,6 +499,8 @@ def make_test_mbox_tarball(self, review_req):
 
                 tar.add(os.path.relpath(tmp.name))
 
+                mbox.close()
+
         return mbox_path
 
     def test_search_mail_archive(self):

ietf/ipr/management/commands/process_email.py

@@ -23,8 +23,12 @@ def add_arguments(self, parser):
 
     def handle(self, *args, **options):
         email = options.get('email', None)
-        binary_input = io.open(email, 'rb') if email else sys.stdin.buffer
-        self.msg_bytes = binary_input.read()
+        if email:
+            binary_input = io.open(email, 'rb')
+            self.msg_bytes = binary_input.read()
+            binary_input.close()
+        else:
+            self.msg_bytes = sys.stdin.buffer.read()
         try:
             process_response_email(self.msg_bytes)
         except ValueError as e:
@@ -44,4 +48,4 @@ def make_failure_message(self, error, **extra):
                 'application', 'octet-stream',  # mime type
                 filename='original-message',
             )
-        return msg
+        return msg

ietf/meeting/tests_views.py

@@ -894,23 +894,27 @@ def build_session_setup(self):
 
     def test_session_draft_tarfile(self):
         session, filenames = self.build_session_setup()
-        url = urlreverse('ietf.meeting.views.session_draft_tarfile', kwargs={'num':session.meeting.number,'acronym':session.group.acronym})
-        response = self.client.get(url)
-        self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.get('Content-Type'), 'application/octet-stream')
-        for filename in filenames:
-            os.unlink(filename)
+        try:
+            url = urlreverse('ietf.meeting.views.session_draft_tarfile', kwargs={'num':session.meeting.number,'acronym':session.group.acronym})
+            response = self.client.get(url)
+            self.assertEqual(response.status_code, 200)
+            self.assertEqual(response.get('Content-Type'), 'application/octet-stream')
+        finally:
+            for filename in filenames:
+                os.unlink(filename)
 
     @skipIf(skip_pdf_tests, skip_message)
     @skip_coverage
     def test_session_draft_pdf(self):
         session, filenames = self.build_session_setup()
-        url = urlreverse('ietf.meeting.views.session_draft_pdf', kwargs={'num':session.meeting.number,'acronym':session.group.acronym})
-        response = self.client.get(url)
-        self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.get('Content-Type'), 'application/pdf')
-        for filename in filenames:
-            os.unlink(filename)
+        try:
+            url = urlreverse('ietf.meeting.views.session_draft_pdf', kwargs={'num':session.meeting.number,'acronym':session.group.acronym})
+            response = self.client.get(url)
+            self.assertEqual(response.status_code, 200)
+            self.assertEqual(response.get('Content-Type'), 'application/pdf')
+        finally:
+            for filename in filenames:
+                os.unlink(filename)
 
     def test_current_materials(self):
         url = urlreverse('ietf.meeting.views.current_materials')
@@ -6411,7 +6415,9 @@ def test_submit_and_approve_multiple_versions(self):
         path = os.path.join(submission.session.meeting.get_materials_path(),'slides')
         filename = os.path.join(path,session.sessionpresentation_set.first().document.name+'-01.txt')
         self.assertTrue(os.path.exists(filename))
-        contents = io.open(filename,'r').read()
+        fd = io.open(filename, 'r')
+        contents = fd.read()
+        fd.close()
         self.assertIn('third version', contents)
 
 
@@ -7946,12 +7952,13 @@ def test_add_proceedings_material_doc(self):
         """Upload proceedings materials document"""
         meeting = self._procmat_test_meeting()
         for mat_type in ProceedingsMaterialTypeName.objects.filter(used=True):
-            mat = self.upload_proceedings_material_test(
-                meeting,
-                mat_type,
-                {'file': self._proceedings_file(), 'external_url': ''},
-            )
-            self.assertEqual(mat.get_href(), f'{mat.document.name}:00')
+            with self._proceedings_file() as fd:
+                mat = self.upload_proceedings_material_test(
+                    meeting,
+                    mat_type,
+                    {'file': fd, 'external_url': ''},
+                )
+                self.assertEqual(mat.get_href(), f'{mat.document.name}:00')
 
     def test_add_proceedings_material_doc_invalid_ext(self):
         """Upload proceedings materials document with disallowed extension"""
@@ -8038,12 +8045,13 @@ def test_replace_proceedings_material(self):
             kwargs=dict(num=meeting.number, material_type=pm_doc.type.slug),
         )
         self.client.login(username='secretary', password='secretary+password')
-        r = self.client.post(pm_doc_url, {'file': self._proceedings_file(), 'external_url': ''})
-        self.assertRedirects(r, success_url)
-        self.assertEqual(meeting.proceedings_materials.count(), 2)
-        pm_doc = meeting.proceedings_materials.get(pk=pm_doc.pk)  # refresh from DB
-        self.assertEqual(pm_doc.document.rev, '01')
-        self.assertEqual(pm_doc.get_href(), f'{pm_doc.document.name}:01')
+        with self._proceedings_file() as fd:
+            r = self.client.post(pm_doc_url, {'file': fd, 'external_url': ''})
+            self.assertRedirects(r, success_url)
+            self.assertEqual(meeting.proceedings_materials.count(), 2)
+            pm_doc = meeting.proceedings_materials.get(pk=pm_doc.pk)  # refresh from DB
+            self.assertEqual(pm_doc.document.rev, '01')
+            self.assertEqual(pm_doc.get_href(), f'{pm_doc.document.name}:01')
 
         # Replace the uploaded document with a URL
         r = self.client.post(pm_doc_url, {'use_url': 'on', 'external_url': 'https://example.com/second'})
@@ -8066,12 +8074,13 @@ def test_replace_proceedings_material(self):
         self.assertEqual(pm_url.get_href(), 'https://example.com/third')
 
         # Now replace the URL doc with an uploaded file
-        r = self.client.post(pm_url_url, {'file': self._proceedings_file(), 'external_url': ''})
-        self.assertRedirects(r, success_url)
-        self.assertEqual(meeting.proceedings_materials.count(), 2)
-        pm_url = meeting.proceedings_materials.get(pk=pm_url.pk)  # refresh from DB
-        self.assertEqual(pm_url.document.rev, '02')
-        self.assertEqual(pm_url.get_href(), f'{pm_url.document.name}:02')
+        with self._proceedings_file() as fd:
+            r = self.client.post(pm_url_url, {'file': fd, 'external_url': ''})
+            self.assertRedirects(r, success_url)
+            self.assertEqual(meeting.proceedings_materials.count(), 2)
+            pm_url = meeting.proceedings_materials.get(pk=pm_url.pk)  # refresh from DB
+            self.assertEqual(pm_url.document.rev, '02')
+            self.assertEqual(pm_url.get_href(), f'{pm_url.document.name}:02')
 
     def test_remove_proceedings_material(self):
         """Proceedings material can be removed"""

ietf/nomcom/management/commands/feedback_email.py

@@ -42,8 +42,11 @@ def handle(self, *args, **options):
         except NomCom.DoesNotExist:
             raise CommandError("NomCom %s does not exist or it isn't active" % year)
 
-        binary_input = io.open(email, 'rb') if email else sys.stdin.buffer
-        self.msg = binary_input.read()
+        if email:
+            with io.open(email, 'rb') as binary_input:
+                self.msg = binary_input.read()
+        else:
+            self.msg = sys.stdin.buffer.read()
 
         try:
             feedback = create_feedback_email(self.nomcom, self.msg)

ietf/nomcom/test_data.py

@@ -94,7 +94,8 @@ def check_comments(encryped, plain, privatekey_file):
 
     decrypted_file.close()
     encrypted_file.close()
-    decrypted_comments = io.open(decrypted_file.name, 'rb').read().decode('utf-8')
+    with io.open(decrypted_file.name, 'rb') as fd:
+        decrypted_comments = fd.read().decode('utf-8')
     os.unlink(encrypted_file.name)
     os.unlink(decrypted_file.name)
 
@@ -116,7 +117,8 @@ def nomcom_test_data():
         nomcom_test_cert_file, privatekey_file = generate_cert()
 
     nomcom.public_key.storage = FileSystemStorage(location=settings.NOMCOM_PUBLIC_KEYS_DIR)
-    nomcom.public_key.save('cert', File(io.open(nomcom_test_cert_file.name, 'r')))
+    with io.open(nomcom_test_cert_file.name, 'r') as fd:
+        nomcom.public_key.save('cert', File(fd))
 
     # chair and member
     create_person(group, "chair", username=CHAIR_USER, email_address='%s%s'%(CHAIR_USER,EMAIL_DOMAIN))

ietf/nomcom/tests.py

@@ -715,7 +715,8 @@ def nominate_view(self, *args, **kwargs):
 
         # save the cert file in tmp
         #nomcom.public_key.storage.location = tempfile.gettempdir()
-        nomcom.public_key.save('cert', File(io.open(self.cert_file.name, 'r')))
+        with io.open(self.cert_file.name, 'r') as fd:
+            nomcom.public_key.save('cert', File(fd))
 
         response = self.client.get(nominate_url)
         self.assertEqual(response.status_code, 200)
@@ -781,7 +782,8 @@ def nominate_newperson_view(self, *args, **kwargs):
 
         # save the cert file in tmp
         #nomcom.public_key.storage.location = tempfile.gettempdir()
-        nomcom.public_key.save('cert', File(io.open(self.cert_file.name, 'r')))
+        with io.open(self.cert_file.name, 'r') as fd:
+            nomcom.public_key.save('cert', File(fd))
 
         response = self.client.get(nominate_url)
         self.assertEqual(response.status_code, 200)
@@ -863,7 +865,8 @@ def add_questionnaire(self, *args, **kwargs):
 
         # save the cert file in tmp
         #nomcom.public_key.storage.location = tempfile.gettempdir()
-        nomcom.public_key.save('cert', File(io.open(self.cert_file.name, 'r')))
+        with io.open(self.cert_file.name, 'r') as fd:
+            nomcom.public_key.save('cert', File(fd))
 
         response = self.client.get(self.add_questionnaire_url)
         self.assertEqual(response.status_code, 200)
@@ -942,7 +945,8 @@ def feedback_view(self, *args, **kwargs):
 
         # save the cert file in tmp
         #nomcom.public_key.storage.location = tempfile.gettempdir()
-        nomcom.public_key.save('cert', File(io.open(self.cert_file.name, 'r')))
+        with io.open(self.cert_file.name, 'r') as fd:
+            nomcom.public_key.save('cert', File(fd))
 
         response = self.client.get(feedback_url)
         self.assertEqual(response.status_code, 200)
@@ -1066,7 +1070,8 @@ def test_encrypted_comments(self):
 
         # save the cert file in tmp
         #nomcom.public_key.storage.location = tempfile.gettempdir()
-        nomcom.public_key.save('cert', File(io.open(self.cert_file.name, 'r')))
+        with io.open(self.cert_file.name, 'r') as fd:
+            nomcom.public_key.save('cert', File(fd))
 
         comment_text = 'Plain text. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.'
         comments = nomcom.encrypt(comment_text)
@@ -1089,7 +1094,8 @@ def setUp(self):
         self.nomcom = get_nomcom_by_year(NOMCOM_YEAR)
         self.cert_file, self.privatekey_file = get_cert_files()
         #self.nomcom.public_key.storage.location = tempfile.gettempdir()
-        self.nomcom.public_key.save('cert', File(io.open(self.cert_file.name, 'r')))
+        with io.open(self.cert_file.name, 'r') as fd:
+            self.nomcom.public_key.save('cert', File(fd))
 
         gen = Position.objects.get(nomcom=self.nomcom,name='GEN')
         rai = Position.objects.get(nomcom=self.nomcom,name='RAI')

ietf/person/views.py

@@ -100,14 +100,14 @@ def photo(request, email_or_name):
     if not size.isdigit():
         return HttpResponse("Size must be integer", status=400)
     size = int(size)
-    img = Image.open(person.photo)
-    img = img.resize((size, img.height*size//img.width))
-    bytes = BytesIO()
-    try:
-        img.save(bytes, format='JPEG')
-        return HttpResponse(bytes.getvalue(), content_type='image/jpg')
-    except OSError:
-        raise Http404
+    with Image.open(person.photo) as img:
+        img = img.resize((size, img.height*size//img.width))
+        bytes = BytesIO()
+        try:
+            img.save(bytes, format='JPEG')
+            return HttpResponse(bytes.getvalue(), content_type='image/jpg')
+        except OSError:
+            raise Http404
 
 
 @role_required("Secretariat")

ietf/review/mailarch.py

@@ -103,6 +103,7 @@ def retrieve_messages_from_mbox(mbox_fileobj):
                 "date": msg["Date"],
                 "utcdate": (utcdate.date().isoformat(), utcdate.time().isoformat()) if utcdate else ("", ""),
             })
+        mbox.close()
 
     return res