From 540c8887fa30a387623df1c9844588239725d690 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 9 Oct 2026 00:04:23 -0400 Subject: [PATCH] fix(schema): a header cut inside a fixed-width field rebuilds as captured (#1458, #1451) When the data ended inside a field, Schema.unpack read the missing octets as zeros and recorded nothing, so from_data(info) wrote that field, and every field after it, at full width: Ethernet(bytes(12) + b'\x08') rebuilt as 14 octets, and a Routing header with 23 of 24 octets as 24. - Schema.unpack records the first field it reads short, and how many octets it got, as __short_read__; Schema.pack writes back only those, and Schema.to_dict/from_dict carry the record like __remainder__. - keep_short_read/replay_short_read carry the record through info: the Link, Internet, Transport and Application bases add it after a parse and cut a from_data rebuild back to it. It is an ordinary info key, so Info.to_dict carries it and from_data(info.to_dict()) replays it too; only a truncated header has it. - The #1454 edge tables drop the #1451/#1458 rows this closes, and the harness its two now-unused defect strings; five SCTP last-chunk pad rows remain, under #1474. Decoded values are unchanged: a truncated number still reads as zero-filled. --- pcapkit/protocols/application/application.py | 23 +- pcapkit/protocols/data/data.py | 6 + pcapkit/protocols/internet/internet.py | 24 +- pcapkit/protocols/link/link.py | 43 +++- pcapkit/protocols/schema/schema.py | 123 +++++++++- pcapkit/protocols/transport/transport.py | 43 +++- tests/protocols/_edge_roundtrip.py | 15 +- .../test_internet_edge_roundtrip_unit.py | 65 +---- .../link/test_link_edge_roundtrip_unit.py | 21 +- .../test_short_read_roundtrip_unit.py | 231 ++++++++++++++++++ .../test_transport_edge_roundtrip_unit.py | 20 +- 11 files changed, 498 insertions(+), 116 deletions(-) create mode 100644 tests/protocols/test_short_read_roundtrip_unit.py diff --git a/pcapkit/protocols/application/application.py b/pcapkit/protocols/application/application.py index b6fa721f6..e767098cd 100644 --- a/pcapkit/protocols/application/application.py +++ b/pcapkit/protocols/application/application.py @@ -16,12 +16,13 @@ from pcapkit.corekit.protochain import ProtoChain from pcapkit.protocols.misc.null import NoPayload from pcapkit.protocols.protocol import _PT, _ST, ProtocolBase +from pcapkit.protocols.schema.schema import keep_short_read, replay_short_read from pcapkit.utilities.exceptions import IntError, UnsupportedCall if TYPE_CHECKING: from typing import IO, Any, NoReturn, Optional - from typing_extensions import Literal + from typing_extensions import Literal, Self __all__ = ['Application'] @@ -82,6 +83,10 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, # call super post-init super().__post_init__(file, length, **kwargs) # type: ignore[arg-type] + # NOTE: A header the data ends inside keeps that in its ``info``, so + # that :meth:`from_data` rebuilds only what was captured (:issue:`1458`). + keep_short_read(self) + # ``read`` may have dispatched the undissected remainder through # ``_decode_next_layer``, which already set the payload and the chain # (basis included); only a protocol that did not gets the empty default @@ -101,6 +106,22 @@ def __index__(cls) -> 'NoReturn': # pylint: disable=invalid-index-returned """ raise IntError(f'{cls.__name__!r} object cannot be interpreted as an integer') + @classmethod + def from_data(cls, data: '_PT | dict[str, Any]', **kwargs: 'Any') -> 'Self': + """Create protocol instance from data. + + Args: + data: Protocol data. + **kwargs: Construction keywords, as for :meth:`ProtocolBase.from_data + `. + + Returns: + Protocol instance, cut back to what was captured when the header + it was parsed from was cut short (:issue:`1458`). + + """ + return replay_short_read(super().from_data(data, **kwargs), data) + ########################################################################## # Utilities. ########################################################################## diff --git a/pcapkit/protocols/data/data.py b/pcapkit/protocols/data/data.py index 7f142dc8c..569c29dbf 100644 --- a/pcapkit/protocols/data/data.py +++ b/pcapkit/protocols/data/data.py @@ -23,3 +23,9 @@ class Data(Info): __next_name__: 'str' #: Next field type, i.e., the type of the payload field. __next_type__: 'Type[ProtocolBase]' + #: Name of the header field the data ended inside, and how many of its + #: octets were there; present only then. Not excluded, so that + #: :meth:`to_dict` carries it and ``from_data(info.to_dict())`` cuts + #: the rebuild back as ``from_data(info)`` does. See + #: :func:`~pcapkit.protocols.schema.schema.keep_short_read`. + __short_read__: 'tuple[str, int]' diff --git a/pcapkit/protocols/internet/internet.py b/pcapkit/protocols/internet/internet.py index d1b2ed3c6..7dd4da035 100644 --- a/pcapkit/protocols/internet/internet.py +++ b/pcapkit/protocols/internet/internet.py @@ -19,6 +19,7 @@ from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.protochain import ProtoChain from pcapkit.protocols.protocol import _PT, _ST, ProtocolBase +from pcapkit.protocols.schema.schema import keep_short_read, replay_short_read from pcapkit.utilities.decorators import beholder from pcapkit.utilities.exceptions import RegistryError from pcapkit.utilities.warnings import RegistryWarning, warn @@ -26,7 +27,7 @@ if TYPE_CHECKING: from typing import IO, Any, Optional, Type - from typing_extensions import Literal + from typing_extensions import Literal, Self __all__ = ['Internet'] @@ -174,6 +175,22 @@ def register(cls, code: 'Enum_TransType', protocol: 'ModuleDescriptor[ProtocolBa f'{incumbent!r} with {protocol!r}', RegistryWarning) cls.__proto__[code] = protocol + @classmethod + def from_data(cls, data: '_PT | dict[str, Any]', **kwargs: 'Any') -> 'Self': + """Create protocol instance from data. + + Args: + data: Protocol data. + **kwargs: Construction keywords, as for :meth:`ProtocolBase.from_data + `. + + Returns: + Protocol instance, cut back to what was captured when the header + it was parsed from was cut short (:issue:`1458`). + + """ + return replay_short_read(super().from_data(data, **kwargs), data) + ########################################################################## # Data models. ########################################################################## @@ -196,8 +213,13 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, So :attr:`data` is cut down to the header's own :attr:`length`, and ``from_data(info).data`` rebuilds it exactly (:issue:`1446`). + A header the data ends inside keeps that in its :attr:`info`, so + that :meth:`from_data` rebuilds only what was captured + (:issue:`1458`). + """ super().__post_init__(file, length, extension=extension, **kwargs) # type: ignore[arg-type] + keep_short_read(self) if extension and file is not None and len(self._data) > self.length: self._data = self._data[:self.length] diff --git a/pcapkit/protocols/link/link.py b/pcapkit/protocols/link/link.py index 081238c27..805e35e67 100644 --- a/pcapkit/protocols/link/link.py +++ b/pcapkit/protocols/link/link.py @@ -21,13 +21,14 @@ from pcapkit.const.reg.ethertype import EtherType as Enum_EtherType from pcapkit.corekit.module import ModuleDescriptor from pcapkit.protocols.protocol import _PT, _ST, ProtocolBase +from pcapkit.protocols.schema.schema import keep_short_read, replay_short_read from pcapkit.utilities.exceptions import RegistryError from pcapkit.utilities.warnings import RegistryWarning, warn if TYPE_CHECKING: - from typing import DefaultDict, Type + from typing import IO, Any, DefaultDict, Optional, Type - from typing_extensions import Literal + from typing_extensions import Literal, Self __all__ = ['Link'] @@ -154,6 +155,44 @@ def register(cls, code: 'Enum_EtherType', protocol: 'ModuleDescriptor[ProtocolBa f'{incumbent!r} with {protocol!r}', RegistryWarning) cls.__proto__[code] = protocol + @classmethod + def from_data(cls, data: '_PT | dict[str, Any]', **kwargs: 'Any') -> 'Self': + """Create protocol instance from data. + + Args: + data: Protocol data. + **kwargs: Construction keywords, as for :meth:`ProtocolBase.from_data + `. + + Returns: + Protocol instance, cut back to what was captured when the header + it was parsed from was cut short (:issue:`1458`). + + """ + return replay_short_read(super().from_data(data, **kwargs), data) + + ########################################################################## + # Data models. + ########################################################################## + + def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, + length: 'Optional[int]' = None, **kwargs: 'Any') -> 'None': + """Post initialisation hook. + + Args: + file: Source packet stream. + length: Length of packet data. + **kwargs: Arbitrary keyword arguments. + + Notes: + A header the data ends inside keeps that in its :attr:`info`, so + that :meth:`from_data` rebuilds only what was captured + (:issue:`1458`). + + """ + super().__post_init__(file, length, **kwargs) # type: ignore[arg-type] + keep_short_read(self) + ########################################################################## # Utilities. ########################################################################## diff --git a/pcapkit/protocols/schema/schema.py b/pcapkit/protocols/schema/schema.py index e270a42b6..e69a95fad 100644 --- a/pcapkit/protocols/schema/schema.py +++ b/pcapkit/protocols/schema/schema.py @@ -26,6 +26,10 @@ from typing_extensions import Self + from pcapkit.protocols.protocol import ProtocolBase + + _PB = TypeVar('_PB', bound=ProtocolBase) + __all__ = ['Schema', 'EnumSchema', 'schema_final'] _VT = TypeVar('_VT') @@ -99,7 +103,8 @@ class marked with a user's own ``from typing import final`` on 3.10 is temp = ['__map__', '__map_reverse__', '__builtin__', '__fields__', '__buffer__', '__updated__', - '__payload__', '__finalised__', '__remainder__', '__remainder_offset__'] + '__payload__', '__finalised__', '__remainder__', '__remainder_offset__', + '__short_read__'] temp.extend(cls.__additional__) for obj in cls.mro(): temp.extend(el for el in dir(obj) if el not in cls.__fields__) @@ -562,7 +567,8 @@ def __update__(self, dict_: 'Optional[Mapping[str, _VT] | Iterable[tuple[str, _V # NOTE: Not a field, but what :meth:`to_dict` emits for the octets an # enclosing ``SchemaField`` span held beyond this schema, and where, # so that a rebuilt schema still re-emits them (:issue:`1380`). - if key in ('__remainder__', '__remainder_offset__'): + # NOTE: Nor is the short read :meth:`unpack` records (:issue:`1458`). + if key in ('__remainder__', '__remainder_offset__', '__short_read__'): self.__dict__[key] = value continue @@ -712,6 +718,10 @@ def to_dict(self) -> 'dict[str, _VT]': that the rebuilt field still packs them (:issue:`1380`). The keys are absent otherwise. + A schema whose data ended inside one of its fields carries the + ``__short_read__`` that :meth:`unpack` recorded, and + :meth:`from_dict` accepts it back, likewise. + """ dict_ = {} # type: dict[str, Any] for (key, value) in self.__dict__.items(): @@ -724,7 +734,7 @@ def to_dict(self) -> 'dict[str, _VT]': else: dict_[out_key] = value - for key in ('__remainder__', '__remainder_offset__'): + for key in ('__remainder__', '__remainder_offset__', '__short_read__'): if key in self.__dict__: dict_[key] = self.__dict__[key] return dict_ @@ -919,9 +929,47 @@ class will consider negative value as a placeholder. self.__buffer__[field.name] = temp self.post_process(packet) + self._cut_short_read() self.__updated__ = False return self.__bytes__() + def _note_short_read(self, name: 'str', length: 'int', byte: 'bytes') -> 'None': + """Record the first field the data ends inside, for :meth:`_cut_short_read`. + + Args: + name: Field name. + length: Octets the field declares. + byte: Octets actually read for it. + + """ + if len(byte) < length and '__short_read__' not in self.__dict__: + self.__dict__['__short_read__'] = (name, len(byte)) + + def _cut_short_read(self) -> 'None': + """Cut the packed buffers back to the octets a short read consumed. + + A schema whose data ended inside one of its fields records, as + ``__short_read__``, that field's name and how many of its octets were + there (see :meth:`unpack`). Its value is read as though the missing + octets were zeros, so packing it writes the field at full width and + every field after it besides, none of which was captured. Only the + octets read are kept here, so a truncated header packs back to exactly + what was captured (:issue:`1458`). + + """ + record = self.__dict__.get('__short_read__') + if record is None: + return + + name, count = record + cut = False + for key in self.__fields__: + if cut: + self.__buffer__[key] = b'' + elif key == name: + self.__buffer__[key] = self.__buffer__[key][:count] + cut = True + def pre_pack(self, packet: 'dict[str, Any]') -> 'None': """Prepare ``packet`` data for packing process. @@ -1017,6 +1065,7 @@ def unpack(cls, data: 'bytes | IO[bytes]', byte = data.read(length) self.__buffer__[field.name] = byte + self._note_short_read(field.name, length, byte) packet[field.name] = byte packet['__length__'] -= length @@ -1036,6 +1085,8 @@ def unpack(cls, data: 'bytes | IO[bytes]', byte = data.read(length) self.__buffer__[field.name] = byte + if not isinstance(field, ForwardMatchField): + self._note_short_read(field.name, length, byte) value = field.unpack(byte, packet.copy()) if isinstance(field, SchemaField) and isinstance(value, Schema): @@ -1380,3 +1431,69 @@ def register(cls, code: '_ET', schema: 'Type[Self]') -> 'None': f'{incumbent!r} with {schema!r}', RegistryWarning) cls.__enum__[code] = schema # type: ignore[index] + + +def keep_short_read(protocol: 'ProtocolBase') -> 'None': + """Carry a parsed header's short read into the protocol's info. + + Args: + protocol: Protocol instance, just parsed. + + Called by the layer base classes once a parse is done. When the data ended + inside one of the header's fields, ``info`` gains the ``__short_read__`` + record :meth:`Schema.unpack` made, so that :func:`replay_short_read` can + cut the rebuild back to what was captured (:issue:`1458`). The key is an + ordinary one, so :meth:`Info.to_dict ` + carries it and ``from_data(info.to_dict())`` replays it too; it is absent + for a header that was captured whole. + + """ + header = cast('Optional[Schema]', getattr(protocol, '__header__', None)) + if header is None: + return + record = header.__dict__.get('__short_read__') + if record is not None: + protocol._info.__update__({'__short_read__': record}) # pylint: disable=protected-access + + +def replay_short_read(protocol: '_PB', data: 'Any') -> '_PB': + """Cut a protocol rebuilt by ``from_data`` back to the octets its header had. + + Args: + protocol: Protocol instance, just built by ``from_data``. + data: The info it was built from. + + Returns: + ``protocol`` itself. + + The rebuild packs every field at full width, the missing octets as the + zeros they were read as. The ``__short_read__`` record that + :func:`keep_short_read` left in ``data`` names the field the capture ended + inside and how much of it there was, so everything from that point on is + dropped (:issue:`1458`). A header cut short has no payload past it. + + """ + record = data.get('__short_read__') if isinstance(data, collections.abc.Mapping) else None + header = cast('Optional[Schema]', getattr(protocol, '__header__', None)) + if record is None or header is None: + return protocol + + name, count = record + offset = 0 + for key in header.__fields__: + if key == name: + offset += min(count, len(header.__buffer__[key])) + break + offset += len(header.__buffer__[key]) + else: + return protocol + + header.__dict__['__short_read__'] = record + header._cut_short_read() # pylint: disable=protected-access + + # pylint: disable=protected-access + protocol._data = protocol._data[:offset] + protocol._file = io.BytesIO(protocol._data) + protocol.__cached__.pop('__len__', None) + protocol._info.__update__({'__short_read__': record}) + return protocol diff --git a/pcapkit/protocols/transport/transport.py b/pcapkit/protocols/transport/transport.py index b18d99a8c..299bb4d76 100644 --- a/pcapkit/protocols/transport/transport.py +++ b/pcapkit/protocols/transport/transport.py @@ -18,14 +18,15 @@ from pcapkit.const.reg.apptype import AppType as Enum_AppType from pcapkit.corekit.module import ModuleDescriptor from pcapkit.protocols.protocol import _PT, _ST, ProtocolBase +from pcapkit.protocols.schema.schema import keep_short_read, replay_short_read from pcapkit.utilities.exceptions import RegistryError, StructError, UnsupportedCall, stacklevel from pcapkit.utilities.logging import DEVMODE, get_logger from pcapkit.utilities.warnings import RegistryWarning, warn if TYPE_CHECKING: - from typing import Any, DefaultDict, Optional, Type + from typing import IO, Any, DefaultDict, Optional, Type - from typing_extensions import Literal + from typing_extensions import Literal, Self from pcapkit.const.reg.apptype import TransportProtocol as Enum_TransportProtocol @@ -161,6 +162,44 @@ def analyze(cls, ports: 'tuple[int, int]', payload: 'bytes', **kwargs: 'Any') -> report = protocol(payload_io, len(payload), **kwargs) # type: ignore[abstract] return report + @classmethod + def from_data(cls, data: '_PT | dict[str, Any]', **kwargs: 'Any') -> 'Self': + """Create protocol instance from data. + + Args: + data: Protocol data. + **kwargs: Construction keywords, as for :meth:`ProtocolBase.from_data + `. + + Returns: + Protocol instance, cut back to what was captured when the header + it was parsed from was cut short (:issue:`1458`). + + """ + return replay_short_read(super().from_data(data, **kwargs), data) + + ########################################################################## + # Data models. + ########################################################################## + + def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, + length: 'Optional[int]' = None, **kwargs: 'Any') -> 'None': + """Post initialisation hook. + + Args: + file: Source packet stream. + length: Length of packet data. + **kwargs: Arbitrary keyword arguments. + + Notes: + A header the data ends inside keeps that in its :attr:`info`, so + that :meth:`from_data` rebuilds only what was captured + (:issue:`1458`). + + """ + super().__post_init__(file, length, **kwargs) # type: ignore[arg-type] + keep_short_read(self) + ########################################################################## # Utilities. ########################################################################## diff --git a/tests/protocols/_edge_roundtrip.py b/tests/protocols/_edge_roundtrip.py index 70d279284..4ce9203f4 100644 --- a/tests/protocols/_edge_roundtrip.py +++ b/tests/protocols/_edge_roundtrip.py @@ -51,23 +51,10 @@ #: Statuses a check can report. ``REJECTED`` is an in-library exception raised #: while parsing; it is a pass for a cut and for a ``REJECTED`` entry only. #: ``PADDED`` is a ``MISMATCH`` whose rebuild is the input followed by zero -#: octets, the signature of a short read zero-filled on parse. +#: octets, the signature of octets the parse never read being written back. STATUSES = ('OK', 'REJECTED', 'PARSE', 'SELF', 'REBUILD', 'MISMATCH', 'PADDED', 'MAKE', 'REPARSE', 'REMAKE', 'TIMEOUT') -#: The defect behind every ``PADDED`` cut, shared by all the family modules. -ZERO_FILLED_SHORT_READ = ( - 'A header cut short inside a fixed-width field parses, because ' - '``FieldBase.unpack`` zero-fills a short read (pcapkit/corekit/fields/field.py:586), ' - 'and nothing records the shortfall, so ``from_data`` rebuilds the field at full width.' -) - -#: The same read, where it is an IPv6 extension header rebuilding itself. -ZERO_FILLED_EXTENSION_HEADER = ( - 'A truncated IPv6 extension header rebuilds from its own ``info`` padded to its ' - 'declared length: the IPv6 extension-header instance of #1458.' -) - class Case(NamedTuple): """One packet to parse and rebuild.""" diff --git a/tests/protocols/internet/test_internet_edge_roundtrip_unit.py b/tests/protocols/internet/test_internet_edge_roundtrip_unit.py index 27d53ad55..59ef86366 100644 --- a/tests/protocols/internet/test_internet_edge_roundtrip_unit.py +++ b/tests/protocols/internet/test_internet_edge_roundtrip_unit.py @@ -18,7 +18,7 @@ import unittest from tests.protocols import _edge_roundtrip as edge -from tests.protocols._edge_roundtrip import Case, Gap, MakeCase, Reject +from tests.protocols._edge_roundtrip import Case, MakeCase, Reject IPV4 = 'pcapkit.protocols.internet.ipv4:IPv4' IPX = 'pcapkit.protocols.internet.ipx:IPX' @@ -250,68 +250,7 @@ def hip_param(code: int, body: bytes) -> bytes: 'ipv4/ihl-4': Reject('ProtocolError', 'resolved to a negative length'), } -KNOWN_FAILURES = ( - Gap(1451, edge.ZERO_FILLED_EXTENSION_HEADER, 'PADDED', '', - ('ipv6-route/unassigned-type/cut*', 'ipv6-route/unassigned-type-24-octets/cut*', - 'ipv6-route/type0-no-address/cut*', 'ipv6-route/type0-reserved-set/cut*', - 'ipv6-route/type2-reserved-set/cut*', 'ipv6-route/rpl-cmpr-0-15-pad7/cut*', - 'ipv6-route/rpl-cmpr-12-13-pad1/cut*', 'ipv6-route/unassigned-type/extension/cut*', - 'ipv6-route/unassigned-type-24-octets/extension/cut*', - 'ipv6-route/type0-no-address/extension/cut*', - 'ipv6-route/type0-reserved-set/extension/cut*', - 'ipv6-route/type2-reserved-set/extension/cut*', - 'ipv6-route/rpl-cmpr-0-15-pad7/extension/cut*', - 'ipv6-route/rpl-cmpr-12-13-pad1/extension/cut*', 'ipv6-frag/reserved-octet-set/cut*', - 'ipv6-frag/every-bit-set/cut*', 'ipv6-frag/atomic/cut*', - 'ipv6-frag/reserved-octet-set/extension/cut*', 'ipv6-frag/every-bit-set/extension/cut*', - 'ipv6-frag/atomic/extension/cut*', 'ah/icv-empty/cut*', 'ah/reserved-set/cut*', - 'ah/icv-12/cut*', 'ah/max-length-1028/cut*', 'ah/icv-empty/extension/cut*', - 'ah/reserved-set/extension/cut*', 'ah/icv-12/extension/cut*', - 'ah/max-length-1028/extension/cut*', 'mh/hoti-reserved-set/cut*', - 'mh/be-unassigned-status/cut*', 'mh/unassigned-type/cut*', - 'mh/unassigned-type-16-octets/cut*', 'mh/unassigned-type-max-length/cut*', - 'mh/hoti-reserved-set/extension/cut*', 'mh/be-unassigned-status/extension/cut*', - 'mh/unassigned-type/extension/cut*', 'mh/unassigned-type-16-octets/extension/cut*', - 'mh/unassigned-type-max-length/extension/cut*', 'esp/spi-seq-only/cut*', - 'hip/no-parameters/cut*', 'hip/packet-type-0-reserved/cut*', - 'hip/packet-type-127-unassigned/cut*', 'hip/version-1/cut*', 'hip/version-15/cut*', - 'hip/every-control-bit/cut*', 'hip/next-tcp-no-payload/cut*', - 'hip/no-parameters/not-extension/cut*', - 'hip/fixed-bit-clear/cut*', 'hip/leading-bit-set/cut*')), - Gap(1458, edge.ZERO_FILLED_SHORT_READ, 'PADDED', '', - ('ipv6-route/rpl-cmpr-0-15-pad7/in-ipv6/cut55', 'ipv6-route/rpl-cmpr-12-13-pad1/in-ipv6/cut55', - 'ipx/min-header/cut*', 'ipx/transport-control-255/cut*', - 'ipx/unassigned-packet-type/cut16', 'ipx/length-short-of-the-data/cut15', - 'ipx/checksum-not-ffff/cut*', 'ipx/length-past-the-data/cut16', - 'ipv6/no-next-header-empty/cut*', 'ipv6/no-next-header-with-octets/cut22', - 'ipv6/class-and-label-max/cut*', 'ipv6/hop-limit-0/cut*', 'ipv6/next-200-unassigned/cut21', - 'ipv6/payload-length-short-of-the-data/cut21', 'ipv6/payload-length-0-with-octets/cut21', - 'ipv6/payload-length-past-the-data/cut21', 'ipv6/hopopt-dstopts-route-frag/cut36', - 'ipv6/ah-then-frag/cut30', 'ipv6/hopopt-then-trailer/cut25', 'ipv6/esp/cut28', - 'ipv6-route/unassigned-type/in-ipv6/cut24', - 'ipv6-route/unassigned-type-24-octets/in-ipv6/cut*', - 'ipv6-route/type0-no-address/in-ipv6/cut24', 'ipv6-route/type0-reserved-set/in-ipv6/cut*', - 'ipv6-route/type2-reserved-set/in-ipv6/cut*', 'ipv6-route/rpl-uncompressed/in-ipv6/cut32', - 'ipv6-route/rpl-cmpr-15-15-x8/in-ipv6/cut28', 'ipv6-route/rpl-cmpr-8-8-x2/in-ipv6/cut32', - 'ipv6-route/rpl-cmpr-15-0-x1/in-ipv6/cut32', 'ipv6-route/rpl-cmpr-0-15-pad7/in-ipv6/cut28', - 'ipv6-route/rpl-cmpr-12-13-pad1/in-ipv6/cut28', - 'ipv6-frag/reserved-octet-set/in-ipv6/cut24', 'ipv6-frag/every-bit-set/in-ipv6/cut24', - 'ipv6-frag/atomic/in-ipv6/cut24', 'ah/icv-empty/in-ipv6/cut26', - 'ah/reserved-set/in-ipv6/cut26', 'ah/icv-12/in-ipv6/cut*', - 'ah/max-length-1028/in-ipv6/cut*', 'mh/brr-reserved-set/in-ipv6/cut28', - 'mh/hoti-reserved-set/in-ipv6/cut*', 'mh/bu-every-flag/in-ipv6/cut28', - 'mh/bu-unassigned-option/in-ipv6/cut28', 'mh/bu-zero-length-option/in-ipv6/cut28', - 'mh/bu-padn-nonzero-contents/in-ipv6/cut28', 'mh/bu-pad1-x4/in-ipv6/cut28', - 'mh/ba-unassigned-status/in-ipv6/cut28', 'mh/be-unassigned-status/in-ipv6/cut*', - 'mh/unassigned-type/in-ipv6/cut24', 'mh/unassigned-type-16-octets/in-ipv6/cut*', - 'mh/unassigned-type-max-length/in-ipv6/cut*', 'hopopt/pad1-x6/in-ipv6/cut24', - 'hopopt/padn-nonzero-contents/in-ipv6/cut24', 'hopopt/padn-0-then-pad1s/in-ipv6/cut24', - 'hopopt/unassigned-skip/in-ipv6/cut24', 'hopopt/unassigned-discard/in-ipv6/cut24', - 'hopopt/unassigned-zero-length/in-ipv6/cut24', - 'hopopt/router-alert-unassigned-value/in-ipv6/cut24', - 'hopopt/jumbo-payload-0/in-ipv6/cut24', 'ipv6-opts/pad1-x6/in-ipv6/cut24', - 'ipv6-opts/padn-nonzero-contents/in-ipv6/cut24', 'ipv6-opts/unassigned/in-ipv6/cut24')), -) +KNOWN_FAILURES = () # type: tuple[edge.Gap, ...] @unittest.skipUnless(edge.HAS_RUNTIME, 'runtime dependencies not installed') diff --git a/tests/protocols/link/test_link_edge_roundtrip_unit.py b/tests/protocols/link/test_link_edge_roundtrip_unit.py index 9dd8fcc7f..2d06ae9b8 100644 --- a/tests/protocols/link/test_link_edge_roundtrip_unit.py +++ b/tests/protocols/link/test_link_edge_roundtrip_unit.py @@ -16,7 +16,7 @@ import unittest from tests.protocols import _edge_roundtrip as edge -from tests.protocols._edge_roundtrip import Case, Gap, MakeCase, Reject +from tests.protocols._edge_roundtrip import Case, MakeCase, Reject ETHERNET = 'pcapkit.protocols.link.ethernet:Ethernet' C_TAG = 'pcapkit.protocols.link.c_tag:C_Tag' @@ -119,24 +119,7 @@ def ospf(type_: int = 1, plen: 'int | None' = None, auth: int = 0, 'l2tpv2/version-3': Reject('ProtocolError', 'L2TPv2: invalid version: 3'), } -KNOWN_FAILURES = ( - Gap(1458, edge.ZERO_FILLED_SHORT_READ, 'PADDED', '', - ('ethernet/header-only-*/cut*', 'ethernet/length-field-0/cut7', - 'ethernet/length-field-1500/cut9', 'ethernet/unassigned-type-ffff/cut8', - 'c-tag/pcp7-dei-vid4095/cut3', 'c-tag/vid0-no-payload/cut*', - 'c-tag/header-only-ipv4-type/cut*', 'arp/hlen0-plen0/cut*', 'arp/reply/cut*', - 'arp/oper-*-unassigned/cut*', 'arp/htype-fffe-unassigned/cut*', - 'arp/ptype-unknown-plen3/cut*', 'arp/ipv6-plen16/cut*', 'arp/hlen255/cut*', - 'arp/18-octet-trailer/cut23', 'rarp/*/cut*', 'l2tpv2/offset-past-the-data', - 'l2tpv2/offset-past-the-data/cut*', 'l2tpv2/minimal/cut*', - 'l2tpv2/control-every-field/cut*', 'l2tpv2/offset-0/cut5', - 'l2tpv2/offset-3-nonzero-pad/cut6', 'l2tpv2/reserved-bits/cut4', 'l2tpv2/priority/cut4', - 'l2tpv2/every-flag-and-reserved/cut*', 'l2tpv2/length-*/cut5', - 'ospf/hello-header-only/cut*', 'ospf/type-*-unassigned/cut*', - 'ospf/auth-simple-password/cut*', 'ospf/auth-ffff-unassigned/cut*', - 'ospf/auth-cryptographic-digest/cut22', 'ospf/packet-length-short-of-the-data/cut13', - 'ospf/packet-length-past-the-data/cut*', 'ospf/version-3/cut*', 'ospf/lsu-body/cut22')), -) +KNOWN_FAILURES = () # type: tuple[edge.Gap, ...] @unittest.skipUnless(edge.HAS_RUNTIME, 'runtime dependencies not installed') diff --git a/tests/protocols/test_short_read_roundtrip_unit.py b/tests/protocols/test_short_read_roundtrip_unit.py new file mode 100644 index 000000000..99a0e102a --- /dev/null +++ b/tests/protocols/test_short_read_roundtrip_unit.py @@ -0,0 +1,231 @@ +# -*- coding: utf-8 -*- +"""A header the data ends inside rebuilds as captured, in every family. + +GitHub issues #1458 and #1451: when the data ended inside a fixed-width field, +the parse read the missing octets as zeros and recorded nothing, so +``from_data(info)`` wrote the field -- and every field after it -- at full +width. ``Ethernet(bytes(12) + b'\\x08')`` rebuilt as 14 octets. + +:meth:`Schema.unpack ` now +records the field the data ends inside and how much of it was read, as +``__short_read__``; :meth:`Schema.pack ` +writes back only those octets, and the layer base classes carry the record +through ``info``. Each packet below is cut at every offset and must either be +refused with an in-library exception or meet ``Cls(raw).data == raw`` and +``Cls.from_data(Cls(raw).info).data == raw``. + +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`. + +""" + +import importlib +import struct +import unittest +import warnings + +from tests._support import reimport_once_per_class + +ETHERNET = 'link.ethernet:Ethernet' +IPV6 = 'internet.ipv6:IPv6' + + +def _ipv6(code: int, body: bytes) -> bytes: + return (bytes([0x60, 0, 0, 0]) + struct.pack('!HBB', len(body), code, 64) + + bytes(15) + b'\x01' + bytes(15) + b'\x02' + body) + + +HIT = bytes(range(32)) + +#: Label, ``module:Class`` under :mod:`pcapkit.protocols`, parse keywords, and +#: one whole packet. +CASES = ( + ('ethernet', ETHERNET, {}, bytes.fromhex('0123456789abfedcba98765488b5')), + ('c-tag', 'link.c_tag:C_Tag', {}, bytes.fromhex('ffff88b5706c')), + ('s-tag', 'link.s_tag:S_Tag', {}, bytes.fromhex('1fff8100000188b5')), + ('arp', 'link.arp:ARP', {}, bytes.fromhex('000186dd06100001') + bytes(range(44))), + ('rarp', 'application.rarp:RARP', {}, bytes.fromhex('0001080006040003') + bytes(range(20))), + ('l2tpv2', 'link.l2tpv2:L2TPv2', {}, bytes.fromhex('fff2000e00000000000000000000')), + ('ospf', 'application.ospf:OSPF', {}, bytes.fromhex('020100180102030400000000abcd0000') + bytes(8)), + ('ipx', 'internet.ipx:IPX', {}, bytes.fromhex('ffff001e0000000000000101010101010451000000000202020202020452')), + ('ipv4', 'internet.ipv4:IPv4', {}, bytes.fromhex('450000141234000040fd7b83c0000201c6336401')), + ('ipv6', IPV6, {}, _ipv6(59, b'')), + ('ipv6-chain', IPV6, {}, bytes.fromhex( + '600000000020004000000000000000000000000000000001000000000000000000000000000000023c' + '000000000000002b000104000000002c00c800000000003b00000000000000')), + ('hopopt', IPV6, {}, _ipv6(0, bytes([17, 1, 5, 2, 0, 0, 1, 8]) + bytes(8))), + ('ipv6-opts', IPV6, {}, _ipv6(60, bytes([17, 1, 5, 2, 0, 0, 1, 8]) + bytes(8))), + ('ipv6-route-0', IPV6, {}, _ipv6(43, bytes([17, 2, 0, 1]) + bytes(4) + b'\x11' * 16)), + ('ipv6-route-2', IPV6, {}, _ipv6(43, bytes([17, 2, 2, 1]) + bytes(4) + b'\x11' * 16)), + ('ipv6-route-unknown', IPV6, {}, _ipv6(43, bytes([17, 1, 9, 1]) + b'\x44' * 12)), + ('ipv6-frag', IPV6, {}, _ipv6(44, bytes([17, 0, 0, 8, 0x12, 0x34, 0x56, 0x78]))), + ('ah', IPV6, {}, _ipv6(51, bytes([17, 4, 0, 0, 0, 0, 1, 0, 0, 0, 0, 7]) + b'\xaa' * 12)), + ('ah-ipv4', 'internet.ah:AH', {}, bytes([17, 4, 0, 0, 0, 0, 1, 0, 0, 0, 0, 7]) + b'\xaa' * 12), + ('mh', IPV6, {}, _ipv6(135, bytes([59, 1, 5, 0, 0, 0, 0, 1, 0x80, 0, 0, 0x10, 1, 2, 0, 0]))), + ('hip', 'internet.hip:HIP', {'extension': True}, bytes.fromhex('3b04102100000000') + HIT), + ('hip-param', IPV6, {}, _ipv6(139, bytes([59, 5, 1, 0x21, 0, 0, 0, 0]) + HIT + + struct.pack('!HH', 63661, 4) + b'\x55' * 4)), + ('shim6', IPV6, {}, _ipv6(140, bytes([59, 1, 0x80, 0, 0, 0, 0, 0]) + b'\x66' * 8)), + ('esp', 'internet.esp:ESP', {}, bytes.fromhex('0000000100000002') + b'\x77' * 16), + ('tcp', 'transport.tcp:TCP', {}, bytes.fromhex('9c409c41000000010000000060020000ffff0000020405b4')), + ('udp', 'transport.udp:UDP', {}, bytes.fromhex('9c409c41000c000061626364')), + ('sctp', 'transport.sctp:SCTP', {}, bytes.fromhex( + '13881389deadbeef000000000003001400000001000100020000000061626364')), + ('ftp', 'application.ftp:FTP', {}, b'XYZW arg\r\n'), + ('http1', 'application.httpv1:HTTP', {}, b'GET / HTTP/1.1\r\n\r\n'), +) + + +def _quiet(case: unittest.TestCase) -> None: + # a cut header warns that its length runs past the data, which is the point + catcher = warnings.catch_warnings() + catcher.__enter__() + case.addCleanup(catcher.__exit__, None, None, None) + warnings.simplefilter('ignore') + + +def _has_fallback(proto: object) -> bool: + from pcapkit.protocols.internet.ipv6_ext import IPv6_Ext + + return any(type(ext) is IPv6_Ext for ext in getattr(proto, 'extension_headers', {}).values()) + + +def _resolve(path: str) -> type: + module, _, name = path.partition(':') + return getattr(importlib.import_module(f'pcapkit.protocols.{module}'), name) + + +class TestShortReadRoundTrip(unittest.TestCase): + """Pin byte-exact round trips of packets cut at every offset.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + _quiet(self) + + def test_every_family_cut_at_every_offset(self) -> None: + from pcapkit.utilities.exceptions import BaseError + + for label, path, kwargs, packet in CASES: + cls = _resolve(path) + for keep in range(1, len(packet) + 1): + raw = packet[:keep] + with self.subTest(case=label, keep=keep): + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + try: + proto = cls(raw, len(raw), **kwargs) + except BaseError: + continue + self.assertEqual(proto.data, raw) + self.assertEqual(type(proto).from_data(proto.info).data, raw) + if _has_fallback(proto): + # from the dict form, IPv6.from_data rebuilds an + # IPv6_Ext standing in for a dedicated parser with + # that parser's class: a separate defect, in ipv6.py + continue + self.assertEqual(type(proto).from_data(proto.info.to_dict()).data, raw) + + def test_extension_header_cut_at_every_offset(self) -> None: + # #1451: each extension header rebuilds from its own info, too + for label, path, _, packet in CASES: + if path != IPV6: + continue + cls = _resolve(path) + for keep in range(41, len(packet) + 1): + raw = packet[:keep] + with self.subTest(case=label, keep=keep): + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + ipv6 = cls(raw, len(raw)) + for ext in ipv6.extension_headers.values(): + self.assertEqual(type(ext).from_data(ext.info).data, ext.data) + self.assertEqual(type(ext).from_data(ext.info.to_dict()).data, ext.data) + + def test_issue_1458_repro(self) -> None: + from pcapkit.protocols.link.ethernet import Ethernet + + raw = bytes(12) + b'\x08' + eth = Ethernet(raw, 13) + self.assertEqual(Ethernet.from_data(eth.info).data, raw) + self.assertEqual(Ethernet.from_data(eth.info.to_dict()).data, raw) + # the value decodes as before, the missing octet read as zero + self.assertEqual(eth.info.type, 0x0800) + + def test_issue_1451_repro(self) -> None: + # a Routing header declaring 24 octets, of which 23 are captured + from pcapkit.protocols.internet.ipv6 import IPv6 + from pcapkit.protocols.internet.ipv6_route import IPv6_Route + + raw = _ipv6(43, (bytes([17, 2, 0, 1]) + bytes(4) + b'\x11' * 16)[:23]) + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + ipv6 = IPv6(raw, len(raw)) + ext = ipv6.extension_headers.get(43) + self.assertIs(type(ext), IPv6_Route) + self.assertEqual(len(ext.data), 23) + self.assertEqual(IPv6_Route.from_data(ext.info).data, ext.data) + self.assertEqual(IPv6.from_data(ipv6.info).data, raw) + self.assertEqual(IPv6.from_data(ipv6.info.to_dict()).data, raw) + + def test_record_rides_in_to_dict_only_when_cut(self) -> None: + from pcapkit.protocols.link.ethernet import Ethernet + + whole = Ethernet(bytes(14), 14) + self.assertNotIn('__short_read__', whole.info) + self.assertIsNone(whole.info.get('__short_read__')) + + self.assertNotIn('__short_read__', whole.info.to_dict()) + self.assertNotIn('__short_read__', list(whole.info)) + + cut = Ethernet(bytes(13), 13) + self.assertEqual(cut.info['__short_read__'], ('type', 1)) + self.assertEqual(cut.info.to_dict()['__short_read__'], ('type', 1)) + + again = Ethernet.from_data(cut.info.to_dict()) + self.assertEqual(again.info['__short_read__'], ('type', 1)) + self.assertEqual(Ethernet.from_data(again.info.to_dict()).data, bytes(13)) + # a dumper writes the record as a list, which replays the same + record = dict(cut.info.to_dict(), __short_read__=['type', 1]) + self.assertEqual(Ethernet.from_data(record).data, bytes(13)) + + +class TestSchemaShortRead(unittest.TestCase): + """Pin the schema-level record and the pack that honours it.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + _quiet(self) + + def test_unpack_records_and_pack_cuts(self) -> None: + from pcapkit.protocols.schema.link.ethernet import Ethernet + + raw = bytes(range(1, 11)) + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + schema = Ethernet.unpack(raw) + self.assertEqual(schema.__dict__['__short_read__'], ('src', 4)) + self.assertEqual(bytes(schema), raw) + + schema.type = 0x86dd # a field after the cut: still not written + self.assertEqual(schema.pack(), raw) + + def test_whole_schema_has_no_record(self) -> None: + from pcapkit.protocols.schema.link.ethernet import Ethernet + + schema = Ethernet.unpack(bytes(14)) + self.assertNotIn('__short_read__', schema.__dict__) + self.assertNotIn('__short_read__', schema.to_dict()) + + def test_record_survives_to_dict_and_from_dict(self) -> None: + from pcapkit.protocols.schema.link.ethernet import Ethernet + + raw = bytes(range(1, 11)) + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + dict_ = Ethernet.unpack(raw).to_dict() + self.assertEqual(dict_['__short_read__'], ('src', 4)) + self.assertEqual(bytes(Ethernet.from_dict(dict_)), raw) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/transport/test_transport_edge_roundtrip_unit.py b/tests/protocols/transport/test_transport_edge_roundtrip_unit.py index 2839cf2f5..475b8eb19 100644 --- a/tests/protocols/transport/test_transport_edge_roundtrip_unit.py +++ b/tests/protocols/transport/test_transport_edge_roundtrip_unit.py @@ -122,17 +122,15 @@ def chunk(type_: int, flags: int, value: bytes, pad: 'bytes | None' = None) -> b KNOWN_FAILURES = ( # ``sctp/last-chunk-unpadded`` is uncut: a final chunk sent without its pad - # octets reads the pad short, and the rebuild writes three zero octets. - Gap(1458, edge.ZERO_FILLED_SHORT_READ, 'PADDED', '', - ('sctp/last-chunk-unpadded', 'sctp/last-chunk-unpadded/cut8', - 'sctp/common-header-only/cut*', 'sctp/data-1-octet-3-pad/cut31', - 'sctp/data-nonzero-pad/cut31', 'sctp/unassigned-type-action-00/cut*', - 'sctp/unassigned-type-action-01/cut8', 'sctp/unassigned-type-action-10/cut23', - 'sctp/shutdown-ack-every-flag/cut8', 'sctp/cookie-ack/cut8', 'sctp/abort-t-bit/cut8', - 'sctp/two-empty-chunks/cut10', 'sctp/heartbeat-empty-info/cut10', 'udp/min-header/cut*', - 'udp/length-0/cut5', 'udp/length-short-of-the-data/cut6', 'udp/length-past-the-data/cut5', - 'udp/length-short-of-the-header/cut5', 'udp/checksum-ffff/cut5', 'udp/ports-0/cut5', - 'udp/ports-65535/cut5')), + # octets rebuilds with three zero octets. The four cuts end inside the last + # chunk's pad the same way. + Gap(1474, 'A last SCTP chunk whose padding is absent rebuilds with it zero-filled: ' + '``padding_length`` (pcapkit/protocols/schema/transport/sctp.py:83), which ' + 'chunks, parameters and causes share, always writes the full padding.', + 'PADDED', '', + ('sctp/last-chunk-unpadded', 'sctp/data-1-octet-3-pad/cut31', + 'sctp/data-nonzero-pad/cut31', 'sctp/unassigned-type-action-00/cut19', + 'sctp/unassigned-type-action-10/cut23')), )