Skip to content

fix(schema): Schema.pack keeps option padding, SchemaField spans and iterated keys (#1289, #1291, #1292) - #1374

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1289-1291-1292-schema-roundtrip
Oct 8, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1289-1291-1292-schema-roundtrip

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — pylint/mypy/isort run on schema.py with the Makefile flags; no finding that origin/main does not already have
  • make test passes, and a test case covers the change — the modules listed below, not the whole suite
  • Added a changelog entry — N/A, added centrally

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Closes #1289
Closes #1291
Refs #1292 (the parse -> from_data remainder is #1380)

All three still reproduced on 13aa677be (after #1331). Fixes, all in protocols/schema/schema.py:

Contract changes: a bytes value shorter than a SchemaField's span now pads to the span (030704 becomes 0307000004), a behaviour change for user schemas. 'x' in schema and schema.get('x') now see post_process values. make() now pads an option area its options leave short.

Tests: new tests/protocols/schema/test_schema_roundtrip_unit.py 6 passed; with origin/main's schema.py 5 fail (the sixth is a guard that builtins stay unreadable). run_unittest_leg.py protocols/internet 529 tests OK; protocols --exclude protocols/internet 927 tests OK; tests/project 389 passed, 1 skipped; test_schema_unit.py 16 passed; test_option_roundtrip_unit.py 6 passed. Per-frame info over 1588 frames in 22 captures is identical to origin/main.

@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one review: running A cross-review is in flight against the current head - no verdict yet and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 7, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 96b32e369: NEEDS CHANGES — the closing keyword for #1292, not the code (Sonnet reviewed; author Opus).

#1292 is only half fixed. Parse → pack is now byte-exact (0302eeee04), but parse → from_data — the round trip the owner's ruling covers — still zero-fills the unread octets: Protocol.from_data (protocol.py:799-847) builds a fresh, never-unpacked schema, so it gives 0302000004, and test_schema_field_keeps_declared_width pins that. Change Closes #1292 to Refs #1292; the remainder is filed as #1380. Impact is user schemas only — none of the 446 shipped schema classes declares a SchemaField directly.

Held up:

Also say in the body: a bytes value shorter than a SchemaField's span now pads to the span (030704 → 0307000004) — a behaviour change for user schemas.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 7, 2026
…iterated keys

- pack: an OptionField records the part of its declared area the packed
  options leave unfilled as __option_padding__, the mirror of unpack, so
  TCP and IPv4 rebuild to the length their header declares (#1289).
- __getitem__: serves every key __iter__ yields, including values a
  post_process stores, so dict(schema) no longer raises KeyError (#1291).
- pack: a SchemaField packs its whole declared span, re-emitting the
  octets its nested schema left unread when the unpack buffer still holds
  them and zero-filling otherwise (refs #1292). A schema rebuilt through
  from_data still zero-fills them; that remainder is #1380.
@JarryShaw
JarryShaw force-pushed the fix/1289-1291-1292-schema-roundtrip branch from 96b32e3 to 9d920e2 Compare October 7, 2026 21:33
@JarryShaw

Copy link
Copy Markdown
Owner Author

Round-2 verdict on 9d920e244: GOOD TO GO. I checked the delta from 96b32e369 myself. The only file touched is tests/protocols/schema/test_schema_roundtrip_unit.py, with three added comment lines on the 0302000004 assertion naming #1380 as the open half. No source changed. The body now reads Closes #1289, Closes #1291 and Refs #1292, with the from_data remainder pointed at #1380. It also states that a short bytes value pads to its SchemaField span (030704 → 0307000004). The round-1 verification carries over.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 7, 2026
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.98% (unit tier, Python 3.14, 9d920e244, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18777 1032 2348 843 90.27%
pcapkit/corekit 1949 84 606 20 94.91%
pcapkit/dumpkit 189 1 68 1 99.22%
pcapkit/foundation 2443 102 852 39 94.39%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 16019 206 4182 177 98.06%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

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

@JarryShaw
JarryShaw merged commit 77cabf1 into main Oct 8, 2026
38 checks passed
@JarryShaw
JarryShaw deleted the fix/1289-1291-1292-schema-roundtrip branch October 8, 2026 00:56
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 8, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 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

1 participant