Skip to content

Commit 7a845ce

Browse files
authored
gh-154842: Reject repack() while a reading handle is open (GH-154843)
ZipFile.repack() moves member data, but a ZipExtFile from an earlier open() keeps its own absolute position, so it silently returned data from the wrong place and a full read failed with a misleading CRC error. Raise ValueError while _fileRefCnt shows an open reading handle, as the writing-handle case already does.
1 parent 5b96d39 commit 7a845ce

3 files changed

Lines changed: 23 additions & 8 deletions

File tree

Doc/library/zipfile.rst

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,9 @@ ZipFile objects
585585
strict_descriptor=True[, chunk_size])
586586

587587
Rewrites the archive to remove unreferenced local file entries, shrinking
588-
its file size. The archive must be opened with mode ``'a'``.
588+
its file size. The archive must be opened with mode ``'a'``, and any file
589+
object returned by :meth:`ZipFile.open` must be closed first, since
590+
repacking moves the member data such objects refer to.
589591

590592
If *removed* is provided, it must be a sequence of :class:`ZipInfo` objects
591593
representing the recently removed members, and only their corresponding

Lib/test/test_zipfile/test_core.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2388,6 +2388,18 @@ def test_repack_writing(self, m_repack):
23882388
zh.repack()
23892389
m_repack.assert_not_called()
23902390

2391+
@mock.patch.object(zipfile, '_ZipRepacker')
2392+
def test_repack_reading(self, m_repack):
2393+
self._prepare_zip_from_test_files(TESTFN, self.test_files)
2394+
with zipfile.ZipFile(TESTFN, 'a') as zh:
2395+
with zh.open(self.test_files[0][0]):
2396+
with self.assertRaises(ValueError):
2397+
zh.repack()
2398+
m_repack.assert_not_called()
2399+
# Allowed once the reading handle is closed.
2400+
zh.repack()
2401+
m_repack.assert_called_once()
2402+
23912403
@mock.patch.object(zipfile, '_ZipRepacker')
23922404
def test_repack_mode_r(self, m_repack):
23932405
self._prepare_zip_from_test_files(TESTFN, self.test_files)

Lib/zipfile/__init__.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2395,15 +2395,16 @@ def repack(self, removed=None, *, strict_descriptor=True,
23952395
truncation."""
23962396
if self.mode != 'a':
23972397
raise ValueError("repack() requires mode 'a'")
2398-
if not self.fp:
2399-
raise ValueError(
2400-
"Attempt to write to ZIP archive that was already closed")
2401-
if self._writing:
2402-
raise ValueError(
2403-
"Can't write to ZIP archive while an open writing handle exists"
2404-
)
24052398

24062399
with self._lock:
2400+
if not self.fp:
2401+
raise ValueError(
2402+
"Attempt to write to ZIP archive that was already closed")
2403+
if self._writing or self._fileRefCnt > 1:
2404+
raise ValueError(
2405+
"Can't repack ZIP archive while an open handle exists"
2406+
)
2407+
24072408
self._writing = True
24082409
try:
24092410
repacker = _ZipRepacker(

0 commit comments

Comments
 (0)