gh-154842: Reject repack() while a reading handle is open (GH-154843)

serhiy-storchaka <[email protected]>
Newsgroups gmane.comp.python.cvs
Message-ID <[email protected]>
https://github.com/python/cpython/commit/7a845ce16548bf94e777984458ea534c5a65a2a8
commit: 7a845ce16548bf94e777984458ea534c5a65a2a8
branch: main
author: Vyron Vasileiadis <[email protected]>
committer: serhiy-storchaka <[email protected]>
date: 2026-08-16T17:24:49Z
summary:

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.

files:
M Doc/library/zipfile.rst
M Lib/test/test_zipfile/test_core.py
M Lib/zipfile/__init__.py

diff --git a/Doc/library/zipfile.rst b/Doc/library/zipfile.rst
index 65bc54e3856a945..cfbbc98a4739b8f 100644
--- a/Doc/library/zipfile.rst
+++ b/Doc/library/zipfile.rst
@@ -585,7 +585,9 @@ ZipFile objects
                            strict_descriptor=True[, chunk_size])
 
    Rewrites the archive to remove unreferenced local file entries, shrinking
-   its file size.  The archive must be opened with mode ``'a'``.
+   its file size.  The archive must be opened with mode ``'a'``, and any file
+   object returned by :meth:`ZipFile.open` must be closed first, since
+   repacking moves the member data such objects refer to.
 
    If *removed* is provided, it must be a sequence of :class:`ZipInfo` objects
    representing the recently removed members, and only their corresponding
diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py
index d0ae7ce787bee32..1c6e3a9f0a9a2de 100644
--- a/Lib/test/test_zipfile/test_core.py
+++ b/Lib/test/test_zipfile/test_core.py
@@ -2388,6 +2388,18 @@ def test_repack_writing(self, m_repack):
                     zh.repack()
         m_repack.assert_not_called()
 
+    @mock.patch.object(zipfile, '_ZipRepacker')
+    def test_repack_reading(self, m_repack):
+        self._prepare_zip_from_test_files(TESTFN, self.test_files)
+        with zipfile.ZipFile(TESTFN, 'a') as zh:
+            with zh.open(self.test_files[0][0]):
+                with self.assertRaises(ValueError):
+                    zh.repack()
+            m_repack.assert_not_called()
+            # Allowed once the reading handle is closed.
+            zh.repack()
+        m_repack.assert_called_once()
+
     @mock.patch.object(zipfile, '_ZipRepacker')
     def test_repack_mode_r(self, m_repack):
         self._prepare_zip_from_test_files(TESTFN, self.test_files)
diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py
index dd1f7fb9e802048..7a81aa8f44c8f4c 100644
--- a/Lib/zipfile/__init__.py
+++ b/Lib/zipfile/__init__.py
@@ -2395,15 +2395,16 @@ def repack(self, removed=None, *, strict_descriptor=True,
         truncation."""
         if self.mode != 'a':
             raise ValueError("repack() requires mode 'a'")
-        if not self.fp:
-            raise ValueError(
-                "Attempt to write to ZIP archive that was already closed")
-        if self._writing:
-            raise ValueError(
-                "Can't write to ZIP archive while an open writing handle exists"
-            )
 
         with self._lock:
+            if not self.fp:
+                raise ValueError(
+                    "Attempt to write to ZIP archive that was already closed")
+            if self._writing or self._fileRefCnt > 1:
+                raise ValueError(
+                    "Can't repack ZIP archive while an open handle exists"
+                )
+
             self._writing = True
             try:
                 repacker = _ZipRepacker(

_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3//lists/python-checkins.python.org
Member address: [email protected]
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.