Skip to content

Commit 0fae5ce

Browse files
committed
fix(protocols): compose IPv6_Ext.id() from its direct concrete members (#2462)
- IPv6_Ext.id() returns the class names of its direct concrete subclasses (HOPOPT, IPv6_Route, IPv6_Frag, ESP, AH, IPv6_Opts, MH, HIP), so selecting 'IPv6_Ext' matches every extension header. - HOPOPT, IPv6_Frag, IPv6_Opts, IPv6_Route, MH and HIP narrow id() to their own class name, as AH and ESP already do; their id() output is unchanged from main. - Document id() on the family and member pages. - Tests derive the member set from IPv6_Ext.__subclasses__() and check protochain selection on parsed HOPOPT, IPv6-Opts and IPv6-Frag packets. Narrow pytest selection: 387 passed, 15 skipped; isort, pylint and mypy add no findings on the touched files.
1 parent afc9103 commit 0fae5ce

2 files changed

Lines changed: 35 additions & 67 deletions

File tree

‎pcapkit/protocols/internet/ipv6_ext.py‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -380,24 +380,16 @@ def protochain(self) -> 'ProtoChain | NoReturn':
380380
##########################################################################
381381

382382
@classmethod
383-
def id(cls) -> 'tuple[Literal["IPv6_Ext"]]':
383+
def id(cls) -> 'tuple[Literal["HOPOPT"], Literal["IPv6_Route"], Literal["IPv6_Frag"], Literal["ESP"], Literal["AH"], Literal["IPv6_Opts"], Literal["MH"], Literal["HIP"]]': # pylint: disable=line-too-long
384384
"""Index ID of the protocol.
385385
386386
Returns:
387-
Index ID of the protocol -- just this class's own name, *not* a
388-
family-wide list of every member's name as :meth:`IPsec.id
389-
<pcapkit.protocols.internet.ipsec.IPsec.id>` returns for its own
390-
family. Unlike ``IPsec``, this class is also the documented
391-
*concrete fallback parser* described above:
392-
:attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6`
393-
registers to it directly, and any :rfc:`6564`-conforming header
394-
whose dedicated class is absent or raised falls back to it too.
395-
Listing the sibling names would make such a fallback-parsed
396-
packet falsely match e.g. ``'AH'`` or ``'HOPOPT'`` in its
397-
protocol chain even though it is neither.
387+
Index ID of every direct concrete subclass, each of which narrows
388+
its own :meth:`id` to itself. A header parsed by this class as the
389+
generic fallback therefore matches every one of these names.
398390
399391
"""
400-
return ('IPv6_Ext',)
392+
return ('HOPOPT', 'IPv6_Route', 'IPv6_Frag', 'ESP', 'AH', 'IPv6_Opts', 'MH', 'HIP')
401393

402394
def read(self, length: 'Optional[int]' = None, *, # pylint: disable=arguments-differ
403395
extension: 'bool' = False, **kwargs: 'Any') -> '_PT':

‎tests/protocols/internet/test_ipv6_ext_unit.py‎

Lines changed: 30 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -299,67 +299,43 @@ def test_alias_is_hyphenated_like_its_siblings(self) -> None:
299299
# so the hyphen matters, not just the dash-free text either side of it.
300300
self.assertEqual(inst.alias.lstrip('IPv6-').lower(), 'ext')
301301

302-
# -- id(): class-name dispatch tuples, not family-wide, not alias strings --
303-
304-
def test_id_pins_every_touched_class_to_its_own_class_name(self) -> None:
305-
"""Every class in this family uses its own Python class name in
306-
``id()``, never the (possibly hyphenated) ``alias`` string -- the
307-
same convention as ``ARP``/``InARP``, ``C_Tag``/``S_Tag`` and
308-
``HTTPv1``/``HTTPv2`` elsewhere in this package. ``IPv6_Ext`` is
309-
pinned to *only* its own name, not a family-wide tuple: see
310-
``test_fallback_parsed_packet_does_not_match_a_sibling_name`` below
311-
for why a family-wide tuple would be a real bug here.
312-
"""
313-
from pcapkit.protocols.internet.hip import HIP
314-
from pcapkit.protocols.internet.hopopt import HOPOPT
302+
# -- id(): the family composes its members, each member narrows -------
303+
304+
def test_id_composes_every_direct_concrete_subclass(self) -> None:
305+
"""Derived from the live hierarchy, so a new member cannot drift."""
306+
import inspect
307+
308+
import pcapkit.protocols # noqa: F401 # pylint: disable=unused-import
315309
from pcapkit.protocols.internet.ipv6_ext import IPv6_Ext
316-
from pcapkit.protocols.internet.ipv6_frag import IPv6_Frag
317-
from pcapkit.protocols.internet.ipv6_opts import IPv6_Opts
318-
from pcapkit.protocols.internet.ipv6_route import IPv6_Route
319-
from pcapkit.protocols.internet.mh import MH
320310

321-
expected = {
322-
IPv6_Ext: ('IPv6_Ext',),
323-
HOPOPT: ('HOPOPT',),
324-
IPv6_Frag: ('IPv6_Frag',),
325-
IPv6_Opts: ('IPv6_Opts',),
326-
IPv6_Route: ('IPv6_Route',),
327-
MH: ('MH',),
328-
HIP: ('HIP',),
329-
}
330-
for klass, want in expected.items():
311+
members = [klass for klass in IPv6_Ext.__subclasses__() if not inspect.isabstract(klass)]
312+
self.assertCountEqual(IPv6_Ext.id(), [klass.__name__ for klass in members])
313+
for klass in members:
331314
with self.subTest(cls=klass.__qualname__):
332-
self.assertEqual(klass.id(), want)
333-
334-
def test_fallback_parsed_packet_does_not_match_a_sibling_name(self) -> None:
335-
"""The regression this fix closes. Before it, ``IPv6_Ext.id()``
336-
listed every family member's alias -- including ``'AH'``,
337-
``'HOPOPT'`` and ``'MH'`` -- so a Shim6-carrying packet (or any
338-
packet whose extension header fell back to this generic parser,
339-
since it is the documented concrete fallback for GitHub issue #917)
340-
would falsely match e.g. ``'AH' in protochain`` even though it
341-
carries none of those protocols. ``ProtoChain.__contains__`` calls
342-
``proto.id()`` on every chain entry
343-
(``pcapkit/corekit/protochain.py``), so this was a real false
344-
positive reachable through a public API, not merely an internal
345-
detail.
346-
347-
Fails on the pre-fix ``IPv6_Ext.id()`` (returns the 7-member
348-
family tuple): ``'AH' in ipv6.protochain`` is ``True`` there.
349-
"""
315+
self.assertEqual(klass.id(), (klass.__name__,))
316+
self.assertIn(klass.__name__.upper(), IPv6_Ext.expand_comp('IPv6_Ext'))
317+
318+
def test_selector_matches_by_member_and_by_family(self) -> None:
319+
"""A member name matches only that member; the family name matches all."""
350320
from pcapkit.const.ipv6.extension_header import ExtensionHeader
351321
from pcapkit.const.reg.transtype import TransType
352322
from pcapkit.protocols.internet.ipv6 import IPv6
353323

354-
shim6_ext = bytes([int(TransType.UDP), 1]) + b'\x00' * 14 # 16 octets, generic rule
355-
udp_payload = _udp_bytes(b'shim6-ok')
356-
raw = _ipv6_bytes(int(ExtensionHeader.Shim6), shim6_ext + udp_payload)
357-
ipv6 = IPv6(io.BytesIO(raw), len(raw))
358-
359-
self.assertIn('IPv6_Ext', ipv6.protochain)
360-
for sibling in ('AH', 'ESP', 'HOPOPT', 'MH'):
361-
with self.subTest(sibling=sibling):
362-
self.assertNotIn(sibling, ipv6.protochain)
324+
generic = bytes([int(TransType.UDP), 0]) + b'\x00' * 6 # (0+1)*8 == 8 octets
325+
headers = {
326+
'HOPOPT': (ExtensionHeader.HOPOPT, generic),
327+
'IPv6_Opts': (ExtensionHeader.IPv6_Opts, generic),
328+
'IPv6_Frag': (ExtensionHeader.IPv6_Frag, bytes([int(TransType.UDP)]) + b'\x00' * 7),
329+
}
330+
for name, (code, ext) in headers.items():
331+
raw = _ipv6_bytes(int(code), ext + _udp_bytes(b'ok'))
332+
ipv6 = IPv6(io.BytesIO(raw), len(raw))
333+
with self.subTest(header=name):
334+
self.assertIn(name, ipv6.protochain)
335+
self.assertIn('IPv6_Ext', ipv6.protochain)
336+
for other in headers:
337+
if other != name:
338+
self.assertNotIn(other, ipv6.protochain)
363339

364340
# -- the version gate: this class only ever activates for IPv6 ----------
365341

0 commit comments

Comments
 (0)