Skip to content

fix(protocols): restoring a registration puts back the shipped ModuleDescriptor (#1504) - #1533

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1504-registrar-restore-descriptor
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1504-registrar-restore-descriptor

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

Every next-layer register resolved a ModuleDescriptor to its class before storing it. The #1364 guard only compares with the incumbent, so after an override, handing back the shipped descriptor stored the class.

  • ProtocolBase.__init_subclass__ snapshots a class's own __proto__ as _shipped_proto. The new _next_layer_entry returns the shipped object, unresolved, for a descriptor equal to the one that code shipped with. Any other descriptor still resolves to its class, as fix(registry): re-registering a stored ModuleDescriptor warns and replaces it with the class #1364's pins expect.
  • ProtocolBase, Link, Internet, Transport (TCP/UDP), SCTP and Frame.register use it. Each keeps its own warn, because tests patch the module-level name.
  • Not fixed: PCAPNG.register (misc/pcapng.py:1021-1035), which this change leaves alone. Gap(1504) narrows to register_linktype/register_pcapng, so this is not Closes.
  • New: tests/protocols/test_register_restores_shipped_descriptor_1504_unit.py, covering 7 site families. It checks the round trip, that the restore imports nothing (a blocked import), and that dispatch resolves the entry when it runs. On main, 28 of 42 subtests fail; here all pass.

Ran (sequentially), all green: tests/foundation/registry, the dispatch and registry modules in tests/protocols, tests/protocols/{link,internet,transport,misc,application}, tests/project, tests/test_docstring_contract.py and tests/test_tier_guard.py.

Part of #1504

…Descriptor (#1504)

- ProtocolBase: snapshot each class's own __proto__ at class creation as
  _shipped_proto, and add _next_layer_entry, which decides what a register
  stores: None for the stored entry (#1364), the shipped descriptor object
  itself, unresolved, for a descriptor equal to the one the code shipped
  with, else the resolved and checked class.
- ProtocolBase, Link, Internet, Transport, SCTP and Frame register use it
  and keep their own warning, so undoing an override leaves the entry a
  descriptor and imports nothing.
- PCAPNG.register (misc/pcapng.py) is not changed here; the Gap(1504) row
  narrows to register_linktype and register_pcapng.
- New per-site tests for the round trip, the restore importing nothing, and
  dispatch resolving the restored entry when it runs.

Part of #1504
@JarryShaw JarryShaw added 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 9, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at e549c9162. Cross-reviewed on Sonnet; the author ran Opus.

  • Reproduced on main, fixed here for TCP, Link, SCTP and Internet. After a restore, the entry is the shipped ModuleDescriptor itself, and nothing gets imported: import_attempts=[] with the module blocked.
  • The snapshot is safe. _shipped_proto is a read-only mapping proxy. The one entry added at import time, Shim6 at ipv6_ext.py:670, is a class, so a restore resolves it to the same object. A subclass that captures an override can only capture a class, which the ModuleDescriptor guard ignores. The worst case is therefore a missed restore, the old behaviour, never a wrong one.
  • Equality is (module, name) namedtuple equality, and it survives pickle, copy and deepcopy.
  • The narrowed Gap(1504) row holds exactly 6 labels, and the check catches deviations both ways. The fix(registry): re-registering a stored ModuleDescriptor warns and replaces it with the class #1364 tests keep their meaning.
  • Tests: 28 of 42 fail before the fix, and two mutants are caught.

Non-blocking:

@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 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Coverage: 90.08% (unit tier, Python 3.14, e549c9162, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 843 2356 756 92.08%
pcapkit/corekit 2161 66 700 33 96.26%
pcapkit/dumpkit 242 2 88 2 98.79%
pcapkit/foundation 2665 98 934 42 95.00%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 17074 238 4604 190 97.98%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 431 4 124 4 98.56%
pcapkit/vendor 4409 2342 1006 157 43.25%

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

@JarryShaw
JarryShaw merged commit 6fadec1 into main Oct 9, 2026
39 checks passed
@JarryShaw
JarryShaw deleted the fix/1504-registrar-restore-descriptor branch October 9, 2026 23:46
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant