Skip to content

Commit ef4d93f

Browse files
authored
fix(sctp): check parameter types in the Supported Address Types and Missing Mandatory Parameter makers (#2288) (#2363)
SCTP._make_param_addrtypes and _make_cause_missing_param shared the conversion "Enum_Parameter.get(item) if isinstance(item, int) else item": True became parameter type 1, False raised EnumValueError, and a str or float item was stored unchecked. Both makers now call one helper, _make_param_types, which raises ProtocolError "SCTP: invalid parameter type: ..." for a bool or any item that is neither a Parameter nor an int. The helper is documented in sctp.rst, since both makers' docstrings refer to it. Test: test_parameter_type_lists_refuse_a_bool_and_a_non_int pins the message for True, False, '5', 5.0 and None in both makers.
1 parent 7c4d7ce commit ef4d93f

3 files changed

Lines changed: 72 additions & 4 deletions

File tree

‎docs/source/pcapkit/protocols/transport/sctp.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ Octets Bits Name Description
5555
.. automethod:: _read_sctp_causes
5656
.. automethod:: _make_sctp_causes
5757
.. automethod:: _make_sctp_cause
58+
.. automethod:: _make_param_types
5859

5960
.. automethod:: _read_chunk_donone
6061
.. automethod:: _read_chunk_data

‎pcapkit/protocols/transport/sctp.py‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,6 +1257,37 @@ def _make_nested_length(self, base: 'int', items: 'Sequence[Schema_Parameter | S
12571257
return padded
12581258
return padded - max(sizes[-1] - items[-1].length, 0)
12591259

1260+
def _make_param_types(self, types: 'Optional[list[Enum_Parameter | int]]') -> 'list[Enum_Parameter]':
1261+
"""Resolve a list of parameter types.
1262+
1263+
Shared by :meth:`_make_param_addrtypes` and
1264+
:meth:`_make_cause_missing_param`, whose ``types`` both list
1265+
parameter types.
1266+
1267+
Args:
1268+
types: parameter types, each given as a
1269+
:class:`~pcapkit.const.sctp.parameter.Parameter` or an
1270+
:obj:`int`
1271+
1272+
Returns:
1273+
The :class:`~pcapkit.const.sctp.parameter.Parameter` for each
1274+
item.
1275+
1276+
Raises:
1277+
ProtocolError: If an item is a :obj:`bool`, or is neither a
1278+
:class:`~pcapkit.const.sctp.parameter.Parameter` nor an
1279+
:obj:`int` (:issue:`2288`).
1280+
1281+
"""
1282+
types_value = [] # type: list[Enum_Parameter]
1283+
for item in types or []:
1284+
# NOTE: ``bool`` is an ``int`` subclass, so ``Parameter.get``
1285+
# would look ``True`` up as parameter type 1 (#2288).
1286+
if isinstance(item, bool) or not isinstance(item, int):
1287+
raise ProtocolError(f'{self.alias}: invalid parameter type: {item!r}')
1288+
types_value.append(Enum_Parameter.get(item))
1289+
return types_value
1290+
12601291
def _read_chunk_donone(self, schema: 'Schema_UnknownChunk', *, chunks: 'Chunks') -> 'Data_UnknownChunk': # pylint: disable=unused-argument
12611292
"""Read SCTP chunk of an unsupported type.
12621293
@@ -2998,12 +3029,15 @@ def _make_param_addrtypes(self, code: 'Enum_Parameter', param: 'Optional[Data_Su
29983029
Returns:
29993030
Constructed parameter schema.
30003031
3032+
Raises:
3033+
ProtocolError: If an item of ``types`` is not a parameter type,
3034+
as :meth:`_make_param_types` checks.
3035+
30013036
"""
30023037
if param is not None:
30033038
types = list(param.types)
30043039

3005-
types_value = [Enum_Parameter.get(item) if isinstance(item, int) else item
3006-
for item in types or []]
3040+
types_value = self._make_param_types(types)
30073041

30083042
return Schema_SupportedAddressTypesParameter(
30093043
type=code,
@@ -3561,6 +3595,10 @@ def _make_cause_missing_param(self, code: 'Enum_CauseCode', cause: 'Optional[Dat
35613595
Returns:
35623596
Constructed error cause schema.
35633597
3598+
Raises:
3599+
ProtocolError: If an item of ``types`` is not a parameter type,
3600+
as :meth:`_make_param_types` checks.
3601+
35643602
Note:
35653603
The count of missing parameters is derived from ``types`` rather
35663604
than taken as an argument, so that it cannot disagree with the list
@@ -3570,8 +3608,7 @@ def _make_cause_missing_param(self, code: 'Enum_CauseCode', cause: 'Optional[Dat
35703608
if cause is not None:
35713609
types = list(cause.types)
35723610

3573-
types_value = [Enum_Parameter.get(item) if isinstance(item, int) else item
3574-
for item in types or []]
3611+
types_value = self._make_param_types(types)
35753612

35763613
return Schema_MissingMandatoryParameterCause(
35773614
code=code,

‎tests/protocols/transport/test_sctp_unit.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,36 @@ def test_constructed_ports_carry_the_same_type_as_parsed_ones(self) -> None:
160160
self.assertEqual(schema.dstport.port, 38412)
161161
self.assertEqual(schema.pack()[:4], b'\x26\xab\x96\x0c')
162162

163+
def test_parameter_type_lists_refuse_a_bool_and_a_non_int(self) -> None:
164+
"""#2288: both makers resolve ``types`` through one shared check.
165+
166+
``bool`` is an :obj:`int` subclass, and a ``Parameter`` member, an
167+
:obj:`int`, a :obj:`str` or a :obj:`float` need distinct handling, so
168+
every type in the list is checked by the same helper.
169+
170+
"""
171+
from pcapkit.const.sctp.cause_code import CauseCode
172+
from pcapkit.const.sctp.parameter import Parameter
173+
from pcapkit.protocols.transport.sctp import SCTP
174+
from pcapkit.utilities.exceptions import ProtocolError
175+
176+
proto = SCTP.__new__(SCTP)
177+
makers = (
178+
(SCTP._make_param_addrtypes, Parameter.Supported_Address_Types),
179+
(SCTP._make_cause_missing_param, CauseCode.Missing_Mandatory_Parameter),
180+
)
181+
for maker, code in makers:
182+
for item in (True, False, '5', 5.0, None):
183+
with self.subTest(maker=maker.__name__, item=item):
184+
with self.assertRaises(ProtocolError) as ctx:
185+
maker(proto, code, types=[Parameter.IPv4_Address, item])
186+
self.assertEqual(str(ctx.exception), f'SCTP: invalid parameter type: {item!r}')
187+
188+
# An ``int`` and a ``Parameter`` resolve to their Parameter member.
189+
with self.subTest(maker=maker.__name__, item='valid'):
190+
self.assertEqual(maker(proto, code, types=[5, Parameter.IPv6_Address]).types,
191+
[Parameter.IPv4_Address, Parameter.IPv6_Address])
192+
163193
def test_checksum_is_the_little_endian_crc32c_of_the_zeroed_packet(self) -> None:
164194
import struct
165195

0 commit comments

Comments
 (0)