Skip to content

fix(foundation): Extractor.output message, make_name registry pollution, scapy engine _offmt - #1113

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1095-extractor-defects
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1095-extractor-defects

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort): ran pylint, mypy and isort on the three files. The new module scores 10.00, and no findings fall on changed lines. The findings that remain are pre-existing.
  • make test passes, and a test case covers the change: see the modules below
  • Added a changelog entry: N/A, added centrally after the wave

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Closes #1095.

  • Extractor.output now names output in its UnsupportedCall. The sibling properties (format, frame, reassembly, trace) were already correct.
  • make_name now uses __output__.get(fmt), so an unknown format is no longer inserted into the defaultdict.
  • The scapy engine's read_frame sets ext._offmt = ofile.kind, as dpkt.py:179 does. History: 74707ebc8 (2023-04-27) removed ext.record_header() from this engine, and that call had set _offmt as a side effect of dumping the global header. DPKT sets _offmt itself, but Scapy never did. This PR follows DPKT and does not restore record_header, whose future is chore(foundation): decide the fate of the unused public Extractor.record_header #1104.

Probe (base b10e7951b → this branch): message 'format' → 'output'. After make_name(..., 'bogus'), 'bogus' in __output__ was True with 8→9 keys and is now False with 8→8. Scapy .format raised AttributeError: ... '_offmt' and now returns json.

Tests: with the fix reverted, tests/foundation/test_extraction_output_offmt_unit.py reports 3 failed. With the fix, it reports 3 passed. The other modules: test_extraction.py 22 passed, engines/test_scapy_engine.py 2 passed, engines/test_runtime_engines.py 5 passed, and tests/project 379 passed, 1 skipped.

…on, scapy engine _offmt

- Extractor.output's UnsupportedCall now names 'output' instead of 'format'.
- Extractor.make_name looks the format up with dict.get, so an unknown format
  is no longer inserted into the __output__ defaultdict before FormatError.
- The scapy engine sets ext._offmt from the dumper's kind, as the other
  engines do, so Extractor.format works after a scapy extraction.

Regression tests in tests/foundation/test_extraction_output_offmt_unit.py.

Closes #1095
@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 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 2c285dd11: GOOD TO GO (ran on Sonnet; author Opus)

  • _offmt fix: scapy now sets _offmt per frame, the same way dpkt.py:179 does. A real scapy run gives .format of json, txt and plist, including files=True.
  • History: confirmed that 74707ebc8 dropped record_header(), which had been setting _offmt as a side effect.
  • make_name: it still raises the same FormatError for an unknown format, and the registry keys are now unchanged.
  • Other subscript: the one at Extractor.__init__ (~1181) can't be reached with an unknown format.
  • Tests: with the source reverted, the 3 new tests fail. test_extraction (22) and test_scapy_engine (2) pass.

Known limits, not regressions:

  • A capture with no frames still leaves scapy's _offmt unset, as it does for DPKT.
  • Scapy with format='pcap' raises TypeError in PCAPIO.__init__. That path is untouched by this PR.

@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 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.74% (unit tier, Python 3.14, 2c285dd11, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1035 2342 846 90.23%
pcapkit/corekit 1874 91 578 22 94.33%
pcapkit/dumpkit 136 0 40 0 100.00%
pcapkit/foundation 2425 145 842 34 92.56%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15653 187 3942 162 98.19%
pcapkit/toolkit 487 71 144 3 84.15%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit 9d561a6 into main Oct 6, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1095-extractor-defects branch October 6, 2026 20:35
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 6, 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(foundation): Extractor.output message, make_name registry pollution, scapy engine _offmt

1 participant