python: identify_stream() raises UnboundLocalError instead of the real OSError on unseekable streams
- Dominant language
- Rust
- Stars
- 18.6k
- Forks
- 1.2k
- Avg merge
- 3d 12h
- Merged PRs (30d)
- 20
Description
### Summary
`Magika.identify_stream()` raises `UnboundLocalError` instead of propagating the real I/O error when the input stream passes every type guard but its `tell()` fails (i.e. a readable-but-unseekable `io.BufferedIOBase`, such as `sys.stdin.buffer` on a pipe or `socket.makefile("rb")`). The real `OSError: [Errno 29] Illegal seek` is discarded as the *context* of a bogus internal error.
### Environment
- `main` @ `e6a4c8e` ("Expose the Rust library as a C library (#1449)")
- `python/` package version `1.0.3`, Python 3.13, macOS arm64
### Reproduction
```python
import io, os
from magika import Magika
m = Magika()
rp, wp = os.pipe()
os.write(wp, b"hello world this is a test\n" * 10)
os.close(wp)
buf = io.BufferedReader(io.FileIO(rp, closefd=True))
print(isinstance(buf, io.BufferedIOBase), buf.readable(), buf.seekable())
# True True False -> passes every guard in identify_stream()
m.identify_stream(buf)
```
Equivalent one-liner:
```bash
echo hi | python3 -c "import sys; from magika import Magika; Magika().identify_stream(sys.stdin.buffer)"
```
### Actual result
```
True True False
Traceback (most recent call last):
File ".../magika/magika.py", line 204, in identify_stream
current_position = stream.tell()
OSError: [Errno 29] Illegal seek
During handling of the above exception, another exception occurred:
...
UnboundLocalError: cannot access local variable 'current_position' where it is not associated with a value
```
### Expected result
The original `OSError` (unseekable stream) propagates unchanged. Magika's own error should never shadow the underlying I/O failure.
### Root cause
`python/src/magika/magika.py:203-209`:
```python
try:
current_position = stream.tell()
result = self._get_result_from_seekable(Seekable(stream))
finally:
# seek to the previous position even in case of exceptions
stream.seek(current_position)
return result
```
`current_position` is bound *inside* the `try` block but referenced unconditionally in `finally`. When the first statement (`stream.tell()`) raises, the name was never bound, so `finally` raises `UnboundLocalError` and replaces the real exception.
### Impact
Callers cannot distinguish "this stream is not seekable" from a Magika internal bug: the traceback names a Magika local variable and the actionable `OSError` is demoted to `__context__`. This is the exact path taken by anyone piping data into a Python tool that forwards `sys.stdin.buffer` to `identify_stream()`.
### Proposed fix
Hoist the `tell()` out of the `try`, so that `finally` only runs once the position is known:
```python
current_position = stream.tell()
try:
result = self._get_result_from_seekable(Seekable(stream))
finally:
stream.seek(current_position)
return result
```
The position-restoring guarantee is preserved, because the only statement remaining inside `try` is the one that needs the restore.
I have a fix plus a regression test in `python/tests/test_magika_python_module.py` (next to `test_magika_module_identify_stream_does_not_alter_position`) and will open a PR shortly.
Contributor guide
Research direction
Start in python/src/magika/magika.py around identify_stream() lines 203-209, then read the nearby position-preservation test in python/tests/test_magika_python_module.py. Run the existing stream tests and add coverage for tell() failing on an unseekable stream; done means the original OSError propagates without an UnboundLocalError.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- api
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100