From 77d383f744f21fba1649339389b04f2a5f104d9b Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 26 Oct 2026 11:26:02 -0400 Subject: [PATCH] fix(arp): prefix the hardware-address messages and reject a bool with ProtocolError (#2256) ARP._make_addr_resolve raised unprefixed, capitalised messages ("Invalid MAC address: ...", "Invalid hardware address: ..."), gave a wrong type the same message as a malformed string, and rejected a bool with a separate FieldValueError that "except ProtocolError" missed. Drop the bool check, as #2232 did for Ethernet and PCAP-NG: any address that is not a str, bytes or bytearray raises "ARP: invalid MAC address: must be a str, bytes or bytearray, not ..." ("hardware address" for a non-Ethernet htype), and a malformed string raises "ARP: invalid MAC address: ...". Accepted forms are unchanged. The 1.5.0 changelog entry for #2166 and the generated CHANGELOG.md are corrected to match. Test: TestARPHardwareAddressType pins the exact messages for bool, int, None, float and a malformed string on Ethernet and Fibre Channel. --- CHANGELOG.md | 2 +- docs/source/changelog/1.5.0.rst | 13 +++--- pcapkit/protocols/link/arp.py | 43 ++++++++----------- .../link/test_arp_proto_addr_length_unit.py | 26 +++++++++-- 4 files changed, 49 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d59325095d..7dcdef608e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -340,7 +340,7 @@ This resolves [#548](https://github.com/JarryShaw/PyPCAPKit/issues/548), which r - a declared length above `sys.maxsize`, such as a PCAP `incl_len` of `2**31 - 16` on GraalPy or 32-bit CPython, raised `OverflowError` from a read or seek. Every length-driven read in `Schema.unpack()`, `Frame.read()` and `PCAPNG.read()` is bounded at `sys.maxsize`, so such a claim gets the overrun handling it gets on 64-bit CPython ([#1975](https://github.com/JarryShaw/PyPCAPKit/issues/1975), [#1979](https://github.com/JarryShaw/PyPCAPKit/pull/1979)). - PCAP-NG blocks cut short by the file or by their own framing no longer abort an extraction. A Decryption Secrets Block whose `secrets_length` runs past the block keeps the declared length and warns, its secrets capped at what the block holds ([#1980](https://github.com/JarryShaw/PyPCAPKit/issues/1980), [#1983](https://github.com/JarryShaw/PyPCAPKit/pull/1983)); an unknown, Custom or systemd journal export block the file truncates gets the "block truncated" warning the other block types give ([#1984](https://github.com/JarryShaw/PyPCAPKit/issues/1984), [#1988](https://github.com/JarryShaw/PyPCAPKit/pull/1988)); and a malformed TLS or WireGuard key-log line warns with `ProtocolWarning` and is left out of `entries`, where it raised `FieldValueError` ([#1985](https://github.com/JarryShaw/PyPCAPKit/issues/1985), [#1989](https://github.com/JarryShaw/PyPCAPKit/pull/1989)). A ZigBee NWK or APS key off its fixed width rebuilds as captured, where it was zero-padded or cut to that width ([#1986](https://github.com/JarryShaw/PyPCAPKit/issues/1986), [#2001](https://github.com/JarryShaw/PyPCAPKit/pull/2001)). - capture timestamps convert without the platform's `fromtimestamp` limits. Windows' C runtime covers only 1970 to 3000, so a PCAP-NG timestamp outside that range fell back to 1970 with `invalid timestamp`, and the Mobility Header's NTP and PMIPv6 timestamps had the same gap. PCAP, PCAP-NG and MH convert through `pcapkit.utilities.datetime.fromtimestamp()`, which gives the stdlib's result on Linux ([#2099](https://github.com/JarryShaw/PyPCAPKit/issues/2099), [#2111](https://github.com/JarryShaw/PyPCAPKit/pull/2111)), and encode through `pcapkit.utilities.datetime.totimestamp()`, since GraalPy's `timestamp()` put a pre-1970 instant with a fraction one second late ([#2134](https://github.com/JarryShaw/PyPCAPKit/issues/2134), [#2136](https://github.com/JarryShaw/PyPCAPKit/pull/2136)). -- ARP's address makers refuse what they cannot pack: a `bool` hardware or protocol address raises `FieldValueError` whatever `htype` or `ptype` is; a hardware address that is none of `str`, `bytes` and `bytearray`, or a protocol address not packed as IPv4 or IPv6 that is none of those and not an `ipaddress` address, raises `ProtocolError`, where both failed later with a bare `TypeError`. A `bytearray` packs as `bytes` ([#2166](https://github.com/JarryShaw/PyPCAPKit/issues/2166), [#2182](https://github.com/JarryShaw/PyPCAPKit/pull/2182)). +- ARP's address makers refuse what they cannot pack: a `bool` protocol address raises `FieldValueError` whatever `ptype` is; a hardware address that is none of `str`, `bytes` and `bytearray` (a `bool` included), or a protocol address not packed as IPv4 or IPv6 that is none of those and not an `ipaddress` address, raises `ProtocolError`, where both failed later with a bare `TypeError`. A `bytearray` packs as `bytes` ([#2166](https://github.com/JarryShaw/PyPCAPKit/issues/2166), [#2182](https://github.com/JarryShaw/PyPCAPKit/pull/2182), [#2256](https://github.com/JarryShaw/PyPCAPKit/issues/2256)). - the Quick-Start rate parses as the `int` the IPv4, HOPOPT, IPv6-Opts and TCP data models declare, where it was a `float` such as `80.0`, and a TCP Fast Open cookie request, the option of length 2, parses with `cookie` as `b''` rather than `None` ([#2167](https://github.com/JarryShaw/PyPCAPKit/issues/2167), [#2185](https://github.com/JarryShaw/PyPCAPKit/pull/2185)). - PCAP-NG's `ns_dnsIP6addr` option and IPv6 name resolution record build with their defaults, `2001:4860:4860::8888` and `::1`, where IPv4 defaults made them raise `FieldValueError` ([#2168](https://github.com/JarryShaw/PyPCAPKit/issues/2168), [#2179](https://github.com/JarryShaw/PyPCAPKit/pull/2179)). - message text: the `epb_flags` and `pack_flags` length errors say `expected 4`, the length they check, rather than `expected 8` ([#2168](https://github.com/JarryShaw/PyPCAPKit/issues/2168), [#2179](https://github.com/JarryShaw/PyPCAPKit/pull/2179)), and the IPv4 TS option's unknown-flag warning and the Mobility Header's prefix error spell `timestamp` and `mobile` ([#2171](https://github.com/JarryShaw/PyPCAPKit/issues/2171), [#2189](https://github.com/JarryShaw/PyPCAPKit/pull/2189)). diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index 0492452f2a..7af7dfc589 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -2698,12 +2698,13 @@ Fixed Linux (:issue:`2099`, :pr:`2111`), and encode through :func:`pcapkit.utilities.datetime.totimestamp`, since GraalPy's ``timestamp()`` put a pre-1970 instant with a fraction one second late (:issue:`2134`, :pr:`2136`). -* ARP's address makers refuse what they cannot pack: a ``bool`` hardware or protocol - address raises :exc:`.FieldValueError` whatever ``htype`` or ``ptype`` is; a hardware - address that is none of ``str``, ``bytes`` and ``bytearray``, or a protocol address - not packed as IPv4 or IPv6 that is none of those and not an ``ipaddress`` address, - raises :exc:`.ProtocolError`, where both failed later with a bare ``TypeError``. A - ``bytearray`` packs as ``bytes`` (:issue:`2166`, :pr:`2182`). +* ARP's address makers refuse what they cannot pack: a ``bool`` protocol address + raises :exc:`.FieldValueError` whatever ``ptype`` is; a hardware address that is + none of ``str``, ``bytes`` and ``bytearray`` (a ``bool`` included), or a protocol + address not packed as IPv4 or IPv6 that is none of those and not an ``ipaddress`` + address, raises :exc:`.ProtocolError`, where both failed later with a bare + ``TypeError``. A ``bytearray`` packs as ``bytes`` (:issue:`2166`, :pr:`2182`, + :issue:`2256`). * the Quick-Start rate parses as the ``int`` the IPv4, HOPOPT, IPv6-Opts and TCP data models declare, where it was a ``float`` such as ``80.0``, and a TCP Fast Open cookie request, the option of length 2, parses with ``cookie`` as ``b''`` rather than diff --git a/pcapkit/protocols/link/arp.py b/pcapkit/protocols/link/arp.py index 9864af3396..0af47798bf 100644 --- a/pcapkit/protocols/link/arp.py +++ b/pcapkit/protocols/link/arp.py @@ -407,40 +407,35 @@ def _make_addr_resolve(self, addr: 'str | bytes', htype: 'int') -> 'bytes': :class:`bytes`. Raises: - FieldValueError: If ``addr`` is a :obj:`bool`, whatever ``htype`` - is. - ProtocolError: If ``htype`` is ``1`` and ``addr`` is not a - well-formed MAC address given as :class:`str`, - :class:`bytes` or :class:`bytearray`, or if ``htype`` is not - ``1`` and ``addr`` is either a :class:`str` that is not *hex* - encoded or none of :class:`str`, :class:`bytes` and - :class:`bytearray`. + ProtocolError: If ``addr`` is not a :class:`str`, :class:`bytes` + or :class:`bytearray` (a :obj:`bool` included), or if + ``htype`` is ``1`` and ``addr`` is not a well-formed MAC + address, or if ``htype`` is not ``1`` and ``addr`` is a + :class:`str` that is not *hex* encoded. + + Notes: + The messages are prefixed with :attr:`self.__class__.__name__ + ` rather than :attr:`self.alias + `, for the reason + :meth:`_make_proto_resolve` gives. """ - # NOTE: checked before the ``htype`` dispatch, for the same reason as - # in ``_make_proto_resolve``: without it a ``bool`` gets the generic - # ``ProtocolError`` the branches below raise for any non-address, - # rather than the ``FieldValueError`` ``_make_proto_resolve`` raises - # for one (#2166). - if isinstance(addr, bool): - raise FieldValueError( - f'{self.__class__.__name__}: invalid hardware address: must not be a bool, ' - f'not {addr!r} -- pass int({addr!r}) if the numeric value is what is wanted') + name = 'MAC address' if htype == Enum_Hardware.Ethernet else 'hardware address' + if not isinstance(addr, (str, bytes, bytearray)): + raise ProtocolError(f'{self.__class__.__name__}: invalid {name}: must be a str, ' + f'bytes or bytearray, not {addr!r}') if htype == Enum_Hardware.Ethernet: - if not isinstance(addr, (str, bytes, bytearray)): - raise ProtocolError(f'Invalid MAC address: {addr!r}') _addr = addr.encode() if isinstance(addr, str) else addr if PAT_MAC_ADDR.fullmatch(_addr) is not None: return bytes.fromhex(_addr.replace(b':', b'').replace(b'-', b'').decode()) - raise ProtocolError(f'Invalid MAC address: {addr!r}') + raise ProtocolError(f'{self.__class__.__name__}: invalid {name}: {addr!r}') if isinstance(addr, str): try: return bytes.fromhex(addr.replace(':', '').replace('-', '')) except ValueError: - raise ProtocolError(f'Invalid hardware address: {addr!r}') from None - if isinstance(addr, (bytes, bytearray)): - return bytes(addr) - raise ProtocolError(f'Invalid hardware address: {addr!r}') + raise ProtocolError(f'{self.__class__.__name__}: invalid {name}: ' + f'{addr!r}') from None + return bytes(addr) def _make_proto_resolve(self, addr: 'IPv4Address | IPv6Address | str | bytes', ptype: 'int', plen: 'Optional[int]' = None) -> 'bytes': diff --git a/tests/protocols/link/test_arp_proto_addr_length_unit.py b/tests/protocols/link/test_arp_proto_addr_length_unit.py index ad5d31142d..16171296e7 100644 --- a/tests/protocols/link/test_arp_proto_addr_length_unit.py +++ b/tests/protocols/link/test_arp_proto_addr_length_unit.py @@ -92,17 +92,35 @@ class TestARPHardwareAddressType(unittest.TestCase): (``TypeError: expected string or bytes-like object``) for Ethernet, and fell through unchanged (``TypeError: object of type 'bool' has no len()``) for any other ``htype``. + + A :obj:`bool` is rejected like any other wrong type, with + :exc:`~pcapkit.utilities.exceptions.ProtocolError` (#2256). """ HTYPES = {'Ethernet': Hardware.Ethernet, 'non-Ethernet': Hardware.Fibre_Channel} - def test_bool_raises_field_value_error(self) -> None: + #: The address noun each ``htype`` uses in its messages. + NOUNS = {'Ethernet': 'MAC address', 'non-Ethernet': 'hardware address'} + + def test_wrong_type_message(self) -> None: for name, htype in self.HTYPES.items(): - for value in (True, False): + for value in (True, False, 5, None, 1.5): with self.subTest(htype=name, value=value): - with self.assertRaises(FieldValueError) as context: + with self.assertRaises(ProtocolError) as context: ARP(htype=htype, sha=value, tha=value) - self.assertIn('must not be a bool', str(context.exception)) + self.assertNotIsInstance(context.exception, FieldValueError) + self.assertEqual( + str(context.exception), + f'ARP: invalid {self.NOUNS[name]}: must be a str, bytes or ' + f'bytearray, not {value!r}') + + def test_malformed_str_message(self) -> None: + for name, htype in self.HTYPES.items(): + with self.subTest(htype=name): + with self.assertRaises(ProtocolError) as context: + ARP(htype=htype, sha='zz', tha='zz') + self.assertEqual(str(context.exception), + f"ARP: invalid {self.NOUNS[name]}: 'zz'") def test_unsupported_type_raises_protocol_error(self) -> None: for name, htype in self.HTYPES.items():