Skip to content

TCP._make_mptcp_addaddr cannot pack: kind/length rejected as fields, then the port predicate KeyErrors on length #541

Description

@JarryShaw

TCP._make_mptcp_addaddr cannot build an ADD_ADDR option end to end. Found while fixing #508; unrelated to that defect, so filed separately.

Measured on main at 122d32795, repo venv, PYTHONSAFEPATH=1, with pcapkit.__file__ asserted to be the repository tree:

>>> t._make_mptcp_addaddr(MPTCPOption.ADD_ADDR, addr_id=1, addr='1.2.3.4')
UnknownFieldWarning: 'kind' is not a valid field name
UnknownFieldWarning: 'length' is not a valid field name
schema: addr=1.2.3.4 port=None
>>> .pack()
KeyError: 'length'

Same result with port=443. The schema object constructs; only pack() fails.

The chain

TCP._make_mptcp_addaddr at pcapkit/protocols/transport/tcp.py:2898-2901 passes kind and length to Schema_MPTCPAddAddress:

return Schema_MPTCPAddAddress(
    kind=cast('Enum_Option', Enum_Option.Multipath_TCP),
    length=4 + (4 if version == 4 else 16) + (2 if port is not None else 0),
    ...

Both are rejected with UnknownFieldWarning, so neither lands in the packet mapping. Then pack() evaluates the port field's condition at pcapkit/protocols/schema/transport/tcp.py:792-794:

port: 'int' = ConditionalField(
    UInt16Field(),
    lambda pkt: pkt['length'] in (10, 22),
)

pkt['length'] is not there, so KeyError: 'length'.

Two things are wrong and it is worth deciding which is the root cause rather than patching the symptom:

  1. The maker computes a correct length and the schema will not accept it. If kind/length are meant to be declared fields on this schema (siblings appear to carry them), the declaration is missing. If they are meant to be supplied by an enclosing option writer, the maker should not be passing them.
  2. The port predicate depends on length, a field derived from whether port itself is present — a circularity that only works if length is materialised before the conditional is evaluated.

The UnknownFieldWarning is the useful signal here: it fires twice, well before the KeyError, and says exactly which fields are being dropped. Worth checking whether other _make_mptcp_* helpers pass kind/length the same way and are merely not exercised.

Coverage

No test builds an ADD_ADDR option through make, which is why this survived. A test that calls pack() — not merely constructs the schema — would have caught it, since construction succeeds and only packing fails. Whatever fix lands should assert on the packed bytes for both the port-present and port-absent cases, since the length arithmetic differs (10 versus 22 for IPv4 versus IPv6 with a port, per the predicate's own tuple).

Note

pcapkit/protocols/transport/tcp.py is touched by PR #539, which leaves this site alone deliberately and records it in a code comment. This wants its own change.

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