Skip to content

MH MN-ID option sizes its identifier from the Python type rather than the subtype, so even the default arguments mis-declare length #448

Description

@JarryShaw

MH._make_opt_mn_id decides how many octets the identifier occupies by inspecting the Python type of the identifier argument, not the subtype that determines the wire format. So for the IPv6_Address subtype, anything other than an ipaddress.IPv6Address declares a length that disagrees with what the schema packs — including the method's own default arguments.

Measured on f50436a8a

Option layout is type (1) + length (1) + subtype (1) + identifier (N), and the length field counts everything after itself, so a correct option satisfies len(packed) == length + 2.

identifier argument declared length packed octets length + 2
'::' — the default 3 19 5 mismatch
'2001:db8::1' (str) 12 19 14 mismatch
ipaddress.ip_address('2001:db8::1') 17 19 19 correct

subtype is Enum_MNIDSubtype.IPv6_Address in all three, which is also its default. So _make_opt_mn_id(type) called with no identifier at all emits a 19-octet option whose length field says 3.

Mechanism

pcapkit/protocols/internet/mh.py:3395-3400:

if isinstance(identifier, ipaddress.IPv6Address):
    id_len = 16
elif isinstance(identifier, int):
    id_len = math.ceil(identifier.bit_length() / 8)
else:
    id_len = len(identifier)

The schema's identifier field packs a 16-octet address whenever the subtype is IPv6_Address, regardless of which Python type carried the value in — '2001:db8::1' is 11 characters, so len(identifier) gives 11 and the option declares 12 where 17 is right. The int branch has the same flaw: an address passed as an integer is sized by its bit length, so math.ceil(bit_length/8) is 1 for ::1.

The signature advertises all four types — identifier: 'bytes | str | IPv6Address | int' = '::' — so three of the four documented input types produce a malformed option for the default subtype, and the default value is one of them.

Fix

Size from subtype_val, which the method has already resolved a few lines above, rather than from the runtime type of identifier: for IPv6_Address the answer is always 16, and the other subtypes size from the encoded identifier. Normalising identifier to its wire form first and then taking len() of that would work for every subtype at once, and would also make the packed bytes and the declared length derive from a single value instead of two independent computations — which is what let them disagree.

Provenance

Reported but deliberately not fixed by #437 (see its "Reported, not fixed"), as pre-existing and outside that PR's registry-completion scope. Recorded in docs/source/pep.rst until now; that page tracks feature requests, not defects, so it is filed here instead.

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

    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions