[3.6] bpo-29110: Fix file object leak in aifc.open (#310)

(cherry picked from commit 03f68b6) (GH-162)
(cherry picked from commit 5dc33ee) (GH-293)

Author: methane

Lib/aifc.py

@@ -303,6 +303,8 @@ class Aifc_read:
     # _ssnd_chunk -- instantiation of a chunk class for the SSND chunk
     # _framesize -- size of one frame in the file
 
+    _file = None  # Set here since __del__ checks it
+
     def initfp(self, file):
         self._version = 0
         self._convert = None
@@ -344,9 +346,15 @@ def initfp(self, file):
 
     def __init__(self, f):
         if isinstance(f, str):
-            f = builtins.open(f, 'rb')
-        # else, assume it is an open file object already
-        self.initfp(f)
+            file_object = builtins.open(f, 'rb')
+            try:
+                self.initfp(file_object)
+            except:
+                file_object.close()
+                raise
+        else:
+            # assume it is an open file object already
+            self.initfp(f)
 
     def __enter__(self):
         return self
@@ -541,18 +549,23 @@ class Aifc_write:
     # _datalength -- the size of the audio samples written to the header
     # _datawritten -- the size of the audio samples actually written
 
+    _file = None  # Set here since __del__ checks it
+
     def __init__(self, f):
         if isinstance(f, str):
-            filename = f
-            f = builtins.open(f, 'wb')
-        else:
-            # else, assume it is an open file object already
-            filename = '???'
-        self.initfp(f)
-        if filename[-5:] == '.aiff':
-            self._aifc = 0
+            file_object = builtins.open(f, 'wb')
+            try:
+                self.initfp(file_object)
+            except:
+                file_object.close()
+                raise
+
+            # treat .aiff file extensions as non-compressed audio
+            if f.endswith('.aiff'):
+                self._aifc = 0
         else:
-            self._aifc = 1
+            # assume it is an open file object already
+            self.initfp(f)
 
     def initfp(self, file):
         self._file = file

Lib/test/test_aifc.py

@@ -1,5 +1,6 @@
-from test.support import findfile, TESTFN, unlink
+from test.support import check_no_resource_warning, findfile, TESTFN, unlink
 import unittest
+from unittest import mock
 from test import audiotests
 from audioop import byteswap
 import io
@@ -149,6 +150,21 @@ def test_skipunknown(self):
         #This file contains chunk types aifc doesn't recognize.
         self.f = aifc.open(findfile('Sine-1000Hz-300ms.aif'))
 
+    def test_close_opened_files_on_error(self):
+        non_aifc_file = findfile('pluck-pcm8.wav', subdir='audiodata')
+        with check_no_resource_warning(self):
+            with self.assertRaises(aifc.Error):
+                # Try opening a non-AIFC file, with the expectation that
+                # `aifc.open` will fail (without raising a ResourceWarning)
+                self.f = aifc.open(non_aifc_file, 'rb')
+
+            # Aifc_write.initfp() won't raise in normal case.  But some errors
+            # (e.g. MemoryError, KeyboardInterrupt, etc..) can happen.
+            with mock.patch.object(aifc.Aifc_write, 'initfp',
+                                   side_effect=RuntimeError):
+                with self.assertRaises(RuntimeError):
+                    self.fout = aifc.open(TESTFN, 'wb')
+
     def test_params_added(self):
         f = self.f = aifc.open(TESTFN, 'wb')
         f.aiff()

Misc/NEWS

@@ -78,6 +78,9 @@ Library
 - bpo-29532: Altering a kwarg dictionary passed to functools.partial()
   no longer affects a partial object after creation.
 
+- bpo-29110: Fix file object leak in aifc.open() when file is given as a
+  filesystem path and is not in valid AIFF format. Patch by Anthony Zhang.
+
 - bpo-22807: Add uuid.SafeUUID and uuid.UUID.is_safe to relay information from
   the platform about whether generated UUIDs are generated with a
   multiprocessing safe method.