Skip to content

lzma: treat empty input as a no-op, not end of stream - #8752

Merged
youknowone merged 1 commit into
RustPython:mainfrom
hyoinandout:lzma-empty-input-not-eof
Sep 20, 2026
Merged

youknowone merged 1 commit into
RustPython:mainfrom
hyoinandout:lzma-empty-input-not-eof

Conversation

@hyoinandout

@hyoinandout hyoinandout commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

One of checkbox below must be checked.

  • I did not use AI to write the code of this patch.
  • This PR follows our AI policy

Summary

LZMADecompressor.decompress(b"") finished the decompressor instead of doing nothing.

decompress_chunks returns whether the codec signalled LZMA_STREAM_END, and it
short-circuits when the chunker has nothing left to feed. That early return answered
"the stream ended" for the case where no input was supplied at all — an empty hand,
not an empty stream.

The caller latches that answer with self.eof |= stream_end, which only ever flips
eof on. A single empty call therefore finished the object for good: needs_input
went false and every later call was rejected by the if self.eof guard with
EOFError, before a single compressed byte had been seen.

import lzma
blob = lzma.compress(b"data")

d = lzma.LZMADecompressor()
d.decompress(b"")   # d.eof was True / needs_input False, now False / True
d.decompress(blob)  # was EOFError: End of stream already reached, now b'data'

The zlib engine returns false from the identical short-circuit in
compression/zlib.rs; lzma had drifted away from it, and this puts the two back in
agreement.

On the bz2 half of the issue: BZ2Decompressor was fixed incidentally by #8638,
which moved the bz2 engine into crates/common. I confirmed current main already
matches CPython there, so this PR adds only a regression test for bz2 and no
behaviour change.

Testing

Run on macOS arm64 (aarch64-apple-darwin) against a local CPython 3.14.7.

The script from the issue now prints the documented False True / b'data' for both
decompressors, matching CPython exactly.

  • Lib/test/test_lzma.py::test_decompressor_chunks_empty — this is precisely the
    reported scenario, and it was marked expectedFailure with this very EOFError.
    Marker removed. I re-checked by reverting only the one-line engine change and
    rebuilding: the test fails with EOFError: End of stream already reached at
    test_lzma.py:152, and passes with the change, so it genuinely pins the regression
    rather than riding along.
  • empty_input_does_not_finish_a_stream — new engine unit test, named after the zlib
    test that already existed and mirrored into bz2, so all three engines stay pinned to
    the same behaviour instead of drifting apart again.
  • test_lzma — 121 run, 1 skipped, SUCCESS
  • test_bz2, test_zlib, test_gzip, test_tarfile — 1009 run, 113 skipped, SUCCESS
  • cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi — clean
  • cargo fmt --check — clean
  • cargo clippy -p rustpython-common --all-features --all-targets -- -D warnings — clean

One note on the capi suite, which AGENTS.md also asks for: cargo test in
crates/capi dies with SIGSEGV on abstract_::iter::tests::next_item. I verified it
crashes identically on unmodified main (2006c63) with this change stashed, so it is
pre-existing and unrelated.

Not verified on Linux or Windows; the change is platform-independent control flow with
no cfg-gated paths.

AI disclosure

Per the AI policy, this patch was written with AI assistance (Claude Code, model
claude-opus-5); the commit carries an Assisted-by: trailer. I reviewed the change,
and the CPython comparison, the fails-without-the-fix check, and the test and lint runs
above were all executed locally on real hardware.

Summary by CodeRabbit

  • Bug Fixes
    • Empty decompression calls no longer incorrectly mark BZ2 or LZMA streams as complete.
    • Compressed data supplied after an empty input is now processed correctly, producing the expected output and end-of-stream status.

decompress_chunks reports whether the codec signalled LZMA_STREAM_END,
and it short-circuits when the chunker has nothing left to feed. That
early return answered "the stream ended" for the case where no input was
supplied at all, so LZMADecompressor.decompress(b"") claimed a finish the
decoder had never signalled.

The caller latches that answer with `self.eof |= stream_end`, which only
ever flips eof on. A single empty call therefore finished the object for
good: needs_input went false and every later call was rejected by the
`if self.eof` guard with EOFError, before a single compressed byte had
been seen. CPython leaves eof false and needs_input true, and accepts
compressed data handed over by a later call.

  d = lzma.LZMADecompressor()
  d.decompress(b"")   # d.eof was True, now False
  d.decompress(blob)  # was EOFError, now b'data'

The zlib engine returns false from the same short-circuit; lzma had
drifted away from it. bz2 was fixed incidentally by RustPython#8638 when its engine
moved into crates/common, so only lzma needed a code change.

Verified on macOS arm64 against CPython 3.14.7: the script from the issue
now prints the documented "False True" and b'data' for both
decompressors. test_decompressor_chunks_empty in Lib/test/test_lzma.py
covers exactly this and no longer needs its expectedFailure marker. The
added engine test is named after the zlib one that already existed and is
mirrored into bz2, so all three engines stay pinned to the same
behaviour. test_lzma, test_bz2, test_zlib, test_gzip, test_tarfile,
cargo test --workspace, cargo fmt --check and cargo clippy
-p rustpython-common --all-features --all-targets -D warnings are clean.

Fixes RustPython#8637

Assisted-by: Claude Code:claude-opus-5
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@github-actions github-actions Bot added the z-ca-2026 Tag to track Contribution Academy 2026 label Sep 20, 2026
@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: 614cc42c-3872-4e50-a694-5d9c8d3b1c50

📥 Commits

Reviewing files that changed from the base of the PR and between 4cc7559 and d05ac3b.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_lzma.py is excluded by !Lib/**
📒 Files selected for processing (2)
  • crates/common/src/compression/bz2.rs
  • crates/common/src/compression/lzma.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change keeps empty decompression calls from marking streams as finished. LZMA updates its EOF result, and new tests verify that LZMA and BZ2 accept compressed data in a later call.

Changes

Compression empty-input handling

Layer / File(s) Summary
LZMA empty-input handling
crates/common/src/compression/lzma.rs
Empty LZMA input returns no output with eof() false. A later call can process compressed data and reach EOF.
BZ2 empty-input regression coverage
crates/common/src/compression/bz2.rs
A test verifies that empty BZ2 input does not finish the stream and that later compressed input produces the original payload.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: youknowone

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main LZMA behavior change. It accurately states that empty input is treated as a no-op rather than end of stream.
Linked Issues check ✅ Passed The changes satisfy issue #8637. crates/common/src/compression/lzma.rs now treats an empty input chunk as non-terminal. The LZMA regression test verifies empty output, eof == false, and successful…
Out of Scope Changes check ✅ Passed The changes are limited to the compression implementations and regression tests required by issue #8637. No unrelated behavior or files are identified in the reviewed pull-request summary.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] lib: cpython/Lib/lzma.py
[x] test: cpython/Lib/test/test_lzma.py (TODO: 9)

dependencies:

  • lzma

dependent tests: (102 tests)

  • lzma: test_lzma test_tarfile
    • shutil: test_argparse test_bz2 test_compileall test_ctypes test_embed test_filecmp test_glob test_httpservers test_importlib test_inspect test_largefile test_launcher test_logging test_modulefinder test_os test_peg_generator test_pkgutil test_py_compile test_reprlib test_sax test_shutil test_site test_string_literals test_subprocess test_support test_sysconfig test_tempfile test_traceback test_unicode_file test_venv test_zoneinfo
      • ctypes.util: test_ctypes
      • ensurepip: test_ensurepip
      • http.server: test_robotparser test_urllib2_localnet test_xmlrpc
      • multiprocessing.util: test_asyncio test_concurrent_futures
      • pathlib: test_ast test_dbm_sqlite3 test_importlib test_json test_pathlib test_pyrepl test_runpy test_tomllib test_tools test_unparse test_winapi test_zipapp test_zipfile test_zstd
      • tempfile: test_asyncio test_bytes test_cmd_line test_compile test_concurrent_futures test_contextlib test_cprofile test_csv test_dis test_doctest test_faulthandler test_fileinput test_generated_cases test_genericalias test_hashlib test_importlib test_linecache test_mailbox test_ntpath test_pickle test_pkg test_posix test_pstats test_pydoc test_pyrepl test_regrtest test_selectors test_shlex test_socket test_sys test_sys_settrace test_tabnanny test_termios test_threadedtempfile test_tokenize test_turtle test_urllib test_urllib2 test_urllib_response test_winconsoleio test_zipfile test_zipfile64
      • webbrowser: test_webbrowser
      • zipapp: test_pdb
      • zipfile: test_zipfile test_zipimport test_zipimport_support
    • zipfile:
      • importlib.metadata: test_importlib

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

@youknowone youknowone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, thanks!

@codspeed

codspeed Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1


Comparing hyoinandout:lzma-empty-input-not-eof (d05ac3b) with main (4cc7559)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@youknowone
youknowone merged commit 4c7cc0a into RustPython:main Sep 20, 2026
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

z-ca-2026 Tag to track Contribution Academy 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

compression: empty input marks BZ2/LZMA decompressors as EOF

2 participants