Skip to content

Fix AsyncZipReader handling of maximum-length ZIP comments#4522

Open
hsusul wants to merge 2 commits into
UKGovernmentBEIS:mainfrom
hsusul:fix-async-zip-max-comment
Open

Fix AsyncZipReader handling of maximum-length ZIP comments#4522
hsusul wants to merge 2 commits into
UKGovernmentBEIS:mainfrom
hsusul:fix-async-zip-max-comment

Conversation

@hsusul

@hsusul hsusul commented Jul 16, 2026

Copy link
Copy Markdown

Summary

AsyncZipReader previously searched only the final 65,536 bytes of an archive when locating the ZIP end-of-central-directory record.

A valid ZIP archive may contain:

  • a 65,535-byte archive comment
  • a 22-byte end-of-central-directory record

This means the parser may need to search the final 65,557 bytes. With the previous limit, valid archives containing comments between 65,515 and 65,535 bytes raised:

ValueError: EOCD not found

This change:

  • adds private constants for the maximum ZIP comment size and minimum EOCD size
  • expands the suffix search window from 65,536 to 65,557 bytes
  • adds a path-based regression test using a valid archive with a 65,535-byte comment
  • verifies both central-directory parsing and member content through AsyncFilesystem

Why this approach

The change is intentionally limited to the EOCD search window.

It does not modify:

  • ZIP64 handling
  • EOCD matching behavior
  • compression handling
  • exception types or messages
  • the direct IO[bytes] path, which already uses Python’s standard-library ZipFile

The fix affects all path-based backends that use AsyncZipReader, including local files, S3, and other fsspec-backed filesystems.

Testing

  • uv run pytest tests/util/test_async_zip.py -k maximum_comment -q

    • 1 passed, 1 skipped, 47 deselected
  • uv run pytest tests/util/test_async_zip.py -k maximum_comment --runtrio -q

    • 1 passed, 1 skipped, 47 deselected
  • uv run pytest tests/util/test_async_zip.py

    • 26 passed, 23 skipped
  • uv run make check

    • Ruff passed
    • formatting unchanged
    • mypy passed with no issues in 1,296 source files

@dragonstyle dragonstyle self-assigned this Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants