Skip to content

Commit c1b6fe4

Browse files
Merge branch 'main' into gh-155245-fix-calendar-OB
2 parents 3c15175 + 5e0c502 commit c1b6fe4

6 files changed

Lines changed: 58 additions & 19 deletions

File tree

Doc/library/zipfile.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,10 @@ ZipFile objects
285285
Added support for specifying member name encoding for reading
286286
metadata in the zipfile's directory and file headers.
287287

288+
.. versionchanged:: next
289+
Deleting a writable, open :class:`zipfile.ZipFile` now emits a
290+
:exc:`ResourceWarning`. Use as a :term:`context manager` or call
291+
:meth:`~zipfile.ZipFile.close` explicitly.
288292

289293
.. method:: ZipFile.close()
290294

Lib/test/test_zipfile/_path/_test_params.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import functools
22
import types
3+
from contextlib import AbstractContextManager
34

45
from ._itertools import always_iterable
56

@@ -9,6 +10,8 @@ def parameterize(names, value_groups):
910
Decorate a test method to run it as a set of subtests.
1011
1112
Modeled after pytest.parametrize.
13+
14+
Context Manager types are entered and exited.
1215
"""
1316

1417
def decorator(func):
@@ -17,6 +20,9 @@ def wrapped(self):
1720
for values in value_groups:
1821
resolved = map(Invoked.eval, always_iterable(values))
1922
params = dict(zip(always_iterable(names), resolved))
23+
for value in params.values():
24+
if isinstance(value, AbstractContextManager):
25+
self.enterContext(value)
2026
with self.subTest(**params):
2127
func(self, **params)
2228

Lib/test/test_zipfile/_path/test_path.py

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import contextlib
21
import io
32
import itertools
43
import pathlib
@@ -83,12 +82,8 @@ def build_alpharep_fixture():
8382

8483

8584
class TestPath(unittest.TestCase):
86-
def setUp(self):
87-
self.fixtures = contextlib.ExitStack()
88-
self.addCleanup(self.fixtures.close)
89-
9085
def zipfile_ondisk(self, alpharep):
91-
tmpdir = pathlib.Path(self.fixtures.enter_context(temp_dir()))
86+
tmpdir = pathlib.Path(self.enterContext(temp_dir()))
9287
buffer = alpharep.fp
9388
alpharep.close()
9489
path = tmpdir / alpharep.filename
@@ -145,7 +140,7 @@ def test_open(self, alpharep):
145140

146141
def test_open_encoding_utf16(self):
147142
in_memory_file = io.BytesIO()
148-
zf = zipfile.ZipFile(in_memory_file, "w")
143+
zf = self.enterContext(zipfile.ZipFile(in_memory_file, "w"))
149144
zf.writestr("path/16.txt", "This was utf-16".encode("utf-16"))
150145
zf.filename = "test_open_utf16.zip"
151146
root = zipfile.Path(zf)
@@ -160,7 +155,7 @@ def test_open_encoding_utf16(self):
160155

161156
def test_open_encoding_errors(self):
162157
in_memory_file = io.BytesIO()
163-
zf = zipfile.ZipFile(in_memory_file, "w")
158+
zf = self.enterContext(zipfile.ZipFile(in_memory_file, "w"))
164159
zf.writestr("path/bad-utf8.bin", b"invalid utf-8: \xff\xff.")
165160
zf.filename = "test_read_text_encoding_errors.zip"
166161
root = zipfile.Path(zf)
@@ -204,7 +199,8 @@ def test_open_write(self):
204199
If the zipfile is open for write, it should be possible to
205200
write bytes or text to it.
206201
"""
207-
zf = zipfile.Path(zipfile.ZipFile(io.BytesIO(), mode='w'))
202+
zip_file = self.enterContext(zipfile.ZipFile(io.BytesIO(), mode='w'))
203+
zf = zipfile.Path(zip_file)
208204
with zf.joinpath('file.bin').open('wb') as strm:
209205
strm.write(b'binary contents')
210206
with zf.joinpath('file.txt').open('w', encoding="utf-8") as strm:
@@ -319,7 +315,7 @@ def test_mutability(self, alpharep):
319315
def huge_zipfile(self):
320316
"""Create a read-only zipfile with a huge number of entries."""
321317
strm = io.BytesIO()
322-
zf = zipfile.ZipFile(strm, "w")
318+
zf = self.enterContext(zipfile.ZipFile(strm, "w"))
323319
for entry in map(str, range(self.HUGE_ZIPFILE_NUM_ENTRIES)):
324320
zf.writestr(entry, entry)
325321
zf.mode = 'r'
@@ -530,7 +526,8 @@ def test_glob_chars(self, alpharep):
530526
]
531527

532528
def test_glob_empty(self):
533-
root = zipfile.Path(zipfile.ZipFile(io.BytesIO(), 'w'))
529+
zip_file = self.enterContext(zipfile.ZipFile(io.BytesIO(), 'w'))
530+
root = zipfile.Path(zip_file)
534531
with self.assertRaises(ValueError):
535532
root.glob('')
536533

@@ -614,7 +611,7 @@ def test_malformed_paths(self):
614611
Paths with dots are treated like regular files.
615612
"""
616613
data = io.BytesIO()
617-
zf = zipfile.ZipFile(data, "w")
614+
zf = self.enterContext(zipfile.ZipFile(data, "w"))
618615
zf.writestr("/one-slash.txt", b"content")
619616
zf.writestr("//two-slash.txt", b"content")
620617
zf.writestr("../parent.txt", b"content")
@@ -632,7 +629,7 @@ def test_unsupported_names(self):
632629
in the zip file.
633630
"""
634631
data = io.BytesIO()
635-
zf = zipfile.ZipFile(data, "w")
632+
zf = self.enterContext(zipfile.ZipFile(data, "w"))
636633
zf.writestr("path?", b"content")
637634
zf.writestr("V: NMS.flac", b"fLaC...")
638635
zf.filename = ''
@@ -647,7 +644,7 @@ def test_backslash_not_separator(self):
647644
In a zip file, backslashes are not separators.
648645
"""
649646
data = io.BytesIO()
650-
zf = zipfile.ZipFile(data, "w")
647+
zf = self.enterContext(zipfile.ZipFile(data, "w"))
651648
zf.writestr(DirtyZipInfo("foo\\bar")._for_archive(zf), b"content")
652649
zf.filename = ''
653650
root = zipfile.Path(zf)

Lib/test/test_zipfile/test_core.py

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,13 @@
2626
from test.support import (
2727
findfile, requires_zlib, requires_bz2, requires_lzma,
2828
requires_zstd, captured_stdout, captured_stderr, requires_subprocess,
29-
cpython_only
29+
cpython_only, gc_collect
3030
)
3131
from test.support.os_helper import (
3232
TESTFN, unlink, rmtree, temp_dir, temp_cwd, fd_count, FakePath
3333
)
3434
from test.support.import_helper import ensure_lazy_imports
35+
from test.support.warnings_helper import check_no_resource_warning
3536

3637

3738
TESTFN2 = TESTFN + "2"
@@ -4058,6 +4059,28 @@ def test_close_on_exception(self):
40584059
except zipfile.BadZipFile:
40594060
self.assertIsNone(zipfp2.fp, 'zipfp is not closed')
40604061

4062+
def test_garbage_collection(self):
4063+
# gh-81954: Warn if a writable zipfile is closed by GC.
4064+
with self.assertWarns(ResourceWarning):
4065+
zipfile.ZipFile(io.BytesIO(), "w")
4066+
gc_collect()
4067+
4068+
# Only warn if there is possible data loss.
4069+
# Properly closed via context manager.
4070+
buf = io.BytesIO()
4071+
with zipfile.ZipFile(buf, "w") as zf:
4072+
zf.writestr("f.txt", b"data")
4073+
4074+
with check_no_resource_warning(self):
4075+
# Read mode: No possible data loss.
4076+
zipfile.ZipFile(buf, "r")
4077+
4078+
# Write with manual explicit close: No pending data.
4079+
zf = zipfile.ZipFile(io.BytesIO(), "w")
4080+
zf.writestr("f.txt", b"data")
4081+
zf.close()
4082+
del zf
4083+
40614084
def test_unsupported_version(self):
40624085
# File has an extract_version of 120
40634086
data = (b'PK\x03\x04x\x00\x00\x00\x00\x00!p\xa1@\x00\x00\x00\x00\x00\x00'
@@ -5510,10 +5533,10 @@ def test_root_folder_in_zipfile(self):
55105533
the zip file, this is a strange behavior, but we should support it.
55115534
"""
55125535
in_memory_file = io.BytesIO()
5513-
zf = zipfile.ZipFile(in_memory_file, "w")
5514-
zf.mkdir('/')
5515-
zf.writestr('./a.txt', 'aaa')
5516-
zf.extractall(TESTFN2)
5536+
with zipfile.ZipFile(in_memory_file, "w") as zf:
5537+
zf.mkdir('/')
5538+
zf.writestr('./a.txt', 'aaa')
5539+
zf.extractall(TESTFN2)
55175540

55185541
def tearDown(self):
55195542
rmtree(TESTFN2)

Lib/zipfile/__init__.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import sys
1313
import threading
1414
import time
15+
lazy import warnings
1516

1617
try:
1718
import zlib # We may need its compression method
@@ -2616,6 +2617,11 @@ def mkdir(self, zinfo_or_directory_name, mode=511):
26162617

26172618
def __del__(self):
26182619
"""Call the "close()" method in case the user forgot."""
2620+
# gh-81954: Warn if writable ZipFile is implicitly closed.
2621+
# GC cleanup order is non-deterministic and can result in data loss.
2622+
if self.fp is not None and self.mode in ('w', 'x', 'a'):
2623+
warnings.warn(f"unclosed ZipFile {self!r}",
2624+
ResourceWarning, source=self, stacklevel=2)
26192625
self.close()
26202626

26212627
def close(self):
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Deleting a writable, open :class:`zipfile.ZipFile` now emits a
2+
:exc:`ResourceWarning`. Use as a :term:`context manager`
3+
or call :meth:`~zipfile.ZipFile.close` explicitly.

0 commit comments

Comments
 (0)