diff --git a/CHANGELOG.md b/CHANGELOG.md index cd5ef2e7c9..e880756b2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -189,6 +189,7 @@ This resolves [#548](https://github.com/JarryShaw/PyPCAPKit/issues/548), which r - `TCP.__proto__` bound `httpv1.HTTP` directly for ports 80 and 8080, so a segment on either port was HTTP/1 by assertion of the port alone; both versions share those ports, so the payload has to decide. Repointed to the generic `pcapkit.protocols.application.http.HTTP` proxy, whose identification became positive only once [#800](https://github.com/JarryShaw/PyPCAPKit/issues/800) and [#814](https://github.com/JarryShaw/PyPCAPKit/pull/814) landed, which is why this waited. `udp.py` already bound the proxy for both ports; this removes the asymmetry its docstring and `docs/source/pep.rst` documented as an open request (prose-only on that side). Protochain over all 23 captures (1,604 frames) is *not* byte-identical, and that is the fix: all 231 HTTP/1.1 frames keep their chain, and nine frames in `options-transport.pcap` change `Ethernet:IPv4:TCP:Raw` to `Ethernet:IPv4:TCP:HTTP/2` -- genuine HTTP/2 frames this library's own `httpv2.HTTP.make` built, previously refused by the HTTP/1 parser. `_guess_version`'s entry count over the corpus goes 0 to 252 (231 HTTP/1.1, 9 HTTP/2, 12 fall-throughs that stay `Raw`). Not labelled breaking, but not free: TCP:80/8080 traffic that is neither valid HTTP/1 nor preface-carrying is now exposed to the fall-through arm, where the direct `httpv1` binding left it `Raw` regardless; a caller depending on that is the one who would notice ([#682](https://github.com/JarryShaw/PyPCAPKit/issues/682)). - **a breaking change to** 8 more bespoke registries: step 2 of [#860](https://github.com/JarryShaw/PyPCAPKit/issues/860), PR 1 of 2 (`AppType` is PR 2, [#874](https://github.com/JarryShaw/PyPCAPKit/pull/874) below), bringing `StatusCode`, `ReturnCode`, `ResponseKind`, `GroupingInformation`, `OptionType`, `FEATCode`, `Command` and `Method` onto `EnumRegistry` and applying [#775](https://github.com/JarryShaw/PyPCAPKit/issues/775)'s mint/unmint ruling to their `get()` as well as `_missing_`. The first five convert 12 unambiguous placeholder branches (`Unassigned`, `Unknown`, `opt_unknown`) to `_unregistered_member`; the three with a custom `__new__` (`StatusCode`, `ReturnCode`, `OptionType`) get an override reconstructing the attributes the base's generic helper would leave unset, and `StatusCode`/`ReturnCode`'s hand-written `get()`, still on the retired `default == -1` convention, is replaced by the base's (no caller relied on the old form). `OptionType` keeps its own `get()` for its multi-namespace dispatch, but a round-2 review found it still minted on both its int/namespace and `str` paths, and the live pcapng parse path (`PCAPNG._make_pcapng_options`) calls it with wire bytes, so parsing an undeclared option code still registered a permanent member; both paths now build an unregistered member. `FEATCode`, `Command` and `Method` mint the literal wire value as its own name rather than any placeholder; per the owner's ruling (*"get will not have sufficient information to create new ones"*: `Command` needs `feat`/`desc`/`type`/`conf` and `Method` needs `safe`/`idempotent`, which a bare wire string lacks), both `_missing_` and each class's own `get()` (a second, independent mint site) now build an unregistered member. `FEATCode`'s crawler now declares all 15 real FEAT-code values from the live IANA table instead of minting 10 of them as a side effect of evaluating `Command`'s rows at import time, the import-time-mutation shape [#861](https://github.com/JarryShaw/PyPCAPKit/pull/861) removed from `FilterType`; pinned count-agnostically so a future IANA update cannot fail a correct regeneration. Untouched, per rulings: `CommandType` stays `IntFlag` (real `A|P` composites in the generated data), and `TransportProtocol`'s `auto()` renumbering and `AppType` are PR 2's. `pcapkit/protocols/schema/misc/pcapng.py`'s `OptionEnumField.post_process` docstring is corrected: it still bypasses `OptionType.get()` directly, now for `pickle` round-trip safety ([#860](https://github.com/JarryShaw/PyPCAPKit/issues/860) already stops the minting that used to be the reason), since neither of `OptionType`'s two paths gets both pickling and correct rendering right at once ([#869](https://github.com/JarryShaw/PyPCAPKit/pull/869)). - `register` on `ProtocolBase`, `Frame`, `PCAPNG`, `SCTP`, `Link`, `Internet` and the concrete `Transport` subclasses (`TCP`, `UDP`) now raises `RegistryError` for a non-class argument, where it raised a bare `TypeError` ("issubclass() arg 1 must be a class") from inside `abc`. Each guard gains an explicit `isinstance(protocol, type)` test ahead of its `issubclass`, after the `ModuleDescriptor` is unwrapped, so a descriptor naming a non-class attribute is rejected too. All seven `Raises:` clauses already promised `RegistryError` if `protocol` "is not a ... subclass", which a non-class satisfied in spirit but not in fact; they now read "is not a class, or not a ... subclass". No guard's target class changes, and a wrong class still raises `RegistryError` as before. `Transport.register` on `Transport` itself still raises `UnsupportedCall` first; only its subclasses reach the guard. Not breaking for exception handling: `RegistryError` subclasses `TypeError` through `BaseError`, so `except TypeError` still catches it, and the type and message text are the only things a handler can key on that change. Two observable side effects do come with it, because `BaseError` is loud by default: the non-class path now emits pcapkit's one `CRITICAL` log record, and outside devmode it installs `sys.excepthook` and `threading.excepthook`, neither of which the bare `TypeError` from `abc` did. That is the wrong-class path's existing behaviour, so the fix makes the two consistent rather than introducing something new. Follows the same fix for the foundation registrars ([#1021](https://github.com/JarryShaw/PyPCAPKit/issues/1021)) ([#1026](https://github.com/JarryShaw/PyPCAPKit/issues/1026)). +- `PCAPNG`'s `ns_dnsname`, `ns_dnsIP4addr` and `ns_dnsIP6addr` scope guards, on both the read and the make side, now say the option "must be in Name Resolution Block", which is the block they check; they used to name the systemd(1) Journal Export Block, with a literal `:manpage:` role in the text. `register_record`'s `RegistryWarning` likewise now says "name resolution record already registered". The make-side `ns_dnsIP4addr`/`ns_dnsIP6addr` messages also drop the lowercase `ns_dnsip4addr`/`ns_dnsip6addr` spelling for the `OptionType` member name. Two more guards named the wrong option: `_make_option_if_rxspeed`'s scope and duplicate guards now say `[if_rxspeed]` rather than `[if_txspeed]`, and `_read_option_epb_queue`'s length guard says `[epb_queue]` rather than `[epb_packetid]`. Only message text changes; the exception and warning types are unchanged ([#1038](https://github.com/JarryShaw/PyPCAPKit/issues/1038)). ### pcapkit.toolkit diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index 26d8eee954..e4b1a232a7 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -2151,6 +2151,18 @@ Fixed so the fix makes the two consistent rather than introducing something new. Follows the same fix for the foundation registrars (:issue:`1021`) (:issue:`1026`). +* ``PCAPNG``'s ``ns_dnsname``, ``ns_dnsIP4addr`` and ``ns_dnsIP6addr`` scope guards, + on both the read and the make side, now say the option "must be in Name Resolution + Block", which is the block they check; they used to name the systemd(1) Journal + Export Block, with a literal ``:manpage:`` role in the text. ``register_record``'s + ``RegistryWarning`` likewise now says "name resolution record already registered". + The make-side ``ns_dnsIP4addr``/``ns_dnsIP6addr`` messages also drop the lowercase + ``ns_dnsip4addr``/``ns_dnsip6addr`` spelling for the ``OptionType`` member name. + Two more guards named the wrong option: ``_make_option_if_rxspeed``'s scope and + duplicate guards now say ``[if_rxspeed]`` rather than ``[if_txspeed]``, and + ``_read_option_epb_queue``'s length guard says ``[epb_queue]`` rather than + ``[epb_packetid]``. Only message text changes; the exception and warning types are unchanged + (:issue:`1038`). pcapkit.toolkit --------------- diff --git a/docs/source/contributing/conventions/process.rst b/docs/source/contributing/conventions/process.rst index c7691d066d..1168a152a6 100644 --- a/docs/source/contributing/conventions/process.rst +++ b/docs/source/contributing/conventions/process.rst @@ -101,7 +101,7 @@ commands down instead of a figure that will be stale by the next merge: The grouping scheme was settled on :issue:`918`: **a section per top-level module, with** ``Added``/``Changed``/``Fixed`` **nested inside each** -- module granularity, not per-file and not per-subpackage. The file carries **9** module-level sections holding -163 entries, and no entry carries an inline kind label:: +164 entries, and no entry carries an inline kind label:: $ grep -cE '^\* \*\*(Added|Changed|Fixed)\*\*' docs/source/changelog/1.5.0.rst 0 diff --git a/pcapkit/protocols/data/misc/pcapng.py b/pcapkit/protocols/data/misc/pcapng.py index 90887f1dd5..ef78076b69 100644 --- a/pcapkit/protocols/data/misc/pcapng.py +++ b/pcapkit/protocols/data/misc/pcapng.py @@ -577,7 +577,7 @@ def __init__(self, type: 'Enum_OptionType', length: 'int', name: 'str') -> 'None @info_final class NS_DNSIP4AddrOption(Option): - """Data model for PCAP-NG ``ns_dnsip4addr`` option.""" + """Data model for PCAP-NG ``ns_dnsIP4addr`` option.""" #: IPv4 address. ip: 'IPv4Address' @@ -588,7 +588,7 @@ def __init__(self, type: 'Enum_OptionType', length: 'int', ip: 'IPv4Address') -> @info_final class NS_DNSIP6AddrOption(Option): - """Data model for PCAP-NG ``ns_dnsip6addr`` option.""" + """Data model for PCAP-NG ``ns_dnsIP6addr`` option.""" #: IPv6 address. ip: 'IPv6Address' diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index 286ac9b663..f8c5fd4f8a 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -952,7 +952,7 @@ def register_record(cls, code: 'Enum_RecordType', meth: 'str | tuple[RecordParse """ if code in cls.__record__: - warn(f'PCAP-NG: [Type {code}] :manpage:`systemd(1)` journal export record already registered', RegistryWarning) + warn(f'PCAP-NG: [Type {code}] name resolution record already registered', RegistryWarning) cls.__record__[code] = meth @classmethod @@ -2962,7 +2962,7 @@ def _read_option_epb_queue(self, schema: 'Schema_EPB_QueueOption', *, raise ProtocolError(f'PCAP-NG: [epb_queue] option must be only one, ' f'but {self._opt[schema.type] + 1} found.') if schema.length != 4: - raise ProtocolError(f'PCAP-NG: [epb_packetid] invalid length (expected 4, got {schema.length})') + raise ProtocolError(f'PCAP-NG: [epb_queue] invalid length (expected 4, got {schema.length})') option = Data_EPB_QueueOption( type=schema.type, @@ -3010,7 +3010,7 @@ def _read_option_ns_dnsname(self, schema: 'Schema_NS_DNSNameOption', *, """ if self._type != Enum_BlockType.Name_Resolution_Block: - raise ProtocolError(f'PCAP-NG: [ns_dnsname] option must be in :manpage:`systemd(1)` Journal Export Block, ' + raise ProtocolError(f'PCAP-NG: [ns_dnsname] option must be in Name Resolution Block, ' f'but found in {self._type} block.') if self._opt[schema.type] > 0: raise ProtocolError(f'PCAP-NG: [ns_dnsname] option must be only one, ' @@ -3036,7 +3036,7 @@ def _read_option_ns_dnsipv4(self, schema: 'Schema_NS_DNSIP4AddrOption', *, """ if self._type != Enum_BlockType.Name_Resolution_Block: - raise ProtocolError(f'PCAP-NG: [ns_dnsIP4addr] option must be in :manpage:`systemd(1)` Journal Export Block, ' + raise ProtocolError(f'PCAP-NG: [ns_dnsIP4addr] option must be in Name Resolution Block, ' f'but found in {self._type} block.') if self._opt[schema.type] > 0: raise ProtocolError(f'PCAP-NG: [ns_dnsIP4addr] option must be only one, ' @@ -3064,7 +3064,7 @@ def _read_option_ns_dnsipv6(self, schema: 'Schema_NS_DNSIP6AddrOption', *, """ if self._type != Enum_BlockType.Name_Resolution_Block: - raise ProtocolError(f'PCAP-NG: [ns_dnsIP6addr] option must be in :manpage:`systemd(1)` Journal Export Block, ' + raise ProtocolError(f'PCAP-NG: [ns_dnsIP6addr] option must be in Name Resolution Block, ' f'but found in {self._type} block.') if self._opt[schema.type] > 0: raise ProtocolError(f'PCAP-NG: [ns_dnsIP6addr] option must be only one, ' @@ -4820,10 +4820,10 @@ def _make_option_if_rxspeed(self, type: 'Enum_OptionType', option: 'Optional[Dat """ if self._type != Enum_BlockType.Interface_Description_Block: - raise ProtocolError(f'PCAP-NG: [if_txspeed] option must be in Interface Description Block, ' + raise ProtocolError(f'PCAP-NG: [if_rxspeed] option must be in Interface Description Block, ' f'but found in {self._type} block.') if self._opt[type] > 0: - raise ProtocolError(f'PCAP-NG: [if_txspeed] option must be only one, ' + raise ProtocolError(f'PCAP-NG: [if_rxspeed] option must be only one, ' f'but {self._opt[type] + 1} found.') if option is not None: @@ -5119,7 +5119,7 @@ def _make_option_ns_dnsname(self, type: 'Enum_OptionType', option: 'Optional[Dat """ if self._type != Enum_BlockType.Name_Resolution_Block: - raise ProtocolError(f'PCAP-NG: [ns_dnsname] option must be in :manpage:`systemd(1)` Journal Export Block, ' + raise ProtocolError(f'PCAP-NG: [ns_dnsname] option must be in Name Resolution Block, ' f'but found in {self._type} block.') if self._opt[type] > 0: raise ProtocolError(f'PCAP-NG: [ns_dnsname] option must be only one, ' @@ -5137,7 +5137,7 @@ def _make_option_ns_dnsname(self, type: 'Enum_OptionType', option: 'Optional[Dat def _make_option_ns_dnsipv4(self, type: 'Enum_OptionType', option: 'Optional[Data_NS_DNSIP4AddrOption]' = None, *, ip: 'str | bytes | IPv4Address | int' = '8.8.8.8', **kwargs: 'Any') -> 'Schema_NS_DNSIP4AddrOption': - """Make PCAP-NG ``ns_dnsip4addr`` option. + """Make PCAP-NG ``ns_dnsIP4addr`` option. Args: type: Option type. @@ -5150,10 +5150,10 @@ def _make_option_ns_dnsipv4(self, type: 'Enum_OptionType', option: 'Optional[Dat """ if self._type != Enum_BlockType.Name_Resolution_Block: - raise ProtocolError(f'PCAP-NG: [ns_dnsip4addr] option must be in :manpage:`systemd(1)` Journal Export Block, ' + raise ProtocolError(f'PCAP-NG: [ns_dnsIP4addr] option must be in Name Resolution Block, ' f'but found in {self._type} block.') if self._opt[type] > 0: - raise ProtocolError(f'PCAP-NG: [ns_dnsip4addr] option must be only one, ' + raise ProtocolError(f'PCAP-NG: [ns_dnsIP4addr] option must be only one, ' f'but {self._opt[type] + 1} found.') if option is not None: @@ -5168,7 +5168,7 @@ def _make_option_ns_dnsipv4(self, type: 'Enum_OptionType', option: 'Optional[Dat def _make_option_ns_dnsipv6(self, type: 'Enum_OptionType', option: 'Optional[Data_NS_DNSIP6AddrOption]' = None, *, ip: 'str | bytes | IPv6Address | int' = '8.8.8.8', **kwargs: 'Any') -> 'Schema_NS_DNSIP6AddrOption': - """Make PCAP-NG ``ns_dnsip6addr`` option. + """Make PCAP-NG ``ns_dnsIP6addr`` option. Args: type: Option type. @@ -5181,10 +5181,10 @@ def _make_option_ns_dnsipv6(self, type: 'Enum_OptionType', option: 'Optional[Dat """ if self._type != Enum_BlockType.Name_Resolution_Block: - raise ProtocolError(f'PCAP-NG: [ns_dnsip6addr] option must be in :manpage:`systemd(1)` Journal Export Block, ' + raise ProtocolError(f'PCAP-NG: [ns_dnsIP6addr] option must be in Name Resolution Block, ' f'but found in {self._type} block.') if self._opt[type] > 0: - raise ProtocolError(f'PCAP-NG: [ns_dnsip6addr] option must be only one, ' + raise ProtocolError(f'PCAP-NG: [ns_dnsIP6addr] option must be only one, ' f'but {self._opt[type] + 1} found.') if option is not None: diff --git a/tests/protocols/misc/test_pcapng_unit.py b/tests/protocols/misc/test_pcapng_unit.py index 890a3da65a..9bd8fca62c 100644 --- a/tests/protocols/misc/test_pcapng_unit.py +++ b/tests/protocols/misc/test_pcapng_unit.py @@ -124,6 +124,118 @@ def test_pcapng_registry_overwrites_warn(self) -> None: record_map[RecordType.nrb_record_end] = originals[2] secrets_map[SecretsType.TLS_Key_Log] = originals[3] + def test_pcapng_record_registry_warning_names_the_name_resolution_record(self) -> None: + # #1038: the warning used to call an ``nrb_record_*`` entry a + # "systemd(1) journal export record", with a literal reST role. + from pcapkit.const.pcapng.record_type import RecordType + from pcapkit.protocols.misc.pcapng import PCAPNG + + record_map = PCAPNG.__dict__['__record__'] + original = record_map[RecordType.nrb_record_end] + try: + with mock.patch('pcapkit.protocols.misc.pcapng.warn') as warn: + PCAPNG.register_record(RecordType.nrb_record_end, 'end') + finally: + record_map[RecordType.nrb_record_end] = original + + message = warn.call_args.args[0] + self.assertIn('name resolution record already registered', message) + self.assertNotIn('journal export', message) + self.assertNotIn(':manpage:', message) + + def test_pcapng_name_resolution_option_guards_name_the_name_resolution_block(self) -> None: + # #1038: the ``ns_dns*`` scope guards check ``Name_Resolution_Block`` + # but used to tell the caller the option belongs in the systemd(1) + # Journal Export Block, and two make-side sites spelled the option + # ``ns_dnsip4addr``/``ns_dnsip6addr``. + from pcapkit.const.pcapng.block_type import BlockType + from pcapkit.const.pcapng.option_type import OptionType + from pcapkit.corekit.multidict import OrderedMultiDict + from pcapkit.protocols.misc.pcapng import PCAPNG + from pcapkit.protocols.schema.misc.pcapng import (NS_DNSIP4AddrOption, NS_DNSIP6AddrOption, + NS_DNSNameOption) + from pcapkit.utilities.exceptions import ProtocolError + + pcapng = object.__new__(PCAPNG) + pcapng._type = BlockType.Enhanced_Packet_Block + pcapng._opt = collections.Counter() + + cases = [ + ('ns_dnsname', lambda: pcapng._read_option_ns_dnsname( + NS_DNSNameOption(type=OptionType.ns_dnsname, length=3, name='dns'), + options=OrderedMultiDict())), + ('ns_dnsIP4addr', lambda: pcapng._read_option_ns_dnsipv4( + NS_DNSIP4AddrOption(type=OptionType.ns_dnsIP4addr, length=4, + ip=ip_address('8.8.8.8')), + options=OrderedMultiDict())), + ('ns_dnsIP6addr', lambda: pcapng._read_option_ns_dnsipv6( + NS_DNSIP6AddrOption(type=OptionType.ns_dnsIP6addr, length=16, + ip=ip_address('2001:4860::8888')), + options=OrderedMultiDict())), + ('ns_dnsname', lambda: pcapng._make_option_ns_dnsname( + OptionType.ns_dnsname, name='dns')), + ('ns_dnsIP4addr', lambda: pcapng._make_option_ns_dnsipv4( + OptionType.ns_dnsIP4addr, ip='8.8.8.8')), + ('ns_dnsIP6addr', lambda: pcapng._make_option_ns_dnsipv6( + OptionType.ns_dnsIP6addr, ip='2001:4860::8888')), + ] + for index, (option, call) in enumerate(cases): + with self.subTest(side='read' if index < 3 else 'make', option=option): + with self.assertRaises(ProtocolError) as ctx: + call() + message = str(ctx.exception) + self.assertIn(f'[{option}] option must be in Name Resolution Block', message) + self.assertNotIn('Journal Export', message) + self.assertNotIn(':manpage:', message) + + # The make-side duplicate guards share the corrected spelling. + pcapng._type = BlockType.Name_Resolution_Block + for option, type_, ip in (('ns_dnsIP4addr', OptionType.ns_dnsIP4addr, '8.8.8.8'), + ('ns_dnsIP6addr', OptionType.ns_dnsIP6addr, '2001:4860::8888')): + with self.subTest(side='make-duplicate', option=option): + pcapng._opt = collections.Counter({type_: 1}) + method = (pcapng._make_option_ns_dnsipv4 if type_ is OptionType.ns_dnsIP4addr + else pcapng._make_option_ns_dnsipv6) + with self.assertRaises(ProtocolError) as ctx: + method(type_, ip=ip) + self.assertIn(f'[{option}] option must be only one', str(ctx.exception)) + + def test_pcapng_guard_messages_name_their_own_option(self) -> None: + # #1038: ``_make_option_if_rxspeed``'s guards said ``[if_txspeed]``, and + # ``_read_option_epb_queue``'s length guard said ``[epb_packetid]``. + from pcapkit.const.pcapng.block_type import BlockType + from pcapkit.const.pcapng.option_type import OptionType + from pcapkit.corekit.multidict import OrderedMultiDict + from pcapkit.protocols.misc.pcapng import PCAPNG + from pcapkit.protocols.schema.misc.pcapng import EPB_QueueOption + from pcapkit.utilities.exceptions import ProtocolError + + pcapng = object.__new__(PCAPNG) + + def message(block_type: BlockType, opt: collections.Counter, call: object) -> str: + pcapng._type = block_type + pcapng._opt = opt + with self.assertRaises(ProtocolError) as ctx: + call() # type: ignore[operator] + return str(ctx.exception) + + make_rxspeed = lambda: pcapng._make_option_if_rxspeed(OptionType.if_rxspeed, speed=1) + with self.subTest(guard='if_rxspeed scope'): + self.assertIn('[if_rxspeed] option must be in Interface Description Block', + message(BlockType.Enhanced_Packet_Block, collections.Counter(), + make_rxspeed)) + with self.subTest(guard='if_rxspeed duplicate'): + self.assertIn('[if_rxspeed] option must be only one', + message(BlockType.Interface_Description_Block, + collections.Counter({OptionType.if_rxspeed: 1}), make_rxspeed)) + with self.subTest(guard='epb_queue length'): + schema = EPB_QueueOption(type=OptionType.epb_queue, length=4, queue_id=5) + object.__setattr__(schema, 'length', 3) + self.assertIn('[epb_queue] invalid length (expected 4, got 3)', + message(BlockType.Enhanced_Packet_Block, collections.Counter(), + lambda: pcapng._read_option_epb_queue( + schema, options=OrderedMultiDict()))) + def test_pcapng_option_registry_preserves_duplicate_numeric_codes(self) -> None: from pcapkit.const.pcapng.option_type import OptionType from pcapkit.protocols.misc.pcapng import PCAPNG, _option_key