GH-128131: Completely support random read access of uncompressed unencrypted files in ZipFile #128143
Conversation
5ec1cff
commented
Dec 21, 2024
•
5ec1cff
commented
- Issue: random access uncompressed unencrypted ZipExtFile #128131
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Closing as a duplicate of #128132. Please do not copy PRs made by others. Thanks. EDIT: It appears that this is either the same person or two people collaborating with each other. Anyway, please do not update two identical PRs. Thanks |
We discussed about this issue and decide that I submit the pr. vvb2060 has already closed his pr. Please reopen my pr, sorry for inconvenience. |
Misc/NEWS.d/next/Library/2024-12-21-03-20-12.gh-issue-128131.QpPmNt.rst
Outdated
Show resolved
Hide resolved
5ec1cff
changed the title
Added. |
…pPmNt.rst Co-authored-by: Bénédikt Tran <[email protected]>
add unittest
5ec1cff
force-pushed
the
5ec1cff-patch-1
branch
from
2c34891 to
57cb51c
Compare
Lib/test/test_zipfile/test_core.py
Outdated
| b = fp.read(100) | ||
| self.assertEqual(b, txt[:100]) | ||
|
|
||
| # seek length must be greater than ZipExtFile.MIN_READ_SIZE (4096) |
There was a problem hiding this comment.
In this case, I would add something like self.assertLess(ZipExtFile.MIN_READ_SIZE, 5000) to be sure that we update the tests if needed.
EDIT: check this outside the "with".
There was a problem hiding this comment.
Can we use ZipExtFile.MIN_READ_SIZE directly ?
5ec1cff
changed the title
Misc/NEWS.d/next/Library/2024-12-21-03-20-12.gh-issue-128131.QpPmNt.rst
Outdated
Show resolved
Hide resolved
5ec1cff
changed the title
5ec1cff
changed the title
There was a problem hiding this comment.
Please address this comment: https://github.com/python/cpython/pull/128143/files#r1894920324.
Lib/test/test_zipfile/test_core.py
Outdated
| arr = fp.read(100) | ||
| self.assertEqual(fp.tell(), 10102) | ||
| self.assertEqual(arr, txt[10002:10102]) | ||
| self.assertLessEqual(sio.bytes_read - old_count, 4096, 'Redundant bytes were read during forward seek and read!') |
There was a problem hiding this comment.
Wrap the lines under 80-chars, e.g.:
diffcount = sio.bytes_read - old_count
self.assertLessEqual(diffcount, 4096, "forward seek: extra bytes read")
Lib/test/test_zipfile/test_core.py
Outdated
| arr = fp.read(100) | ||
| self.assertEqual(fp.tell(), 5199) | ||
| self.assertEqual(arr, txt[5099:5199]) | ||
| self.assertLessEqual(sio.bytes_read - old_count, 4096, 'Redundant bytes were read during backward seek and read!') |
There was a problem hiding this comment.
Ditto (message can be: "backward seek: extra bytes read")
| # disable CRC checking after first seeking - it would be invalid | ||
| self._expected_crc = None | ||
| # seek actual file taking already buffered data into account | ||
| read_offset -= len(self._readbuffer) - self._offset | ||
| self._fileobj.seek(read_offset, os.SEEK_CUR) | ||
| self._left -= read_offset | ||
| self._compress_left -= read_offset |
There was a problem hiding this comment.
This is a good catch that was a bug with read_offset > 0. Are you able to make a test for this too?
5ec1cff
force-pushed
the
5ec1cff-patch-1
branch
from
692109a to
67f05de
Compare
