Skip to content

fix(dpkt): extract(engine='dpkt') keeps a frame dpkt cannot parse instead of leaking AttributeError (#1351) - #1377

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1351-dpkt-parse-error
Oct 8, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1351-dpkt-parse-error

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — run on dpkt.py with the Makefile flags: mypy clean, isort clean, pylint adds only one more _frnum protected-access of the kind the file already has
  • make test passes, and a test case covers the change — the modules listed below, not the whole suite
  • Added a changelog entry — N/A, added centrally

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Closes #1351

dpkt 1.9.8 dpkt/ip6.py:96 raises AttributeError for an IPv6 Fragment header followed by ESP (50) or Destination Options (60). engines/dpkt.py:164 (packet = protocol(pkt)) did not catch it. read_frame now catches the parse failure, warns DPKTWarning (Frame N: dpkt cannot parse the frame (...); kept as raw data) and keeps the frame as the raw packet class. That class is now _get_raw_protocol(), shared with the unrecognised-link-type path.

Why skip-with-warning, not an error: the default engine (ProtocolBase.analyze) falls back to Raw on any exception, and scapy dissects a failure as a Raw layer. Neither ends the extraction, so frame numbering stays in step with them.

Before/after (followups-d5-50.pcap, -60.pcap): AttributeError: 'IP6ESPHeader' / 'IP6DstOptsHeader' object has no attribute 'frag_off' -> 1 frame, bytes intact, one warning.

Tests: new tests/foundation/engines/test_dpkt_parse_error_unit.py passes (2 subtests); with origin/main's dpkt.py both subtests fail with the AttributeError. run_unittest_leg.py toolkit 287 tests OK; test_engine_parity.py 5 passed; test_reassembly_engine_parity.py 5 passed; tests/project 389 passed, 1 skipped. test_runtime_engines.py has 2 failures (test_pcap_run_writes_global_header_for_file_and_stream_modes); they fail the same way on origin/main, outside this change.

… an IPv6 Fragment header followed by ESP or Destination Options

- read_frame: a frame dpkt fails to parse is kept as a raw packet with a
  DPKTWarning, as the default and scapy engines fall back to a raw
  payload, instead of letting the third-party exception end the
  extraction. dpkt 1.9.8 raises AttributeError from ip6.py for an IPv6
  Fragment header followed by ESP or Destination Options (#1351).
- _get_raw_protocol: the raw packet class, shared with the
  unrecognised-link-type path.
@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 7, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on ef2deb8bb: GOOD TO GO (Sonnet reviewed; author Opus).

  • Repro: on main, the Fragment+ESP capture raises a raw AttributeError (IP6ESPHeader … frag_off), and so does the Fragment+DstOpts capture (IP6DstOptsHeader). Here each yields 1 RawPacket frame whose 70 wire bytes appear verbatim in the file, plus exactly one DPKTWarning that names the exception.
  • Exception scope is right: the except Exception wraps only protocol(pkt), dpkt's own constructor. pcapkit's adapters (attach_buffer, packet2dict, reassembly) sit outside the try, so pcapkit bugs are not swallowed, and KeyboardInterrupt/SystemExit pass through. It mirrors ProtocolBase.analyze (protocol.py:712). dpkt raises at least six exception types from different paths, so narrowing the list would bring fix(dpkt): extract(engine='dpkt') lets a raw AttributeError escape on an IPv6 Fragment header followed by ESP or Destination Options #1351 back on the next one.
  • Engines agree: default, scapy and dpkt each give 1 frame for both captures. pyshark is unavailable on 3.14 and was not tested.
  • Downstream: extract(engine='dpkt', reassembly=True, trace=True) completes on both captures. The fallback frame still carries fix(toolkit): dpkt reassembly adapters build their data from dpkt's re-serialised bytes and mutate the packet #1335's buffer, so packet2bytes returns the exact wire bytes.
  • Sweep and tests: a dpkt sweep over every fixture capture raised nothing and never hit the new warning, so the fallback hides no other failure. The toolkit leg ran 287 tests with 0 failures, both parity modules pass, and the revert proof fails 2 subtests.

Nits: the test does not pin packet2bytes or reassembly on the fallback frame. The comment's "other engines fall back the same way" is exact for the default engine and loose for scapy.

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

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage: 87.06% (unit tier, Python 3.14, ef2deb8bb, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18777 1032 2348 843 90.27%
pcapkit/corekit 1949 123 606 42 92.37%
pcapkit/dumpkit 189 1 68 1 99.22%
pcapkit/foundation 2450 300 852 96 84.98%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 16013 588 4178 266 94.97%
pcapkit/toolkit 539 96 168 17 78.93%
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 89336d0 into main Oct 8, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1351-dpkt-parse-error branch October 8, 2026 01:00
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 8, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 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(dpkt): extract(engine='dpkt') lets a raw AttributeError escape on an IPv6 Fragment header followed by ESP or Destination Options

1 participant