diff --git a/Lib/aifc.py b/Lib/aifc.py index 7ebdbeb68c1093..d5764a33d054a7 100644 --- a/Lib/aifc.py +++ b/Lib/aifc.py @@ -294,6 +294,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 @@ -335,9 +337,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 @@ -532,18 +540,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 diff --git a/Lib/test/test_aifc.py b/Lib/test/test_aifc.py index ab5143787b0d44..eaa24b63a26d68 100644 --- a/Lib/test/test_aifc.py +++ b/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 os @@ -150,6 +151,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() diff --git a/Misc/NEWS b/Misc/NEWS index bab13da4342dc4..8a7e1b5cd9c16e 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -32,6 +32,9 @@ Extension Modules Library ------- +- 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. + - Issue #28961: Fix unittest.mock._Call helper: don't ignore the name parameter anymore. Patch written by Jiajun Huang.