diff --git a/pcapkit/protocols/internet/ah.py b/pcapkit/protocols/internet/ah.py index 5546c22c5..11f99570b 100644 --- a/pcapkit/protocols/internet/ah.py +++ b/pcapkit/protocols/internet/ah.py @@ -52,11 +52,17 @@ class AH(IPsec[Data_AH, Schema_AH], IPv6_Ext[Data_AH, Schema_AH], """This class implements Authentication Header. Double-inherited (GitHub issue #917): ``AH`` is both a member of the - IPsec family and an IPv6 extension header -- IANA's - ``protocol-numbers-1.csv`` marks it ``Y`` in the *IPv6 Extension Header* - column (:rfc:`4302`), and this package's own + IPsec family and an IPv6 extension header -- IANA's *IPv6 Extension + Header Types* registry lists it at 51 (:rfc:`4302#section-3.1.1` has + it appear after the hop-by-hop, routing and fragmentation extension + headers in the IPv6 header chain), and this package's own :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` registry - agrees. :class:`~pcapkit.protocols.internet.ipsec.IPsec` is first in the + agrees. The same section separately states that, in the context of + IPv4, AH is placed after the IP header and before the next-layer + protocol -- the primary-source evidence that it also travels + directly as an IPv4 payload, which is what qualifies it for a base + besides :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`. + :class:`~pcapkit.protocols.internet.ipsec.IPsec` is first in the bases so that its :meth:`~pcapkit.protocols.internet.ipsec.IPsec.id` keeps precedence. diff --git a/pcapkit/protocols/internet/esp.py b/pcapkit/protocols/internet/esp.py index 88cfcd591..7a1f50585 100644 --- a/pcapkit/protocols/internet/esp.py +++ b/pcapkit/protocols/internet/esp.py @@ -976,12 +976,19 @@ class ESP(IPsec[Data_ESP, Schema_ESP], IPv6_Ext[Data_ESP, Schema_ESP], """This class implements Encapsulating Security Payload. Double-inherited (GitHub issue #917), mirroring - :class:`~pcapkit.protocols.internet.ah.AH`: IANA's - ``protocol-numbers-1.csv`` marks ``ESP`` ``Y`` in the *IPv6 Extension - Header* column (:rfc:`4303`), and this package's own + :class:`~pcapkit.protocols.internet.ah.AH`: IANA's *IPv6 Extension + Header Types* registry lists ``ESP`` at 50 (:rfc:`4303#section-3.1.1` + has it appear after the hop-by-hop, routing and fragmentation + extension headers in the IPv6 header chain), and this package's own :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` registry - agrees (``ESP = 50``), so it must honour the same extension-mode contract - as its siblings. :attr:`payload` and :attr:`protochain` come from + agrees (``ESP = 50``), so it must honour the same extension-mode + contract as its siblings. The same section separately states that, + in the context of IPv4, ESP is placed after the IP header and + before the next-layer protocol -- the primary-source evidence that + it also travels directly as an IPv4 payload, which is what + qualifies it for a base besides + :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`. + :attr:`payload` and :attr:`protochain` come from :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`; :attr:`protocol` is spelled out below for the reason given there. diff --git a/tests/protocols/internet/test_ah_unit.py b/tests/protocols/internet/test_ah_unit.py index 56330c007..619690b18 100644 --- a/tests/protocols/internet/test_ah_unit.py +++ b/tests/protocols/internet/test_ah_unit.py @@ -114,6 +114,76 @@ def test_ah_read_properties_and_ipsec_id(self) -> None: self.assertTrue(post_proto._extf) post_init.assert_called_once() + def test_docstring_cites_the_extension_header_registry(self) -> None: + """GitHub issue #931. + + The class docstring's extension-header *membership* claim must + rest on IANA's authoritative *IPv6 Extension Header Types* + registry (and this package's own registry, generated from it), + not the ``protocol-numbers-1.csv`` *IPv6 Extension Header* column + that #926 repointed the const generator away from -- that column + is a derived signal, not the registry :rfc:`8200#section-4` names + as authoritative, and disagreed with it on header 147 + (``BIT_EMU``). + + :rfc:`4302#section-3.1.1` is cited too, but only for what it + actually says: an IPv6-context placement recommendation ("should + appear after hop-by-hop, routing, and fragmentation extension + headers"), not a membership assertion -- membership is the + registry's claim, per + ``docs/source/contributing/conventions.rst``'s "being *in* that + registry is what makes something an extension header" ruling. + Rephrasing the placement sentence as "places it among the IPv6 + extension headers" overstated the RFC; that was caught in review + on the second version of this fix. + + The section's separate IPv4-context sentence ("placed after the + IP header ... before the next layer protocol") is legitimate + prose here too -- it is the primary-source evidence that AH + also travels directly as an IPv4 payload, exactly as + :mod:`~pcapkit.protocols.internet.hip` cites its own IPv4 + appendix for the same reason. Per + ``docs/source/contributing/conventions.rst``'s "own-protocolhood + on its own is not sufficient" ruling (MH is the settling case), + that fact *qualifies* an already-standalone header for a base + besides :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`; + it does not *make* the header standalone, and the RFC has no + say in which base is named. An earlier revision of this + docstring's ``ESP`` counterpart claimed the RFC "makes it a + standalone protocol" and "names ``IPsec`` as the second base", + the latter positionally false and unsupported by the cited + section; both were caught in review on the third version of + this fix. So this test targets the exact wrong phrasing from + the earlier rounds rather than banning "IPv4" outright, which + would forbid the legitimate use kept above. + """ + import re + + from pcapkit.protocols.internet.ah import AH + + # Normalise the docstring's line-wrapping before matching, since the + # citation may be split across lines by hand-wrapping. + doc = ' '.join((AH.__doc__ or '').split()) + self.assertNotIn('protocol-numbers-1.csv', doc) + self.assertNotIn('among the IPv6 extension headers', doc) + self.assertNotIn('second base', doc) + self.assertNotIn('directly after an IPv4 header', doc) + + # Pin the claim, not just the phrase: whatever the RFC-placement + # parenthetical attached directly to the registry-membership + # sentence says, it must not smuggle in IPv4 evidence for that + # claim, regardless of how a future revision words it. + membership_clause = re.search(r'registry lists it at 51 \((.*?)\)', doc) + self.assertIsNotNone(membership_clause, 'could not locate the membership clause') + self.assertNotIn('IPv4', membership_clause.group(1)) + + self.assertIn('IPv6 Extension Header Types', doc) + self.assertIn(':rfc:`4302#section-3.1.1`', doc) + self.assertIn('hop-by-hop, routing and fragmentation', doc) + self.assertIn('IPv6 header chain', doc) + self.assertIn('travels directly as an IPv4 payload', doc) + self.assertIn('qualifies it for a base besides', doc) + if __name__ == '__main__': unittest.main() diff --git a/tests/protocols/internet/test_esp_unit.py b/tests/protocols/internet/test_esp_unit.py index 4f4ad5970..995420c32 100644 --- a/tests/protocols/internet/test_esp_unit.py +++ b/tests/protocols/internet/test_esp_unit.py @@ -1038,6 +1038,79 @@ def test_ipv6_extension_header_position(self) -> None: self.assertIn(str(ESP.__index__()), [str(key) for key in ipv6.extension_headers]) self.assertIn('ESP', str(ipv6.protochain)) + def test_docstring_cites_the_extension_header_registry(self) -> None: + """GitHub issue #931. + + The class docstring's extension-header *membership* claim must + rest on IANA's authoritative *IPv6 Extension Header Types* + registry (and this package's own registry, generated from it), + not the ``protocol-numbers-1.csv`` *IPv6 Extension Header* column + that #926 repointed the const generator away from -- that column + is a derived signal, not the registry :rfc:`8200#section-4` names + as authoritative, and disagreed with it on header 147 + (``BIT_EMU``). + + :rfc:`4303#section-3.1.1` is cited too, but only for what it + actually says: an IPv6-context placement recommendation ("should + appear after hop-by-hop, routing, and fragmentation extension + headers"), not a membership assertion -- membership is the + registry's claim, per + ``docs/source/contributing/conventions.rst``'s "being *in* that + registry is what makes something an extension header" ruling. + Rephrasing the placement sentence as "places it among the IPv6 + extension headers" overstated the RFC and contradicted this same + docstring's ``Note:`` (which says RFC 8200 declines ESP + extension-header status); that was caught in review on the + second version of this fix. + + The section's separate IPv4-context sentence ("placed after the + IP header ... before the next layer protocol") is legitimate + prose here too -- it is the primary-source evidence that ESP + also travels directly as an IPv4 payload, exactly as + :mod:`~pcapkit.protocols.internet.hip` cites its own IPv4 + appendix for the same reason. Per + ``docs/source/contributing/conventions.rst``'s "own-protocolhood + on its own is not sufficient" ruling (MH is the settling case), + that fact *qualifies* an already-standalone header for a base + besides :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`; + it does not *make* the header standalone, and the RFC has no + say in which base is named (``IPsec`` versus ``Internet``) -- + an earlier revision claimed the RFC "makes it a standalone + protocol" and "names ``IPsec`` as the second base", the latter + positionally false (``ESP.__bases__`` puts ``IPsec`` first, not + second) and unsupported by the cited section; both were caught + in review on the third version of this fix. So this test + targets the exact wrong phrasing from the earlier rounds rather + than banning "IPv4" outright, which would forbid the legitimate + use kept above. + """ + import re + + from pcapkit.protocols.internet.esp import ESP + + # Normalise the docstring's line-wrapping before matching, since the + # citation may be split across lines by hand-wrapping. + doc = ' '.join((ESP.__doc__ or '').split()) + self.assertNotIn('protocol-numbers-1.csv', doc) + self.assertNotIn('among the IPv6 extension headers', doc) + self.assertNotIn('second base', doc) + self.assertNotIn('directly after an IPv4 header', doc) + + # Pin the claim, not just the phrase: whatever the RFC-placement + # parenthetical attached directly to the registry-membership + # sentence says, it must not smuggle in IPv4 evidence for that + # claim, regardless of how a future revision words it. + membership_clause = re.search(r'registry lists ``ESP`` at 50 \((.*?)\)', doc) + self.assertIsNotNone(membership_clause, 'could not locate the membership clause') + self.assertNotIn('IPv4', membership_clause.group(1)) + + self.assertIn('IPv6 Extension Header Types', doc) + self.assertIn(':rfc:`4303#section-3.1.1`', doc) + self.assertIn('hop-by-hop, routing and fragmentation', doc) + self.assertIn('IPv6 header chain', doc) + self.assertIn('travels directly as an IPv4 payload', doc) + self.assertIn('qualifies it for a base besides', doc) + if __name__ == '__main__': unittest.main()