Skip to content

Fix IOWrapper indexing the path instead of the file - #16

Merged
ESultanik merged 1 commit into
masterfrom
iowrapper-fixes
Aug 7, 2026
Merged

Fix IOWrapper indexing the path instead of the file#16
ESultanik merged 1 commit into
masterfrom
iowrapper-fixes

Conversation

@ESultanik

Copy link
Copy Markdown
Owner

Stacked on #15.

IOWrapper advertises itself as a collections.abc.Sequence, but dispatched on isinstance(..., Sequence)/Sized at call time — and str is itself a Sequence. So for a file path, every sequence operation silently applied to the path text rather than the file's contents:

>>> open('/tmp/f.bin','wb').write(bytes(range(20)))
>>> len(IOWrapper('/tmp/f.bin'))
14                      # the length of the path string
>>> IOWrapper('/tmp/f.bin')[5]
'_'                     # a character of the path, not even an int

The source kind is now decided once in __init__ — path, stdin, stream, or bytes — so indexing always reads content. Four further defects fall out of that:

  • The integer branch never seeked, so every index returned byte 0.
  • The stepped-slice branch did ret.append(r) with the one-byte bytes from read(1); bytearray.append needs an int, so it raised TypeError.
  • __enter__ recorded any handle that merely differed from self.wrapped, so IOWrapper('-') closed sys.stdin on exit. Ownership is now explicit: only handles we opened get closed.
  • '-' returned the text sys.stdin, so reads yielded str and callers doing b[0] & 0xff hit TypeError. It now returns sys.stdin.buffer.

Negative indices and open-ended slices are handled, and the length is cached (via os.path.getsize for paths).

get_length seeks to the end instead of reading the whole stream in 1 KiB chunks, keeping a chunked fallback for non-seekable streams at 1 MiB.

Dead code replaced

AutoUnzippingStream and GzipIOWrapper — both referenced nowhere — are replaced by an auto_unzip context manager built on ExitStack. Two improvements over what they did: layers tear down outermost-first (the old __exit__ closed the underlying stream before the gzip wrapper), and buffering sits on top of the gzip layer rather than beneath it, which is where it matters since the varint decoder reads a byte at a time. Nothing consumes it yet; the CLI decrypt path adopts it next.

Verification

tests/test_iowrapper.py: 58 cases across path, str-path, bytes, bytearray, stream, and int-iterable sources, plus a subprocess check that stdin is read as binary and survives the context manager. Two xfail markers removed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01367hFob9sd4xpDmVT4uoFy

`IOWrapper` advertises itself as a `collections.abc.Sequence`, but dispatched on
`isinstance(..., Sequence)`/`Sized` at call time -- and `str` is itself a
Sequence. So for a file path, every sequence operation silently applied to the
path text rather than the file's contents:

    >>> open('/tmp/f.bin','wb').write(bytes(range(20)))
    >>> len(IOWrapper('/tmp/f.bin'))
    14                      # the length of '/tmp/f.bin'... plus tmp dir
    >>> IOWrapper('/tmp/f.bin')[5]
    '_'                     # a character of the path, not even an int

The source kind is now decided once in `__init__` -- path, stdin, stream or
bytes -- so indexing always reads content. Four further defects fall out of that:

  * The integer branch never seeked, so every index returned byte 0.
  * The stepped-slice branch did `ret.append(r)` with the one-byte `bytes` from
    `read(1)`; `bytearray.append` needs an int, so it raised TypeError.
  * `__enter__` recorded any handle that merely differed from `self.wrapped`, so
    `IOWrapper('-')` closed **sys.stdin** on exit. Ownership is now explicit: only
    handles we opened get closed.
  * `'-'` returned the *text* `sys.stdin`, so reads yielded `str` and callers doing
    `b[0] & 0xff` hit TypeError. It now returns `sys.stdin.buffer`.

Negative indices and open-ended slices are handled, and the length is cached
(via `os.path.getsize` for paths).

`get_length` seeks to the end instead of reading the whole stream in 1 KiB
chunks, keeping a chunked fallback for non-seekable streams at 1 MiB.

`AutoUnzippingStream` and `GzipIOWrapper` -- both dead code, referenced nowhere --
are replaced by an `auto_unzip` context manager built on ExitStack. Two
improvements over what they did: layers tear down outermost-first (the old
`__exit__` closed the underlying stream before the gzip wrapper), and buffering
sits *on top* of the gzip layer rather than beneath it, which is where it
matters, since the varint decoder reads a byte at a time. Nothing consumes it
yet; the CLI decrypt path adopts it next.

Adds `tests/test_iowrapper.py`: 58 cases across path, str-path, bytes, bytearray,
stream and int-iterable sources, plus a subprocess check that stdin is read as
binary and survives the context manager. Two xfail markers removed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01367hFob9sd4xpDmVT4uoFy
@ESultanik
ESultanik force-pushed the progress-portability branch from 36f70db to 794125c Compare August 7, 2026 19:25
Base automatically changed from progress-portability to master August 7, 2026 19:30
@ESultanik
ESultanik merged commit 5ad5d9e into master Aug 7, 2026
11 checks passed
@ESultanik
ESultanik deleted the iowrapper-fixes branch August 7, 2026 19:31
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.

1 participant