Repository navigation
Limit zip file entries to 100MB by default - #406
claude[bot] wants to merge 1 commit into
Conversation
Ports sillsdev/machine#481. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-authored-by: Eli C. Lowry <83078660+Enkidu93@users.noreply.github.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| for actual_entry_name in self._archive.namelist(): | ||
| if actual_entry_name.lower() == file_name.lower(): | ||
| return BytesIO(self._archive.read(actual_entry_name)) | ||
| with open_bounded_stream(self._archive, actual_entry_name) as stream: |
There was a problem hiding this comment.
Minor: F1. No test checks that the corpora enforce the limit. tests/utils/test_zip_entry_utils.py covers only the helper. Nothing checks that ZipParatextProjectFileHandler.open, ZipEntryStreamContainer.open_stream or DblBundleTextCorpus refuse an oversized entry, so going back to archive.read would still pass CI. Searched tests/ for open_bounded_stream|BoundedStream|BadZipFile: only the new test file matches. A test for each call site is needed (AGENTS.md: focused tests with every behavior change).
| if entry.file_size > max_uncompressed_size: | ||
| raise BadZipFile("Entry uncompressed size exceeds maximum allowed limit.") | ||
|
|
||
| if entry.compress_size > 0 and entry.file_size / entry.compress_size > max_compression_ratio: |
There was a problem hiding this comment.
FYI: F2. The 100:1 ratio check has no minimum entry size, so a small, very repetitive entry is refused. Deflate shrinks a few KB of padding or repeated empty XML elements to tens of bytes, which is over 100:1, and loading the Paratext project then raises BadZipFile. The callers cannot pass a looser limit. This matches C# OpenBoundedStream, so it is parity and not a port defect. Unverified: I could not run Python here.
| def readinto(self, buffer) -> int: # pyright: ignore[reportIncompatibleMethodOverride] | ||
| data = self._inner_stream.read(len(buffer)) | ||
| self._total_bytes_read += len(data) | ||
| if self._total_bytes_read > self._max_size: |
There was a problem hiding this comment.
FYI: F3. In Python this runtime limit probably never fires for a zip entry, so the PR body claim that it "covers entries whose headers understate their size" may not hold. CPython ZipExtFile trims its output to the central-directory file_size (data[:self._left]), and open_bounded_stream has already checked that size against the same limit. An understated header ends in a CRC BadZipFile instead. .NET DeflateStream has no such trim, which is why C# needs the check. Unverified: I could not run Python here.
|
Review summary
Surface changed: Findings:
Reviewed at 6a53b79 |
Zip entries read through the Paratext and DBL corpora are now refused when they are larger than 100MB or compressed more than 100:1. Ports machine PR #481.
Changes
open_bounded_streaminmachine/utils/zip_entry_utils.pyis the counterpart ofOpenBoundedStream. It raisesBadZipFilewhen the declared size or compression ratio exceeds the limits.BoundedStreaminmachine/utils/bounded_stream.pyraisesOSErrorif more than the limit is actually read, which covers entries whose headers understate their size.ZipEntryStreamContainer,ZipParatextProjectFileHandlerandDblBundleTextCorpusnow open entries through it. Before, they usedarchive.readorarchive.openwith no limit.Write,SetLengthand seek handling is not ported. The Python stream is read-only, since only reads are used.Tests
tests/utils/test_zip_entry_utils.pyports the four C# tests and adds an empty-entry case../local_check.sh --agent-strict: 868 passed, 3 skipped.Closes #358
🤖 Generated with Claude Code
This change is