fix(ethernet,pcapng): reject a non-string MAC or EUI address (#2222)

pcapkit/protocols/link/ethernet.py

@@ -232,12 +232,16 @@ def _make_mac_addr(self, addr: 'str | bytes | bytearray') -> 'bytes':
             MAC address packed as 6 octets.
 
         Raises:
-            ProtocolError: If ``addr`` is not a ``:``- or ``-``-separated
-                *hex* encoded MAC address.
+            ProtocolError: If ``addr`` is not a :class:`str`, :class:`bytes`
+                or :class:`bytearray` (a :obj:`bool` included), or is not a
+                ``:``- or ``-``-separated *hex* encoded MAC address.
 
         """
+        if not isinstance(addr, (str, bytes, bytearray)):
+            raise ProtocolError(f'{self.alias}: invalid MAC address: must be a str, bytes or '
+                                f'bytearray, not {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.alias}: invalid MAC address: {addr!r}')

pcapkit/protocols/misc/pcapng.py

@@ -2118,15 +2118,19 @@ def _make_mac_addr(self, addr: 'str | bytes | bytearray') -> 'bytes':
             MAC address packed as 6 octets.
 
         Raises:
-            ProtocolError: If ``addr`` is not a ``:``- or ``-``-separated
-                *hex* encoded MAC address.
+            ProtocolError: If ``addr`` is not a :class:`str`, :class:`bytes`
+                or :class:`bytearray` (a :obj:`bool` included), or is not a
+                ``:``- or ``-``-separated *hex* encoded MAC address.
 
         """
+        if not isinstance(addr, (str, bytes, bytearray)):
+            raise ProtocolError(f'PCAP-NG: invalid MAC address: must be a str, bytes or '
+                                f'bytearray, not {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'PCAP-NG: invalid MAC address: {addr!r}')
 
     def _make_eui_addr(self, addr: 'str | bytes | bytearray') -> 'bytes':
         """Make EUI address.
@@ -2138,15 +2142,19 @@ def _make_eui_addr(self, addr: 'str | bytes | bytearray') -> 'bytes':
             EUI address packed as 8 octets.
 
         Raises:
-            ProtocolError: If ``addr`` is not a ``:``- or ``-``-separated
-                *hex* encoded EUI-64 address.
+            ProtocolError: If ``addr`` is not a :class:`str`, :class:`bytes`
+                or :class:`bytearray` (a :obj:`bool` included), or is not a
+                ``:``- or ``-``-separated *hex* encoded EUI-64 address.
 
         """
+        if not isinstance(addr, (str, bytes, bytearray)):
+            raise ProtocolError(f'PCAP-NG: invalid EUI address: must be a str, bytes or '
+                                f'bytearray, not {addr!r}')
         _addr = addr.encode() if isinstance(addr, str) else addr
 
         if PAT_EUI_ADDR.fullmatch(_addr) is not None:
             return bytes.fromhex(_addr.replace(b':', b'').replace(b'-', b'').decode())
-        raise ProtocolError(f'invalid EUI address: {addr!r}')
+        raise ProtocolError(f'PCAP-NG: invalid EUI address: {addr!r}')
 
     def _decode_next_layer(self, dict_: 'Data_PCAPNG', proto: 'Optional[int]' = None,
                            length: 'Optional[int]' = None, *, packet: 'Optional[dict[str, Any]]' = None) -> 'Data_PCAPNG':  # pylint: disable=arguments-differ

tests/protocols/link/test_ethernet_mac_type_unit.py

@@ -0,0 +1,51 @@
+# -*- coding: utf-8 -*-
+"""Ethernet rejects a non-string MAC address with an in-library error.
+
+GitHub issue #2222: :meth:`Ethernet._make_mac_addr
+<pcapkit.protocols.link.ethernet.Ethernet._make_mac_addr>` passed a
+:obj:`bool`, :class:`int`, :obj:`None` or :class:`float` straight to the MAC
+pattern, which raised a bare :class:`TypeError` that neither
+:class:`~pcapkit.utilities.exceptions.ProtocolError` nor :class:`ValueError`
+catches. Anything not a :class:`str`, :class:`bytes` or :class:`bytearray`,
+a :obj:`bool` included, raises
+:class:`~pcapkit.utilities.exceptions.ProtocolError`.
+
+Every case builds its own frame in memory and reads no capture.
+
+"""
+
+import unittest
+
+from pcapkit.protocols.link.ethernet import Ethernet
+from pcapkit.utilities.exceptions import ProtocolError
+
+
+class TestEthernetMacAddressType(unittest.TestCase):
+    """Pin the error raised for each non-string MAC address type."""
+
+    def test_non_string_raises_protocol_error(self) -> None:
+        for field in ('dst', 'src'):
+            for value in (True, False, 5, None, 1.5):
+                with self.subTest(field=field, value=value):
+                    with self.assertRaises(ProtocolError) as context:
+                        Ethernet(**{field: value})
+                    self.assertEqual(
+                        str(context.exception),
+                        f'Ethernet: invalid MAC address: must be a str, bytes or bytearray, '
+                        f'not {value!r}')
+
+    def test_malformed_string_message(self) -> None:
+        with self.assertRaises(ProtocolError) as context:
+            Ethernet(dst='not-a-mac')
+        self.assertEqual(str(context.exception), "Ethernet: invalid MAC address: 'not-a-mac'")
+
+    def test_supported_forms_still_pack(self) -> None:
+        raw = bytes.fromhex('0123456789ab')
+        for value in ('01:23:45:67:89:ab', b'01-23-45-67-89-ab',
+                      bytearray(b'01:23:45:67:89:ab')):
+            with self.subTest(value=value):
+                self.assertEqual(Ethernet(dst=value).data[:6], raw)
+
+
+if __name__ == '__main__':
+    unittest.main()

tests/protocols/misc/test_pcapng_addr_type_2222_unit.py

@@ -0,0 +1,86 @@
+# -*- coding: utf-8 -*-
+"""GitHub issue #2222: PCAP-NG rejects a non-string MAC or EUI address.
+
+:meth:`PCAPNG._make_mac_addr
+<pcapkit.protocols.misc.pcapng.PCAPNG._make_mac_addr>` and
+:meth:`PCAPNG._make_eui_addr
+<pcapkit.protocols.misc.pcapng.PCAPNG._make_eui_addr>` passed a :obj:`bool`,
+:class:`int`, :obj:`None` or :class:`float` straight to the address pattern,
+which raised a bare :class:`TypeError`. Anything not a :class:`str`,
+:class:`bytes` or :class:`bytearray`, a :obj:`bool` included, raises
+:class:`~pcapkit.utilities.exceptions.ProtocolError`.
+Each case reaches the maker through the ``if_MACaddr`` or ``if_EUIaddr``
+option of an Interface Description Block.
+
+:mod:`pcapkit` is imported inside each test, after
+:func:`~tests._support.reimport_once_per_class`.
+
+"""
+
+import unittest
+from typing import TYPE_CHECKING
+
+from tests.protocols.misc.test_pcapng_roundtrip2_unit import PCAPNGTestCase
+
+if TYPE_CHECKING:
+    from typing import Any
+
+
+class TestPCAPNGAddressType(PCAPNGTestCase):
+    """Pin the error raised for each non-string ``if_MACaddr``/``if_EUIaddr`` value."""
+
+    def _idb(self, option: 'str', interface: 'Any') -> 'Any':
+        """Build an Interface Description Block carrying one address option."""
+        from pcapkit.const.pcapng.block_type import BlockType
+        from pcapkit.const.pcapng.option_type import OptionType
+        from pcapkit.const.reg.linktype import LinkType
+        from pcapkit.protocols.misc.pcapng import PCAPNG
+
+        return PCAPNG(num=2, sct=1, ctx=self._context(), type=BlockType.Interface_Description_Block,
+                      block={'linktype': LinkType.ETHERNET, 'snaplen': 65535,
+                             'options': [(OptionType[option], {'interface': interface})]})
+
+    OPTIONS = ('if_MACaddr', 'if_EUIaddr')
+
+    #: Address kind each option's error message names.
+    KINDS = {'if_MACaddr': 'MAC', 'if_EUIaddr': 'EUI'}
+
+    def test_non_string_raises_protocol_error(self) -> None:
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        for option in self.OPTIONS:
+            for value in (True, False, 5, None, 1.5):
+                with self.subTest(option=option, value=value):
+                    with self.assertRaises(ProtocolError) as context:
+                        self._idb(option, value)
+                    self.assertEqual(
+                        str(context.exception),
+                        f'PCAP-NG: invalid {self.KINDS[option]} address: must be a str, bytes '
+                        f'or bytearray, not {value!r}')
+
+    def test_malformed_string_message(self) -> None:
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        for option in self.OPTIONS:
+            with self.subTest(option=option):
+                with self.assertRaises(ProtocolError) as context:
+                    self._idb(option, 'bad')
+                self.assertEqual(str(context.exception),
+                                 f"PCAP-NG: invalid {self.KINDS[option]} address: 'bad'")
+
+    def test_supported_forms_still_pack(self) -> None:
+        from pcapkit.const.pcapng.option_type import OptionType
+
+        cases = {
+            'if_MACaddr': ('01:23:45:67:89:ab', '01-23-45-67-89-ab'),
+            'if_EUIaddr': ('02:34:56:ff:fe:78:9a:bc', '02-34-56-ff-fe-78-9a-bc'),
+        }
+        for option, (text, dashed) in cases.items():
+            for value in (text, dashed.encode(), bytearray(text.encode())):
+                with self.subTest(option=option, value=value):
+                    block = self._idb(option, value)
+                    self.assertEqual(block.info.options[OptionType[option]].interface, text)
+
+
+if __name__ == '__main__':
+    unittest.main()