Skip to content

gh-156002: Keep reading through third-party zipfile decompressors - #157180

Open
rasmusfaber wants to merge 5 commits into
python:mainfrom
rasmusfaber:gh-156002-third-party-decompressors
Open

gh-156002: Keep reading through third-party zipfile decompressors#157180
rasmusfaber wants to merge 5 commits into
python:mainfrom
rasmusfaber:gh-156002-third-party-decompressors

Conversation

@rasmusfaber

@rasmusfaber rasmusfaber commented Sep 8, 2026

Copy link
Copy Markdown

Follow-up to GH-156003, as discussed in #156002 (comment).

The change to bound decompression made ZipExtFile._read1() call decompress(data, max_length) on non-deflate decompressors and check needs_input. Many third-party decompressors (zipfile-zstd, zipfile-deflate64, and others) that are installed by replacing _get_decompressor() (zipfile-zstd, zipfile-deflate64, ...) do not support those, and end up failing with AttributeError: ... has no attribute '_needs_input'.

This PR:

  • gives _decompressor_needs_input() a default argument for decompressors that report neither needs_input nor _needs_input (no three-state return);
  • decides once per decompressor instance, when ZipExtFile creates it (construction and seek()), whether it takes the bounded path: only decompressors that report needs_input do, which is all of the stdlib ones. Anything else is read unbounded, exactly as before gh-156002: Bound zipfile decompression for bzip2/LZMA/Zstandard #156003;
  • adds a test with a fake third-party compression method whose decompressor has a one-argument decompress() and no needs_input; it fails with the AttributeError above without the fix.

Verified with the real packages by applying the same three hunks to the 3.13 backport (#156738): a Deflate64 archive written by 7-Zip reads through zipfile-deflate64, and a Zstandard member reads through zipfile-zstd, on Python 3.13.15 where both currently fail. test_zipfile, test_zstd, test_zipimport, test_zipapp and test_shutil pass on a debug build of main with _zstd enabled; the *BoundedDecompressTests still pass for every stdlib method.

Should ride along with the open backports #156737#156741.

Claude Fable 5.1 and GPT-6 Astra both reviewed and gave input, as well as drafted PR description and commit messages.

pythonGH-156003 made ZipExtFile._read1() call decompress(data, max_length) on
non-deflate decompressors and consult needs_input before reading more. A
decompressor installed by replacing _get_decompressor() (zipfile-zstd,
zipfile-deflate64, ...) may support neither, and every read through it
then failed with AttributeError.

Give _decompressor_needs_input() a default for decompressors that report
nothing, and only take the bounded path for decompressors that do report
needs_input (the stdlib ones); others are read unbounded, as before.
@python-cla-bot

python-cla-bot Bot commented Sep 8, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@rasmusfaber
rasmusfaber marked this pull request as ready for review September 8, 2026 13:59
Calling decompress() with one argument is a maintenance burden,
so mark it as deprecated in 3.16.
@encukou

encukou commented Sep 9, 2026

Copy link
Copy Markdown
Member

Thanks! I'll push to this PR directly, hope that's fine.

  • The comments should be clear about monkey-patching not being supported.
  • Adding _needs_input (and not needs_input) to LZMADecompressor was not 100% necessary (see the review; this PR pushes toward the public name to simplify implementation.
  • I don't like using the existence of needs_input as a proxy for decompress taking two arguments; I prefer catching the TypeError.
  • The fallback to one-argument decompress() is a maintenance burden; let's schedule removal. (The DeprecationWarning should only be for 3.16, not in the backports.)
  • In the tests, check that the data is actually "compressed" (via swapcase()).

Could you check if this still works for you?

@rasmusfaber

Copy link
Copy Markdown
Author

Thanks! I'll push to this PR directly, hope that's fine.

Of course. I fixed a small typo that caused CI to fail, and a few other tiny nits.

Could you check if this still works for you?

Yes. This still works fine. I will see if I can get fixes merged in the third-party decompressors so they will continue to work when the fallback is removed (and be safe against CVE-2026-15310) even without a proper public API.

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.

2 participants