diff --git a/CHANGELOG.md b/CHANGELOG.md index 48ba96649..49cc9d432 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). @@ -46,6 +46,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 a7b803d68..93c724f49 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 @@ -527,6 +531,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