Skip to content

fix(schema): finalise each non-final Schema subclass on its own state (#1498) - #1499

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1498-schema-child-finalisation
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1498-schema-child-finalisation

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

Schema.__new__ read the inherited __finalised__, so once a non-final parent had been built its non-final children were never finalised and to_dict() leaked __map__, __map_reverse__, __buffer__ and __updated__. The check now reads cls.__dict__.get('__finalised__', FinalisedState.NONE), mirroring #1496 for Info. The docstring Warning describing the @final-under-BASE escape becomes a Note, since that gap closes too.

Shipped instances: the Quick-Start option schemas of HOPOPT, IPv6-Opts and IPv4 (non-final children of non-final Option).

tests/protocols/schema/test_schema_child_finalisation_1498_unit.py runs both construction orders in fresh interpreters, for a synthetic pair and the three shipped ones. It also covers a child declared after its parent's BASE, and a bare @final under a BASE parent. On origin/main's schema.py: 6 failed (4 parent-first subtests and the 2 direct tests). The child-first subtests pass there, as expected.

Run here with an FSIZE cap: the new test, tests/test_final_enforcement.py, tests/corekit/test_schema_roundtrip_unit.py, tests/protocols/internet, tests/protocols/transport, tests/protocols/test_capture_roundtrip_runtime.py, tests/project and tests/test_docstring_contract.py. All pass. Per-file hashes of repr(frame.info.to_dict()) over the 23 examples/captures files are identical on both trees.

Closes #1498

…#1498)

Schema.__new__ read __finalised__ through inheritance, so once a non-final
parent had been built its non-final children read the parent's BASE and
were never finalised: they kept the __excluded__ copied at declaration and
to_dict() leaked __map__, __map_reverse__, __buffer__ and __updated__.
Read the class's own __dict__ instead, as #1496 does for Info. This also
closes the documented gap where a bare @Final under a BASE ancestor
escaped the guard, so its Warning becomes a Note.

The shipped instances are the Quick-Start option schemas of HOPOPT,
IPv6-Opts and IPv4.
@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 68728b13a. Cross-reviewed on Sonnet; the author ran on Opus.

  • The fix works. I measured it myself: on origin/main's schema.py the new test gives 6 failed, 1 passed; at head it gives 3 passed, 8 subtests.
  • Broader than the PR's own cases. Building all 446 Schema subclasses parent-first leaks internal keys from 36 classes on main, among them HIP Parameter and HostIdentity, HTTP/2 FrameType, and several Options. At head the count is 0. Child-first gives 0 on both trees. At head, __init__, __fields__ and __excluded__ match between the two orders for every class.
  • Nothing shipped trips the new refusal. The census raises 0 errors. Unpacking all 446 classes with empty and 64-zero-byte buffers gives identical outcomes on both trees, with 0 SchemaError.
  • Same approach as fix(corekit): finalise each non-final Info subclass on its own state (#1490) #1496, with no shared file (git merge-tree clean).
  • Regression runs: the per-capture to_dict() hashes are identical across all 23 captures. Internet, transport and tests/project give 1111 passed; the enforcement and schema round trips give 39 passed.

Non-blocking: the NOTE in schema_final (schema.py:167-179) still explains a BASE-inheritance hazard that the own-__dict__ read removes. #1496's info_final has the same note.

@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: 89.65% (unit tier, Python 3.14, 68728b13a, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 958 2356 871 90.99%
pcapkit/corekit 2115 66 680 33 96.17%
pcapkit/dumpkit 242 2 88 2 98.79%
pcapkit/foundation 2648 98 930 48 94.80%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 17086 238 4628 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 a608b6e into main Oct 9, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1498-schema-child-finalisation branch October 9, 2026 20:42
@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.

fix(schema): a non-final Schema subclass built after its parent leaks internal keys from to_dict()

1 participant