Skip to content

four MPTCP ProtocolError messages carry a doubled separator, rendering as 'TCP: : [OptNo 30] 1: ...' #649

Description

@JarryShaw

Four ProtocolError messages in the MPTCP option handlers carry a doubled : separator — f'{self.alias}: : [OptNo …]' — so they render as TCP: : [OptNo 30] 1: invalid flags combination with an empty field between the protocol alias and the option number. Cosmetic, but it is the string a user sees when an MP_JOIN or DSS option is rejected.

Measured on 375e9d411; pcapkit/protocols/transport/tcp.py is byte-identical on current origin/main (da381f259).

The sites — all four, all in pcapkit/protocols/transport/tcp.py

file:line Enclosing method Message
tcp.py:1564 _read_mptcp_join (def :1544) invalid flags combination
tcp.py:2818 _make_mptcp_join (def :2800) invalid flags combination
tcp.py:2984 _make_mptcp_dss (def :2944) missing required fields
tcp.py:2986 _make_mptcp_dss (def :2944) missing required fields

Verbatim, tcp.py:1564:

        raise ProtocolError(f'{self.alias}: : [OptNo {schema.kind}] {schema.subtype}: invalid flags combination')

and tcp.py:2818:

        raise ProtocolError(f'{self.alias}: : [OptNo {Enum_Option.Multipath_TCP}] {subtype}: invalid flags combination')

grep -rn "alias}: : " pcapkit/ returns nothing outside this file, so the defect is confined to these four lines.

Measured

Rendering the f-strings with the real values they are given — self.alias == 'TCP', kind == Option.Multipath_TCP (30), subtype == MPTCPOption.MP_JOIN (1):

tcp.py:1564 -> 'TCP: : [OptNo 30] 1: invalid flags combination'
tcp.py:2818 -> 'TCP: : [OptNo 30] 1: invalid flags combination'
tcp.py:2984 -> 'TCP: : [OptNo 30] 1: missing required fields'

Why it is a defect rather than a style preference

The rest of the file is consistent and does not do this. Counting the alias-prefixed messages in tcp.py:

doubled  '{self.alias}: : '  :  4
single   '{self.alias}: '    : 28

The 28 all read f'{self.alias}: [OptNo {schema.kind}] …' — e.g. tcp.py:1527, rendering as:

'TCP: [OptNo 30] invalid format'

So the four are the outliers against a 28-site house form in the same file.

Consequence

Purely presentational: the exception type, the option number and the subtype are all correct, and nothing downstream parses these strings. The cost is that a user reading TCP: : [OptNo 30] 1: sees an empty field and may reasonably wonder which value failed to interpolate.

Note the test that exercises the read-path message does not pin the prefix — tests/protocols/transport/test_tcp_mptcp_join_flag_ordering_unit.py:404 asserts only self.assertIn('invalid flags combination', str(caught.exception)) — so fixing the four lines needs no test change. Other tests quote the correct single-separator form in their docstrings (e.g. tests/protocols/transport/test_tcp_mptcp_subtype_unit.py:279, tests/protocols/transport/test_tcp_mptcp_capable_length_unit.py:195), which is further evidence of the intended shape.

Notes

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions