Skip to content

Commit e48062c

Browse files
authored
fix(protocols): four more maker guards name the rejected argument (#2407)
1 parent aba2478 commit e48062c

5 files changed

Lines changed: 267 additions & 4 deletions

File tree

‎pcapkit/protocols/application/httpv2.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,12 @@ def make(self, # type: ignore[override]
359359
Returns:
360360
Constructed packet data.
361361
362+
Raises:
363+
ProtocolError: If ``frame`` is none of the forms this maker reads --
364+
raw :obj:`bytes`, a :obj:`dict` of frame arguments, an
365+
:class:`~pcapkit.protocols.data.application.httpv2.HTTP` data
366+
model or a :class:`~pcapkit.protocols.schema.schema.Schema`.
367+
362368
.. versionchanged:: 1.5.0
363369
364370
* ``reserved`` is a new parameter before ``sid``, so a 1.4.x
@@ -400,7 +406,8 @@ def make(self, # type: ignore[override]
400406
elif isinstance(frame, Schema):
401407
frame_val = frame
402408
else:
403-
raise ProtocolError(f'HTTP/2: [Type {type_val}] invalid format')
409+
raise ProtocolError(f'HTTP/2: [Type {type_val}] invalid frame: must be bytes, a dict, '
410+
f'an HTTP data model or a Schema, not {frame!r}')
404411

405412
flags_val = {} # type: dict[str, int]
406413
for bit in range(8):

‎pcapkit/protocols/internet/hopopt.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1536,7 +1536,8 @@ def _make_opt_pad(self, code: 'Enum_Option', opt: 'Optional[Data_PadOption]' = N
15361536
pad = getattr(opt, 'pad', b'')
15371537

15381538
if code == Enum_Option.Pad1 and length != 0:
1539-
warn(f'{self.alias}: [OptNo {code}] invalid format', ProtocolWarning)
1539+
warn(f'{self.alias}: [OptNo {code}] invalid length: must be 0 for a Pad1 option, '
1540+
f'not {length!r}; making a PadN', ProtocolWarning)
15401541
code = Enum_Option.PadN # type: ignore[assignment]
15411542

15421543
return Schema_PadOption(

‎pcapkit/protocols/internet/ipv6_opts.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1524,7 +1524,8 @@ def _make_opt_pad(self, code: 'Enum_Option', opt: 'Optional[Data_PadOption]' = N
15241524
pad = getattr(opt, 'pad', b'')
15251525

15261526
if code == Enum_Option.Pad1 and length != 0:
1527-
warn(f'{self.alias}: [OptNo {code}] invalid format', ProtocolWarning)
1527+
warn(f'{self.alias}: [OptNo {code}] invalid length: must be 0 for a Pad1 option, '
1528+
f'not {length!r}; making a PadN', ProtocolWarning)
15281529
code = Enum_Option.PadN # type: ignore[assignment]
15291530

15301531
return Schema_PadOption(

‎pcapkit/protocols/misc/pcapng.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1490,6 +1490,12 @@ def make(self,
14901490
Returns:
14911491
Constructed packet data.
14921492
1493+
Raises:
1494+
ProtocolError: If ``block`` is none of the forms this maker reads --
1495+
raw :obj:`bytes`, a :obj:`dict` of block arguments, a
1496+
:class:`~pcapkit.protocols.data.misc.pcapng.PCAPNG` data model
1497+
or a :class:`~pcapkit.protocols.schema.schema.Schema`.
1498+
14931499
"""
14941500
type_val = cast('Enum_BlockType',
14951501
self._make_index(type, type_default, namespace=type_namespace,
@@ -1535,7 +1541,8 @@ def make(self,
15351541
elif isinstance(block, Schema):
15361542
block_val = block
15371543
else:
1538-
raise ProtocolError(f'PCAP-NG: [Type {type_val}] invalid format')
1544+
raise ProtocolError(f'PCAP-NG: [Type {type_val}] invalid block: must be bytes, a dict, '
1545+
f'a PCAPNG data model or a Schema, not {block!r}')
15391546

15401547
return Schema_PCAPNG(
15411548
type=type_val,
Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
# -*- coding: utf-8 -*-
2+
"""GitHub issue #2406: the makers #2296 did not list name their argument.
3+
4+
#2296 swept ``_make_*`` guards off the *reader's* text --
5+
``<alias>: [... N] invalid format`` -- and onto the house form established by
6+
#2232 and #2257, ``<prefix>: <lowercase description>: must be X, not {value!r}``.
7+
The maker-side sites below were outside its checklist and kept the generic
8+
wording:
9+
10+
* :meth:`HOPOPT._make_opt_pad
11+
<pcapkit.protocols.internet.hopopt.HOPOPT._make_opt_pad>` and
12+
:meth:`IPv6_Opts._make_opt_pad
13+
<pcapkit.protocols.internet.ipv6_opts.IPv6_Opts._make_opt_pad>`, the twins of
14+
the ``Pad1`` length warning #2296 fixed on :class:`~pcapkit.protocols.internet.mh.MH`;
15+
* :meth:`HTTP.make <pcapkit.protocols.application.httpv2.HTTP.make>` and
16+
:meth:`PCAPNG.make <pcapkit.protocols.misc.pcapng.PCAPNG.make>`, the
17+
dispatchers whose unsupported-``frame``/``block`` guard is the same shape as
18+
``MH.make``'s.
19+
20+
The parse side is deliberately untouched, as in #2296: ``invalid format`` is the
21+
right thing for a reader to say. :class:`ReadersKeepInvalidFormat` below pins
22+
that division on the one pair where the reader and the maker guard the very same
23+
thing -- ``_read_opt_pad`` and ``_make_opt_pad`` both reject a ``Pad1`` carrying
24+
a non-zero length.
25+
26+
Every message is pinned whole, and every maker's valid input is exercised in the
27+
same class, so a guard that starts refusing good input regresses here too.
28+
29+
:mod:`pcapkit` is imported inside each test, after
30+
:func:`~tests._support.reimport_once_per_class`.
31+
32+
"""
33+
34+
from __future__ import annotations
35+
36+
import unittest
37+
import warnings
38+
from typing import TYPE_CHECKING
39+
40+
from tests._support import reimport_once_per_class
41+
42+
if TYPE_CHECKING:
43+
from typing import Any
44+
45+
46+
class MakerMessageTestCase(unittest.TestCase):
47+
"""Re-import :mod:`pcapkit` once per class, as the suite's convention asks."""
48+
49+
def setUp(self) -> None:
50+
reimport_once_per_class(self)
51+
52+
def protocol(self, cls: 'Any') -> 'Any':
53+
"""An uninitialised protocol instance, for calling a maker directly.
54+
55+
The makers under test read nothing off ``self`` but ``alias``, so there
56+
is no packet to construct first -- and constructing one would route the
57+
bad argument through a schema that raises its own error instead.
58+
59+
"""
60+
return object.__new__(cls)
61+
62+
63+
class IPv6ExtensionPadOptionWarnings(MakerMessageTestCase):
64+
"""``_make_opt_pad`` warns about ``length``, not about a format.
65+
66+
HOPOPT and IPv6-Opts carry byte-identical copies of the option machinery,
67+
down to the ``Pad1`` guard, so both are driven from one table: the only
68+
thing that differs in the message is the alias each protocol reports.
69+
70+
"""
71+
72+
def protocols(self) -> 'list[tuple[Any, str]]':
73+
"""Each padding-option maker, paired with the alias it names itself by."""
74+
from pcapkit.protocols.internet.hopopt import HOPOPT
75+
from pcapkit.protocols.internet.ipv6_opts import IPv6_Opts
76+
77+
return [(HOPOPT, 'HOPOPT'), (IPv6_Opts, 'IPv6-Opts')]
78+
79+
def test_pad1_with_a_length_warns_by_name(self) -> None:
80+
from pcapkit.const.ipv6.option import Option
81+
from pcapkit.utilities.warnings import ProtocolWarning
82+
83+
code = Option.Pad1
84+
for cls, alias in self.protocols():
85+
protocol = self.protocol(cls)
86+
for length in (1, 3):
87+
with self.subTest(protocol=alias, length=length):
88+
with warnings.catch_warnings(record=True) as caught:
89+
warnings.simplefilter('always')
90+
schema = protocol._make_opt_pad(code, length=length)
91+
messages = [str(w.message) for w in caught
92+
if issubclass(w.category, ProtocolWarning)]
93+
self.assertEqual(len(messages), 1, messages)
94+
self.assertEqual(
95+
messages[0],
96+
f'{alias}: [OptNo {code}] invalid length: '
97+
f'must be 0 for a Pad1 option, not {length!r}; making a PadN')
98+
self.assertEqual(schema.type, Option.PadN)
99+
100+
def test_pad1_with_no_length_does_not_warn(self) -> None:
101+
from pcapkit.const.ipv6.option import Option
102+
from pcapkit.utilities.warnings import ProtocolWarning
103+
104+
for cls, alias in self.protocols():
105+
protocol = self.protocol(cls)
106+
with self.subTest(protocol=alias):
107+
with warnings.catch_warnings(record=True) as caught:
108+
warnings.simplefilter('always')
109+
schema = protocol._make_opt_pad(Option.Pad1, length=0)
110+
self.assertEqual([str(w.message) for w in caught
111+
if issubclass(w.category, ProtocolWarning)], [])
112+
self.assertEqual(schema.type, Option.Pad1)
113+
114+
def test_padn_with_a_length_still_builds(self) -> None:
115+
from pcapkit.const.ipv6.option import Option
116+
from pcapkit.utilities.warnings import ProtocolWarning
117+
118+
for cls, alias in self.protocols():
119+
protocol = self.protocol(cls)
120+
with self.subTest(protocol=alias):
121+
with warnings.catch_warnings(record=True) as caught:
122+
warnings.simplefilter('always')
123+
schema = protocol._make_opt_pad(Option.PadN, length=2, pad=b'\x00\x00')
124+
self.assertEqual([str(w.message) for w in caught
125+
if issubclass(w.category, ProtocolWarning)], [])
126+
self.assertEqual(schema.type, Option.PadN)
127+
self.assertEqual(schema.len, 2)
128+
self.assertEqual(schema.pad, b'\x00\x00')
129+
130+
131+
class HTTPFrameMessage(MakerMessageTestCase):
132+
"""``HTTP.make`` names ``frame`` and the forms it reads."""
133+
134+
def test_unreadable_frame_names_the_argument(self) -> None:
135+
from pcapkit.const.http.frame import Frame
136+
from pcapkit.protocols.application.httpv2 import HTTP
137+
from pcapkit.utilities.exceptions import ProtocolError
138+
139+
http = self.protocol(HTTP)
140+
code = Frame.DATA
141+
for frame in (5, 1.5, 'frame', []):
142+
with self.subTest(frame=frame):
143+
with self.assertRaises(ProtocolError) as context:
144+
http.make(type=code, frame=frame)
145+
self.assertEqual(
146+
str(context.exception),
147+
f'HTTP/2: [Type {code}] invalid frame: must be bytes, a dict, '
148+
f'an HTTP data model or a Schema, not {frame!r}')
149+
150+
def test_raw_frame_still_builds(self) -> None:
151+
from pcapkit.const.http.frame import Frame
152+
from pcapkit.protocols.application.httpv2 import HTTP
153+
154+
http = self.protocol(HTTP)
155+
schema = http.make(type=Frame.DATA, frame=b'\x01\x02')
156+
self.assertEqual(schema.frame, b'\x01\x02')
157+
self.assertEqual(schema.length, 2)
158+
159+
def test_an_omitted_frame_is_not_rejected(self) -> None:
160+
from pcapkit.const.http.frame import Frame
161+
from pcapkit.protocols.application.httpv2 import HTTP
162+
163+
# ``frame=None`` dispatches to the frame type's own constructor (#2274)
164+
# rather than reaching the guard, so the guard has not widened to it.
165+
http = self.protocol(HTTP)
166+
self.assertEqual(http.make(type=Frame.DATA, frame=None).type, Frame.DATA)
167+
168+
169+
class PCAPNGBlockMessage(MakerMessageTestCase):
170+
"""``PCAPNG.make`` names ``block`` and the forms it reads."""
171+
172+
def protocol(self, cls: 'Any') -> 'Any':
173+
"""As the base helper, but with the section context the dispatcher reads.
174+
175+
``PCAPNG.make`` settles the byte order before handing the block over,
176+
and reads :attr:`PCAPNG._ctx` to do it. :data:`None` is what an
177+
out-of-capture construction carries, and sends it to the default
178+
``__packet__`` byte order.
179+
180+
"""
181+
protocol = super().protocol(cls)
182+
protocol._ctx = None
183+
return protocol
184+
185+
def test_unreadable_block_names_the_argument(self) -> None:
186+
from pcapkit.const.pcapng.block_type import BlockType
187+
from pcapkit.protocols.misc.pcapng import PCAPNG
188+
from pcapkit.utilities.exceptions import ProtocolError
189+
190+
pcapng = self.protocol(PCAPNG)
191+
code = BlockType.Enhanced_Packet_Block
192+
for block in (5, None, 1.5, 'block'):
193+
with self.subTest(block=block):
194+
with self.assertRaises(ProtocolError) as context:
195+
pcapng.make(type=code, block=block)
196+
self.assertEqual(
197+
str(context.exception),
198+
f'PCAP-NG: [Type {code}] invalid block: must be bytes, a dict, '
199+
f'a PCAPNG data model or a Schema, not {block!r}')
200+
201+
def test_raw_block_still_builds(self) -> None:
202+
from pcapkit.const.pcapng.block_type import BlockType
203+
from pcapkit.protocols.misc.pcapng import PCAPNG
204+
205+
pcapng = self.protocol(PCAPNG)
206+
schema = pcapng.make(type=BlockType.Enhanced_Packet_Block, block=b'\x00\x01')
207+
self.assertEqual(schema.type, BlockType.Enhanced_Packet_Block)
208+
self.assertEqual(schema.block, b'\x00\x01')
209+
210+
211+
class ReadersKeepInvalidFormat(MakerMessageTestCase):
212+
"""The parse side still says ``invalid format``, as #2296 asks.
213+
214+
``_read_opt_pad`` and ``_make_opt_pad`` reject the same thing -- a ``Pad1``
215+
option carrying a non-zero length -- so the pair is where a sweep over the
216+
makers is most likely to catch a reader by accident.
217+
218+
"""
219+
220+
def readers(self) -> 'list[tuple[Any, Any, str]]':
221+
"""Each padding-option reader, its schema class and its alias."""
222+
from pcapkit.protocols.internet.hopopt import HOPOPT
223+
from pcapkit.protocols.internet.ipv6_opts import IPv6_Opts
224+
from pcapkit.protocols.schema.internet.hopopt import PadOption as Schema_HOPOPTPad
225+
from pcapkit.protocols.schema.internet.ipv6_opts import PadOption as Schema_IPv6OptsPad
226+
227+
return [(HOPOPT, Schema_HOPOPTPad, 'HOPOPT'),
228+
(IPv6_Opts, Schema_IPv6OptsPad, 'IPv6-Opts')]
229+
230+
def test_the_reader_still_reports_an_invalid_format(self) -> None:
231+
from pcapkit.const.ipv6.option import Option
232+
from pcapkit.corekit.multidict import OrderedMultiDict
233+
from pcapkit.utilities.exceptions import ProtocolError
234+
235+
code = Option.Pad1
236+
for cls, schema_cls, alias in self.readers():
237+
protocol = self.protocol(cls)
238+
with self.subTest(protocol=alias):
239+
schema = schema_cls(type=code, len=3, pad=b'')
240+
with self.assertRaises(ProtocolError) as context:
241+
protocol._read_opt_pad(schema, options=OrderedMultiDict())
242+
self.assertEqual(str(context.exception),
243+
f'{alias}: [OptNo {code}] invalid format')
244+
245+
246+
if __name__ == '__main__':
247+
unittest.main()

0 commit comments

Comments
 (0)