Skip to content

fix(protocol): main is red, IPv4 make() with an unparsable raw item no longer raises the wrapped error - #1395

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1394-main-red-make-raw-item
Oct 8, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1394-main-red-make-raw-item

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests

  • Followed the coding style — isort -l100 -ppcapkit clean; pylint/mypy on protocol.py report nothing new on the changed lines (the 117-col raise matches the pre-existing one)

  • make test passes, and a test case covers the change — only the modules listed below

  • Changelog entry — N/A, added centrally

  • fix — corrects a defect


Root cause. #1344 pads the IPv4 option area once, at the end. So [b'\x02', b'\x94\x04\x00\x00'] now packs as 02 94 …: option 2 claims length 148. The schema accepts that, and _read_ipv4_options rejects it at ipv4.py:659 (IPv4: invalid format). That check runs in read(), outside the try that #1352 put around the schema re-parse only (protocol.py:737-742). Before #1344, each item got its own padding, 02 01 01 00. That gave a negative data length, which raised inside the try.

Which side is right. #1352's side. The input is still malformed, so the test stays as it is. unpack() now also wraps read() when the made header holds raw items. The wrapped message adds the original error's text after a colon (… given to make(): IPv4: invalid format), so a regex on the reader's message still matches.

Other protocols. The same escape happened in TCP, HOPOPT, IPv6-Opts, MH, SCTP and HIP. A fuzz of 400 raw items per protocol on main let 517 bare read() errors out: IPv4 375, MH 70, TCP 36, HOPOPT 15, IPv6-Opts 13, SCTP 8. HIP's escape is the ESP_INFO case in the tests. With this fix, all of them are wrapped. Errors raised inside make() itself are unchanged (TCP _make_tcp_options enum lookup, IPv6-Route type check).

Tests. New module tests/protocols/test_make_raw_item_read_unit.py, with 8 cases that read() rejects, plus a well-formed guard. Revert proof: with git show origin/main:pcapkit/protocols/protocol.py written over the file, all 8 subtests fail, plus the original test_unparsable_raw_item_raises_protocol_error. With the fix, these pass:

  • test_protocol_make_unit.py: 12 passed
  • new module: 2 passed, 8 subtests
  • leg protocols/internet: 546 tests, OK
  • leg protocols --exclude protocols/internet: 968 tests, OK
  • tests/project: 389 passed, 1 skipped

Contract note: the ProtocolError text for a malformed raw make() item now ends with the original message.

Closes #1394

…o longer raises the wrapped error

#1344 pads the IPv4 option area once at the end, so the schema now
accepts octets read() then rejects (an option whose length runs past the
area), and the reader's bare ProtocolError escaped the wrap #1352 put
around the schema re-parse only. unpack() now wraps the read as well when
the made header holds raw items, and the wrapped message carries the
original one. The same escape is fixed for TCP, HOPOPT, IPv6-Opts, MH,
SCTP and HIP.

Closes #1394
@JarryShaw JarryShaw added the fix Pull requests that fix a defect (fix: subject prefix) label Oct 7, 2026
@JarryShaw JarryShaw added the review: running A cross-review is in flight against the current head - no verdict yet label Oct 7, 2026
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Coverage: 87.50% (unit tier, Python 3.14, 0c22e7d27, 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 2018 129 628 42 92.25%
pcapkit/dumpkit 189 1 68 1 99.22%
pcapkit/foundation 2443 347 852 103 82.40%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 16233 398 4276 261 96.57%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 429 47 122 2 85.30%
pcapkit/vendor 4409 2359 1006 158 42.84%

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

@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 0c22e7d27: GOOD TO GO (Sonnet reviewed; author Opus). This unblocks main (#1394).

  • Scope of the wrap. It applies only in the _make_raw and _holds_raw(...) branch, so make() without raw items is unchanged. It catches (BaseError, struct.error) only. BaseError is an Exception subclass, so KeyboardInterrupt, MemoryError and RecursionError propagate. raise … from error keeps __cause__.
  • The message change is needed. main's own test at test_protocol_make_unit.py:162 expects [ParamNo 65] invalid format in the message. I checked that line myself.
  • Tests. test_protocol_make_unit plus the new module give 14/0. The revert proof gives 9 failed: the original test and all 8 per-protocol cases (IPv4 overrun and short, TCP, HOPOPT, IPv6-Opts, MH, SCTP, HIP).
  • CI. Green at this head (38/0), and the branch is up to date with main.

Nit: a ProtocolError raised by read() gets wrapped again, so the message nests. __cause__ is still kept.

An earlier review run went past its time budget and was stopped before reporting; this verdict comes from a narrower re-run.

@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 8, 2026
@JarryShaw
JarryShaw merged commit 970fed9 into main Oct 8, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1394-main-red-make-raw-item branch October 8, 2026 01:07
@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

Development

Successfully merging this pull request may close these issues.

fix(protocol): main is red, IPv4 make() with an unparsable raw item no longer raises the wrapped error

1 participant