Skip to content

Commit 82d68c7

Browse files
committed
gh-154842: Reject repack() while a reading handle is open
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 8b048eb commit 82d68c7

3 files changed

Lines changed: 20 additions & 1 deletion

File tree

Doc/library/zipfile.rst

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

583583
Rewrites the archive to remove unreferenced local file entries, shrinking
584-
its file size. The archive must be opened with mode ``'a'``.
584+
its file size. The archive must be opened with mode ``'a'``, and any file
585+
object returned by :meth:`ZipFile.open` must be closed first, since
586+
repacking moves the member data those objects read from.
585587

586588
If *removed* is provided, it must be a sequence of :class:`ZipInfo` objects
587589
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
@@ -2382,6 +2382,18 @@ def test_repack_writing(self, m_repack):
23822382
zh.repack()
23832383
m_repack.assert_not_called()
23842384

2385+
@mock.patch.object(zipfile, '_ZipRepacker')
2386+
def test_repack_reading(self, m_repack):
2387+
self._prepare_zip_from_test_files(TESTFN, self.test_files)
2388+
with zipfile.ZipFile(TESTFN, 'a') as zh:
2389+
with zh.open(self.test_files[0][0]):
2390+
with self.assertRaises(ValueError):
2391+
zh.repack()
2392+
m_repack.assert_not_called()
2393+
# Allowed once the reading handle is closed.
2394+
zh.repack()
2395+
m_repack.assert_called_once()
2396+
23852397
@mock.patch.object(zipfile, '_ZipRepacker')
23862398
def test_repack_mode_r(self, m_repack):
23872399
self._prepare_zip_from_test_files(TESTFN, self.test_files)

Lib/zipfile/__init__.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2404,6 +2404,11 @@ def repack(self, removed=None, *, strict_descriptor=True,
24042404
raise ValueError(
24052405
"Can't write to ZIP archive while an open writing handle exists"
24062406
)
2407+
if self._fileRefCnt > 1:
2408+
raise ValueError(
2409+
"Can't repack the ZIP archive while an open reading handle "
2410+
"exists. Close the reading handle before repacking."
2411+
)
24072412

24082413
with self._lock:
24092414
self._writing = True

0 commit comments

Comments
 (0)