From 5e16278729cde506d508785af51464a4010b3bd1 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Fri, 26 Jun 2026 16:20:26 -0700 Subject: [PATCH 1/2] gh-81954: Warn when ZipFile is closed with unwritten data Bring `zipfile.ZipFile` into alignment with other buffered file writers like `GzipFile` and `TextIOWrapper` by emitting a `ResourceWarning` if there is unwritten data and it is closed implicitly. ZipFile should be closed either by using it as a context manager or explicitly calling `.close()`. --- Doc/library/zipfile.rst | 4 +++ Lib/test/test_zipfile/_path/test_path.py | 7 ++++ Lib/test/test_zipfile/test_core.py | 33 ++++++++++++++++--- Lib/zipfile/__init__.py | 8 +++++ ...6-06-26-16-15-55.gh-issue-81954.MfUjgS.rst | 3 ++ 5 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-06-26-16-15-55.gh-issue-81954.MfUjgS.rst diff --git a/Doc/library/zipfile.rst b/Doc/library/zipfile.rst index 98d2a5e5cdf00e..3470c2f9211f74 100644 --- a/Doc/library/zipfile.rst +++ b/Doc/library/zipfile.rst @@ -285,6 +285,10 @@ ZipFile objects Added support for specifying member name encoding for reading metadata in the zipfile's directory and file headers. + .. versionchanged:: next + Deleting a :class:`zipfile.ZipFile` which contains unwritten data before + it is closed now emits a :exc:`ResourceWarning`. Use as a + :term:`context manager` or call :meth:`~zipfile.ZipFile.close` explicitly. .. method:: ZipFile.close() diff --git a/Lib/test/test_zipfile/_path/test_path.py b/Lib/test/test_zipfile/_path/test_path.py index e7931b6f394075..97e1377743644a 100644 --- a/Lib/test/test_zipfile/_path/test_path.py +++ b/Lib/test/test_zipfile/_path/test_path.py @@ -6,6 +6,7 @@ import stat import sys import unittest +import warnings import zipfile import zipfile._path @@ -86,6 +87,12 @@ class TestPath(unittest.TestCase): def setUp(self): self.fixtures = contextlib.ExitStack() self.addCleanup(self.fixtures.close) + # These tests use zipfiles as fixtures which do not get closed. Ignore + # the ResourceWarning. + self.fixtures.enter_context(warnings.catch_warnings()) + warnings.filterwarnings( + 'ignore', category=ResourceWarning, message='unclosed ZipFile' + ) def zipfile_ondisk(self, alpharep): tmpdir = pathlib.Path(self.fixtures.enter_context(temp_dir())) diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index 4f20209927e7b3..cc3c0833cd2d9c 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -25,12 +25,13 @@ from test.support import ( findfile, requires_zlib, requires_bz2, requires_lzma, requires_zstd, captured_stdout, captured_stderr, requires_subprocess, - cpython_only + cpython_only, gc_collect ) from test.support.os_helper import ( TESTFN, unlink, rmtree, temp_dir, temp_cwd, fd_count, FakePath ) from test.support.import_helper import ensure_lazy_imports +from test.support.warnings_helper import check_no_resource_warning TESTFN2 = TESTFN + "2" @@ -4051,6 +4052,28 @@ def test_close_on_exception(self): except zipfile.BadZipFile: self.assertIsNone(zipfp2.fp, 'zipfp is not closed') + def test_garbage_collection(self): + # gh-81954: Warn if a writable zipfile is closed by GC. + with self.assertWarns(ResourceWarning): + zipfile.ZipFile(io.BytesIO(), "w") + gc_collect() + + # Only warn if there is possible data loss. + # Properly closed via context manager. + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + zf.writestr("f.txt", b"data") + + with check_no_resource_warning(self): + # Read mode: No possible data loss. + zipfile.ZipFile(buf, "r") + + # Write with manual explicit close: No pending data. + zf = zipfile.ZipFile(io.BytesIO(), "w") + zf.writestr("f.txt", b"data") + zf.close() + del zf + def test_unsupported_version(self): # File has an extract_version of 120 data = (b'PK\x03\x04x\x00\x00\x00\x00\x00!p\xa1@\x00\x00\x00\x00\x00\x00' @@ -5503,10 +5526,10 @@ def test_root_folder_in_zipfile(self): the zip file, this is a strange behavior, but we should support it. """ in_memory_file = io.BytesIO() - zf = zipfile.ZipFile(in_memory_file, "w") - zf.mkdir('/') - zf.writestr('./a.txt', 'aaa') - zf.extractall(TESTFN2) + with zipfile.ZipFile(in_memory_file, "w") as zf: + zf.mkdir('/') + zf.writestr('./a.txt', 'aaa') + zf.extractall(TESTFN2) def tearDown(self): rmtree(TESTFN2) diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py index 418933a2e8d9e8..9c5b409b38b569 100644 --- a/Lib/zipfile/__init__.py +++ b/Lib/zipfile/__init__.py @@ -12,6 +12,7 @@ import sys import threading import time +lazy import warnings try: import zlib # We may need its compression method @@ -2616,6 +2617,13 @@ def mkdir(self, zinfo_or_directory_name, mode=511): def __del__(self): """Call the "close()" method in case the user forgot.""" + # gh-81954: Warn if ZipFile is implicitly closed with unwritten end + # records. GC cleanup order is non-deterministic and can result in data + # loss. + if (self.fp is not None and self.mode in ('w', 'x', 'a') + and self._didModify): + warnings.warn(f"unclosed ZipFile {self!r}", + ResourceWarning, source=self, stacklevel=2) self.close() def close(self): diff --git a/Misc/NEWS.d/next/Library/2026-06-26-16-15-55.gh-issue-81954.MfUjgS.rst b/Misc/NEWS.d/next/Library/2026-06-26-16-15-55.gh-issue-81954.MfUjgS.rst new file mode 100644 index 00000000000000..9d0de6ee1a9a15 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-06-26-16-15-55.gh-issue-81954.MfUjgS.rst @@ -0,0 +1,3 @@ +Deleting a :class:`zipfile.ZipFile` which contains unwritten data before it +is closed now emits a :exc:`ResourceWarning`. Use as a :term:`context manager` +or call :meth:`~zipfile.ZipFile.close` explicitly. From 03007df8252c2687a5d599c0c1ff3056290b7954 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sun, 5 Jul 2026 17:47:40 -0700 Subject: [PATCH 2/2] Move zipfile.Path tests to enter/exit context managers --- Lib/test/test_zipfile/_path/_test_params.py | 11 +++++-- Lib/test/test_zipfile/_path/test_path.py | 32 +++++++-------------- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/Lib/test/test_zipfile/_path/_test_params.py b/Lib/test/test_zipfile/_path/_test_params.py index 00a9eaf2f99c1a..1dfa3b4c08ffae 100644 --- a/Lib/test/test_zipfile/_path/_test_params.py +++ b/Lib/test/test_zipfile/_path/_test_params.py @@ -1,5 +1,6 @@ import functools import types +from contextlib import AbstractContextManager, ExitStack from ._itertools import always_iterable @@ -9,6 +10,8 @@ def parameterize(names, value_groups): Decorate a test method to run it as a set of subtests. Modeled after pytest.parametrize. + + Context Manager types are entered and exited. """ def decorator(func): @@ -17,8 +20,12 @@ def wrapped(self): for values in value_groups: resolved = map(Invoked.eval, always_iterable(values)) params = dict(zip(always_iterable(names), resolved)) - with self.subTest(**params): - func(self, **params) + with ExitStack() as fixtures: + for value in params.values(): + if isinstance(value, AbstractContextManager): + fixtures.enter_context(value) + with self.subTest(**params): + func(self, **params) return wrapped diff --git a/Lib/test/test_zipfile/_path/test_path.py b/Lib/test/test_zipfile/_path/test_path.py index 97e1377743644a..b00491ab4cbdb7 100644 --- a/Lib/test/test_zipfile/_path/test_path.py +++ b/Lib/test/test_zipfile/_path/test_path.py @@ -1,4 +1,3 @@ -import contextlib import io import itertools import pathlib @@ -6,7 +5,6 @@ import stat import sys import unittest -import warnings import zipfile import zipfile._path @@ -84,18 +82,8 @@ def build_alpharep_fixture(): class TestPath(unittest.TestCase): - def setUp(self): - self.fixtures = contextlib.ExitStack() - self.addCleanup(self.fixtures.close) - # These tests use zipfiles as fixtures which do not get closed. Ignore - # the ResourceWarning. - self.fixtures.enter_context(warnings.catch_warnings()) - warnings.filterwarnings( - 'ignore', category=ResourceWarning, message='unclosed ZipFile' - ) - def zipfile_ondisk(self, alpharep): - tmpdir = pathlib.Path(self.fixtures.enter_context(temp_dir())) + tmpdir = pathlib.Path(self.enterContext(temp_dir())) buffer = alpharep.fp alpharep.close() path = tmpdir / alpharep.filename @@ -152,7 +140,7 @@ def test_open(self, alpharep): def test_open_encoding_utf16(self): in_memory_file = io.BytesIO() - zf = zipfile.ZipFile(in_memory_file, "w") + zf = self.enterContext(zipfile.ZipFile(in_memory_file, "w")) zf.writestr("path/16.txt", "This was utf-16".encode("utf-16")) zf.filename = "test_open_utf16.zip" root = zipfile.Path(zf) @@ -167,7 +155,7 @@ def test_open_encoding_utf16(self): def test_open_encoding_errors(self): in_memory_file = io.BytesIO() - zf = zipfile.ZipFile(in_memory_file, "w") + zf = self.enterContext(zipfile.ZipFile(in_memory_file, "w")) zf.writestr("path/bad-utf8.bin", b"invalid utf-8: \xff\xff.") zf.filename = "test_read_text_encoding_errors.zip" root = zipfile.Path(zf) @@ -211,7 +199,8 @@ def test_open_write(self): If the zipfile is open for write, it should be possible to write bytes or text to it. """ - zf = zipfile.Path(zipfile.ZipFile(io.BytesIO(), mode='w')) + zip_file = self.enterContext(zipfile.ZipFile(io.BytesIO(), mode='w')) + zf = zipfile.Path(zip_file) with zf.joinpath('file.bin').open('wb') as strm: strm.write(b'binary contents') with zf.joinpath('file.txt').open('w', encoding="utf-8") as strm: @@ -326,7 +315,7 @@ def test_mutability(self, alpharep): def huge_zipfile(self): """Create a read-only zipfile with a huge number of entries.""" strm = io.BytesIO() - zf = zipfile.ZipFile(strm, "w") + zf = self.enterContext(zipfile.ZipFile(strm, "w")) for entry in map(str, range(self.HUGE_ZIPFILE_NUM_ENTRIES)): zf.writestr(entry, entry) zf.mode = 'r' @@ -537,7 +526,8 @@ def test_glob_chars(self, alpharep): ] def test_glob_empty(self): - root = zipfile.Path(zipfile.ZipFile(io.BytesIO(), 'w')) + zip_file = self.enterContext(zipfile.ZipFile(io.BytesIO(), 'w')) + root = zipfile.Path(zip_file) with self.assertRaises(ValueError): root.glob('') @@ -621,7 +611,7 @@ def test_malformed_paths(self): Paths with dots are treated like regular files. """ data = io.BytesIO() - zf = zipfile.ZipFile(data, "w") + zf = self.enterContext(zipfile.ZipFile(data, "w")) zf.writestr("/one-slash.txt", b"content") zf.writestr("//two-slash.txt", b"content") zf.writestr("../parent.txt", b"content") @@ -639,7 +629,7 @@ def test_unsupported_names(self): in the zip file. """ data = io.BytesIO() - zf = zipfile.ZipFile(data, "w") + zf = self.enterContext(zipfile.ZipFile(data, "w")) zf.writestr("path?", b"content") zf.writestr("V: NMS.flac", b"fLaC...") zf.filename = '' @@ -654,7 +644,7 @@ def test_backslash_not_separator(self): In a zip file, backslashes are not separators. """ data = io.BytesIO() - zf = zipfile.ZipFile(data, "w") + zf = self.enterContext(zipfile.ZipFile(data, "w")) zf.writestr(DirtyZipInfo("foo\\bar")._for_archive(zf), b"content") zf.filename = '' root = zipfile.Path(zf)