From 18414583bdf3c9ce7ea17674121663dfae5585e1 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 1 Nov 2026 14:12:44 -0500 Subject: [PATCH 1/2] docs(conventions): write the rulings from their threads onto the pages (#2557) - utilities-or-corekit: compat holds compatibility implementations and re-exports only, behind version gates; a platform-independent helper such as fromtimestamp gets its own module. States what the #1709 probe test matches. - protocol-layer-placement: misc also holds the checksum pseudo-headers, with the IP_Pseudo placement ruling; the internet row and the routing section read RFC 1812's list as the Internet Layer protocols it names, not as everything internet holds. The OSPF class docstring and the layer-placement test's docstring say the same. - reassembly-integration: the intro names where points 1-3 were ruled, and that point 4 rests on the DTLS case #2469 settled. - protocol-family-grouping: AH and ESP are selected by IPsec and IPv6_Ext only where IPv4 carries them. - mint-criterion: CommandType caches in the value table, as Flags does. - A ruling cited to an issue whose thread does not hold it says so. Closes #2557 --- .../conventions/documentation.rst | 5 ++++ .../conventions/mint-criterion.rst | 26 +++++++++++-------- .../conventions/protocol-family-grouping.rst | 8 ++++-- .../conventions/protocol-layer-placement.rst | 19 ++++++++++---- .../conventions/reassembly-integration.rst | 8 ++++-- .../conventions/utilities-or-corekit.rst | 16 ++++++++++++ pcapkit/protocols/application/ospf.py | 4 +-- .../application/test_layer_placement_unit.py | 2 +- 8 files changed, 65 insertions(+), 23 deletions(-) diff --git a/docs/source/contributing/conventions/documentation.rst b/docs/source/contributing/conventions/documentation.rst index dc4d6a7d9a..9ad73d93df 100644 --- a/docs/source/contributing/conventions/documentation.rst +++ b/docs/source/contributing/conventions/documentation.rst @@ -204,6 +204,11 @@ was asked for and given -- and it survives the work that implemented it. So cite issue a rule was settled on, and describe a change by what it did rather than by its number. Ruled on :issue:`719`. +**Cite a ruling given in a pull request's review by the issue that pull request +closes**, as ``ruled in review of (:issue:`N`)``. Where it closes none, cite the +pull request with ``:pr:`` and say that no issue holds the ruling. Ruled in review of +:pr:`2599` (:issue:`2557`). + **The changelog and** :file:`tests/` **are both exempt, for related reasons.** A changelog entry exists so a reader can find the change, and the pull-request number *is* that pointer; converting it would delete the thing the entry is for -- diff --git a/docs/source/contributing/conventions/mint-criterion.rst b/docs/source/contributing/conventions/mint-criterion.rst index 4b96fd0b3d..5efdb91c13 100644 --- a/docs/source/contributing/conventions/mint-criterion.rst +++ b/docs/source/contributing/conventions/mint-criterion.rst @@ -59,11 +59,11 @@ Measured against the tree, by class: :mod:`l2tp.type `, :mod:`mh.dhcp_support_mode ` and :mod:`vlan.priority_level `. -* **Cache in the value table only**: :class:`~pcapkit.const.tcp.flags.Flags` and the - Mobility Header flag registries, such as - :class:`~pcapkit.const.mh.binding_ack_flag.BindingACKFlag`. They range-check and hand - back to ``super()._missing_``, which for an ``aenum.IntFlag`` reaches - ``_create_pseudo_member_`` and ends in +* **Cache in the value table only**: :class:`~pcapkit.const.tcp.flags.Flags`, + :class:`~pcapkit.const.ftp.command.CommandType` and the Mobility Header flag + registries, such as :class:`~pcapkit.const.mh.binding_ack_flag.BindingACKFlag`. They + range-check and hand back to ``super()._missing_``, which for an ``aenum.IntFlag`` + reaches ``_create_pseudo_member_`` and ends in ``cls._value2member_map_.setdefault(value, pseudo_member)``. * **Raise for any unknown value**: :class:`~pcapkit.const.reg.apptype.apptype.AppType`, :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader`, @@ -214,7 +214,9 @@ only ``ast.Name`` nodes finds every mint and no unmint at all: :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` and :class:`~pcapkit.const.pcapng.tls_key_label.TLSKeyLabel` (they define no ``_missing_``), and does not see the transport subclasses, which inherit theirs. - Reconcile it against a runtime probe for the split in the list above. + :class:`~pcapkit.const.ftp.command.CommandType` is one of those flag registries, and + the ``UNMINT`` lines for :file:`pcapkit/const/ftp/command.py` are the file's other + registries. Reconcile it against a runtime probe for the split in the list above. .. warning:: @@ -222,8 +224,10 @@ only ``ast.Name`` nodes finds every mint and no unmint at all: **mutates the class**. A probe is not a read: it installs something that every later lookup then finds. :class:`~pcapkit.const.mh.cga_type.CGAType` grows ``__members__``, ``_member_map_`` and ``_value2member_map_``; - :class:`~pcapkit.const.tcp.flags.Flags` and the Mobility Header flag registries - grow ``_value2member_map_`` only. Snapshot ``{member.value for member in Cls}`` - before any lookup regardless -- that snapshot cannot see the flag registries' cache, - so compare ``len(Cls._value2member_map_)`` too -- and use a throwaway process per - registry when comparing behaviour across revisions. + :class:`~pcapkit.const.tcp.flags.Flags`, + :class:`~pcapkit.const.ftp.command.CommandType` and the Mobility Header flag + registries grow ``_value2member_map_`` only. Snapshot + ``{member.value for member in Cls}`` before any lookup regardless -- that snapshot + cannot see the flag registries' cache, so compare ``len(Cls._value2member_map_)`` + too -- and use a throwaway process per registry when comparing behaviour across + revisions. diff --git a/docs/source/contributing/conventions/protocol-family-grouping.rst b/docs/source/contributing/conventions/protocol-family-grouping.rst index 313b9fa75a..d00eb988fb 100644 --- a/docs/source/contributing/conventions/protocol-family-grouping.rst +++ b/docs/source/contributing/conventions/protocol-family-grouping.rst @@ -119,8 +119,12 @@ A parsed RARP takes ``DRARP`` as its alias for operation 5, and A family-inclusive member is selected by any sibling's registry key (a key of ``pcapkit.protocols.__proto__``), because each expands to the shared family name (:ref:`below `). A narrow member is selected by its own name -and by the key of each family whose ``id()`` lists it: ``AH`` and ``ESP`` by ``IPsec`` -and by ``IPv6_Ext``, ``HTTPv1`` and ``HTTPv2`` by ``HTTP``. +and by the key of each family whose ``id()`` lists it: ``HTTPv1`` and ``HTTPv2`` by +``HTTP``, and ``AH`` and ``ESP`` by ``IPsec`` and by ``IPv6_Ext`` where IPv4 carries +them. Under IPv6 they are no layer of their own but entries of +:attr:`IPv6.extension_headers `, +so on an ``Ethernet:IPv6:AH`` frame ``'AH' in``, ``'IPsec' in`` and ``'IPv6_Ext' in`` +are all ``False``; only ``frame.protochain`` matches them. Chained Records ~~~~~~~~~~~~~~~ diff --git a/docs/source/contributing/conventions/protocol-layer-placement.rst b/docs/source/contributing/conventions/protocol-layer-placement.rst index 7daa114b92..cfa2f78a81 100644 --- a/docs/source/contributing/conventions/protocol-layer-placement.rst +++ b/docs/source/contributing/conventions/protocol-layer-placement.rst @@ -31,15 +31,17 @@ The subpackages, functionally rather than by carrier: * - :mod:`~pcapkit.protocols.link` - frames, and link-address or tagging protocols * - :mod:`~pcapkit.protocols.internet` - - layer-3 addressing and the forwarding path itself -- - :rfc:`1812#section-4.1` confines it to IP, ICMP and IGMP + - layer-3 addressing and the forwarding path itself: IP, ICMP and IGMP, the + Internet Layer protocols :rfc:`1812#section-4.1` names, and with them IPX and + the IPsec and IPv6 extension headers (:ref:`extension-header-subclassing`) * - :mod:`~pcapkit.protocols.transport` - IANA transport protocols * - :mod:`~pcapkit.protocols.application` - protocols that are **users of the stack rather than part of its forwarding path** * - :mod:`~pcapkit.protocols.misc` - - file formats and layerless sentinels + - file formats, layerless sentinels, and the checksum pseudo-headers in + :mod:`~pcapkit.protocols.misc.checksum` .. important:: @@ -86,8 +88,8 @@ Routing Protocols: Control Plane, Not Data Plane :rfc:`1812` is Standards Track and organises itself by layer. Its §7 is titled **"APPLICATION LAYER - ROUTING PROTOCOLS"**, with OSPF at §7.2.2, while :rfc:`1812#section-4.1` reads *"This chapter and chapter 5 discuss the protocols used -at the Internet Layer: IP, ICMP, and IGMP."* So a routing protocol is application-layer -and the internet layer holds only IP, ICMP and IGMP. +at the Internet Layer: IP, ICMP, and IGMP."* So a routing protocol is application-layer, +not one of the Internet Layer protocols §4.1 names. **X.200 does not contradict this, and must not be cited as though it did.** X.200 §7.5.2.1 says the Network Layer provides transport *"independence of routing and relay @@ -160,6 +162,13 @@ Each is deliberate. Do not "fix" any of them. ``application/ → link/`` dependency is the honest consequence of letting layer and parser disagree. +**A pseudo-header protocol is in** :mod:`~pcapkit.protocols.misc`, **not beside the +layer it is built from.** :class:`~pcapkit.protocols.misc.checksum.ip_pseudo.IP_Pseudo` +is built from an IPv4 or IPv6 layer but is no layer on the wire, and its ``__layer__`` +is :obj:`None`. Pseudo-header protocols share :mod:`pcapkit.protocols.misc.checksum`, +ruled in review of the IP pseudo-header for :issue:`2471`, whose thread does not hold +it. + **The dispatch tier is decoupled from the subpackage.** A protocol's subpackage says nothing about which registry reaches it, and no dispatch key depends on it: diff --git a/docs/source/contributing/conventions/reassembly-integration.rst b/docs/source/contributing/conventions/reassembly-integration.rst index 75fcf67fdc..a2c4b602f4 100644 --- a/docs/source/contributing/conventions/reassembly-integration.rst +++ b/docs/source/contributing/conventions/reassembly-integration.rst @@ -4,8 +4,12 @@ Reassembly Integration ----------------------- Where a new reassembly engine under :mod:`pcapkit.foundation.reassembly` plugs into -the rest of the package. The owner's ruling, given on the DTLS reassembly work of -:issue:`2440`. +the rest of the package. Points 1 to 3 are the owner's ruling on the DTLS reassembly +work of :issue:`2440`, whose pull-request review applies it; the issue itself does not +hold it. Point 4 rests on :pr:`2469`, which settled the DTLS case alone: in review of +how DTLS sizes a handshake message's buffer, the owner had DTLS reassembly follow its +RFCs. +No issue holds that ruling. The Rule ~~~~~~~~ diff --git a/docs/source/contributing/conventions/utilities-or-corekit.rst b/docs/source/contributing/conventions/utilities-or-corekit.rst index 871dd7b5a1..959942e004 100644 --- a/docs/source/contributing/conventions/utilities-or-corekit.rst +++ b/docs/source/contributing/conventions/utilities-or-corekit.rst @@ -17,6 +17,22 @@ import, one inside a function and one under ``TYPE_CHECKING`` alike. :file:`tests/project/test_utilities_import_boundary_1735_unit.py` checks every module under :file:`pcapkit/utilities/` for it. +:mod:`pcapkit.utilities.compat` **holds compatibility implementations and re-exports +only.** Where a name or behaviour differs by interpreter version, ``compat`` re-exports +the right one behind a :data:`sys.version_info` gate rather than a ``getattr`` or +``hasattr`` probe. That was ruled in review of the threading excepthook and +``AppType`` union fixes for :issue:`1691` and :issue:`1694`, neither of whose threads +holds it, and applied to the package's other probes on :issue:`1709`. +:file:`tests/project/test_stdlib_feature_probes_1709_unit.py` fails on a ``getattr`` +or ``hasattr`` call naming its attribute in a string literal, where the receiver is a +builtin class or a module the file binds with an ``import`` statement, or where the +attribute is one the test lists with the interpreter version that added it; the calls +it exempts pass. A helper that has to behave the same on every platform is +neither a compatibility implementation nor a re-export, so it gets a module of its +own: that is why :func:`~pcapkit.utilities.datetime.fromtimestamp` is in +:mod:`pcapkit.utilities.datetime`, ruled in review of the timestamp fix for +:issue:`2099`, whose thread does not hold it. + :mod:`pcapkit.corekit` **is for the types the packet model is built from**: what :mod:`pcapkit.protocols`, :mod:`pcapkit.const` and :mod:`pcapkit.foundation` subclass, instantiate or decorate with. The question is *is this part of what a diff --git a/pcapkit/protocols/application/ospf.py b/pcapkit/protocols/application/ospf.py index 2e5c8bf64e..943814fbe8 100644 --- a/pcapkit/protocols/application/ospf.py +++ b/pcapkit/protocols/application/ospf.py @@ -104,8 +104,8 @@ class OSPF(Application[Data_OSPF, Schema_OSPF], A routing protocol computes the forwarding table rather than forwarding packets, so it is a *user* of the stack rather than part of its forwarding path. :rfc:`1812#section-7` places it accordingly, titling that chapter - "APPLICATION LAYER - ROUTING PROTOCOLS" with OSPF at §7.2.2, while - :rfc:`1812#section-4.1` confines the internet layer to IP, ICMP and IGMP. + "APPLICATION LAYER - ROUTING PROTOCOLS" with OSPF at §7.2.2, while the + Internet Layer protocols :rfc:`1812#section-4.1` names are IP, ICMP and IGMP. Hence :class:`~pcapkit.protocols.application.application.Application` as the base, and ``layer == 'Application'``. See :doc:`/contributing/conventions/protocol-layer-placement`. diff --git a/tests/protocols/application/test_layer_placement_unit.py b/tests/protocols/application/test_layer_placement_unit.py index d34f45142d..cd3337f334 100644 --- a/tests/protocols/application/test_layer_placement_unit.py +++ b/tests/protocols/application/test_layer_placement_unit.py @@ -4,7 +4,7 @@ Settled on GitHub issue #719: layer is decided by what a protocol is *for*, not by what encapsulates it, with the IETF as the single source of truth. :rfc:`1812#section-7` is titled "APPLICATION LAYER - ROUTING PROTOCOLS" with OSPF at -§7.2.2 and confines the internet layer to IP, ICMP and IGMP at §4.1; +§7.2.2, while the Internet Layer protocols §4.1 names are IP, ICMP and IGMP; :rfc:`1122#section-1.1.3` lists RARP in the application layer while ARP sits in the Link Layer chapter at §2.3.2. The rule and its citations are written down at :doc:`/contributing/conventions/protocol-layer-placement`. ``FTP_DATA`` follows the From 3ac73ccf755a766c6a96a19261b6fddcec8e3774 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 1 Nov 2026 14:18:39 -0500 Subject: [PATCH 2/2] docs(conventions): record the SSL family and the dispatcher criterion (#2563) - protocol-family-grouping: a new SSL Family section records the grouping ruled on #2441: SSL over SSLv2, TLS and DTLS, one TLS class with a dynamic alias, SSLv3 as the thin subclass, and how SSLv2 is reached on port 443. - Dispatching Bases opens with the criterion: a base is bound, and dispatches, only where the registry code cannot tell the members apart (ruled for IGMP on #2444); IP, IPsec and SSL are never bound (ruled for SSL on #2441). - Member Shapes lists SSLv2, TLS and DTLS; Grouping by Name names SSL. - The intro leaves each ruling to name its own thread. Closes #2563 --- .../conventions/protocol-family-grouping.rst | 63 ++++++++++++++++--- 1 file changed, 53 insertions(+), 10 deletions(-) diff --git a/docs/source/contributing/conventions/protocol-family-grouping.rst b/docs/source/contributing/conventions/protocol-family-grouping.rst index d00eb988fb..e9e86bd75e 100644 --- a/docs/source/contributing/conventions/protocol-family-grouping.rst +++ b/docs/source/contributing/conventions/protocol-family-grouping.rst @@ -5,8 +5,8 @@ Protocol Family Grouping A protocol family is a base class named after the family plus its member classes. What each class returns from :meth:`~pcapkit.protocols.protocol.Protocol.id` is what -the selectors on :class:`~pcapkit.protocols.protocol.Protocol` match against. The -rulings were set on :pr:`2441`, :pr:`2443`, :pr:`2446`, :issue:`2462` and :issue:`2517`. +the selectors on :class:`~pcapkit.protocols.protocol.Protocol` match against. Each +ruling below names the thread it was set on. The ``id()`` Criterion ~~~~~~~~~~~~~~~~~~~~~~ @@ -43,18 +43,57 @@ Grouping by Name **Members share a base because they share a name, not a wire format.** HTTP/1 and HTTP/2 share :class:`~pcapkit.protocols.application.http.HTTP`, IPv4 and IPv6 share -:class:`~pcapkit.protocols.internet.ip.IP`, and AH and ESP share -:class:`~pcapkit.protocols.internet.ipsec.IPsec`. :pr:`2441`. - -:class:`~pcapkit.protocols.internet.ip.IP` and -:class:`~pcapkit.protocols.internet.ipsec.IPsec` define no ``read()``. The HTTP class -does, but yields no instance of itself (:ref:`below `). +:class:`~pcapkit.protocols.internet.ip.IP`, AH and ESP share +:class:`~pcapkit.protocols.internet.ipsec.IPsec`, and SSLv2 and SSLv3 share +:class:`~pcapkit.protocols.application.ssl.SSL` (:ref:`below `). +:pr:`2441`. + +.. _protocol-family-ssl: + +The SSL Family +~~~~~~~~~~~~~~ + +**SSLv2 and SSLv3 share a base by name, though only SSLv3 shares TLS's record +layer.** The owner asked on :pr:`2441` for this grouping to be recorded here; no issue +holds the ruling. By the ``id()`` criterion, +:meth:`SSL.id() ` is +``('SSLv2', 'TLS', 'DTLS')``, without SSLv3, which subclasses TLS. + +**One** :class:`~pcapkit.protocols.application.tls.TLS` **class reads SSLv3 through +TLS 1.3**, in the ARP shape (:ref:`protocol-family-shapes`): ``id()`` stays +``('TLS',)`` and ``alias`` names the version read. Only record version ``0x0300`` is +branded SSLv3, in ``name`` as well as ``alias``; every later version is TLS. +:class:`~pcapkit.protocols.application.tls.SSLv3` ``('SSLv3',)`` narrows ``id()`` and +nothing else, as :class:`~pcapkit.protocols.link.arp.InARP` does under ARP, and nothing +dispatches to it: an SSLv3 record on TCP port 443 parses as TLS with alias ``SSLv3``. +:class:`~pcapkit.protocols.application.dtls.DTLS` ``('DTLS',)`` reads DTLS 1.0, 1.2 and +1.3 the same way. + +TCP port 443 binds TLS and UDP port 5684 binds DTLS. +:class:`~pcapkit.protocols.application.sslv2.SSLv2` ``('SSLv2',)`` is bound to no port: +on 443, TLS offers SSLv2 a segment whose first record it rejects, and a record SSLv2 +takes stays a TLS layer with alias ``SSLv2``. So ``'SSL' in`` selects both TLS and +DTLS layers, while ``'SSLv3' in`` and ``'SSLv2' in`` select no layer port 443 parsed; +only ``frame.protochain`` matches those two, by alias. .. _protocol-family-dispatch: Dispatching Bases ~~~~~~~~~~~~~~~~~ +**A registry binds a family's base only where its code cannot tell the members apart, +and there the base picks the member.** IP protocol number 2 is IGMP's in every version, +and the TCP and UDP ports bound to HTTP are the family's, so +:class:`~pcapkit.protocols.internet.igmp.IGMP`'s ``_dispatch`` and +:meth:`~pcapkit.protocols.application.http.HTTP._dispatch_version` read the octets to +choose. Every registry that reaches IPv4, IPv6, AH, ESP, TLS or DTLS has a code for +that member alone, so no registry binds :class:`~pcapkit.protocols.internet.ip.IP`, +:class:`~pcapkit.protocols.internet.ipsec.IPsec` or +:class:`~pcapkit.protocols.application.ssl.SSL`, and none of the three defines +``read()``: none needs a dispatcher there. Ruled for IGMP on :pr:`2444` and for SSL on +:pr:`2441`; no issue holds either ruling. The asymmetry is deliberate, and it concerns +the registry only; parsing or building through a base directly is the rule below. + **A versioned family's base dispatches on both paths** (:issue:`2524`). Parsing octets through it, constructing it from fields and its ``from_data`` give the member the octets, fields or data model describe, never an instance of the base. In @@ -95,7 +134,8 @@ Member Shapes (:ref:`protocol-family-canonical-names`); :class:`HTTPv1 ` ``('HTTP/1',)`` and :class:`HTTPv2 ` - ``('HTTP/2',)`` + ``('HTTP/2',)``; :class:`~pcapkit.protocols.application.sslv2.SSLv2` + ``('SSLv2',)`` * - Family-inclusive - the member's label plus the family name - :class:`~pcapkit.protocols.link.c_tag.C_Tag` ``('C_Tag', 'VLAN')``, @@ -105,7 +145,10 @@ Member Shapes - fixed, whichever protocol was read - :class:`~pcapkit.protocols.link.arp.ARP` ``('ARP',)`` reads the operation code and reports ARP, RARP, DRARP or InARP through - :attr:`~pcapkit.protocols.protocol.Protocol.alias` + :attr:`~pcapkit.protocols.protocol.Protocol.alias`; + :class:`~pcapkit.protocols.application.tls.TLS` ``('TLS',)`` reports SSLv3 or a + TLS version, and :class:`~pcapkit.protocols.application.dtls.DTLS` + ``('DTLS',)`` a DTLS version (:ref:`protocol-family-ssl`) Nothing dispatches to :class:`~pcapkit.protocols.link.arp.InARP` ``('InARP',)``: under EtherType ``0x0806``, operation 8 parses as ARP with alias ``InARP``.