fix(ipx)!: keep the octets past the IPX Length field as captured (#1433)

pcapkit/protocols/data/internet/ipx.py

@@ -53,4 +53,8 @@ class IPX(Protocol):
     src: 'Address'
 
     if TYPE_CHECKING:
+        #: Octets captured past the Packet Length, present only when there are
+        #: any.
+        trailer: 'bytes'
+
         def __init__(self, chksum: 'bytes', len: 'int', count: 'int', type: 'Packet', dst: 'Address', src: 'Address') -> 'None': ...  # pylint: disable=unused-argument,super-init-not-called,multiple-statements,redefined-builtin,line-too-long

pcapkit/protocols/internet/ipx.py

@@ -109,6 +109,15 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_IPX':
             src=self._read_ipx_address(schema.src),
         )
 
+        # NOTE: Octets captured past the Packet Length belong to this layer,
+        # since its own length field is what leaves them out of the payload: an
+        # Ethernet carrier hands the whole rest of its frame here, padding
+        # included, and the rebuild keeps them as captured (:issue:`1433`).
+        if schema.trailer:
+            ipx.__update__([
+                ('trailer', schema.trailer),
+            ])
+
         return self._decode_next_layer(ipx, ipx.type, ipx.len - 30)
 
     def make(self,
@@ -122,6 +131,7 @@ def make(self,
              dst: 'bytes' = b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
              src: 'bytes' = b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
              payload: 'bytes | ProtocolBase | Schema' = b'',
+             trailer: 'bytes' = b'',
              **kwargs: 'Any') -> 'Schema_IPX':
         """Make (construct) packet data.
 
@@ -137,6 +147,8 @@ def make(self,
             dst: Destination Address.
             src: Source Address.
             payload: Payload data.
+            trailer: Octets after the packet, outside the Packet Length, such as
+                link-layer padding; written as is.
             **kwargs: Arbitrary keyword arguments.
 
         Returns:
@@ -156,6 +168,7 @@ def make(self,
             dst=dst,
             src=src,
             payload=payload,
+            trailer=trailer,
         )
 
     ##########################################################################
@@ -201,6 +214,8 @@ def _make_data(cls, data: 'Data_IPX') -> 'dict[str, Any]':  # type: ignore[overr
             'dst': cls._make_ipx_address(data.dst),
             'src': cls._make_ipx_address(data.src),
             'payload': cls._make_payload(data),
+            # NOTE: And the octets captured past the Packet Length (:issue:`1433`).
+            'trailer': data.get('trailer', b''),
         }
 
     @staticmethod

pcapkit/protocols/schema/internet/ipx.py

@@ -34,7 +34,13 @@ class IPX(Schema):
     src: 'bytes' = BytesField(length=12)
     #: Payload.
     payload: 'bytes' = PayloadField(length=lambda pkt: pkt['len'] - 30)
+    #: Octets captured past the Packet Length, such as Ethernet minimum-frame
+    #: padding. A :class:`~pcapkit.corekit.fields.misc.PayloadField` with no
+    #: length takes whatever the payload leaves on unpack and writes its value
+    #: verbatim on pack, so the octets survive whatever they hold.
+    trailer: 'bytes' = PayloadField(default=b'')
 
     if TYPE_CHECKING:
         def __init__(self, chksum: 'bytes', len: 'int', count: 'int', type: 'Enum_Packet',
-                     dst: 'bytes', src: 'bytes', payload: 'bytes | ProtocolBase | Schema') -> 'None': ...
+                     dst: 'bytes', src: 'bytes', payload: 'bytes | ProtocolBase | Schema',
+                     trailer: 'bytes' = b'') -> 'None': ...

tests/protocols/internet/test_ipx_trailer_kept_unit.py

@@ -0,0 +1,99 @@
+# -*- coding: utf-8 -*-
+"""Octets captured past an IPX packet's own length survive a rebuild.
+
+GitHub issue #1433: :class:`~pcapkit.protocols.link.ethernet.Ethernet` hands
+the whole rest of the frame to the next layer, but
+:class:`~pcapkit.protocols.internet.ipx.IPX` reads only its Packet Length. The
+octets after that -- most often the padding of a 60-octet minimum Ethernet
+frame -- were kept by no ``info``, so
+:meth:`~pcapkit.protocols.protocol.ProtocolBase.from_data` dropped them: the
+frame below rebuilt as 44 octets of its 60.
+
+The IPX layer owns them as ``trailer``, since its own length field is what
+leaves them out of the payload, and so the IPX packet rebuilds byte for byte
+on its own as well as inside a frame.
+
+Every case builds its own octets in memory and reads no capture. Classes are
+imported inside each test, after :func:`~tests._support.reimport_once_per_class`,
+c.f. ``tests/protocols/link/test_ethernet_mac_roundtrip_unit.py``.
+
+"""
+
+import unittest
+
+from tests._support import reimport_once_per_class
+
+#: Destination and source MAC addresses.
+MACS = bytes.fromhex('001122334455' '66778899aabb')
+#: Destination and source IPX addresses.
+ADDRS = bytes(range(1, 25))
+
+
+def _ipx(length: int) -> bytes:
+    """Return a 30-octet IPX header whose Packet Length is ``length``."""
+    return bytes(2) + length.to_bytes(2, 'big') + b'\x00\x00' + ADDRS
+
+
+#: Ethernet frames whose IPX packet is shorter than what the frame carries.
+FRAMES = {
+    'zero-padding': MACS + b'\x81\x37' + _ipx(30) + bytes(16),
+    'junk-padding': MACS + b'\x81\x37' + _ipx(30) + b'\xde\xad' * 8,
+}
+
+#: Bare IPX packets that end at or before their own length, and their payload.
+NO_TRAILER = {
+    'exact': (_ipx(34) + b'abcd', b'abcd'),
+    'truncated': (_ipx(40) + b'abcd', b'abcd'),
+}
+
+
+class TestIPXTrailerKept(unittest.TestCase):
+    """Pin the octets past an IPX packet's own length through ``from_data``."""
+
+    def setUp(self) -> None:
+        reimport_once_per_class(self)
+
+    def test_frame_rebuilds_byte_for_byte(self) -> None:
+        from pcapkit.protocols.link.ethernet import Ethernet
+
+        for name, frame in FRAMES.items():
+            with self.subTest(frame=name):
+                parsed = Ethernet(frame, len(frame))
+                self.assertEqual(len(parsed.data), 60)
+                self.assertEqual(parsed.info.ipx.len, 30)
+                self.assertEqual(parsed.info.ipx.trailer, frame[44:])
+                self.assertNotIn('trailer', parsed.info)
+                self.assertEqual(Ethernet.from_data(parsed.info).data, frame)
+
+    def test_packet_rebuilds_byte_for_byte(self) -> None:
+        from pcapkit.protocols.internet.ipx import IPX
+
+        packet = _ipx(34) + b'abcd' + b'\xee' * 5
+        parsed = IPX(packet, len(packet))
+        self.assertEqual(parsed.info.trailer, b'\xee' * 5)
+        self.assertEqual(bytes(parsed.__header__.payload), b'abcd')
+        self.assertEqual(IPX.from_data(parsed.info).data, packet)
+
+    def test_no_trailer_key_without_excess(self) -> None:
+        from pcapkit.protocols.internet.ipx import IPX
+
+        for name, (packet, payload) in NO_TRAILER.items():
+            with self.subTest(packet=name):
+                parsed = IPX(packet, len(packet))
+                self.assertNotIn('trailer', parsed.info)
+                self.assertEqual(bytes(parsed.__header__.payload), payload)
+                self.assertEqual(IPX.from_data(parsed.info).data, packet)
+
+    def test_make_writes_trailer_outside_the_length(self) -> None:
+        from pcapkit.protocols.internet.ipx import IPX
+
+        made = IPX(payload=b'abcd', trailer=b'\x00\x01')
+        self.assertEqual(made.info.len, 34)
+        self.assertEqual(len(made.data), 36)
+        self.assertEqual(made.data[-6:], b'abcd\x00\x01')
+        self.assertEqual(made.info.trailer, b'\x00\x01')
+        self.assertEqual(IPX(made.data, len(made.data)).info.trailer, b'\x00\x01')
+
+
+if __name__ == '__main__':
+    unittest.main()