Skip to content

fix(corekit): read a counted list's cut item as a short read, not a negative length (#2749) - #2755

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2749-igmpv3-cut-record-warning
Nov 4, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2749-igmpv3-cut-record-warning

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

A standalone IGMPv3 Report cut inside a group record logged SchemaWarning: packet length < 0 once per field past the cut. It already read TRUNCATED, with the right short-read record and a byte-exact rebuild; only the warning was wrong.

  • ListField marks each schema item it counts with a __data_end__ key. Schema.unpack skips the negative-length warning for an item that carries it, so a cut counted item reads quietly.
  • Uncounted lists, items past the count and cut headers still warn. Frames parsed through IPv4 or Ethernet are unchanged, because the count is unset there. A probe of 1,920 parses on base and with the fix differed only in the removed warnings.
  • New tests: one cuts at every offset in the first, middle and last record, with controls that must still warn. One sweeps every schema-item list in the registry for the same cut. Reverting the fix fails both.

Closes #2749

…egative length (#2749)

- A standalone IGMPv3 Report cut inside a group record read TRUNCATED with its
  short-read record, and also warned `SchemaWarning: packet length < 0` for each
  field of the record past the cut. Report.records counts its records and runs
  to the end of the data, and each record is sized by the octets left to it, so
  Schema.unpack read the cut record's fields as overrunning a given length.
- ListField now reads a schema item it counts with `__data_end__` in its packet;
  Schema.unpack clears the key as it starts and warns no negative length for a
  schema whose length ends with the data. Uncounted lists, items past the count
  and cut headers warn as before; inside a frame the count is unset, so frames
  parsed through IPv4 are unchanged.
- Tests: the Report cut inside each record's fixed fields, source list and aux
  data, with a malformed record behind IPv4 still warning; a sweep over every
  list of schema items in the schema registry, with malformed-length controls.

Closes #2749
@JarryShaw JarryShaw added bug Issues reporting a defect (set by the bug report template; a default, not an assessment) review: running A cross-review is in flight against the current head - no verdict yet labels Nov 4, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Nov 4, 2026
@github-actions github-actions Bot added this to the PyPCAPKit v1.6 milestone Nov 4, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at b9f4f21db, from a Sonnet review.

  • Scope: only Report.records passes count=. __data_end__ lives in a packet copy that Schema.unpack pops before any field runs, so it cannot reach nested schemas or siblings. It appears in no info, to_dict or repr across 247 dumps.
  • Behaviour: 247 standalone and IPv4-framed cases, base vs head, differ only in the warnings. Status, short-read records, record counts and rebuild octets are identical, and no framed case changed.
  • The new modules fail on base (212 and 29 failures). The short-read, schema round-trip, IGMP and tests/corekit suites pass, and so do Project tests, Lint and deploy-pages.
  • Non-blocking: a standalone record with an overstated length now reads as a quiet cut, which it cannot be told apart from; it is still TRUNCATED. The phantom zero-filled record behind IPv4 predates this PR and is filed as fix(internet): a framed IGMPv3 Report cut inside a record parses a phantom zero-filled record #2756.

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

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Coverage: 94.45% (unit tier, Python 3.15, b9f4f21db, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 20918 175 2908 156 98.57%
pcapkit/corekit 2890 76 928 37 96.83%
pcapkit/dumpkit 286 3 102 3 98.45%
pcapkit/foundation 3505 27 1216 32 98.75%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 22786 155 5906 127 99.00%
pcapkit/toolkit 961 26 342 8 96.62%
pcapkit/utilities 559 7 150 5 98.31%
pcapkit/vendor 5590 2286 1280 214 55.75%

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

@JarryShaw
JarryShaw merged commit 453e388 into main Nov 4, 2026
72 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Nov 4, 2026
@JarryShaw
JarryShaw deleted the fix/2749-igmpv3-cut-record-warning branch November 4, 2026 04:25
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 4, 2026
JarryShaw added a commit that referenced this pull request Nov 4, 2026
- Fixed: a cut header's rebuild keeps the next layer its parse had, where
  1.5.x rebuilt a cut Ethernet header's Raw as NoPayload, and that Raw
  carries no error (#2759), a new entry; old values measured against v1.5.1.
- Amended unreleased entries (#2758): a cut list of schema items ends with
  the data, the item the cut lands inside zero-filled and none past it, in
  the ListField, IGMP and HTTP/2 SETTINGS entries, the last also rebuilding at
  its declared Length or a recomputed one once edited.
- Folded (#2755): a counted item the data ends inside reads as a quiet short
  read, into the ListField and IGMP entries.
- CHANGELOG.md regenerated; util/changelog_md.py --check passes.
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)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(internet): a standalone IGMPv3 Report cut inside a record warns 'packet length < 0', repeatedly

1 participant