Skip to content

python: identify_stream() raises UnboundLocalError instead of the real OSError on unseekable streams #1473

Open

Description

@kaluli123123

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

Reproduction

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:

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:

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:

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.

Activity

  1. kaluli123123 commented on Sep 14, 2026

    @kaluli123123
    Author

    PR: #1475

    Root cause. identify_stream() bound current_position inside the try block while referencing it unconditionally in finally. A readable-but-unseekable io.BufferedIOBase passes every guard in the method, so stream.tell() raises, the name is never bound, and finally raises UnboundLocalError — demoting the actionable OSError to __context__.

    Fix boundary. One statement moved in python/src/magika/magika.py (tell() hoisted above the try), plus one regression test. No public API change, and no behavior change for seekable streams: the position-restoring guarantee holds because the only statement left inside try is the one that needs the restore.

    Focused tests.

    • uv run pytest tests/test_magika_python_module.py -k identify_stream -q -> 2 passed, 41 deselected.
    • Reverting only the source fix makes the new test fail with exactly the reported UnboundLocalError at src/magika/magika.py:208.
    • Whole file: uv run pytest tests/test_magika_python_module.py -q -> 42 passed, 1 skipped.
    • ruff format --check, ruff check, mypy src tests -> all clean.

    The new test uses an io.BufferedIOBase subclass whose tell()/seek() raise OSError(ESPIPE) rather than a real OS pipe, so it is deterministic and also valid on Windows (#789).

    Gates I did not clear. I did not run the Rust/JS/Go suites or CI (no permission to trigger it), and the CLA is not signed on this account yet. The PR is open and ready for review; I have not merged anything and have left this issue open.

  2. kaluli123123 commented on Sep 14, 2026

    @kaluli123123
    Author

    Correction to my earlier gate report: GitHub now reports cla/google SUCCESS and check-changes SUCCESS on the linked PR; four zizmor jobs are SKIPPED. My earlier assertion that the CLA was unsigned was not verified and was incorrect. No language test-suite result is currently reported by GitHub. The PR remains OPEN and non-draft; no merge or issue closure was performed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions