From 2d6a778f7335cd21f8e7bc4d12f2162441bc2e69 Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Fri, 21 Mar 2025 11:21:40 +0100 Subject: [PATCH] [3.12] gh-131492, gh-131461: handle exceptions in GzipFile constructor while owning resources (GH-131462) (#131519) (cherry picked from commit ce79274e9f093bd06d2285c9af48dbcbc92173de) Co-authored-by: Thomas Grainger Co-authored-by: Victor Stinner --- Lib/gzip.py | 100 ++++++++++-------- Lib/test/test_tarfile.py | 11 +- ...-03-20-08-32-49.gh-issue-131492.saC2cA.rst | 1 + 3 files changed, 63 insertions(+), 49 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-03-20-08-32-49.gh-issue-131492.saC2cA.rst diff --git a/Lib/gzip.py b/Lib/gzip.py index 01a791e5824..ecfe3a9cd7e 100644 --- a/Lib/gzip.py +++ b/Lib/gzip.py @@ -195,51 +195,58 @@ def __init__(self, filename=None, mode=None, raise ValueError("Invalid mode: {!r}".format(mode)) if mode and 'b' not in mode: mode += 'b' - if fileobj is None: - fileobj = self.myfileobj = builtins.open(filename, mode or 'rb') - if filename is None: - filename = getattr(fileobj, 'name', '') - if not isinstance(filename, (str, bytes)): - filename = '' - else: - filename = os.fspath(filename) - origmode = mode - if mode is None: - mode = getattr(fileobj, 'mode', 'rb') + + try: + if fileobj is None: + fileobj = self.myfileobj = builtins.open(filename, mode or 'rb') + if filename is None: + filename = getattr(fileobj, 'name', '') + if not isinstance(filename, (str, bytes)): + filename = '' + else: + filename = os.fspath(filename) + origmode = mode + if mode is None: + mode = getattr(fileobj, 'mode', 'rb') - if mode.startswith('r'): - self.mode = READ - raw = _GzipReader(fileobj) - self._buffer = io.BufferedReader(raw) - self.name = filename + if mode.startswith('r'): + self.mode = READ + raw = _GzipReader(fileobj) + self._buffer = io.BufferedReader(raw) + self.name = filename - elif mode.startswith(('w', 'a', 'x')): - if origmode is None: - import warnings - warnings.warn( - "GzipFile was opened for writing, but this will " - "change in future Python releases. " - "Specify the mode argument for opening it for writing.", - FutureWarning, 2) - self.mode = WRITE - self._init_write(filename) - self.compress = zlib.compressobj(compresslevel, - zlib.DEFLATED, - -zlib.MAX_WBITS, - zlib.DEF_MEM_LEVEL, - 0) - self._write_mtime = mtime - self._buffer_size = _WRITE_BUFFER_SIZE - self._buffer = io.BufferedWriter(_WriteBufferStream(self), - buffer_size=self._buffer_size) - else: - raise ValueError("Invalid mode: {!r}".format(mode)) + elif mode.startswith(('w', 'a', 'x')): + if origmode is None: + import warnings + warnings.warn( + "GzipFile was opened for writing, but this will " + "change in future Python releases. " + "Specify the mode argument for opening it for writing.", + FutureWarning, 2) + self.mode = WRITE + self._init_write(filename) + self.compress = zlib.compressobj(compresslevel, + zlib.DEFLATED, + -zlib.MAX_WBITS, + zlib.DEF_MEM_LEVEL, + 0) + self._write_mtime = mtime + self._buffer_size = _WRITE_BUFFER_SIZE + self._buffer = io.BufferedWriter(_WriteBufferStream(self), + buffer_size=self._buffer_size) + else: + raise ValueError("Invalid mode: {!r}".format(mode)) - self.fileobj = fileobj + self.fileobj = fileobj - if self.mode == WRITE: - self._write_gzip_header(compresslevel) + if self.mode == WRITE: + self._write_gzip_header(compresslevel) + except: + # Avoid a ResourceWarning if the write fails, + # eg read-only file or KeyboardInterrupt + self._close() + raise @property def mtime(self): @@ -368,11 +375,14 @@ def close(self): elif self.mode == READ: self._buffer.close() finally: - self.fileobj = None - myfileobj = self.myfileobj - if myfileobj: - self.myfileobj = None - myfileobj.close() + self._close() + + def _close(self): + self.fileobj = None + myfileobj = self.myfileobj + if myfileobj is not None: + self.myfileobj = None + myfileobj.close() def flush(self,zlib_mode=zlib.Z_SYNC_FLUSH): self._check_not_closed() diff --git a/Lib/test/test_tarfile.py b/Lib/test/test_tarfile.py index e28d0311826..ae55bebbc80 100644 --- a/Lib/test/test_tarfile.py +++ b/Lib/test/test_tarfile.py @@ -1615,10 +1615,13 @@ def write(self, data): raise exctype f = BadFile() - with self.assertRaises(exctype): - tar = tarfile.open(tmpname, self.mode, fileobj=f, - format=tarfile.PAX_FORMAT, - pax_headers={'non': 'empty'}) + with ( + warnings_helper.check_no_resource_warning(self), + self.assertRaises(exctype), + ): + tarfile.open(tmpname, self.mode, fileobj=f, + format=tarfile.PAX_FORMAT, + pax_headers={'non': 'empty'}) self.assertFalse(f.closed) diff --git a/Misc/NEWS.d/next/Library/2025-03-20-08-32-49.gh-issue-131492.saC2cA.rst b/Misc/NEWS.d/next/Library/2025-03-20-08-32-49.gh-issue-131492.saC2cA.rst new file mode 100644 index 00000000000..0f52dec7ce8 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-03-20-08-32-49.gh-issue-131492.saC2cA.rst @@ -0,0 +1 @@ +Fix a resource leak when constructing a :class:`gzip.GzipFile` with a filename fails, for example when passing an invalid ``compresslevel``.