Skip to content

fix(extraction): accept a file object without a name as fin (#1506) - #1509

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1506-extract-file-object
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1506-extract-file-object

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

fin is typed str | IO[bytes] and documented as "file name to be read or a binary IO object" on both extract() and Extractor, so streams are in the contract. An io.BytesIO hit two crashes in a row, and the path-based engines a third:

  • make_name read fin.name. A stream without a str name (none, or the int of a descriptor-backed file) is now labelled <BytesIO>-style. Output names come from fout alone, so nofile=False, fout=None writes out.<ext> as for a path — no error needed.
  • the magic number (and the PCAP-NG engine's block type) are read with peek, which IO[bytes] lacks. A seekable stream without it is wrapped in _PeekableStream, a read-and-seek-back shim with no finaliser, so the caller's stream stays open (Extractor._cleanup closes the caller-supplied stream and leaks the file it opened itself #610).
  • scapy, pyshark, pypcap and pcap_ct hand _ifnm to their library as a path, so an unnamed stream would open the <BytesIO> label as a file (FileNotFoundError from engines/scapy.py:182). Extractor.__init__ now refuses these with UnsupportedCall, worded like the existing pypcap/pcap_ct refusal, before the engine runs. dpkt, pypcapfile and the default engine read _ifile and keep working; a stream with a real str name is not refused.

New tests/foundation/test_extraction_unnamed_stream_unit.py, 9 tests: 8 fail on origin/main's extraction.py, and the scapy and pyshark refusal cases fail on the previous revision (45a293871); all 9 pass here. pyshark rules itself out on CPython 3.14, so its case clears unsupported_reason to test the refusal itself. tests/foundation, tests/interface, tests/project and tests/test_docstring_contract.py pass after rebasing onto a608b6e83.

Closes #1506

@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 9, 2026
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: NEEDS CHANGES at 45a293871. Cross-reviewed on Sonnet; the author ran on Opus.

Required: with an unnamed stream, engine='scapy' now fails with a confusing error. I reproduced it myself: extract(fin=io.BytesIO(data), nofile=True, engine='scapy') raises FileNotFoundError: [Errno 2] No such file or directory: '<BytesIO>' from engines/scapy.py:182 (sniff(offline=ext._ifnm)). pyshark opens ext._ifnm as a path too (engines/pyshark.py:228). Fix: in extraction.py, refuse engines that need a path when fin has no real str name, with an in-library error. That would match what pypcap and pcap_ct already do (UnsupportedCall, engines/pypcap.py:290, engines/pcap_ct.py:375).

What checked out:

  • _PeekableStream returns the right bytes at mid-stream, after a partial read and at EOF, and always restores the position. A BufferedReader is not wrapped. The caller's stream is left open.
  • Per-frame to_dict() hashes are identical between path and BytesIO input across six PCAP and PCAP-NG captures, nanosecond included. The <BytesIO> label never reaches an output filename or file. dpkt and pypcapfile work with a stream.
  • Test runs: the new test 5 passed; tests/foundation 346; tests/interface 22; tests/integration 91; tests/project 419.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
`fin` is typed and documented as `str | IO[bytes]`, but an `io.BytesIO`
crashed before parsing:

- `Extractor.make_name` read `fin.name`; a stream without a `str` name is
  now labelled after its type, e.g. `<BytesIO>`. Output names come from
  `fout` alone, so nothing else depends on it.
- the magic number and the PCAP-NG block type are read with `peek`, which
  `IO[bytes]` lacks; a seekable stream without one is wrapped in
  `_PeekableStream`, a read-and-seek-back shim with no finaliser, so the
  caller's stream is never closed behind their back (#610).
- scapy, pyshark, pypcap and pcap_ct open `_ifnm` as a path, so with an
  unnamed stream they would open the `<BytesIO>` label as a file; they now
  refuse it with `UnsupportedCall` before the engine runs.
@JarryShaw
JarryShaw force-pushed the fix/1506-extract-file-object branch from 45a2938 to d97bc17 Compare October 9, 2026 21:01
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 9, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at d97bc1796. Round 2 was cross-reviewed on Sonnet; the author ran Opus.

  • The round-1 finding is fixed. With a BytesIO given as fin, scapy now raises UnsupportedCall instead of FileNotFoundError. dpkt, pypcapfile and the default engine still read the stream, both pcap and pcapng.
  • Fail-before and pass-after, re-derived independently by the reviewer and by me:
    • with the PR's extraction.py: 9 passed;
    • with main's: 8 failed, 1 passed;
    • with 45a293871's: 2 failed, 7 passed.
  • The refusal at :1328 runs after _select_engine() at :1308. An engine that falls back to the default is not refused. A non-str .name (an int from os.fdopen, or bytes) is refused.
  • The existing extraction tests pass: 47 passed, 101 subtests.
  • Not run here: the pyshark, pypcap and pcap_ct refusals, because those engines are unavailable on 3.14. The reviewer exercised them by patching out the fallback.
  • Nits, not blocking: pypcap and pcap_ct have no refusal test of their own. The message says "has no name" even when the name is an int or bytes.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@JarryShaw
JarryShaw merged commit 6af87f6 into main Oct 9, 2026
40 checks passed
@JarryShaw
JarryShaw deleted the fix/1506-extract-file-object branch October 9, 2026 21:21
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(extraction): extract(fin=...) with a file object raises AttributeError

1 participant