diff --git a/docs/source/pcapkit/protocols/transport/sctp.rst b/docs/source/pcapkit/protocols/transport/sctp.rst index dd0dcdd276..a394c1611b 100644 --- a/docs/source/pcapkit/protocols/transport/sctp.rst +++ b/docs/source/pcapkit/protocols/transport/sctp.rst @@ -55,6 +55,7 @@ Octets Bits Name Description .. automethod:: _read_sctp_causes .. automethod:: _make_sctp_causes .. automethod:: _make_sctp_cause + .. automethod:: _make_param_types .. automethod:: _read_chunk_donone .. automethod:: _read_chunk_data diff --git a/pcapkit/protocols/transport/sctp.py b/pcapkit/protocols/transport/sctp.py index 2af5492854..9a5b66dec1 100644 --- a/pcapkit/protocols/transport/sctp.py +++ b/pcapkit/protocols/transport/sctp.py @@ -1257,6 +1257,37 @@ def _make_nested_length(self, base: 'int', items: 'Sequence[Schema_Parameter | S return padded return padded - max(sizes[-1] - items[-1].length, 0) + def _make_param_types(self, types: 'Optional[list[Enum_Parameter | int]]') -> 'list[Enum_Parameter]': + """Resolve a list of parameter types. + + Shared by :meth:`_make_param_addrtypes` and + :meth:`_make_cause_missing_param`, whose ``types`` both list + parameter types. + + Args: + types: parameter types, each given as a + :class:`~pcapkit.const.sctp.parameter.Parameter` or an + :obj:`int` + + Returns: + The :class:`~pcapkit.const.sctp.parameter.Parameter` for each + item. + + Raises: + ProtocolError: If an item is a :obj:`bool`, or is neither a + :class:`~pcapkit.const.sctp.parameter.Parameter` nor an + :obj:`int` (:issue:`2288`). + + """ + types_value = [] # type: list[Enum_Parameter] + for item in types or []: + # NOTE: ``bool`` is an ``int`` subclass, so ``Parameter.get`` + # would look ``True`` up as parameter type 1 (#2288). + if isinstance(item, bool) or not isinstance(item, int): + raise ProtocolError(f'{self.alias}: invalid parameter type: {item!r}') + types_value.append(Enum_Parameter.get(item)) + return types_value + def _read_chunk_donone(self, schema: 'Schema_UnknownChunk', *, chunks: 'Chunks') -> 'Data_UnknownChunk': # pylint: disable=unused-argument """Read SCTP chunk of an unsupported type. @@ -2998,12 +3029,15 @@ def _make_param_addrtypes(self, code: 'Enum_Parameter', param: 'Optional[Data_Su Returns: Constructed parameter schema. + Raises: + ProtocolError: If an item of ``types`` is not a parameter type, + as :meth:`_make_param_types` checks. + """ if param is not None: types = list(param.types) - types_value = [Enum_Parameter.get(item) if isinstance(item, int) else item - for item in types or []] + types_value = self._make_param_types(types) return Schema_SupportedAddressTypesParameter( type=code, @@ -3561,6 +3595,10 @@ def _make_cause_missing_param(self, code: 'Enum_CauseCode', cause: 'Optional[Dat Returns: Constructed error cause schema. + Raises: + ProtocolError: If an item of ``types`` is not a parameter type, + as :meth:`_make_param_types` checks. + Note: The count of missing parameters is derived from ``types`` rather 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 if cause is not None: types = list(cause.types) - types_value = [Enum_Parameter.get(item) if isinstance(item, int) else item - for item in types or []] + types_value = self._make_param_types(types) return Schema_MissingMandatoryParameterCause( code=code, diff --git a/tests/protocols/transport/test_sctp_unit.py b/tests/protocols/transport/test_sctp_unit.py index 942d6dee10..d893424173 100644 --- a/tests/protocols/transport/test_sctp_unit.py +++ b/tests/protocols/transport/test_sctp_unit.py @@ -160,6 +160,36 @@ def test_constructed_ports_carry_the_same_type_as_parsed_ones(self) -> None: self.assertEqual(schema.dstport.port, 38412) self.assertEqual(schema.pack()[:4], b'\x26\xab\x96\x0c') + def test_parameter_type_lists_refuse_a_bool_and_a_non_int(self) -> None: + """#2288: both makers resolve ``types`` through one shared check. + + ``bool`` is an :obj:`int` subclass, and a ``Parameter`` member, an + :obj:`int`, a :obj:`str` or a :obj:`float` need distinct handling, so + every type in the list is checked by the same helper. + + """ + from pcapkit.const.sctp.cause_code import CauseCode + from pcapkit.const.sctp.parameter import Parameter + from pcapkit.protocols.transport.sctp import SCTP + from pcapkit.utilities.exceptions import ProtocolError + + proto = SCTP.__new__(SCTP) + makers = ( + (SCTP._make_param_addrtypes, Parameter.Supported_Address_Types), + (SCTP._make_cause_missing_param, CauseCode.Missing_Mandatory_Parameter), + ) + for maker, code in makers: + for item in (True, False, '5', 5.0, None): + with self.subTest(maker=maker.__name__, item=item): + with self.assertRaises(ProtocolError) as ctx: + maker(proto, code, types=[Parameter.IPv4_Address, item]) + self.assertEqual(str(ctx.exception), f'SCTP: invalid parameter type: {item!r}') + + # An ``int`` and a ``Parameter`` resolve to their Parameter member. + with self.subTest(maker=maker.__name__, item='valid'): + self.assertEqual(maker(proto, code, types=[5, Parameter.IPv6_Address]).types, + [Parameter.IPv4_Address, Parameter.IPv6_Address]) + def test_checksum_is_the_little_endian_crc32c_of_the_zeroed_packet(self) -> None: import struct