Skip to content

Commit 5c03bf4

Browse files
committed
refactor(foundation): declare each registry's code kind as a class attribute, not a module table (#2343)
Problem: _DESTINATION_KINDS mapped a protocol class to the Enum registry _check_code validates its code against, found by walking the destination's MRO for whichever ancestor owns __proto__ and looking that owner up in the table. The pairing lived apart from the __proto__ it describes, and a new destination needed both a new __proto__ and a new row in a module dict elsewhere. Fix: each of the seven destination classes (Link, Internet, Frame, PCAPNG, TCP, UDP, SCTP) now declares its own __proto_key__ class attribute, alongside __proto__, pairing its Enum registry with whether a bare int is also valid. register_protocol_code reads it straight off the owner the MRO-walk already finds (via vars(), not getattr, so a class keeping a registry of its own -- Loopback, IPX -- does not inherit its parent's pairing). _check_code also takes the destination class explicitly now, so its error names which registry refused, not just which enum: "TCP: code must be ... not True" rather than "code must be ... not True". Every other call site of _check_code (register_ethertype, register_transtype, every option/parameter registrar) passes its own target the same way. Two call sites write into two registries keyed alike and pass one of the two as target, rather than checking twice for no behavioural difference: register_linktype (Frame and PCAPNG) names Frame; register_apptype (TCP and/or UDP, resolved after the check) names TCP. Test: test_a_registry_of_its_own_does_not_inherit_its_parent_pairing pins the Loopback/IPX case; test_a_rejection_names_the_destination_registry and test_every_rejection_names_the_destination_registry (24 subtests, one per call site) pin the new error prefix; test_every_destination_kind_row_is_enforced now walks every ProtocolBase subclass declaring __proto_key__ rather than a fixed dict's keys, so a new undeclared pairing is caught too.
1 parent 7ad0efb commit 5c03bf4

17 files changed

Lines changed: 307 additions & 82 deletions

File tree

‎docs/source/pcapkit/protocols/internet/internet.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ which is a base class for internet layer protocols, e.g. :class:`~pcapkit.protoc
2828
:no-value:
2929
.. autoattribute:: __proto__
3030
:no-value:
31+
.. autoattribute:: __proto_key__
32+
:no-value:
3133
.. autoattribute:: _exthdr_depth
3234

3335
.. autodata:: pcapkit.protocols.internet.internet._exthdr_rebuilt

‎docs/source/pcapkit/protocols/link/link.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,5 @@ and :class:`~pcapkit.protocols.link.vlan.VLAN`.
2222
.. autoattribute:: __layer__
2323
.. autoattribute:: __proto__
2424
:no-value:
25+
.. autoattribute:: __proto_key__
26+
:no-value:

‎docs/source/pcapkit/protocols/misc/pcap.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,8 @@ whose structure is described as below:
143143

144144
.. autoattribute:: __proto__
145145
:no-value:
146+
.. autoattribute:: __proto_key__
147+
:no-value:
146148

147149
.. automethod:: __post_init__
148150
.. automethod:: __index__

‎docs/source/pcapkit/protocols/misc/pcapng.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ which implements an extractor for PCAP-NG file format [*]_, and the
194194

195195
.. autoattribute:: __proto__
196196
:no-value:
197+
.. autoattribute:: __proto_key__
198+
:no-value:
197199
.. autoattribute:: __block__
198200
:no-value:
199201
.. autoattribute:: __option__

‎docs/source/pcapkit/protocols/transport/sctp.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ Octets Bits Name Description
139139

140140
.. autoattribute:: __proto__
141141
:no-value:
142+
.. autoattribute:: __proto_key__
143+
:no-value:
142144
.. autoattribute:: __chunk__
143145
:no-value:
144146
.. autoattribute:: __parameter__

‎docs/source/pcapkit/protocols/transport/tcp.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ Octets Bits Name Description
131131

132132
.. autoattribute:: __proto__
133133
:no-value:
134+
.. autoattribute:: __proto_key__
135+
:no-value:
134136
.. autoattribute:: __option__
135137
:no-value:
136138
.. autoattribute:: __mp_option__

‎docs/source/pcapkit/protocols/transport/udp.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ Octets Bits Name Description
3333

3434
.. autoattribute:: __proto__
3535
:no-value:
36+
.. autoattribute:: __proto_key__
37+
:no-value:
3638

3739
.. automethod:: __index__
3840

‎pcapkit/foundation/registry/protocols.py‎

Lines changed: 63 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -423,25 +423,6 @@ def _register_name(protocol: 'Type[ProtocolBase]', *targets: 'tuple[type, Any]')
423423
Enum_LinkType: (Frame, PCAPNG),
424424
}
425425

426-
#: Key type of each ``__proto__`` dispatch registry a ``code=`` target can name.
427-
#:
428-
#: Maps the class owning the registry to the enum its typed registrar checks
429-
#: ``code`` against, and whether a bare :class:`int` is a valid key there too --
430-
#: the same :func:`_check_code` arguments that registrar passes. A destination is
431-
#: looked up by the class it inherits ``__proto__`` from, so a subclass sharing
432-
#: :class:`Link`'s registry is checked as :class:`Link` is, while one keeping a
433-
#: registry of its own, such as :class:`~pcapkit.protocols.link.loopback.Loopback`,
434-
#: names no entry here and is not checked (:issue:`2268`).
435-
_DESTINATION_KINDS: 'dict[Type[ProtocolBase], tuple[Type[EnumRegistry], bool]]' = {
436-
Link: (Enum_EtherType, False), # type: ignore[type-abstract]
437-
Internet: (Enum_TransType, False), # type: ignore[type-abstract]
438-
Frame: (Enum_LinkType, False),
439-
PCAPNG: (Enum_LinkType, False),
440-
TCP: (Enum_AppType, True),
441-
UDP: (Enum_AppType, True),
442-
SCTP: (Enum_PayloadProtocolIdentifier, True),
443-
}
444-
445426

446427
def _iter_code_targets(code: 'Any') -> 'Iterator[tuple[Type[ProtocolBase], Any]]':
447428
"""Flatten a ``code=`` argument into ``(destination, key)`` pairs.
@@ -549,26 +530,43 @@ def register_protocol_code(protocol: 'Type[ProtocolBase]', code: 'Any') -> 'None
549530
an enum member's type names no known destination registry --
550531
inference refuses rather than guesses; or if a key is not of the
551532
type its destination's registry is keyed by, as its typed registrar
552-
would reject it, e.g. ``{Frame: TransType.ICMP}``. Every target is
553-
checked before any is registered, so a rejected ``code`` leaves
554-
every registry as it was (:issue:`2268`).
533+
would reject it, e.g. ``{Frame: TransType.ICMP}``. What a registry is
534+
keyed by is read from the
535+
:attr:`__proto_key__ <pcapkit.protocols.link.link.Link.__proto_key__>`
536+
declared by the class owning it, so a destination sharing another's
537+
registry is checked as its owner is, and one whose owner declares no
538+
pairing is not checked. Every target is checked before any is
539+
registered, so a rejected ``code`` leaves every registry as it was
540+
(:issue:`2268`).
555541
556542
"""
557543
targets = list(_iter_code_targets(code))
558544
for destination, key in targets:
559545
owner = next((klass for klass in getattr(destination, '__mro__', ())
560546
if '__proto__' in vars(klass)), None)
561-
if owner in _DESTINATION_KINDS:
562-
kind, number = _DESTINATION_KINDS[owner]
563-
_check_code(key, kind, number=number)
547+
if owner is None:
548+
continue
549+
550+
# NOTE: the pairing is read out of the owner's own ``vars()`` rather than
551+
# with ``getattr``, which walks the MRO and would hand a class keeping a
552+
# registry of its own its parent's pairing --
553+
# :attr:`Loopback.__proto__ <pcapkit.protocols.link.loopback.Loopback.__proto__>`
554+
# is keyed by address family rather than by EtherType, so inheriting
555+
# :class:`Link`'s would reject every key it does accept. A class is
556+
# checked against the registry it owns, or not at all.
557+
pairing = vars(owner).get('__proto_key__')
558+
if pairing is not None:
559+
kind, number = pairing
560+
_check_code(key, kind, owner, number=number)
564561

565562
for destination, key in targets:
566563
destination.register(key, protocol)
567564
logger.debug('registered %s into %s.__proto__: %s', protocol.__name__,
568565
destination.__name__, key)
569566

570567

571-
def _check_code(code: 'Any', kind: 'type', *, number: 'bool' = False) -> 'None':
568+
def _check_code(code: 'Any', kind: 'type', target: 'Type[ProtocolBase]', *,
569+
number: 'bool' = False) -> 'None':
572570
"""Reject a registration key that is not a member of the registrar's enum.
573571
574572
Each registrar typed on one enum calls this before touching any registry,
@@ -582,6 +580,10 @@ def _check_code(code: 'Any', kind: 'type', *, number: 'bool' = False) -> 'None':
582580
Args:
583581
code: Registration key handed to the registrar.
584582
kind: Enum the registrar is typed on.
583+
target: Protocol class whose registry ``code`` is destined for, named in
584+
the error message so a rejection says *whose* registry rejected the
585+
key rather than only what it wanted -- the registrars share one
586+
message, and several of them are typed on the same enum.
585587
number: Whether a bare :class:`int` is a valid key too, as for the
586588
registrars keyed on a port number or a payload protocol identifier.
587589
@@ -600,8 +602,9 @@ def _check_code(code: 'Any', kind: 'type', *, number: 'bool' = False) -> 'None':
600602
return
601603
expected = f'an int or a member of {kind.__name__}' if number else f'a member of {kind.__name__}'
602604
if member:
603-
raise RegistryError(f'code must be {expected}, not a member of {type(code).__name__}: {code!r}')
604-
raise RegistryError(f'code must be {expected}, not {code!r}')
605+
raise RegistryError(f'{target.__name__}: code must be {expected}, not a member of '
606+
f'{type(code).__name__}: {code!r}')
607+
raise RegistryError(f'{target.__name__}: code must be {expected}, not {code!r}')
605608

606609

607610
###############################################################################
@@ -646,7 +649,10 @@ def register_linktype(code: 'LinkType', module: 'str | ModuleDescriptor[Protocol
646649
subclass.
647650
648651
"""
649-
_check_code(code, Enum_LinkType)
652+
# NOTE: this writes into both Frame's and PCAP-NG's registries, which are
653+
# keyed alike, so the key check is the same for either and Frame names it.
654+
# :func:`register_pcap` and :func:`register_pcapng` name one each.
655+
_check_code(code, Enum_LinkType, Frame)
650656
if isinstance(module, str):
651657
module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_))
652658

@@ -691,7 +697,7 @@ def register_pcap(code: 'LinkType', module: 'str | ModuleDescriptor[ProtocolBase
691697
subclass.
692698
693699
"""
694-
_check_code(code, Enum_LinkType)
700+
_check_code(code, Enum_LinkType, Frame)
695701
if isinstance(module, str):
696702
module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_))
697703

@@ -735,7 +741,7 @@ def register_pcapng(code: 'LinkType', module: 'str | ModuleDescriptor[ProtocolBa
735741
subclass.
736742
737743
"""
738-
_check_code(code, Enum_LinkType)
744+
_check_code(code, Enum_LinkType, PCAPNG)
739745
if isinstance(module, str):
740746
module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_))
741747

@@ -784,7 +790,7 @@ def register_ethertype(code: 'EtherType', module: 'str | ModuleDescriptor[Protoc
784790
subclass.
785791
786792
"""
787-
_check_code(code, Enum_EtherType)
793+
_check_code(code, Enum_EtherType, Link) # type: ignore[type-abstract]
788794
if isinstance(module, str):
789795
module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_))
790796

@@ -833,7 +839,7 @@ def register_transtype(code: 'TransType', module: 'str | ModuleDescriptor[Protoc
833839
subclass.
834840
835841
"""
836-
_check_code(code, Enum_TransType)
842+
_check_code(code, Enum_TransType, Internet) # type: ignore[type-abstract]
837843
if isinstance(module, str):
838844
module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_))
839845

@@ -870,7 +876,7 @@ def register_ipv4_option(code: 'IPv4_OptionNumber', meth: 'str | tuple[IPv4_Opti
870876
is a name with no matching ``_read_opt_*`` method.
871877
872878
"""
873-
_check_code(code, IPv4_OptionNumber)
879+
_check_code(code, IPv4_OptionNumber, IPv4)
874880
if isinstance(meth, str) and not hasattr(IPv4, f'_read_opt_{meth}'):
875881
raise RegistryError('method must be a valid IPv4 option parser function')
876882

@@ -904,7 +910,7 @@ def register_hip_parameter(code: 'HIP_Parameter', meth: 'str | tuple[HIP_Paramet
904910
is a name with no matching ``_read_param_*`` method.
905911
906912
"""
907-
_check_code(code, HIP_Parameter)
913+
_check_code(code, HIP_Parameter, HIP)
908914
if isinstance(meth, str) and not hasattr(HIP, f'_read_param_{meth}'):
909915
raise RegistryError('method must be a valid HIP parameter parser function')
910916

@@ -938,7 +944,7 @@ def register_hopopt_option(code: 'IPv6_Option', meth: 'str | tuple[HOPOPT_Option
938944
is a name with no matching ``_read_opt_*`` method.
939945
940946
"""
941-
_check_code(code, IPv6_Option)
947+
_check_code(code, IPv6_Option, HOPOPT)
942948
if isinstance(meth, str) and not hasattr(HOPOPT, f'_read_opt_{meth}'):
943949
raise RegistryError('method must be a valid HOPOPT option parser function')
944950

@@ -972,7 +978,7 @@ def register_ipv6_opts_option(code: 'IPv6_Option', meth: 'str | tuple[IPv6_Opts_
972978
is a name with no matching ``_read_opt_*`` method.
973979
974980
"""
975-
_check_code(code, IPv6_Option)
981+
_check_code(code, IPv6_Option, IPv6_Opts)
976982
if isinstance(meth, str) and not hasattr(IPv6_Opts, f'_read_opt_{meth}'):
977983
raise RegistryError('method must be a valid IPv6-Opts option parser function')
978984

@@ -1006,7 +1012,7 @@ def register_ipv6_route_routing(code: 'IPv6_Routing', meth: 'str | tuple[IPv6_Ro
10061012
is a name with no matching ``_read_data_type_*`` method.
10071013
10081014
"""
1009-
_check_code(code, IPv6_Routing)
1015+
_check_code(code, IPv6_Routing, IPv6_Route)
10101016
if isinstance(meth, str) and not hasattr(IPv6_Route, f'_read_data_type_{meth}'):
10111017
raise RegistryError('method must be a valid IPv6-Route routing data parser function')
10121018

@@ -1040,7 +1046,7 @@ def register_mh_message(code: 'MH_Packet', meth: 'str | tuple[MH_PacketParser, M
10401046
is a name with no matching ``_read_msg_*`` method.
10411047
10421048
"""
1043-
_check_code(code, MH_Packet)
1049+
_check_code(code, MH_Packet, MH)
10441050
if isinstance(meth, str) and not hasattr(MH, f'_read_msg_{meth}'):
10451051
raise RegistryError('method must be a valid MH message type parser function')
10461052

@@ -1074,7 +1080,7 @@ def register_mh_option(code: 'MH_Option', meth: 'str | tuple[MH_OptionParser, MH
10741080
is a name with no matching ``_read_opt_*`` method.
10751081
10761082
"""
1077-
_check_code(code, MH_Option)
1083+
_check_code(code, MH_Option, MH)
10781084
if isinstance(meth, str) and not hasattr(MH, f'_read_opt_{meth}'):
10791085
raise RegistryError('method must be a valid MH option parser function')
10801086

@@ -1113,7 +1119,7 @@ def register_mh_extension(code: 'MH_CGAExtension', meth: 'str | tuple[MH_Extensi
11131119
that take one (:issue:`516`).
11141120
11151121
"""
1116-
_check_code(code, MH_CGAExtension)
1122+
_check_code(code, MH_CGAExtension, MH)
11171123
if isinstance(meth, str) and not hasattr(MH, f'_read_ext_{meth}'):
11181124
raise RegistryError('method must be a valid MH CGA extension function')
11191125

@@ -1227,7 +1233,12 @@ def register_apptype(code: 'int | Enum_AppType', module: 'str | ModuleDescriptor
12271233
protocol per argument, with no alias (:issue:`806`).
12281234
12291235
"""
1230-
_check_code(code, Enum_AppType, number=True)
1236+
# NOTE: which of TCP's and UDP's registries this writes into depends on
1237+
# ``*transport``, which is resolved below -- after this check, so a bad
1238+
# ``code`` is still reported before a bad transport protocol. The two are
1239+
# keyed alike, so the check itself is the same for either and TCP names it.
1240+
# :func:`register_tcp` and :func:`register_udp` name one each.
1241+
_check_code(code, Enum_AppType, TCP, number=True)
12311242

12321243
# NOTE: ``class_`` sits at the sibling position -- positional, ahead of
12331244
# ``*transport`` -- per maintainer ruling, so a third positional that is
@@ -1356,7 +1367,7 @@ def register_tcp(code: 'int | Enum_AppType', module: 'str | ModuleDescriptor[Pro
13561367
member of another enum.
13571368
13581369
"""
1359-
_check_code(code, Enum_AppType, number=True)
1370+
_check_code(code, Enum_AppType, TCP, number=True)
13601371
if isinstance(code, Enum_AppType):
13611372
code = code.port
13621373
if isinstance(module, str):
@@ -1395,7 +1406,7 @@ def register_tcp_option(code: 'TCP_Option', meth: 'str | tuple[TCP_OptionParser,
13951406
is a name with no matching ``_read_mode_*`` method.
13961407
13971408
"""
1398-
_check_code(code, TCP_Option)
1409+
_check_code(code, TCP_Option, TCP)
13991410
if isinstance(meth, str) and not hasattr(TCP, f'_read_mode_{meth}'):
14001411
raise RegistryError('method must be a TCP option parser function')
14011412

@@ -1429,7 +1440,7 @@ def register_tcp_mp_option(code: 'TCP_MPTCPOption', meth: 'str | tuple[TCP_MPOpt
14291440
is a name with no matching ``_read_mptcp_*`` method.
14301441
14311442
"""
1432-
_check_code(code, TCP_MPTCPOption)
1443+
_check_code(code, TCP_MPTCPOption, TCP)
14331444
if isinstance(meth, str) and not hasattr(TCP, f'_read_mptcp_{meth}'):
14341445
raise RegistryError('method must be a MPTCP option parser function')
14351446

@@ -1470,7 +1481,7 @@ def register_udp(code: 'int | Enum_AppType', module: 'str | ModuleDescriptor[Pro
14701481
member of another enum.
14711482
14721483
"""
1473-
_check_code(code, Enum_AppType, number=True)
1484+
_check_code(code, Enum_AppType, UDP, number=True)
14741485
if isinstance(code, Enum_AppType):
14751486
code = code.port
14761487
if isinstance(module, str):
@@ -1524,7 +1535,7 @@ def register_sctp(code: 'int | SCTP_PayloadProtocolIdentifier', module: 'str | M
15241535
:class:`~pcapkit.const.reg.apptype.sctp.SCTP` port enum.
15251536
15261537
"""
1527-
_check_code(code, Enum_PayloadProtocolIdentifier, number=True)
1538+
_check_code(code, Enum_PayloadProtocolIdentifier, SCTP, number=True)
15281539
if isinstance(module, str):
15291540
module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_))
15301541

@@ -1566,7 +1577,7 @@ def register_http_frame(code: 'HTTP_Frame', meth: 'str | tuple[HTTP_FrameParser,
15661577
is a name with no matching ``_read_http_*`` method.
15671578
15681579
"""
1569-
_check_code(code, HTTP_Frame)
1580+
_check_code(code, HTTP_Frame, HTTPv2)
15701581
if isinstance(meth, str) and not hasattr(HTTPv2, f'_read_http_{meth}'):
15711582
raise RegistryError('method must be a frame parser function')
15721583

@@ -1605,7 +1616,7 @@ def register_pcapng_block(code: 'PCAPNG_BlockType', meth: 'str | tuple[PCAPNG_Bl
16051616
is a name with no matching ``_read_block_*`` method.
16061617
16071618
"""
1608-
_check_code(code, PCAPNG_BlockType)
1619+
_check_code(code, PCAPNG_BlockType, PCAPNG)
16091620
if isinstance(meth, str) and not hasattr(PCAPNG, f'_read_block_{meth}'):
16101621
raise RegistryError('method must be a block parser function')
16111622

@@ -1639,7 +1650,7 @@ def register_pcapng_option(code: 'PCAPNG_OptionType', meth: 'str | tuple[PCAPNG_
16391650
is a name with no matching ``_read_option_*`` method.
16401651
16411652
"""
1642-
_check_code(code, PCAPNG_OptionType)
1653+
_check_code(code, PCAPNG_OptionType, PCAPNG)
16431654
if isinstance(meth, str) and not hasattr(PCAPNG, f'_read_option_{meth}'):
16441655
raise RegistryError('method must be a option parser function')
16451656

@@ -1673,7 +1684,7 @@ def register_pcapng_record(code: 'PCAPNG_RecordType', meth: 'str | tuple[PCAPNG_
16731684
is a name with no matching ``_read_record_*`` method.
16741685
16751686
"""
1676-
_check_code(code, PCAPNG_RecordType)
1687+
_check_code(code, PCAPNG_RecordType, PCAPNG)
16771688
if isinstance(meth, str) and not hasattr(PCAPNG, f'_read_record_{meth}'):
16781689
raise RegistryError('method must be a name resolution record parser function')
16791690

@@ -1707,7 +1718,7 @@ def register_pcapng_secrets(code: 'PCAPNG_SecretsType', meth: 'str | tuple[PCAPN
17071718
is a name with no matching ``_read_secrets_*`` method.
17081719
17091720
"""
1710-
_check_code(code, PCAPNG_SecretsType)
1721+
_check_code(code, PCAPNG_SecretsType, PCAPNG)
17111722
if isinstance(meth, str) and not hasattr(PCAPNG, f'_read_secrets_{meth}'):
17121723
raise RegistryError('method must be a decryption secrets parser function')
17131724

0 commit comments

Comments
 (0)