Skip to content

Conversation

@danifus
Copy link
Contributor

@danifus danifus commented Jul 12, 2024

Fixes: gh-121639
Related to: #81719

Add ZipExtFile._init_read() to perform initialization and any reinitialization if necessary. The name _init_read() was chosen to match the surrounding _init_decrypter() method but these might need to lose the leading _ in a different PR if we're going to allow subclassing ZipExtFile.

Since we now check the decryption check byte in _init_decrypter(), I think it would also make sense for _init_decrypter() to return the decrypter (or nothing) rather than the check byte but that would change the (_internal) api so I haven't added it here. Happy to update this PR with that change if it is acceptable.

raise RuntimeError("Bad password for file %r" % self._zinfo.orig_filename)
return h

def _init_read(self):
Copy link
Member

@emmatyping emmatyping Oct 21, 2025

Choose a reason for hiding this comment

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

Suggested change
def _init_read(self):
def _reset_read_state(self):

if h != check_byte:
raise RuntimeError("Bad password for file %r" % zipinfo.orig_filename)

self._init_read()
Copy link
Member

@emmatyping emmatyping Oct 21, 2025

Choose a reason for hiding this comment

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

"init" implies there is some one time setup/initialization going on to me. I'd suggest maybe _reset_read_state

Suggested change
self._init_read()
self._reset_read_state()

self._offset = 0
self._decompressor = _get_decompressor(self._compress_type)
self._eof = False
self._init_read()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self._init_read()
self._reset_read_state()

Copy link
Member

@emmatyping emmatyping left a comment

Choose a reason for hiding this comment

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

Thanks for this, this is a really nice refactor! I have one suggestion on naming but seems good otherwise.

@bedevere-app
Copy link

bedevere-app bot commented Oct 21, 2025

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

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.

zipfile: Deduplicate reinitialization setup for ZipExtFile when seeking

2 participants