indygreg / indygreg/python-zstandard

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

Open Beginner friendly
#295 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
642
Forks
116
Avg merge
1d 14h
Merged PRs (30d)
5

Description

## Summary

`ZstdCompressionReader.readinto(buf)` and `.readinto1(buf)` correctly copy compressed bytes into the caller's buffer, but the reader's internal `bytesCompressed` counter is not updated. As a result, `stream_reader.tell()` returns `0` after any number of successful `readinto()` calls, even though bytes were in fact written. The equivalent `.read()` path correctly advances the counter.

## Impact

- **Severity:** Silent data-correctness bug — no crash, no exception, just a wrong value from `tell()`. Any caller that relies on `tell()` to measure progress, compute offsets, or compare positions will silently malfunction.
- **Reachability:** Standard `io.RawIOBase` idioms — any use of `readinto()` / `readinto1()` on a `stream_reader`. Common in performance-sensitive decode pipelines that reuse a pre-allocated buffer.
- **Version:** 0.25.0 (commit `7a77a75`).
- **Platform:** Platform-independent.

## Reproducer

```python
import zstandard, io

data = b'hello world ' * 10000

# read() — tell() works correctly
comp1 = zstandard.ZstdCompressor()
r1 = comp1.stream_reader(io.BytesIO(data))
r1.__enter__()
while r1.read(1024):
pass
print("read tell:", r1.tell()) # 29 (correct: total compressed bytes)
r1.__exit__(None, None, None)

# readinto() — tell() stuck at 0
comp2 = zstandard.ZstdCompressor()
r2 = comp2.stream_reader(io.BytesIO(data))
r2.__enter__()
buf = bytearray(1024)
while r2.readinto(buf):
pass
print("readinto tell:", r2.tell()) # 0 — BUG (should match read() path)
r2.__exit__(None, None, None)
```

## Root cause

`readinto` / `readinto1` build a `ZSTD_outBuffer` **on the stack** that wraps the caller's buffer:

```c
ZSTD_outBuffer output = {dest, dest_size, 0};
zresult = ZSTD_compressStream2(cctx, &output, &input, ZSTD_e_continue);
```

After the call, `output.pos` holds the number of bytes written to `dest`. The reader then updates its position by reading from `self->output.pos` — the **persistent** struct, which the local-struct call never touched. So `self->output.pos` stays at zero, and `bytesCompressed` never advances.

The `read()` path uses `self->output` directly (not a local copy), so the persistent field *is* updated by `ZSTD_compressStream2`. That's why `tell()` works after `read()` but not after `readinto`.

## Affected sites

- `c-ext/compressionreader.c` — `readinto` and `readinto1` method bodies.

(Same pattern may warrant a look on the decompression side as well, although the main analysis only flagged compression.)

## Suggested fix

Two options; either is minimal.

### Option A — advance from the local struct

Read `bytesCompressed` from the local `output.pos` before it goes out of scope:

```c
ZSTD_outBuffer output = {dest, dest_size, 0};
zresult = ZSTD_compressStream2(cctx, &output, &input, ZSTD_e_continue);
/* ... */
self->bytesCompressed += output.pos; /* was: effectively + self->output.pos, i.e. 0 */
```

### Option B — share the persistent struct

If you'd rather the two methods share the `read()` bookkeeping path, use `self->output` directly instead of a stack-local struct, and update `self->output.dst` / `self->output.size` to point at the caller's buffer before the call. Slightly more invasive but avoids duplicating the position-update logic.

## Methodology

Found via [cext-review-toolkit](https://github.com/devdanzin/cext-review-toolkit) (Tree-sitter-based static analysis with structured naive/informed review passes). Reproducer verified live on CPython 3.14.3 debug build — `read()` path returns `tell() == 29` (matches the compressed-output length); `readinto()` path returns `tell() == 0` after the exact same compressed sequence is consumed. Happy to open a PR.

*Discovery, root-cause analysis, and issue drafting were performed by [Claude Code](https://www.anthropic.com/claude-code) and reviewed by a human before filing.*

## Full report

Complete multi-agent analysis (48 FIX findings across 13 categories, plus a reproducer appendix): https://gist.github.com/devdanzin/b86039ac097141579590c1a0f3a43605

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in c-ext/compressionreader.c at the readinto and readinto1 method bodies, comparing their local ZSTD_outBuffer with the read() path's persistent output bookkeeping. Run the supplied Python reproducer and verify that tell() after readinto() and readinto1() advances consistently with read(); inspect the decompression-side pattern if relevant.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, python
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.