Fix IOWrapper indexing the path instead of the file - #16
Merged
Conversation
`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
force-pushed
the
progress-portability
branch
from
August 7, 2026 19:25
36f70db to
794125c
Compare
ESultanik
force-pushed
the
iowrapper-fixes
branch
from
August 7, 2026 19:25
80759c9 to
6c96d30
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #15.
IOWrapperadvertises itself as acollections.abc.Sequence, but dispatched onisinstance(..., Sequence)/Sizedat call time — andstris itself a Sequence. So for a file path, every sequence operation silently applied to the path text rather than the file's contents: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:ret.append(r)with the one-bytebytesfromread(1);bytearray.appendneeds an int, so it raisedTypeError.__enter__recorded any handle that merely differed fromself.wrapped, soIOWrapper('-')closedsys.stdinon exit. Ownership is now explicit: only handles we opened get closed.'-'returned the textsys.stdin, so reads yieldedstrand callers doingb[0] & 0xffhitTypeError. It now returnssys.stdin.buffer.Negative indices and open-ended slices are handled, and the length is cached (via
os.path.getsizefor paths).get_lengthseeks 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
AutoUnzippingStreamandGzipIOWrapper— both referenced nowhere — are replaced by anauto_unzipcontext manager built onExitStack. 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. Twoxfailmarkers removed.🤖 Generated with Claude Code
https://claude.ai/code/session_01367hFob9sd4xpDmVT4uoFy