Skip to content

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

Merged
serhiy-storchaka merged 2 commits into
python:mainfrom
fedonman:zipfile-repack-open-readers
Aug 16, 2026
Merged

gh-154842: Reject repack() while a reading handle is open#154843
serhiy-storchaka merged 2 commits into
python:mainfrom
fedonman:zipfile-repack-open-readers

Conversation

@fedonman

@fedonman fedonman commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

ZipFile.repack() moves member data to lower offsets, 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 BadZipFile: Bad CRC-32. Raise ValueError while an open reading handle exists, as the writing-handle case already does.

The test fails without the guard. repack() is new in 3.16, so no NEWS entry.

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.
@read-the-docs-community

read-the-docs-community Bot commented Jul 28, 2026

Copy link
Copy Markdown

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add NEWS. This feature was already released in alpha/beta versions.

Sorry, it is 3.16 only, no NEWS.

Comment thread Lib/zipfile/__init__.py Outdated
raise ValueError(
"Can't write to ZIP archive while an open writing handle exists"
)
if self._fileRefCnt > 1:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would not it be better to move all two (or even three) checks into the with self._lock block?

And maybe harmonize or consolidate the two last errors? Something like "Can't repack ZIP archive while an open handle exists".

@fedonman fedonman Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both done, thanks.

The fp/_writing/_fileRefCnt checks guarded state that repack() then mutates
inside `with self._lock`, so the check and the `_writing = True` it gates were
not atomic. Move them into the lock. The writing-handle and reading-handle
errors describe the same condition, so state it once.
@fedonman
fedonman force-pushed the zipfile-repack-open-readers branch from 5d6c2cc to bfe8238 Compare August 16, 2026 16:51

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍

@serhiy-storchaka
serhiy-storchaka enabled auto-merge (squash) August 16, 2026 16:57
@serhiy-storchaka
serhiy-storchaka merged commit 7a845ce into python:main Aug 16, 2026
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants