From 98eb6e4ddaf3ada34a114aa4e0ba6f57e2c43bb1 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 21 Sep 2026 17:15:57 -0400 Subject: [PATCH] fix(l2tp): refuse a version nibble other than 2, and settle what IP protocol 115 is (#548) * L2TPv2.read() accepted any version nibble, so an L2TPv3 datagram parsed as v2 with a tunnel and session ID read out of v3's Control Connection ID. RFC 2661 section 3.1 fixes Ver at 2 and reserves 1 for L2F, and L2TPv2.version already documented that "a datagram carrying any other value is a different protocol reached through a different class" -- but nothing enforced it, so the hard-coded Literal[2] property and info.version answered 2 and 3 for the same octets. read() now raises ProtocolError, which degrades the payload to Raw through the existing beholder path with the reason recorded. This affects real captures: RFC 3931 section 4.1.2 puts L2TPv3 on port 1701 too, the port UDP.__proto__ already binds, so Ethernet:IPv4:UDP:L2TPv2:Raw with invented field values becomes Ethernet:IPv4:UDP:Raw with the octets preserved. * #548 asked for TransType.L2TP (115) to be registered and proposed binding L2TPv2 there. Not done, and the reasoning is now recorded in pcapkit/protocols/link/l2tp.py rather than only in a test: RFC 3931 section 4.1.1 gives 115 to L2TPv3 over IP, whose session header is "free of any restrictions imposed by coexistence with L2TPv2 and L2F" and carries no version nibble at all, so a v2 parser cannot even detect that the datagram is not its own. Measured, that binding reported version=4, tunnelid=0x5678 and sessionid=0xff03 from the top half of a Session ID and two octets of the PPP frame behind it. 115 is a missing class, not a missing registration, and stays unbound; no L2TPv3 dissector was invented here. * register_protocol_code's worked example registered L2TPv2 at 115, so a caller following it got exactly that misparse. It names L2TPv3 instead -- the shape is unchanged and v3 genuinely is reachable both ways. * Added tests/protocols/test_dispatch_reachability_unit.py, the coverage #548 asked for: every ProtocolBase descendant whose __index__ returns an enum member must be reachable under that code in the registry its enum type designates, read from the same _CODE_DESTINATIONS table that backs code= so the audit cannot drift from the mechanism. 23 claims verified, no gaps, plus a case that injects a gap to prove the audit has teeth. Unit tier green: 1186 passed, 8 skipped, 2698 subtests. New tests in tests/protocols/link/test_l2tp_version_unit.py proven to fail without the fix (22 failed / 5 passed before, 7 passed / 21 subtests after); pcapkit/protocols/link/l2tpv2.py at 100% statement and branch coverage. --- CHANGELOG.md | 6 +- docs/source/changelog/1.5.0.rst | 46 ++- pcapkit/foundation/registry/protocols.py | 15 +- pcapkit/protocols/link/l2tp.py | 35 ++- pcapkit/protocols/link/l2tpv2.py | 36 ++- .../protocols/link/test_l2tp_version_unit.py | 289 ++++++++++++++++++ .../test_dispatch_reachability_unit.py | 230 ++++++++++++++ .../test_protocol_code_registration_unit.py | 20 +- 8 files changed, 658 insertions(+), 19 deletions(-) create mode 100644 tests/protocols/link/test_l2tp_version_unit.py create mode 100644 tests/protocols/test_dispatch_reachability_unit.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 22b5fc2b4..8655f0ce6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ The largest release since 1.0, and the first recorded here as it happened rather - **Changed** -- `Probe`, `CipherSuite` and `IntegritySuite` are `Info` subclasses rather than `typing.NamedTuple`, and no `NamedTuple` remains in the package. They are Mappings now, so `len()` and iteration yield field names rather than values. - **Changed** -- renames with no compatibility alias left behind: `HoleDiscriptor` is spelled `HoleDescriptor` and its package alias `TCP_HoleDiscriptor` is `TCP_HoleDescriptor` (#350); PCAP-NG `Option` subclasses spell the namespace class keyword `ns=` instead of `namespace=` (#439); and `examples/sample` and `examples/samples` -- one letter apart, holding different things -- are now `examples/captures` and `examples/generators`. - **Changed** -- subclass registration is **opt-in** for `Engine`, `Reassembly`, `TraceFlow` and `dumpkit`'s `Dumper` (#514). Each registers if and only if its registry keyword is given -- `engine=` for `Engine`, `protocol=` for `Reassembly` and `TraceFlow`, `fmt=` for `Dumper`. Previously an absent keyword fell back to the class' own name, so *every* subclass of the public class was registered, and declining meant subclassing the parallel `*Base` class under an alias -- which is what every built-in does, and why the public classes had **0** subclasses between them against the `*Base` classes' 9, 5, 2 and 3. **This breaks out-of-tree code that subclasses one of the four and relies on the derived key**; pass the keyword, or call the matching `register_*` function. Nothing the library ships is affected, and the `*Base` classes remain importable. Two things that were silent are now loud: an unrecognised class keyword raises `UnsupportedCall` instead of being swallowed by `**kwargs` -- which used to register the class under its own name, so passing `name=` to a `Reassembly` subclass silently ignored the key it was given, `protocol=` being the real one -- and `Dumper`'s `ext=` without `fmt=` likewise. A class attribute is not an opt-in: `__engine_name__` and `__protocol_name__` still set the name a class reports, registered or not. Each metaclass also gained a class-level `registry` property mirroring `EnumSchema.registry`. As a side effect a `Dumper` subclass no longer touches the filesystem while its `class` statement runs: inferring `fmt` from the `kind` property meant instantiating the class against a `NamedTemporaryFile` mid-definition. `Engine`'s keyword is `engine=` rather than the `name=` this first shipped with, because `name` cannot be passed as a class keyword at all on Python 3.10: `mcls`, `name`, `bases` and `namespace` collide with `abc.ABCMeta.__new__`'s own parameters, which are positional-or-keyword before 3.11 and positional-only from 3.11, so a class statement naming any of the four raises `TypeError` from the metaclass before the hook is reached. Those four are the whole of the `ABCMeta.__new__` collision surface, measured on 3.10.21, 3.11.15 and 3.14.7; `engine=`, `protocol=` and `fmt=` are all outside it, so the documented registration path works on every supported version. There is no `name=` alias -- a keyword that worked on some interpreters and not others is the trap being removed, not a compatibility measure. -- **Added** -- `Protocol`/`ProtocolBase` gain a `code=` class keyword for the next-layer dispatch registries -- `Link.__proto__`, `Internet.__proto__`, `TCP.__proto__`, `UDP.__proto__`, `SCTP.__proto__`, `Frame.__proto__` and `PCAPNG.__proto__` -- the same opt-in treatment #514 gave `Engine`, `Reassembly`, `TraceFlow` and `Dumper` above, extended to the one family that needed a registration key invented rather than merely un-guarded. Omitting `code` leaves a subclass unregistered, exactly as before this keyword existed: the built-in dispatch tables are still populated by literal assignment in each layer module, not by `__init_subclass__`, so nothing the library ships moves. `code` accepts a bare enum member, whose *type* infers the destination -- `EtherType` means `Link`, `TransType` means `Internet`, `PayloadProtocolIdentifier` means `SCTP`, and `LinkType` means *both* `Frame` **and** `PCAPNG`, deterministically, mirroring what `register_linktype` already does by hand -- or a `{destination: key}` mapping, required for a raw `int` such as a TCP/UDP port number, which cannot say by itself which transport it belongs to. Either form may appear in an iterable, so one declaration can register a class into several registries at once, e.g. a `L2TP` subclass reachable both by IP protocol number and by a UDP port. The explicit mapping form is accepted even for a key whose type could be inferred -- being more explicit than required is never an error. Inference refuses rather than guesses: an enum member whose type names no known destination raises `RegistryError` instead of silently doing nothing or picking an arbitrary registry, and an unrecognised class keyword raises `UnsupportedCall`, matching the other four families. Backed by the new `pcapkit.foundation.registry.protocols.register_protocol_code`, which can also be called directly to register a class that declined at class-definition time. This is the mechanism #548 (`TransType.L2TP` registered nowhere) needs and does not yet use -- fixing it is now a one-declaration change, left for its own issue rather than folded in here. +- **Added** -- `Protocol`/`ProtocolBase` gain a `code=` class keyword for the next-layer dispatch registries -- `Link.__proto__`, `Internet.__proto__`, `TCP.__proto__`, `UDP.__proto__`, `SCTP.__proto__`, `Frame.__proto__` and `PCAPNG.__proto__` -- the same opt-in treatment #514 gave `Engine`, `Reassembly`, `TraceFlow` and `Dumper` above, extended to the one family that needed a registration key invented rather than merely un-guarded. Omitting `code` leaves a subclass unregistered, exactly as before this keyword existed: the built-in dispatch tables are still populated by literal assignment in each layer module, not by `__init_subclass__`, so nothing the library ships moves. `code` accepts a bare enum member, whose *type* infers the destination -- `EtherType` means `Link`, `TransType` means `Internet`, `PayloadProtocolIdentifier` means `SCTP`, and `LinkType` means *both* `Frame` **and** `PCAPNG`, deterministically, mirroring what `register_linktype` already does by hand -- or a `{destination: key}` mapping, required for a raw `int` such as a TCP/UDP port number, which cannot say by itself which transport it belongs to. Either form may appear in an iterable, so one declaration can register a class into several registries at once, e.g. a `L2TP` subclass reachable both by IP protocol number and by a UDP port. The explicit mapping form is accepted even for a key whose type could be inferred -- being more explicit than required is never an error. Inference refuses rather than guesses: an enum member whose type names no known destination raises `RegistryError` instead of silently doing nothing or picking an arbitrary registry, and an unrecognised class keyword raises `UnsupportedCall`, matching the other four families. Backed by the new `pcapkit.foundation.registry.protocols.register_protocol_code`, which can also be called directly to register a class that declined at class-definition time. This was written up as the mechanism #548 (`TransType.L2TP` registered nowhere) needs, with fixing that issue described here as "now a one-declaration change". Investigating #548 found otherwise -- 115 is an [RFC 3931](https://datatracker.ietf.org/doc/html/rfc3931) L2TPv3-over-IP header with no class to dispatch to, so the declaration would have pointed the [RFC 2661](https://datatracker.ietf.org/doc/html/rfc2661) parser at it. See the corresponding **Fixed** entry below; the mechanism itself is unaffected, and its worked example now names `L2TPv3` rather than `L2TPv2`. - **Changed** -- extraction is around 46% faster on a 1,117-frame HTTP capture, with byte-identical output (#420). A reassembled datagram's payload is now analysed on first read rather than eagerly, which cuts IP reassembly's own cost by 90.7% and TCP's by 23.7% -- IP reassembly submits a datagram for every frame, fragmented or not (#424). Flow tracing over the same capture went from 1416.6 ms to 744.0 ms, because the flow dumper had been handing each record to a `Frame` constructor that re-dissected the whole protocol stack to return bytes it had just been given; options are no longer parsed twice either (#427). All output compared byte-for-byte across the sample captures in each case. - **Fixed** -- next-layer, option, chunk, block and parameter dispatch all read `defaultdict` registries, so a lookup miss inserted the key into class-level state shared by every later instance, after which a legitimate `register_*` call warned that the code was already registered. Every read now goes through a lookup that does not grow the table, and `IPv4.__option__` and `HIP.__parameter__` became inspectable class attributes rather than names assembled at call time (#426, #428, #429, #434). One break comes with it: a tuple-registered handler pair written to the documented `OptionParser`/`OptionConstructor` signature now works where it could previously never be called at all, and a pair written with an explicit leading `self` -- the only shape that used to work -- now does not. - **Fixed** -- the identical defect one layer up, in the schema layer's own `EnumSchema.registry`: `Option.registry[code]` for an unregistered `code` inserted the default schema under that code, so a single lookup made an unassigned TCP option number, e.g. `156`, read back as registered for the rest of the process. `EnumSchema.__enum__` is now built (or, when a subclass seeds it manually in its own class body -- `PCAPNG.Option`'s namespaced mapping, `TCP.MPTCP`'s plain one) as a retention-safe mapping that still returns the registered default on a miss, it just stops recording it; `.registry` keeps returning the same object it always did, so nothing that held a reference to it is affected (#555). @@ -45,6 +45,10 @@ The largest release since 1.0, and the first recorded here as it happened rather - **Fixed** -- two more defects #541 exposed rather than caused, both since it let construction reach code that had never run before. `MPTCP.subtype` was still `typing.TYPE_CHECKING`-only, an annotation rather than a field, so `TCP(options=[(Enum_Option.Multipath_TCP, ...)])` raised `AttributeError: ... has no attribute 'subtype'` for every subtype but `MP_JOIN`: the convenience constructor builds a schema in memory and reads it straight back through `_read_mptcp_*` with no byte round trip, so `_MPTCP.post_process` -- the only code that ever set `subtype` -- never ran. Fixed on the construction path (`TCP._make_mode_mp`) rather than by adding a third real field the way `kind`/`length` got in #541: unlike those two, `subtype` is already packed as 4 bits of each subtype's own `test` bitfield, and a second, independent field for the same bits would either double-encode them or need a "derive, don't pack" field kind this library does not have (#566). Separately, `_make_mptcp_capable` wrote `length=20 if rkey is None else 32` where [RFC 8684](https://datatracker.ietf.org/doc/html/rfc8684) section 3.1 gives 12 and 20, and `MPTCPCapable.rkey`'s own condition (`pkt['length'] != 32`) dropped the receiver's key for exactly the length the maker used to mean "key present" -- so a spec-correct, key-absent MP_CAPABLE could not be built at all, and a key-present one silently lost its key on the wire. Both, and the matching guard in `_read_mptcp_capable`, now agree on 12/20. **This changes MP_CAPABLE's packed output**: a 20-octet, key-present option built or parsed under the old code becomes 12 octets with no key, or 20 octets with the key actually present, depending on which the caller meant (#567). - **Fixed** -- IPv6-Route's RPL routing data was five octets wide at the front where [RFC 6554](https://datatracker.ietf.org/doc/html/rfc6554) section 3 gives four, and three further defects sat stacked behind it. `CmprI`, `CmprE` and `Pad` are each a 4-bit field, sharing one 32-bit word with a 20-bit `Reserved`, but the schema declared `cmpr_i` and `cmpr_e` as whole octets -- so a constructed two-address header packed to 41 octets while the `Hdr Ext Len` of 5 derived from that inflated data area declared 48. Correcting the width is a **wire-format change**, and it reshapes the schema: `RPL(cmpr_i=..., cmpr_e=...)` is now `RPL(cmpr={'cmpr_i': ..., 'cmpr_e': ...})`, beside the `pad={'pad_len': ...}` that was already there. Behind it, the reader's `header.length % 16` guard read `Hdr Ext Len` as an octet count and assumed 16-octet addresses, which an SRH only carries when `CmprI` and `CmprE` are both 0 -- the unit confusion #487 fixed for Source Route and Type 2, flagged and deliberately left by #489 for want of a working RPL round trip to validate a replacement against. It is replaced by section 4.2's own address-count arithmetic, `n = (((Hdr Ext Len * 8) - Pad - (16 - CmprE)) / (16 - CmprI)) + 1`, which the reader now requires to close -- non-negative and whole. `RPL.post_process` subtracted `pad_len` a second time from a buffer whose own length callback had already taken it off, losing one address per `16 - CmprI` octets of padding, and set `ip` only when it had parsed octets -- so once the guard stopped rejecting every constructed header, `IPv6_Route(type=..., data={'ip': [...]})` raised a bare `AttributeError: 'RPL' object has no attribute 'ip'` from the reader. And `_make_data_type_rpl` computed `Pad` as `8 - length % 8` without the outer `% 8`, so an already-aligned address vector was handed a full 8 octets of padding where section 3 requires that when `CmprI` and `CmprE` are both 0, `Pad` MUST carry a value of 0. The narrower `cmpr_i` also removes a latent divide-by-zero: a whole octet could hold 16, making `16 - CmprI` zero, where a 4-bit field tops out at 15. `ipv6-route-type/RPL_Source_Route_Header` round-trips now and its `EXPECTED_FAILURES` entry is deleted; as with the guard it replaces, none of this has been checked against a real RPL capture (#564). - **Changed** -- `ModuleDescriptor.klass` reads an already-imported module out of `sys.modules` rather than re-entering `importlib.import_module`, which matters because next layer dispatch resolves a descriptor there on a per-frame path. A registry *hit* holding a `ModuleDescriptor` is resolved once and written back, but a *miss* deliberately is not -- recording a miss in a class-level `collections.defaultdict` is the defect #425/#428 fixed at this layer and #560 fixed at the schema layer -- so every unrecognised frame resolved the same fallback descriptor again: 48 of the 52 `ModuleDescriptor.klass` resolutions an extraction of `many_interfaces.pcapng` performs, and 4 of the 7 on `ipv4.pcap`. `import_module` keeps real per-call work for a module `sys.modules` already holds, so that resolution now costs ~117 ns rather than ~436 ns and the whole miss path ~526 ns rather than ~883 ns, on CPython 3.14.7. **The scale is worth stating plainly: this is not measurable in** `extract()` **wall clock.** 48 avoided calls is ~17 us against a ~37 ms extraction, two orders of magnitude inside this host's run-to-run variance, and the "~40% of cumulative time" reading that prompted the work was an artifact of `_import_next_layer` being a recursive-descent dispatcher -- its *self* time is 0.58%, while `aenum.extend_enum` is 16.7%. What the change is taken for is its shape rather than its speed: nothing memoises the resolved class, anywhere, so `sys.modules` stays the only module cache in play and its invalidation is the interpreter's. A memo of the class would serve the pre-reload class after an `importlib.reload` forever, and an instance of it fails `isinstance` against the live one. Proposed by `@Ts-Boom` in #563, whose profiling found the miss path; the implementation differs because that one added a second, never-invalidated cache of resolved classes (#574). +- **Fixed** -- `L2TPv2` parsed any version nibble, so an L2TPv3 datagram was reported as v2 with a tunnel and session ID read out of v3's Control Connection ID. [RFC 2661](https://datatracker.ietf.org/doc/html/rfc2661) §3.1 fixes `Ver` at 2 and reserves 1 for L2F, and `L2TPv2.version` already documented that "a datagram carrying any other value is a different protocol reached through a different class" -- but nothing enforced it, so the hard-coded `Literal[2]` property and `info.version` disagreed on the same octets, answering 2 and 3. `read` now raises `ProtocolError`, which degrades the payload to `Raw` through the existing `beholder` path with the reason recorded. **This affects real captures**: [RFC 3931](https://datatracker.ietf.org/doc/html/rfc3931) §4.1.2 puts L2TPv3 on port 1701 too, the port `UDP.__proto__` already binds, so `Ethernet:IPv4:UDP:L2TPv2:Raw` with invented field values becomes `Ethernet:IPv4:UDP:Raw` with the octets preserved. + +This is the resolution of #548, which reported `TransType.L2TP` (115) as registered nowhere and proposed binding `L2TPv2` there. That binding is wrong rather than merely awkward: [RFC 3931](https://datatracker.ietf.org/doc/html/rfc3931) §4.1.1 gives 115 to *L2TPv3 over IP*, whose session header is "free of any restrictions imposed by coexistence with L2TPv2 and L2F" and carries **no version nibble at all**, so a v2 parser cannot even detect that the datagram is not its own. Measured, it produced `version=4`, `tunnelid=0x5678` and `sessionid=0xff03` from the top half of a Session ID and two octets of the PPP frame behind it. 115 is a missing *class*, not a missing registration, and stays unbound until an `L2TPv3` class exists; no dissector was invented here to fill it. The reasoning is now recorded in `pcapkit.protocols.link.l2tp` rather than only in a test, and `register_protocol_code`'s worked example -- which named `L2TPv2` at 115 -- names `L2TPv3` instead (#548). +- **Added** -- `tests/protocols/test_dispatch_reachability_unit.py`, the coverage #548 asked for: every `ProtocolBase` descendant whose `__index__` returns an enum member is checked to be reachable under that code in the registry its enum *type* designates, read from the same `_CODE_DESTINATIONS` table backing `code=` so the two cannot drift. Where `test_dispatch_registry_unit.py` walks the 38 entries that exist and checks each parses, this walks the classes and catches one nothing registered at all -- the shape in which `OSPF` once shipped reachable from no table. 23 claims verified, no gaps; a companion case injects a gap and confirms the audit reports it, so the guard cannot rot into a permanently green no-op (#548). Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` and `1.5.0b2` (both 2026-09-18) and `1.5.0b3` (2026-09-19), all published as prereleases and so resolved only by `pip install --pre`. `1.5.0b1` half-shipped: the tag, the GitHub release and the Conda deployments landed, but PyPI rejected the wheel because `twine check` found a Sphinx-only `:mod:` role in `README.rst`, which `pyproject.toml` declares as the dynamic long description. `1.5.0b2` is what reshipped it -- the release workflow is version-driven, so an existing version cannot republish -- and `1.5.0b3` followed the CI change that stops a TestPyPI outage from costing a release its wheels (#497, #498). diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index 1927d5f65..067dc56bc 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -205,9 +205,13 @@ pull requests between #326 and #509. ``UnsupportedCall``, matching the other four families. Backed by the new ``pcapkit.foundation.registry.protocols.register_protocol_code``, which can also be called directly to register a class that declined at class-definition - time. This is the mechanism #548 (``TransType.L2TP`` registered nowhere) - needs and does not yet use -- fixing it is now a one-declaration change, left - for its own issue rather than folded in here. + time. This was written up as the mechanism #548 (``TransType.L2TP`` + registered nowhere) needs, with fixing that issue described here as "now a + one-declaration change". Investigating #548 found otherwise -- 115 is an + :rfc:`3931` L2TPv3-over-IP header with no class to dispatch to, so the + declaration would have pointed the :rfc:`2661` parser at it. See the + corresponding **Fixed** entry below; the mechanism itself is unaffected, and + its worked example now names ``L2TPv3`` rather than ``L2TPv2``. * **Changed** -- extraction is around 46% faster on a 1,117-frame HTTP capture, with byte-identical output (#420). A reassembled datagram's payload is now analysed on first read rather than eagerly, which cuts IP reassembly's own @@ -478,6 +482,42 @@ pull requests between #326 and #509. the live one. Proposed by ``@Ts-Boom`` in #563, whose profiling found the miss path; the implementation differs because that one added a second, never-invalidated cache of resolved classes (#574). +* **Fixed** -- ``L2TPv2`` parsed any version nibble, so an L2TPv3 datagram was + reported as v2 with a tunnel and session ID read out of v3's Control + Connection ID. :rfc:`2661` §3.1 fixes ``Ver`` at 2 and reserves 1 for L2F, and + ``L2TPv2.version`` already documented that "a datagram carrying any other + value is a different protocol reached through a different class" -- but nothing + enforced it, so the hard-coded ``Literal[2]`` property and ``info.version`` + disagreed on the same octets, answering 2 and 3. ``read`` now raises + ``ProtocolError``, which degrades the payload to ``Raw`` through the existing + ``beholder`` path with the reason recorded. **This affects real captures**: + :rfc:`3931` §4.1.2 puts L2TPv3 on port 1701 too, the port ``UDP.__proto__`` + already binds, so ``Ethernet:IPv4:UDP:L2TPv2:Raw`` with invented field values + becomes ``Ethernet:IPv4:UDP:Raw`` with the octets preserved. + + This is the resolution of #548, which reported ``TransType.L2TP`` (115) as + registered nowhere and proposed binding ``L2TPv2`` there. That binding is + wrong rather than merely awkward: :rfc:`3931` §4.1.1 gives 115 to *L2TPv3 over + IP*, whose session header is "free of any restrictions imposed by coexistence + with L2TPv2 and L2F" and carries **no version nibble at all**, so a v2 parser + cannot even detect that the datagram is not its own. Measured, it produced + ``version=4``, ``tunnelid=0x5678`` and ``sessionid=0xff03`` from the top half + of a Session ID and two octets of the PPP frame behind it. 115 is a missing + *class*, not a missing registration, and stays unbound until an ``L2TPv3`` + class exists; no dissector was invented here to fill it. The reasoning is now + recorded in ``pcapkit.protocols.link.l2tp`` rather than only in a test, and + ``register_protocol_code``'s worked example -- which named ``L2TPv2`` at 115 -- + names ``L2TPv3`` instead (#548). +* **Added** -- ``tests/protocols/test_dispatch_reachability_unit.py``, the + coverage #548 asked for: every ``ProtocolBase`` descendant whose ``__index__`` + returns an enum member is checked to be reachable under that code in the + registry its enum *type* designates, read from the same + ``_CODE_DESTINATIONS`` table backing ``code=`` so the two cannot drift. Where + ``test_dispatch_registry_unit.py`` walks the 38 entries that exist and checks + each parses, this walks the classes and catches one nothing registered at all + -- the shape in which ``OSPF`` once shipped reachable from no table. 23 claims + verified, no gaps; a companion case injects a gap and confirms the audit + reports it, so the guard cannot rot into a permanently green no-op (#548). Preceded by ``1.5.0a1`` (2026-09-15), ``1.5.0b1`` and ``1.5.0b2`` (both 2026-09-18) and ``1.5.0b3`` (2026-09-19), all published as prereleases and so diff --git a/pcapkit/foundation/registry/protocols.py b/pcapkit/foundation/registry/protocols.py index f47361f16..88ae8ad45 100644 --- a/pcapkit/foundation/registry/protocols.py +++ b/pcapkit/foundation/registry/protocols.py @@ -164,7 +164,7 @@ def register_protocol(protocol: 'Type[Protocol]') -> 'None': #: Enum type -> the class(es) owning the :attr:`ProtocolBase.__proto__ #: ` dispatch registry #: keyed by that enum type -- the "registry-of-registries" that lets -#: ``code=`` infer a destination from a key's own type, per GH-514. This is +#: ``code=`` infer a destination from a key's own type, per #514. This is #: not an invention: it is exactly the targeting #: :func:`register_ethertype`, :func:`register_transtype`, #: :func:`register_linktype` and :func:`register_sctp` already hard-code by @@ -259,12 +259,23 @@ def register_protocol_code(protocol: 'Type[Protocol]', code: 'Any') -> 'None': .. code-block:: python - register_protocol_code(L2TPv2, [TransType.L2TP, {UDP: 1701}]) + register_protocol_code(L2TPv3, [TransType.L2TP, {UDP: 1701}]) The explicit mapping form is accepted for any key, even one whose type could be inferred -- being more explicit than required is never an error. + Note: + That example names ``L2TPv3``, which this package does not implement + yet, rather than :class:`~pcapkit.protocols.link.l2tpv2.L2TPv2`. It is + v3 that is genuinely reachable both ways: :rfc:`3931` §4.1.1 puts it + directly over IP on protocol 115 and §4.1.2 puts it over UDP on port + 1701. :class:`L2TPv2 ` answers on + port 1701 only, and registering *it* at ``TransType.L2TP`` would point + the :rfc:`2661` parser at a v3-over-IP header -- see + :class:`~pcapkit.protocols.link.l2tp.L2TP` and GitHub issue #548 for + what that produced when measured. + Args: protocol: Protocol class to register. code: Registration key(s); see above. diff --git a/pcapkit/protocols/link/l2tp.py b/pcapkit/protocols/link/l2tp.py index 43eebbc3c..388f36310 100644 --- a/pcapkit/protocols/link/l2tp.py +++ b/pcapkit/protocols/link/l2tp.py @@ -33,13 +33,28 @@ --------------------------- **L2TPv3** [:rfc:`3931`] has a different session header and a different control -message header from v2, and is reachable two ways -- over UDP port 1701 like v2, -and directly over IP as **protocol number 115**. That second route is why +message header from v2, and is reachable two ways -- over UDP port 1701 like v2 +(§4.1.2), and directly over IP as **protocol number 115** (§4.1.1: *"L2TPv3 over +IP (both versions) utilizes the IANA-assigned IP protocol ID 115"*). That second +route is why :attr:`Internet.__proto__ ` leaves 115 unbound today: the binding waits on an ``L2TPv3`` class, not on a different framing decision. It also means v3 is the first member of this family to have a real :meth:`~pcapkit.protocols.protocol.Protocol.__index__`. +GitHub issue #548 proposed closing that gap by binding +:class:`~pcapkit.protocols.link.l2tpv2.L2TPv2` at 115 instead, which does not +work and is worth recording so it is not proposed again. Over IP the v3 session +header is, in :rfc:`3931` §4.1.1's own words, *"free of any restrictions imposed +by coexistence with L2TPv2 and L2F"* -- a data message opens with the raw 32-bit +Session ID and carries **no version nibble at all**, so there is nothing a v2 +parser could even test to recognise that the datagram is not its own. Measured, +that binding reported ``version=4``, ``tunnelid=0x5678`` and ``sessionid=0xff03`` +for a v3-over-IP datagram: a complete header assembled out of the top half of a +Session ID and the first two octets of the PPP frame behind it. 115 is a missing +*class*, not a missing registration, and until that class exists an undissected +payload is the honest answer. + **L2F** [:rfc:`2341`] is reached when the version nibble reads ``1``. It is *not* an earlier version of L2TP: :rfc:`2661` §3.1 requires ``Ver`` to be 2 and reserves the value 1 "to permit detection of L2F packets should they arrive intermixed @@ -54,8 +69,16 @@ Selecting a version ------------------- -Nothing dispatches on the version nibble yet, because only one version exists. -When a second lands, the mechanism it wants already has a precedent in +Nothing *dispatches* on the version nibble yet, because only one version exists +-- but :meth:`L2TPv2.read ` does +**check** it, and refuses anything other than ``2``. That is the half of the +mechanism which is useful with one version implemented: it keeps v3 traffic on +port 1701 (:rfc:`3931` §4.1.2 shares the port, so this is ordinary capture +traffic rather than a corner case) from being reported as v2 with a tunnel and +session ID read out of v3's Control Connection ID. + +When a second version lands, the remaining half -- delegation rather than refusal +-- already has a precedent in :class:`~pcapkit.protocols.application.http.HTTP`, which reads a version and delegates to a per-version class. L2TP is the easier case: HTTP has to *trial-parse* each candidate in @@ -63,7 +86,9 @@ format carries no version field, whereas L2TP states its version explicitly in those four bits. So a deterministic switch on ``Ver`` is enough, and no new registry is needed -- the class bound at UDP 1701 reads two octets, masks out the -nibble, and hands the datagram to the matching class. +nibble, and hands the datagram to the matching class. Note the switch belongs on +the **UDP** path only: over IP protocol 115 there is no nibble to switch on, per +the §4.1.1 note above. .. [*] https://en.wikipedia.org/wiki/Layer_2_Tunneling_Protocol diff --git a/pcapkit/protocols/link/l2tpv2.py b/pcapkit/protocols/link/l2tpv2.py index 13c23762f..4c58fad5c 100644 --- a/pcapkit/protocols/link/l2tpv2.py +++ b/pcapkit/protocols/link/l2tpv2.py @@ -58,7 +58,7 @@ from pcapkit.protocols.data.link.l2tp import Flags as Data_Flags from pcapkit.protocols.link.l2tp import L2TP from pcapkit.protocols.schema.link.l2tp import L2TP as Schema_L2TP -from pcapkit.utilities.exceptions import UnsupportedCall +from pcapkit.utilities.exceptions import ProtocolError, UnsupportedCall if TYPE_CHECKING: from enum import IntEnum as StdlibEnum @@ -88,11 +88,20 @@ class L2TPv2(L2TP[Data_L2TP, Schema_L2TP], The protocol is dispatched from :attr:`UDP.__proto__ ` at port 1701. + Only the :rfc:`2661` framing is read: :meth:`read` refuses a datagram whose + version nibble is not ``2``, so a v3 datagram arriving on port 1701 + (:rfc:`3931` §4.1.2 shares the port) degrades to + :class:`~pcapkit.protocols.misc.raw.Raw` instead of being reported as v2. + Note: IANA protocol number 115 (``L2TP``) is deliberately left unbound. It references :rfc:`3931`, i.e. **L2TPv3**, whose session and control message headers are a different shape -- so the binding waits on an - ``L2TPv3`` class rather than on this one. + ``L2TPv3`` class rather than on this one. Binding *this* class there was + proposed in GitHub issue #548 and does not work: over IP the v3 session + header carries no version nibble at all, so this class cannot recognise + that the datagram is not its own. See + :class:`~pcapkit.protocols.link.l2tp.L2TP` for the measurement. As with :class:`~pcapkit.protocols.link.ospf.OSPF`, the class subclasses :class:`~pcapkit.protocols.link.link.Link` and so reports @@ -124,6 +133,13 @@ def version(self) -> 'Literal[2]': carrying any other value is a different protocol reached through a different class. c.f. :class:`~pcapkit.protocols.link.l2tp.L2TP`. + This is enforced rather than merely asserted -- :meth:`read` refuses a + datagram whose nibble is not ``2``, so this hard-coded answer cannot + disagree with + :attr:`info.version ` + on the same octets. It did before that guard landed, reporting ``2`` + here and ``3`` there. + """ return 2 @@ -171,6 +187,22 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_L2TP': schema = self.__header__ _flag = schema.flags + # NOTE: :rfc:`2661` §3.1 fixes ``Ver`` at 2 and reserves 1 "to permit + # detection of L2F packets should they arrive intermixed with L2TP + # packets", while :rfc:`3931` uses 3 -- so a datagram carrying any other + # nibble is a different protocol, exactly as ``version`` documents. + # Refuse it rather than parse it: every field after this word has a + # different meaning (or no meaning) in another version, so continuing + # reports a tunnel and session ID assembled out of octets that are + # neither. Reported as GitHub issue #548, where an :rfc:`3931` §4.1.1 + # L2TPv3-over-IP datagram yielded ``version=4``, ``tunnelid=0x5678`` and + # ``sessionid=0xff03`` -- read out of the top half of a Session ID and + # the first two octets of the PPP frame behind it. This is also what + # makes the hard-coded ``Literal[2]`` of ``version`` true, instead of + # disagreeing with ``info.version`` on the same datagram. + if _flag['version'] != 2: + raise ProtocolError(f'{self.alias}: invalid version: {_flag["version"]}') + flags = Data_Flags( type=Enum_Type(_flag['type']), len=bool(_flag['len']), diff --git a/tests/protocols/link/test_l2tp_version_unit.py b/tests/protocols/link/test_l2tp_version_unit.py new file mode 100644 index 000000000..965e54a29 --- /dev/null +++ b/tests/protocols/link/test_l2tp_version_unit.py @@ -0,0 +1,289 @@ +# -*- coding: utf-8 -*- +"""L2TPv2 accepts only the version nibble :rfc:`2661` fixes. + +GitHub issue #548 reported that IANA protocol number 115 (``TransType.L2TP``) +is registered nowhere, so an L2TP-over-IP capture falls through to +:class:`~pcapkit.protocols.misc.raw.Raw`, and proposed binding +:class:`~pcapkit.protocols.link.l2tpv2.L2TPv2` there. Measurement says that +binding would be wrong, and these tests are what pin that down. + +:rfc:`3931` §4.1.1 ("L2TPv3 over IP") is what protocol 115 designates: +*"L2TPv3 over IP (both versions) utilizes the IANA-assigned IP protocol ID +115."* The same section notes the v3 session header over IP is *"free of any +restrictions imposed by coexistence with L2TPv2 and L2F"* -- meaning that over +IP a v3 **data** message opens with the raw Session ID and carries no version +nibble anywhere. So 115 is not a second door onto the :rfc:`2661` framing +:class:`L2TPv2` implements; it is a different header that only an ``L2TPv3`` +class can read, and no such class exists in this tree. + +What *was* genuinely missing is the guard that makes that reasoning +enforceable. +:attr:`L2TPv2.version ` +is annotated ``Literal[2]``, returns a hard-coded ``2``, and its +docstring already promises that *"a datagram carrying any other value is a +different protocol reached through a different class"* -- but +:meth:`~pcapkit.protocols.link.l2tpv2.L2TPv2.read` stored the wire nibble +unchecked, so the class reported ``version == 2`` while its own parsed data +reported ``3`` for the same octets. These tests hold the two to the same +answer. + +Every case builds its own octets in memory and reads no capture under +:file:`examples/captures/`, so this belongs to the unit tier. + +""" +from __future__ import annotations + +import importlib.util +import io +import os +import struct +import tempfile +import unittest + +from tests._support import close_extractor, purge_modules + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + + +def l2tp_data(version: 'int' = 2) -> bytes: + """An L2TPv2 data message with every optional field absent. + + Matches :func:`tests.protocols.test_dispatch_bindings_unit.l2tp_data` and + :data:`examples.generators.dispatch._L2TP_DATA` at ``version=2``; the + parameter exists so a case can put a *different* nibble in bits 12-15 + while holding every other octet identical. + + """ + # bit0=type, bit1=len, bit4=seq, bit6=offset, bit7=prio, bits12-15=version + return struct.pack('!HHH', version, 0x1234, 0x5678) + b'\xff\x03\x00\x21PPP' + + +def l2tpv3_over_ip_data() -> bytes: + """An :rfc:`3931` §4.1.1 L2TPv3-over-IP data message. + + Over IP the v3 session header opens with the 32-bit Session ID -- there is + no flags word and no version nibble to inspect, which is precisely why a + v2 parser cannot decline this by reading a version field. + + """ + return struct.pack('!I', 0x12345678) + b'\xff\x03\x00\x21' + b'PPPPAYLOAD' + + +def ipv4(proto: 'int', payload: bytes) -> bytes: + """A minimal IPv4 header carrying ``payload`` under protocol ``proto``.""" + total = 20 + len(payload) + return struct.pack('!BBHHHBBH4s4s', 0x45, 0, total, 1, 0, 64, proto, 0, + bytes((10, 0, 0, 1)), bytes((10, 0, 0, 2))) + payload + + +def ethernet(etype: 'int', payload: bytes) -> bytes: + """A minimal Ethernet II header carrying ``payload`` under ``etype``.""" + return (b'\x00\x11\x22\x33\x44\x55' + b'\x66\x77\x88\x99\xAA\xBB' + + struct.pack('!H', etype) + payload) + + +def make_pcap(*frames: bytes) -> str: + """Write ``frames`` to a little-endian LINKTYPE_ETHERNET PCAP file.""" + path = os.path.join(tempfile.mkdtemp(prefix='pcapkit-l2tp-'), 'l2tp.pcap') + with open(path, 'wb') as file: + # little endian, v2.4, LINKTYPE_ETHERNET + file.write(struct.pack(' None: + purge_modules(['pcapkit']) + + def parse(self, data: bytes): + """Parse ``data`` as :class:`L2TPv2` directly, with no dispatch.""" + from pcapkit.protocols.link.l2tpv2 import L2TPv2 + + return L2TPv2(io.BytesIO(data), len(data)) + + def extract(self, *frames: bytes): + """Extract synthesised ``frames`` and return the frame list.""" + import pcapkit + + extraction = pcapkit.extract(fin=make_pcap(*frames), nofile=True, + store=True) + self.addCleanup(close_extractor, extraction) + return extraction.frame + + ########################################################################## + # The version nibble. + ########################################################################## + + def test_version_two_still_parses(self) -> None: + """The RFC 2661 framing is unaffected -- this is the regression guard.""" + l2tp = self.parse(l2tp_data(2)) + + self.assertEqual(l2tp.version, 2) + self.assertEqual(l2tp.info.version, 2) + self.assertEqual(l2tp.info.tunnelid, 0x1234) + self.assertEqual(l2tp.info.sessionid, 0x5678) + + def test_a_version_nibble_other_than_two_is_refused(self) -> None: + """:rfc:`2661` §3.1 fixes ``Ver`` at 2, so 0, 1 and 3 are not L2TPv2. + + Version 1 is reserved by :rfc:`2661` §3.1 *"to permit detection of L2F + packets should they arrive intermixed with L2TP packets"*, and version + 3 is :rfc:`3931`. Neither is this class's protocol, and the nibble is + the only thing in the header that says so. + + """ + from pcapkit.utilities.exceptions import ProtocolError + + for version in (0, 1, 3, 4, 15): + with self.subTest(version=version): + with self.assertRaises(ProtocolError) as caught: + self.parse(l2tp_data(version)) + self.assertIn('version', str(caught.exception).lower()) + + def test_no_accepted_datagram_disagrees_with_the_version_property(self) -> None: + """``L2TPv2.version`` and ``info.version`` cannot report different numbers. + + Before this guard the class answered ``version == 2`` from a hard-coded + ``Literal[2]`` property while ``info.version`` carried whatever the + wire said, so one datagram had two versions depending on which + attribute a consumer read. + + """ + from pcapkit.utilities.exceptions import ProtocolError + + for version in range(16): + with self.subTest(version=version): + try: + l2tp = self.parse(l2tp_data(version)) + except ProtocolError: + continue # refused, so it reports nothing at all + self.assertEqual(l2tp.version, l2tp.info.version) + + ########################################################################## + # Issue #548: IP protocol 115. + ########################################################################## + + def test_ip_protocol_115_has_no_class_to_dispatch_to(self) -> None: + """115 is :rfc:`3931` L2TPv3 over IP, and no ``L2TPv3`` class exists. + + The companion assertion to + :meth:`tests.protocols.test_dispatch_bindings_unit.DispatchBindingTests.test_l2tp_over_ip_waits_on_an_l2tpv3_class`, + kept next to the version guard because the guard is what makes the + reasoning enforceable rather than merely asserted. + + """ + import pkgutil + + import pcapkit.protocols.link as linkpkg + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.internet.internet import Internet + + self.assertEqual(int(TransType.L2TP), 115) + self.assertNotIn(TransType.L2TP, Internet.__proto__) + + modules = {info.name for info in pkgutil.iter_modules(linkpkg.__path__)} + self.assertIn('l2tpv2', modules) + self.assertNotIn('l2tpv3', modules) + + def test_l2tpv3_over_ip_degrades_to_raw_rather_than_a_fabricated_header(self) -> None: + """Binding ``L2TPv2`` at 115 must not invent an L2TPv2 header. + + This is the acceptance test for issue #548's actual resolution. It + performs the registration the issue asked for -- and that + :func:`~pcapkit.foundation.registry.protocols.register_protocol_code` + once documented as its worked example -- then feeds it a genuine + :rfc:`3931` §4.1.1 v3-over-IP data message. + + Measured before the version guard landed, that combination reported + ``version=4``, ``tunnelid=0x5678`` and ``sessionid=0xff03``: a + confident header assembled out of the top half of a Session ID and the + first two octets of a PPP frame. Degrading to + :class:`~pcapkit.protocols.misc.raw.Raw` is the honest answer, and is + what the library does for any payload it has no dissector for. + + """ + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.internet.internet import Internet + from pcapkit.protocols.link.l2tpv2 import L2TPv2 + + snapshot = dict(Internet.__proto__) + self.addCleanup(lambda: (Internet.__proto__.clear(), + Internet.__proto__.update(snapshot))) + Internet.register(TransType.L2TP, L2TPv2) + + payload = l2tpv3_over_ip_data() + frame = self.extract(ethernet(0x0800, ipv4(115, payload)))[0] + ipv4_info = frame.info.to_dict()['ethernet']['ipv4'] + + # No fabricated L2TPv2 header: the payload stays opaque. + self.assertNotIn('l2tp', ipv4_info) + self.assertNotIn('L2TPv2', str(frame.protochain)) + + # It degrades through :func:`~pcapkit.utilities.decorators.beholder`, + # which re-parses as ``Raw`` and passes ``alias=proto`` -- so an + # enumeration key renders under its own name rather than as ``Raw``, + # per that function's own note. The payload is preserved verbatim and + # the reason is recorded, which is the whole point of degrading. + self.assertEqual(str(frame.protochain), 'Ethernet:IPv4:L2TP') + self.assertEqual(ipv4_info['raw']['protocol'], TransType.L2TP) + self.assertEqual(ipv4_info['raw']['packet'], payload) + self.assertIn('invalid version', ipv4_info['raw']['error']) + + def test_l2tpv3_over_udp_1701_no_longer_parses_as_l2tpv2(self) -> None: + """:rfc:`3931` §4.1.2 puts L2TPv3 on port 1701 too, and that is a live capture. + + This is the part of the change that fixes a real dissection rather than + guarding a hypothetical registration: port 1701 is bound today, so + before the version guard a v3 datagram arriving on it was dissected as + L2TPv2 and reported ``tunnelid=0x1234``/``sessionid=0x5678`` read out + of v3's Control Connection ID. :rfc:`3931` §3.2.1 requires ``Ver`` to + be 3, so the nibble is exactly the signal that this is not v2. + + """ + def udp(src: 'int', dst: 'int', payload: bytes) -> bytes: + return struct.pack('!HHHH', src, dst, 8 + len(payload), 0) + payload + + payload = l2tp_data(3) + frame = self.extract(ethernet( + 0x0800, ipv4(17, udp(1701, 1701, payload)), + ))[0] + udp_info = frame.info.to_dict()['ethernet']['ipv4']['udp'] + + self.assertEqual(str(frame.protochain), 'Ethernet:IPv4:UDP:Raw') + self.assertNotIn('l2tp', udp_info) + # A bare port is not an enumeration, so this one does render as ``Raw``. + self.assertEqual(udp_info['raw']['protocol'], 1701) + self.assertEqual(udp_info['raw']['packet'], payload) + + def test_udp_port_1701_is_unaffected_by_the_guard(self) -> None: + """The encapsulation that *is* L2TPv2's still dissects end to end. + + :rfc:`2661` puts L2TPv2 on UDP port 1701, which is the binding + ``UDP.__proto__`` already carries and the one this change must leave + alone. + + """ + def udp(src: 'int', dst: 'int', payload: bytes) -> bytes: + return struct.pack('!HHHH', src, dst, 8 + len(payload), 0) + payload + + frame = self.extract(ethernet( + 0x0800, ipv4(17, udp(1701, 1701, l2tp_data(2))), + ))[0] + + self.assertEqual(str(frame.protochain), 'Ethernet:IPv4:UDP:L2TPv2:Raw') + l2tp = frame.info.to_dict()['ethernet']['ipv4']['udp']['l2tp'] + self.assertEqual(l2tp['version'], 2) + self.assertEqual(l2tp['tunnelid'], 0x1234) + self.assertEqual(l2tp['sessionid'], 0x5678) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/test_dispatch_reachability_unit.py b/tests/protocols/test_dispatch_reachability_unit.py new file mode 100644 index 000000000..a996bcab5 --- /dev/null +++ b/tests/protocols/test_dispatch_reachability_unit.py @@ -0,0 +1,230 @@ +# -*- coding: utf-8 -*- +"""Every class that declares a dispatch code is reachable under it. + +GitHub issue #548 asked for this directly: + + No test asserts that every ``TransType`` member with an implementing class + is reachable through dispatch, which is why this survived. Such a test + would be broadly useful -- it would catch the next missing registration + rather than just this one. + +:file:`test_dispatch_registry_unit.py` walks the problem from the other end: it +takes each of the 38 ``__proto__`` entries that *exist* and checks the class it +names can parse a packet. That cannot see a class nobody registered at all, +which is exactly how :class:`~pcapkit.protocols.link.ospf.OSPF` shipped +reachable from no table (fixed in #436). This module walks it from the class +side instead -- every +:class:`~pcapkit.protocols.protocol.ProtocolBase` descendant whose +:meth:`~pcapkit.protocols.protocol.Protocol.__index__` returns an enum member +is *claiming* to be reachable under that code, so the claim is checked. + +Which registry a code belongs in is not restated here. It is read from +:data:`~pcapkit.foundation.registry.protocols._CODE_DESTINATIONS`, the enum +type -> destination table #570 shipped to back ``code=``, so the audit and the +registration mechanism cannot drift apart: an enum type added to that table +gets audited here the same day. + +Nothing is parsed and no capture under :file:`examples/captures/` is read, so +this belongs to the unit tier. + +""" +from __future__ import annotations + +import importlib.util +import pkgutil +import unittest +from typing import TYPE_CHECKING, NamedTuple + +from tests._support import purge_modules + +if TYPE_CHECKING: + from typing import Any + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + + +class Gap(NamedTuple): + """One class that declares a code it cannot be reached under.""" + + #: Fully qualified name of the class making the claim. + protocol: 'str' + #: ``__name__`` of the class owning the registry it should appear in. + destination: 'str' + #: The code its ``__index__`` returned. + code: 'Any' + #: What the registry actually holds under that code, or :data:`None`. + found: 'Any' + + +def _resolve(entry: 'Any') -> 'tuple[Any, Any]': + """The ``(module, name)`` a registry entry names. + + An entry is either a + :class:`~pcapkit.corekit.module.ModuleDescriptor` -- the lazy form the + built-in tables use -- or an already-imported class. + + """ + module = getattr(entry, 'module', None) + if module is not None: + return module, getattr(entry, 'name', None) + return getattr(entry, '__module__', None), getattr(entry, '__name__', None) + + +def _descendants(cls: 'type') -> 'Any': + """Every subclass of ``cls``, transitively.""" + for sub in cls.__subclasses__(): + yield sub + yield from _descendants(sub) + + +def audit() -> 'tuple[list[Gap], int]': + """Walk every indexed protocol class and report the ones nothing reaches. + + Returns: + The gaps found, and how many reachable claims were verified -- the + second number is what stops a silently-empty walk from passing as a + clean audit. + + """ + import pcapkit.protocols as protopkg + from pcapkit.foundation.registry.protocols import _CODE_DESTINATIONS + from pcapkit.protocols.protocol import ProtocolBase + + # Import every protocol module, or ``__subclasses__`` sees only whatever + # this process happened to touch first. + for info in pkgutil.walk_packages(protopkg.__path__, prefix='pcapkit.protocols.'): + importlib.import_module(info.name) + + gaps = [] # type: list[Gap] + checked = 0 + + for cls in sorted(set(_descendants(ProtocolBase)), + key=lambda item: (item.__module__, item.__name__)): + try: + code = cls.__index__() # type: ignore[call-arg] + except Exception: # pylint: disable=broad-except + continue # declares no code, so claims no reachability + if code is None: + continue # an abstract base, reached by nothing + + destinations = _CODE_DESTINATIONS.get(type(code)) + if destinations is None: + continue # no registry is keyed by this enum type + + for destination in destinations: + entry = destination.__proto__.get(code) + module, name = _resolve(entry) if entry is not None else (None, None) + + # Two spellings both count as reachable, for reasons that are + # properties of the tables rather than of this audit: + # + # * the same *name*, because a built-in entry may name the package + # re-export rather than the defining module -- ``Frame.__proto__`` + # holds ``pcapkit.protocols.link:Ethernet``, not + # ``pcapkit.protocols.link.ethernet:Ethernet``; + # * the same *module*, because the project's rule is that a shared + # index may share a module -- ``InARP`` shares ``ARP``'s, and + # ``DRARP`` shares ``RARP``'s -- and a distinct one may not, per + # ``test_vlan_indices_follow_the_one_module_per_index_rule``. Both + # classes answer to the code and the module they share is what + # dispatches between them, so the sibling's name is not a mismatch. + if name == cls.__name__ or module == cls.__module__: + checked += 1 + continue + + gaps.append(Gap(f'{cls.__module__}.{cls.__name__}', + destination.__name__, code, name)) + + return gaps, checked + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class DispatchReachabilityTests(unittest.TestCase): + """No protocol class declares a code that nothing dispatches.""" + + def setUp(self) -> None: + purge_modules(['pcapkit']) + + def test_every_indexed_class_is_reachable_under_the_code_it_declares(self) -> None: + """The invariant itself, over every class rather than a hand-picked few.""" + gaps, checked = audit() + + self.assertEqual(gaps, [], 'protocol classes declare codes nothing dispatches: ' + + '; '.join(f'{gap.protocol} says {gap.code!r} but ' + f'{gap.destination}.__proto__ holds ' + f'{gap.found!r}' for gap in gaps)) + + # A walk that found nothing to check would also report no gaps, so pin + # the floor. 23 claims verified when this landed -- the 21 distinct + # classes plus ``InARP`` and ``DRARP`` reaching their siblings' entries, + # and ``Ethernet`` counted once for ``Frame`` and once for ``PCAPNG``. + self.assertGreaterEqual(checked, 23) + + def test_the_audit_detects_a_code_that_nothing_dispatches(self) -> None: + """The guard has teeth: inject a gap and confirm the audit reports it. + + Without this, a bug that made :func:`audit` return early -- an import + that silently failed, an ``except`` that swallowed too much -- would + leave a permanently green test asserting nothing at all. + + """ + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.internet.internet import Internet + from pcapkit.protocols.link.link import Link + + # ``TransType.L2TP`` is genuinely unbound (issue #548), so a class + # claiming it is exactly the shape this audit exists to catch. + self.assertNotIn(TransType.L2TP, Internet.__proto__) + + class Unreachable(Link): # pylint: disable=abstract-method + """Declares 115 and is registered nowhere.""" + + @classmethod + def __index__(cls) -> 'Any': + return TransType.L2TP + + try: + gaps, _ = audit() + names = [gap.protocol for gap in gaps] + self.assertTrue(any(name.endswith('Unreachable') for name in names), + f'audit missed the injected gap; reported {names}') + finally: + # ``__subclasses__`` holds a weak reference, but the class is only + # collected once nothing in this frame names it. + del Unreachable + + def test_transtype_l2tp_is_unbound_because_v3_has_no_class(self) -> None: + """115's gap is a missing *class*, not a missing registration. + + Issue #548 read the empty slot as an oversight. :rfc:`3931` §4.1.1 is + what fills it: *"L2TPv3 over IP (both versions) utilizes the + IANA-assigned IP protocol ID 115."* So 115 names the v3-over-IP header, + which no class in this tree implements -- and this audit deliberately + does **not** flag it, because nothing claims 115 in the first place. + :class:`~pcapkit.protocols.link.l2tpv2.L2TPv2` raises from + ``__index__`` rather than returning it. + + """ + import pcapkit.protocols.link as linkpkg + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.internet.internet import Internet + from pcapkit.protocols.link.l2tp import L2TP + from pcapkit.protocols.link.l2tpv2 import L2TPv2 + from pcapkit.utilities.exceptions import UnsupportedCall + + self.assertNotIn(TransType.L2TP, Internet.__proto__) + for cls in (L2TP, L2TPv2): + with self.subTest(cls=cls.__name__): + with self.assertRaises(UnsupportedCall): + cls.__index__() + + modules = {info.name for info in pkgutil.iter_modules(linkpkg.__path__)} + self.assertNotIn('l2tpv3', modules) + + gaps, _ = audit() + self.assertNotIn(TransType.L2TP, [gap.code for gap in gaps]) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/test_protocol_code_registration_unit.py b/tests/protocols/test_protocol_code_registration_unit.py index 948e65cdd..055fb3e98 100644 --- a/tests/protocols/test_protocol_code_registration_unit.py +++ b/tests/protocols/test_protocol_code_registration_unit.py @@ -251,12 +251,20 @@ class FakeProtocol: reg_udp.assert_called_once_with(54323, FakeProtocol) def test_iterable_mixes_inferred_and_explicit_targets(self) -> None: - """The L2TPv2 shape: one class, an inferred entry and an explicit one. - - ``L2TPv2`` is a ``Link``-layer class reachable both by an IP protocol - number (inferred: ``TransType`` -> ``Internet``) and by a UDP port - (explicit, since a bare port cannot say TCP or UDP) -- see GH-514's - design thread and the adjacent GH-548. + """The L2TPv3 shape: one class, an inferred entry and an explicit one. + + An ``L2TPv3`` class -- which this package does not implement yet -- would + be a ``Link``-layer class reachable both by an IP protocol number + (inferred: ``TransType`` -> ``Internet``, :rfc:`3931` §4.1.1) and by a + UDP port (explicit, since a bare port cannot say TCP or UDP, + :rfc:`3931` §4.1.2) -- see #514's design thread. + + Deliberately *not* ``L2TPv2``, which this docstring named until #548 was + settled: v2 answers on port 1701 only, and registering it at + ``TransType.L2TP`` points the :rfc:`2661` parser at a v3-over-IP header. + The mechanism under test is unchanged either way -- the class here is a + stand-in and nothing is really registered -- but the example should not + recommend a binding the library refuses. """ from pcapkit.const.reg.transtype import TransType from pcapkit.foundation.registry.protocols import register_protocol_code