Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Doc/library/zipfile.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
7 changes: 7 additions & 0 deletions Lib/test/test_zipfile/_path/test_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import stat
import sys
import unittest
import warnings
import zipfile
import zipfile._path

Expand Down Expand Up @@ -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()))
Expand Down
33 changes: 28 additions & 5 deletions Lib/test/test_zipfile/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions Lib/zipfile/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import sys
import threading
import time
lazy import warnings

try:
import zlib # We may need its compression method
Expand Down Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Loading