Skip to content

fix(foundation): reject a registrar code from the wrong enum (#2224) - #2245

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2224-registrar-enum-type
Oct 26, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2224-registrar-enum-type

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — bug fix

Description

Closes #2224. _check_code accepted a member of any enum, so register_linktype(TransType.ICMP, X) replaced the shipped LinkType.ETHERNET entry, since ICMP is 1.

  • Fix: _check_code(code, kind, *, number=False) passes only isinstance(code, kind). With number=True it also passes a plain int. Each registrar passes the enum it is annotated with, and those const imports move from TYPE_CHECKING to runtime.
  • Int-accepting registrars: register_tcp, register_udp and register_apptype (AppType) and register_sctp (PayloadProtocolIdentifier) keep plain int. Per the owner's ruling on fix(foundation): registrars accept a member of the wrong enum as the code #2224, they now refuse anything else, such as None or '80'. Their Raises: sections say so.
  • Test: tests/foundation/registry/test_registrar_code_check_unit.py has one wrong-enum case per registrar. Where the value types allow, the wrong member is equal by value to one of the registrar's own members. Registries are snapshotted and restored.
  • Fail-without-fix: on origin/main, 24 failed.
  • Tests run: tests/foundation gives 522 passed. The dispatch registry, docstring contract and tier guard give 147 passed.

Problem: _check_code only required some enum member. The registry enums
compare equal to their values, so a member of another enum was stored
and could replace a shipped entry, e.g. register_linktype(TransType.ICMP,
...) overwrote Frame.__proto__[LinkType.ETHERNET].

Fix: _check_code takes the enum the registrar is typed on and checks the
type, not equality. register_tcp, register_udp, register_sctp and
register_apptype call it too, with number=True, so a plain int is still
accepted but an IntEnum member of another enum is not.

Test: test_registrar_code_check_unit gains a wrong-enum case for every
registrar, each equal by value to one of its own members where possible,
asserting RegistryError with every registry snapshotted and unchanged.
@JarryShaw JarryShaw added bug Issues reporting a defect (set by the bug report template; a default, not an assessment) fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 26, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Review: GOOD TO GO at 039216bdf (Sonnet).

  • Imports: no cycles; cold import pcapkit and import pcapkit.foundation.registry.protocols succeed. -X importtime: head 508–525 ms against main 506–523 ms.
  • Refusal: each of the 24 registrars checks its annotated enum. TransType.ICMP to register_linktype, register_pcap or register_ethertype raises RegistryError. register_tcp(None) and register_tcp('80') are refused, per the ruling.
  • Fail-without-fix: 24 failed on origin/main. On the head, the foundation, dispatch registry, docstring contract and tier guard tests give 669 passed.
  • Not blocking: register_tcp(True) passes as an int.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 26, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.56% (unit tier, Python 3.15, 039216bdf, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19599 881 2618 867 92.11%
pcapkit/corekit 2589 74 816 37 96.51%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 3019 23 1042 25 98.82%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17433 218 4584 185 98.15%
pcapkit/toolkit 869 36 298 10 95.20%
pcapkit/utilities 504 7 146 5 98.15%
pcapkit/vendor 4432 1988 1018 167 51.87%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Ready to merge at 039216bdf. Closes #2224 (labelled fix, not breaking, per the owner's ruling on #2224).

  • Review: Sonnet GOOD TO GO, first round. No import cycles and no import-time cost.
  • CI: all checks complete, Required checks passed SUCCESS, 0 failed.
  • KTLO: 12 legs, no warnings.
  • Ordering: merged into main and run. foundation leg: 743 tests, 0 failures.
  • Conflicts: none against the current main.

@JarryShaw
JarryShaw merged commit 20c8b2f into main Oct 26, 2026
70 checks passed
@JarryShaw
JarryShaw deleted the fix/2224-registrar-enum-type branch October 26, 2026 14:51
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 26, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issues reporting a defect (set by the bug report template; a default, not an assessment) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(foundation): registrars accept a member of the wrong enum as the code

1 participant