bpo-5950: Support reading zips with comments in zipimport#9548
Conversation
|
Looks good to me. |
warsaw
left a comment
warsaw
left a comment
There was a problem hiding this comment.
Thank you for adding support for a long standing request! I have a few questions about code details, but in general it LGTM.
| raise ZipImportError(f"can't open Zip file: {archive!r}", path=archive) | ||
|
|
||
| with fp: | ||
| end_cent_dir_size = 22 |
There was a problem hiding this comment.
Perhaps make this a module constant, all upper cased? Also, what does the abbreviation "cent" mean?
There was a problem hiding this comment.
Done. "cent" means "central".
| if len(buffer) != end_cent_dir_size: | ||
| raise ZipImportError(f"can't read Zip file: {archive!r}", path=archive) | ||
| if buffer[:4] != b'PK\x05\x06': | ||
| string_end_archive = b'PK\x05\x06' |
There was a problem hiding this comment.
This is another candidate for a module constant.
| except OSError: | ||
| raise ZipImportError(f"can't read Zip file: {archive!r}", | ||
| path=archive) | ||
| max_comment_len = (1 << 16) - 1 |
There was a problem hiding this comment.
This is another candidate for a module constant.
|
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 |
|
Thank you, @warsaw, for the comments. I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @warsaw: please review the changes made to this pull request. |
|
@warsaw: Please replace |
5 participants
Some of the code is based on
_EndRecData()in Lib/zipfile.py.https://bugs.python.org/issue5950