Skip to content

Parsing a packet pollutes the shared __proto__ registry, so a later register_* warns about a protocol nobody registered #421

Description

@JarryShaw

ProtocolBase._import_next_layer indexes a defaultdict at
pcapkit/protocols/protocol.py:1300, so a lookup miss inserts the key. The
registry is class-level and shared, so ordinary parsing mutates it.

Reproduced on main:

ICMP in Internet.__proto__ before: False
ICMP in Internet.__proto__ after:  True
register_transtype now warns: ['protocol 1 already registered, overwriting']

Parsing a single 24-byte IPv4 packet carrying proto=1 inserts
TransType.ICMP -> Raw into Internet.__proto__. A subsequent, entirely
legitimate register_transtype(TransType.ICMP, …) then reports that ICMP was
"already registered, overwriting" — about a registration that never happened.

Same pattern at pcapkit/protocols/internet/internet.py:248 and
pcapkit/protocols/internet/ipv6.py:409.

Consequences

  • A spurious RegistryWarning that sends the reader looking for a conflicting
    registration that does not exist.
  • The registry grows one entry per distinct unknown code seen, for the process
    lifetime, on a class attribute shared by every instance.
  • It makes "is this protocol registered?" un-answerable by inspection, because
    the answer depends on what has been parsed.

There is already a template for the fix

SCTP._import_next_layer overrides the base method precisely to avoid this and
documents why at pcapkit/protocols/transport/sctp.py:818-831. The same
treatment is wanted for Frame, PCAPNG, Link, Internet, IPv6, TCP and
UDP — or, better, in the base method so each subclass does not have to
remember.

Note this is related to but distinct from the __output__ defaultdict
discussed earlier: there the injection is load-bearing memoisation, deliberately
kept. Here nothing depends on the insertion — the value inserted is just Raw.

Found while auditing for #419.

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