From 3075bd3d23ea626efdfa9335af485f1cb190b8d3 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Thu, 8 Oct 2026 15:11:00 -0400 Subject: [PATCH] fix(ipx)!: keep the octets past the IPX Length field as captured (#1433) Ethernet hands IPX the whole rest of its frame, but IPX reads only its Packet Length, so the octets past it (most often minimum-frame padding) were in no info and from_data dropped them: a padded 60-octet frame rebuilt as 44. - schema: add a length-less trailer PayloadField after the payload - read(): record it as info.trailer, only when non-empty - make(trailer=): write it after the packet, outside the Packet Length - _make_data(): round-trip it BREAKING: IPX info gains a trailer key on padded frames. --- pcapkit/protocols/data/internet/ipx.py | 4 + pcapkit/protocols/internet/ipx.py | 15 +++ pcapkit/protocols/schema/internet/ipx.py | 8 +- .../internet/test_ipx_trailer_kept_unit.py | 99 +++++++++++++++++++ 4 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 tests/protocols/internet/test_ipx_trailer_kept_unit.py diff --git a/pcapkit/protocols/data/internet/ipx.py b/pcapkit/protocols/data/internet/ipx.py index 62c2de7bc7..4716a0b843 100644 --- a/pcapkit/protocols/data/internet/ipx.py +++ b/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 diff --git a/pcapkit/protocols/internet/ipx.py b/pcapkit/protocols/internet/ipx.py index 176a9e642b..9c3b7e94f5 100644 --- a/pcapkit/protocols/internet/ipx.py +++ b/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 diff --git a/pcapkit/protocols/schema/internet/ipx.py b/pcapkit/protocols/schema/internet/ipx.py index 79fa2017e7..16b27f4f71 100644 --- a/pcapkit/protocols/schema/internet/ipx.py +++ b/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': ... diff --git a/tests/protocols/internet/test_ipx_trailer_kept_unit.py b/tests/protocols/internet/test_ipx_trailer_kept_unit.py new file mode 100644 index 0000000000..22232c2237 --- /dev/null +++ b/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()