Skip to content

Commit 79949ec

Browse files
authored
fix(schema): a SchemaField's unread octets are lost through from_data, so a direct SchemaField rebuilds zero-filled (#1397)
Schema.unpack now records the octets of a direct SchemaField's span that the nested schema left unread on the nested schema as __remainder__, with the width it read as __remainder_offset__. to_dict() emits both as keys of the nested mapping, from_dict() accepts them back, and Schema.pack re-emits the remainder wherever the nested value goes, as long as the nested schema still packs to that width; a nested schema that shrank or grew is zero-filled as before. from_dict(to_dict()) and Protocol.from_data now rebuild byte-exactly. The keys are absent when nothing was left unread. Closes #1380
1 parent 979dc37 commit 79949ec

3 files changed

Lines changed: 269 additions & 13 deletions

File tree

‎pcapkit/protocols/schema/schema.py‎

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ class marked with a user's own ``from typing import final`` on 3.10 is
9999

100100
temp = ['__map__', '__map_reverse__', '__builtin__',
101101
'__fields__', '__buffer__', '__updated__',
102-
'__payload__', '__finalised__']
102+
'__payload__', '__finalised__', '__remainder__', '__remainder_offset__']
103103
temp.extend(cls.__additional__)
104104
for obj in cls.mro():
105105
temp.extend(el for el in dir(obj) if el not in cls.__fields__)
@@ -374,6 +374,11 @@ class Schema(Mapping[str, _VT], Generic[_VT], metaclass=SchemaMeta):
374374
__buffer__: 'dict[str, bytes]'
375375
#: Flag for whether the schema is recently updated.
376376
__updated__: 'bool'
377+
#: Octets of an enclosing :class:`~pcapkit.corekit.fields.misc.SchemaField`
378+
#: span that this nested schema left unread; set only when non-empty.
379+
__remainder__: 'bytes'
380+
#: Octets this nested schema read before :attr:`__remainder__`.
381+
__remainder_offset__: 'int'
377382

378383
#: Flag for finalised class initialisation.
379384
__finalised__: 'FinalisedState' = FinalisedState.NONE
@@ -554,6 +559,13 @@ def __update__(self, dict_: 'Optional[Mapping[str, _VT] | Iterable[tuple[str, _V
554559
data_iter = itertools.chain(dict_, kwargs.items())
555560

556561
for (key, value) in data_iter:
562+
# NOTE: Not a field, but what :meth:`to_dict` emits for the octets an
563+
# enclosing ``SchemaField`` span held beyond this schema, and where,
564+
# so that a rebuilt schema still re-emits them (:issue:`1380`).
565+
if key in ('__remainder__', '__remainder_offset__'):
566+
self.__dict__[key] = value
567+
continue
568+
557569
if key not in self.__buffer__:
558570
warn(f'{key!r} is not a valid field name', UnknownFieldWarning)
559571
continue
@@ -691,6 +703,15 @@ def to_dict(self) -> 'dict[str, _VT]':
691703
types, such as :obj:`list`, :obj:`tuple`, :obj:`set`, etc., we
692704
shall not convert them into :obj:`dict` and remain them intact.
693705
706+
Note:
707+
A schema unpacked as the value of a
708+
:class:`~pcapkit.corekit.fields.misc.SchemaField` that left part of
709+
the field's span unread carries those octets under a
710+
``__remainder__`` key, and the number of octets it did read under
711+
``__remainder_offset__``. :meth:`from_dict` accepts both back, so
712+
that the rebuilt field still packs them (:issue:`1380`). The keys
713+
are absent otherwise.
714+
694715
"""
695716
dict_ = {} # type: dict[str, Any]
696717
for (key, value) in self.__dict__.items():
@@ -702,6 +723,10 @@ def to_dict(self) -> 'dict[str, _VT]':
702723
dict_[out_key] = value.to_dict()
703724
else:
704725
dict_[out_key] = value
726+
727+
for key in ('__remainder__', '__remainder_offset__'):
728+
if key in self.__dict__:
729+
dict_[key] = self.__dict__[key]
705730
return dict_
706731

707732
def to_bytes(self) -> 'bytes':
@@ -862,19 +887,31 @@ class will consider negative value as a placeholder.
862887
if isinstance(field, SchemaField) and len(temp) < field.length:
863888
# NOTE: :meth:`unpack` hands a nested schema its whole declared
864889
# span, and the schema may read less of it than that. The octets
865-
# it left unread are still in the buffer :meth:`unpack` stored, so
866-
# they are re-emitted from there, as long as the buffer still
867-
# starts with what the nested schema packs to now; otherwise, or
868-
# for a schema that was never unpacked, the span is zero-filled.
890+
# it left unread travel with the nested value as its
891+
# ``__remainder__`` -- on the schema itself, or as a key of the
892+
# mapping :meth:`to_dict` made of it -- so they are re-emitted
893+
# from there, as long as the nested schema still packs to the
894+
# ``__remainder_offset__`` octets it was parsed at. Re-emitting
895+
# them after a nested schema that has shrunk or grown would put
896+
# stale octets at a shifted offset, so then, or for a schema that
897+
# never had any, the rest of the span is zero-filled.
869898
# Either way the field keeps the width its length field declares
870-
# (:issue:`1292`).
899+
# (:issue:`1292`), and a schema rebuilt through :meth:`from_dict`
900+
# packs what the parsed one did (:issue:`1380`).
871901
#
872902
# A field reached through a
873903
# :class:`~pcapkit.corekit.fields.misc.SwitchField` is not handled
874904
# here: the PCAP-NG block selector declares the rest of the
875905
# *stream* as its span, which is no part of the field.
876-
buffer = self.__buffer__[field.name]
877-
tail = buffer[len(temp):field.length] if buffer.startswith(temp) else b''
906+
if isinstance(data, Schema):
907+
record = data.__dict__ # type: Mapping[str, Any]
908+
elif isinstance(data, collections.abc.Mapping):
909+
record = data
910+
else:
911+
record = {}
912+
tail = record.get('__remainder__', b'')
913+
if record.get('__remainder_offset__') != len(temp) or len(temp) + len(tail) > field.length:
914+
tail = b''
878915
temp += tail + bytes(field.length - len(temp) - len(tail))
879916
self.__buffer__[field.name] = temp
880917

@@ -998,6 +1035,16 @@ def unpack(cls, data: 'bytes | IO[bytes]',
9981035
self.__buffer__[field.name] = byte
9991036

10001037
value = field.unpack(byte, packet.copy())
1038+
if isinstance(field, SchemaField) and isinstance(value, Schema):
1039+
# NOTE: Kept on the nested schema rather than only in
1040+
# ``__buffer__``, so that the octets it left unread go wherever
1041+
# the value goes, :meth:`to_dict` included; see :meth:`pack`.
1042+
# The width read is summed from the nested buffers rather than
1043+
# taken with :func:`len`, which may pack the schema again.
1044+
consumed = sum(map(len, value.__buffer__.values()))
1045+
if len(byte) > consumed:
1046+
value.__dict__['__remainder__'] = byte[consumed:]
1047+
value.__dict__['__remainder_offset__'] = consumed
10011048
setattr(self, field.name, value)
10021049

10031050
packet[field.name] = value
Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
# -*- coding: utf-8 -*-
2+
"""A :class:`~pcapkit.corekit.fields.misc.SchemaField`'s unread octets survive a rebuild.
3+
4+
GitHub issue #1380: a nested schema may read less than its
5+
:class:`~pcapkit.corekit.fields.misc.SchemaField`'s declared span. Parsing then
6+
packing kept the octets it left unread (#1292), but a schema rebuilt through
7+
:meth:`~pcapkit.protocols.schema.schema.Schema.from_dict`, or a protocol rebuilt
8+
through :meth:`~pcapkit.protocols.protocol.ProtocolBase.from_data`, had never
9+
been unpacked and zero-filled them. They now travel with the nested value as its
10+
``__remainder__``.
11+
12+
No shipped schema declares a :class:`~pcapkit.corekit.fields.misc.SchemaField`
13+
directly, so the schemas and the protocol here are built in the test. Everything
14+
from :mod:`pcapkit` is imported inside each test, after
15+
:func:`~tests._support.reimport_once_per_class`, not at module load.
16+
17+
"""
18+
19+
import unittest
20+
import warnings
21+
22+
from tests._support import reimport_once_per_class
23+
24+
#: ``n`` = 3, a 3-octet span whose nested schema reads only ``02``, then ``tail``.
25+
RAW = bytes.fromhex('0302eeee04')
26+
27+
28+
def _nested_schemas(): # type: ignore[no-untyped-def]
29+
"""An outer schema whose nested schema reads less than its span."""
30+
from pcapkit.corekit.fields.misc import SchemaField
31+
from pcapkit.corekit.fields.numbers import UInt8Field
32+
from pcapkit.protocols.schema.schema import Schema, schema_final
33+
34+
@schema_final
35+
class Inner(Schema):
36+
x: int = UInt8Field()
37+
38+
@schema_final
39+
class Outer(Schema):
40+
n: int = UInt8Field()
41+
inner: Inner = SchemaField(length=lambda pkt: pkt['n'], schema=Inner)
42+
tail: int = UInt8Field()
43+
44+
return Inner, Outer
45+
46+
47+
#: A 6-octet span whose nested schema reads ``k`` = 2 and ``aabb``, then ``tail``.
48+
SIZED_RAW = bytes.fromhex('02aabbccddee04')
49+
50+
51+
def _sized_schemas(): # type: ignore[no-untyped-def]
52+
"""An outer schema whose nested schema's own width depends on its values."""
53+
from pcapkit.corekit.fields.misc import SchemaField
54+
from pcapkit.corekit.fields.numbers import UInt8Field
55+
from pcapkit.corekit.fields.strings import BytesField
56+
from pcapkit.protocols.schema.schema import Schema, schema_final
57+
58+
@schema_final
59+
class Inner(Schema):
60+
k: int = UInt8Field()
61+
body: bytes = BytesField(length=lambda pkt: pkt['k'])
62+
63+
@schema_final
64+
class Outer(Schema):
65+
inner: Inner = SchemaField(length=6, schema=Inner)
66+
tail: int = UInt8Field()
67+
68+
return Inner, Outer
69+
70+
71+
def _protocol(outer): # type: ignore[no-untyped-def]
72+
"""A protocol over ``outer`` whose data model holds the nested schema's mapping."""
73+
from pcapkit.corekit.infoclass import info_final
74+
from pcapkit.corekit.protochain import ProtoChain
75+
from pcapkit.protocols.data.data import Data
76+
from pcapkit.protocols.protocol import ProtocolBase
77+
78+
@info_final
79+
class OuterData(Data):
80+
n: int
81+
inner: dict
82+
tail: int
83+
84+
class OuterProtocol(ProtocolBase, schema=outer, data=OuterData): # type: ignore[call-arg,misc]
85+
__layer__ = 'Internet'
86+
87+
@property
88+
def name(self): # type: ignore[no-untyped-def]
89+
return 'Outer'
90+
91+
@property
92+
def length(self): # type: ignore[no-untyped-def]
93+
return len(self.__header__)
94+
95+
def read(self, length=None, **kwargs): # type: ignore[no-untyped-def]
96+
from pcapkit.protocols.misc.null import NoPayload
97+
98+
schema = self.__header__
99+
self._next = NoPayload()
100+
self._protos = ProtoChain(type(self), self.alias, basis=self._next.protochain)
101+
# NOTE: A made header still holds the mapping ``make`` was given.
102+
inner = schema.inner if isinstance(schema.inner, dict) else schema.inner.to_dict()
103+
return OuterData(n=schema.n, inner=inner, tail=schema.tail)
104+
105+
def make(self, n=0, inner=None, tail=0, **kwargs): # type: ignore[no-untyped-def]
106+
return outer(n=n, inner=inner, tail=tail)
107+
108+
@classmethod
109+
def __index__(cls): # type: ignore[no-untyped-def,override]
110+
return 250
111+
112+
return OuterProtocol
113+
114+
115+
class TestSchemaFieldRemainder(unittest.TestCase):
116+
"""Pin the unread octets of a nested schema through every rebuild."""
117+
118+
def setUp(self) -> None:
119+
reimport_once_per_class(self)
120+
121+
def test_to_dict_carries_the_remainder(self) -> None:
122+
_, outer = _nested_schemas()
123+
124+
parsed = outer.unpack(RAW, len(RAW), None)
125+
self.assertEqual(parsed.to_dict(), {
126+
'n': 3, 'inner': {'x': 2, '__remainder__': b'\xee\xee', '__remainder_offset__': 1}, 'tail': 4,
127+
})
128+
129+
def test_from_dict_rebuilds_byte_exactly(self) -> None:
130+
_, outer = _nested_schemas()
131+
132+
parsed = outer.unpack(RAW, len(RAW), None)
133+
with warnings.catch_warnings():
134+
warnings.simplefilter('error')
135+
rebuilt = outer.from_dict(parsed.to_dict())
136+
packed = rebuilt.pack()
137+
self.assertEqual(packed, RAW)
138+
139+
def test_parsed_nested_schema_keeps_its_remainder(self) -> None:
140+
_, outer = _nested_schemas()
141+
142+
parsed = outer.unpack(RAW, len(RAW), None)
143+
self.assertEqual(outer(n=3, inner=parsed.inner, tail=4).pack(), RAW)
144+
145+
def test_remainder_is_not_a_field(self) -> None:
146+
_, outer = _nested_schemas()
147+
148+
inner = outer.unpack(RAW, len(RAW), None).inner
149+
self.assertEqual(dict(inner), {'x': 2})
150+
self.assertEqual(bytes(inner), b'\x02')
151+
with self.assertRaises(KeyError):
152+
inner['__remainder__'] # pylint: disable=pointless-statement,unsubscriptable-object
153+
154+
def test_remainder_that_no_longer_fits_is_dropped(self) -> None:
155+
_, outer = _nested_schemas()
156+
157+
data = outer.unpack(RAW, len(RAW), None).to_dict()
158+
data['n'] = 2
159+
self.assertEqual(outer.from_dict(data).pack(), bytes.fromhex('02020004'))
160+
161+
def test_shrunk_in_place_is_zero_filled(self) -> None:
162+
_, outer = _sized_schemas()
163+
164+
parsed = outer.unpack(SIZED_RAW, len(SIZED_RAW), None)
165+
parsed.inner.body = b'\xaa'
166+
parsed.inner.k = 1
167+
self.assertEqual(parsed.pack(), bytes.fromhex('01aa0000000004'))
168+
169+
def test_shrunk_through_from_dict_is_zero_filled(self) -> None:
170+
_, outer = _sized_schemas()
171+
172+
data = outer.unpack(SIZED_RAW, len(SIZED_RAW), None).to_dict()
173+
data['inner'].update(k=1, body=b'\xaa')
174+
self.assertEqual(outer.from_dict(data).pack(), bytes.fromhex('01aa0000000004'))
175+
176+
def test_grown_through_from_dict_is_zero_filled(self) -> None:
177+
_, outer = _sized_schemas()
178+
179+
data = outer.unpack(SIZED_RAW, len(SIZED_RAW), None).to_dict()
180+
data['inner'].update(k=4, body=b'\xaa\xbb\xcc\xdd')
181+
self.assertEqual(outer.from_dict(data).pack(), bytes.fromhex('04aabbccdd0004'))
182+
183+
def test_sized_round_trips_byte_exactly(self) -> None:
184+
_, outer = _sized_schemas()
185+
186+
parsed = outer.unpack(SIZED_RAW, len(SIZED_RAW), None)
187+
self.assertEqual(parsed.pack(), SIZED_RAW)
188+
self.assertEqual(outer.from_dict(parsed.to_dict()).pack(), SIZED_RAW)
189+
190+
def test_no_remainder_without_unread_octets(self) -> None:
191+
_, outer = _nested_schemas()
192+
193+
raw = bytes.fromhex('010204')
194+
parsed = outer.unpack(raw, len(raw), None)
195+
self.assertNotIn('__remainder__', parsed.to_dict()['inner'])
196+
self.assertEqual(outer.from_dict(parsed.to_dict()).pack(), raw)
197+
198+
def test_protocol_from_data_rebuilds_byte_exactly(self) -> None:
199+
_, outer = _nested_schemas()
200+
protocol = _protocol(outer)
201+
202+
parsed = protocol(RAW, len(RAW))
203+
self.assertEqual(bytes(protocol.from_data(parsed.info)), RAW)
204+
self.assertEqual(bytes(protocol.from_data(parsed.info.to_dict())), RAW)
205+
206+
207+
if __name__ == '__main__':
208+
unittest.main()

‎tests/protocols/schema/test_schema_roundtrip_unit.py‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
1212
GitHub issue #1292: a :class:`~pcapkit.corekit.fields.misc.SchemaField` read
1313
its whole declared span but packed only what its nested schema consumed, so
14-
the octets the nested schema left unread were lost.
14+
the octets the nested schema left unread were lost; GitHub issue #1380: a
15+
schema rebuilt through ``from_dict`` still zero-filled them.
1516
1617
Every case builds its own octets in memory and reads no capture. Everything
1718
from :mod:`pcapkit` is imported inside each test, after
@@ -98,10 +99,10 @@ def test_schema_field_keeps_declared_width(self) -> None:
9899

99100
raw = bytes.fromhex('0302eeee04')
100101
rebuilt = outer.from_dict(outer.unpack(raw, len(raw), None).to_dict())
101-
# NOTE: The width is kept but the unread octets are zero-filled, since a
102-
# rebuilt schema was never unpacked; that open half is #1380, which will
103-
# change this expectation to ``0302eeee04``.
104-
self.assertEqual(rebuilt.pack(), bytes.fromhex('0302000004'))
102+
# NOTE: The unread octets travel through ``to_dict`` as the nested
103+
# value's ``__remainder__`` (#1380); a nested schema that never had any
104+
# is zero-filled to the declared width.
105+
self.assertEqual(rebuilt.pack(), raw)
105106
self.assertEqual(outer(n=3, inner=inner(x=2), tail=4).pack(), bytes.fromhex('0302000004'))
106107

107108

0 commit comments

Comments
 (0)