Skip to content

Fix stream reader readinto position accounting - #351

Open
CAOShurong wants to merge 1 commit into
indygreg:mainfrom
CAOShurong:codex/295-correct-stream-reader-tell
Open

CAOShurong wants to merge 1 commit into
indygreg:mainfrom
CAOShurong:codex/295-correct-stream-reader-tell

Conversation

@CAOShurong

Copy link
Copy Markdown

Fixes #295.

The C readinto() and readinto1() paths compress into a local output buffer,
but at EOF they increment bytesCompressed from self->output.pos. This can
leave tell() unchanged or wrap the counter after earlier output. Use the local
output.pos - oldPos instead, without changing compression or buffer ownership.

The regressions check cumulative emitted bytes after every call, bytes/stream
sources, small and multi-block inputs, mixed reads, guarded writable views,
repeated EOF and invalid/closed calls. They also verify decompression round trips.
The unchanged C backend fails two of the three new methods; CFFI passes them.

Validation on Windows x64, CPython 3.13.1, after building an sdist/wheel and
installing the wheel non-editably:

  • pytest --import-mode=importlib <checkout>/tests/test_compressor_stream_reader.py -q:
    22 passed on C and CFFI, twice each.
  • pytest --import-mode=importlib <checkout>/tests -q: C 251 passed/44 skipped;
    CFFI 227 passed/68 skipped, twice each. All 292 pre-existing test outcomes match
    the unchanged baseline.
  • With ZSTD_SLOW_TESTS=1, compression-reader fuzzing: 16 passed per backend
    (--hypothesis-seed=295, default profile).
  • ruff check ., changed-file format check, configured mypy (33 files), and
    git diff --check passed. C extension built with warnings as errors.

The setuptools test_suite/tests_require warnings are unchanged from baseline.
An empty bytes source stalled in the unchanged baseline and was excluded from
the additional API probe; an empty BytesIO source is covered by the regression.
Rust, other platforms/Python versions, and the full CI fuzzing profile were not
run locally.

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.

Silent data-correctness bug: readinto() / readinto1() on stream_reader return tell() == 0 after successful reads

1 participant