Repository navigation
fix(toolkit): dpkt walks IP layers off the octets as the default engine does (#1609, #1626, #1651, #1654) - #1662
Merged
Conversation
…ne does (#1609, #1626, #1651, #1654) The dpkt adapters walked what dpkt decoded, which is not what the default engine dissects. They now read every IP layer, extension header and TCP segment off the record octets, by the default engine's rules: - #1651: an IPv6 payload ends at its Payload Length, or at a jumbogram's Jumbo Payload Length; a zero one with no Jumbo Payload option has none. - #1654: IP behind a VLAN tag stack dpkt leaves undecoded is read too. - #1609: TCP and IP inputs stop at 32 extension headers in a row and at the 64-layer frame bound. - #1626: Mobility, HIP and Shim6 are walked, and so is a Fragment header with another header behind it, in a frame dpkt cannot decode as well, which is read by its link type (attach_linktype). - packet2dict maps a frame kept raw to {} rather than raising. Closes #1651 Closes #1654 Part of #1609, #1626
A frame dpkt cannot decode is kept as raw octets, which do not say what link layer they start with, so the adapters could hand over nothing for it. The engine now stashes the link type beside the timestamp and the record octets, and the adapters read the frame by it. Part of #1626
dpkt's adapters now stop the chain at EXTENSION_HEADER_LIMIT, so the ipv6-33 and ipv6-1000 dpkt cases agree with default, and a gap that no longer fails is an error in the harness. Part of #1609
JarryShaw
added
fix
Owner
Author
|
Verdict: GOOD TO GO at The reviewer re-measured dpkt against the default engine:
Disclosed behaviour change, for you to accept: the dpkt adapters now read dpkt's serialisation of the network layer. A hand-built packet therefore reports dpkt's recomputed TCP checksum, and a duck-typed non-dpkt object returns Three commits sit outside the toolkit:
This conflicts with the scapy PR only in |
Contributor
|
Coverage: 89.68% (unit tier, Python 3.14,
Per-file detail: the |
3 of 13 tasks
JarryShaw
removed
the
review: good-to-go
JarryShaw
added a commit
that referenced
this pull request
Oct 13, 2026
… headers as the default engine does (#1609, #1626, #1650) (#1663) * fix(toolkit): scapy walks link types, layer bounds and IPv6 extension headers as the default engine does (#1609, #1626, #1650) - #1650: the first layer was whatever Scapy dissected for the link type, so TCP and IP fragments at LINUX_SLL, LINUX_SLL2, 12 and PPP were handed on. _network now starts the walk only at NULL, ETHERNET, LOOP, RAW, IPV4 and IPV6, and reads a loopback family by the default engine's rules (2 is IPv4; 24, 28, 30 are IPv6) in its byte order, re-dissecting the packet where Scapy's host-dependent bindings differ. A packet built by hand takes its link type from its first layer. - #1609: _dissected now counts each layer, and stops at FRAME_LAYER_LIMIT, so nothing is read past layer 64, behind VLAN tags included. - #1626: _ipv6_chain walks an IPv6 extension header chain over the octets, up to EXTENSION_HEADER_LIMIT and the frame bound, keeping Scapy's layers while they match. TCP, IPv4 and IPv6 behind AH, Mobility, HIP, Shim6 and AH with Next Header 0 are now read, and so is the Fragment header behind them; IPv6 reassembly takes it from the octets. New tests/toolkit/test_scapy_linktype_and_layer_bounds_1609_1626_1650_unit.py compares scapy with the default engine over 90 cases; all 6 tests fail against main and pass with the fix, on 3.14 and 3.11. Closes #1650 Part of #1609, #1626 * test(engines): drop the scapy gap the extension header bound now closes (#1609) scapy now stops an IPv6 extension header chain at EXTENSION_HEADER_LIMIT, so ipv6-33/scapy/* come back OK and the recorded Gap fails the module. The Gap is removed and the docstring says why scapy agrees; the dpkt and tshark gaps stay. Part of #1609 * docs(protocols): the scapy toolkit stops at the extension header bound (#1609) The Extension Header Chains section listed scapy among the engines whose own parsers walk past EXTENSION_HEADER_LIMIT; it now stops there, as the pypcapfile toolkit does. Part of #1609 * docs(protocols): the scapy toolkit stops at the frame layer bound (#1609) The FRAME_LAYER_LIMIT section listed scapy among the engines that do not apply the bound; it now counts the layers it walks and stops at it. Part of #1609 * docs(protocols): the dpkt toolkit stops at both bounds too (#1609) #1662 made the dpkt adapters stop at EXTENSION_HEADER_LIMIT and at FRAME_LAYER_LIMIT, but both sections still listed dpkt among the engines whose own parsers walk past them. Only pyshark, and pypcapfile for the frame bound, are left there. Part of #1609
This was referenced Oct 13, 2026
JarryShaw
added a commit
that referenced
this pull request
Oct 14, 2026
Third-party engines are best effort (#1687), so the dpkt code that exists only to match the default engine comes out: - revert #1662, #1655, #1624, #1622, #1614 and the dpkt hunks of #1605: the octet walk and its link-type stash, IPv6 in IPv4 and IPv4 in IPv6, refusing MPLS and ISL, the tunnel and extension-header walks, the IPv4 header checks and the layer bounds - keep #1690's timestamps, #1594's parser choice, #1621 and #1653 - re-land as guards: a layer dpkt's own lengths misplace (#1646) is re-serialised, not refused with ProtocolError; a Total Length below the IHL keeps the header whole (#1616), and a header the capture cuts is no input; packet2dict and packet2chain take a str payload and a raw frame - delete the modules #1662 and #1655 added; the 1615/1616 module keeps only what its #1616 tests use, as no scapy test imports it now; dpkt's comparisons with the default engine assert constants or the capture's octets, or are dropped - drop pypcapfile's VLAN_TAGS, VLAN_TAG_LEN and IPV6_EXTENSION_HEADERS, which only dpkt imported, and the 1668 module's _Agreement base, which nothing subclasses now - the docs drop the depth claims and name what dpkt leaves out silently The dpkt modules, the shared modules that run dpkt, tests/foundation/engines and the project guards pass on 3.14 and 3.11 (dpkt 1.9.8, pypcapfile 0.12.0).
JarryShaw
added a commit
that referenced
this pull request
Oct 14, 2026
Third-party engines are best effort (#1687), so the dpkt code that exists only to match the default engine comes out: - revert #1662, #1655, #1624, #1622, #1614 and the dpkt hunks of #1605: the octet walk and its link-type stash, IPv6 in IPv4 and IPv4 in IPv6, refusing MPLS and ISL, the tunnel and extension-header walks, the IPv4 header checks and the layer bounds - keep #1690's timestamps, #1594's parser choice, #1621 and #1653 - re-land as guards: a layer dpkt's own lengths misplace (#1646) is re-serialised, not refused with ProtocolError; a Total Length below the IHL keeps the header whole (#1616), and a header the capture cuts is no input; packet2dict and packet2chain take a str payload and a raw frame - delete the modules #1662 and #1655 added; the 1615/1616 module keeps only what its #1616 tests use, as no scapy test imports it now; dpkt's comparisons with the default engine assert constants or the capture's octets, or are dropped - drop pypcapfile's VLAN_TAGS, VLAN_TAG_LEN and IPV6_EXTENSION_HEADERS, which only dpkt imported, and the 1668 module's _Agreement base, which nothing subclasses now - the docs drop the depth claims and name what dpkt leaves out silently The dpkt modules, the shared modules that run dpkt, tests/foundation/engines and the project guards pass on 3.14 and 3.11 (dpkt 1.9.8, pypcapfile 0.12.0).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please follow the guide below
You will be asked some questions, please read them carefully and answer honestly
Put an
xinto all the boxes [ ] relevant to your pull request (like that [x])Use Preview tab to see how your pull request will actually look like
Searched for similar pull requests
Followed the coding style (
make pylint,make mypy,make isort)make testpasses, and a test case covers the changeAdded a changelog entry under
docs/source/changelog/and regeneratedCHANGELOG.md, if the change is user-visible — N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657What is the purpose of your pull request?
Tick the commit type your subject line carries.
fix— corrects a defectfeat— adds a featureperf— changes performance, not behaviourrefactor— changes neither behaviour nor performancetest— tests onlydocs— documentation onlyci— workflows or build toolingrelease— bumps the version or rolls up a distributionchore— anything elseDescription of your pull request and other information
The dpkt adapters walked what dpkt decoded. They now read every IP layer, extension header and TCP segment off the record octets, by the default engine's rules (
_ip_layers,_ipv6_chain,_tcp_carrierinpcapkit/toolkit/dpkt.py):attach_linktype), and the adapters read the frame by it.packet2dictmaps a frame kept raw to{}. Before, with file output, the first frame dpkt could not decode ended the extraction withAttributeError.Measured on 3.14 (dpkt 1.9.8, scapy 2.7.0) and 3.11 (dpkt 1.9.8, scapy 2.8.0, pypcapfile 0.12.0), over 294 built cases (every issue row plus controls) and the 23 fixture captures. dpkt now agrees with default field for field on all 294 cases (159 on
main). The 135 dpkt cells that changed are exactly the ones that disagreed. No default, scapy or pypcapfile cell changed, and no fixture changed.New
tests/toolkit/test_dpkt_ip_walk_1609_1626_1651_1654_unit.py: all 6 tests fail onmain(83 subtest failures) and pass here (115 subtests), on both interpreters. Over the dpkt test files, coverage (lines and branches) ofpcapkit/toolkit/dpkt.pygoes from 98% to 99%, and of the dpkt engine from 89% to 91%.A packet built by hand has no record octets, so it is now read from dpkt's serialisation of its network layer. Its TCP header therefore carries the checksum dpkt fills in, and that pin is updated. The duck-typed fakes in
test_dpkt_unit.pyare replaced with real frames. The engine and two shared tests (the #1604 gap, and the #1606_decode_ipassert) each get a separate commit.Closes #1651
Closes #1654
Part of #1609, #1626