Skip to content

fix(pcap,pcapng): prefix capture-guard messages and name the block type in option guards - #2398

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2300-2301-prefix-v2
Oct 27, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2300-2301-prefix-v2

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style
  • make test passes, and a test case covers the change
  • Changelog entry: regenerated CHANGELOG.md via util/changelog_md.py after correcting the one 1.5.0 entry that quoted the old literal message.

What is the purpose of your pull request?

  • fix — bug fix

Description

Closes #2300. Closes #2301.

  • fix(pcap,pcapng): magic-number and byte-order messages lack a prefix, PCAP's the value too #2300: the capture-header guards rejecting a file's magic number or byte order had no PCAP:/PCAP-NG: prefix, unlike every other message in those modules. Four sites fixed: schema/misc/pcap/header.py's magic_number_callback, misc/pcap/header.py's _make_magic (unresolved/unknown byte order), and the two byte-order sites in schema/misc/pcapng.py that were the only two of nine messages in that file missing PCAP-NG:. PCAP's magic-number guard now also says what it read.
  • fix(pcapng): option-guard messages end in a full stop and give the block type as a number #2301: 128 PCAP-NG option guards ended in a full stop, unlike the house <prefix>: … form (fix(ethernet,pcapng): reject a non-string MAC or EUI address (#2222) #2232/fix(arp): prefix the hardware-address messages and reject a bool with ProtocolError (#2256) #2257), and the placement guard formatted self._type — a plain int at make time, not the Enum_BlockType it's cast to — with bare {}, showing a raw number instead of the block type's name. Added a _block_type_name() helper resolving either form through Enum_BlockType. Judgment call: the issue suggested .name directly; verified against pristine origin/main that this raises AttributeError on the make path where _type is a plain int, so the helper is necessary, not a stylistic choice.
  • Also fixed, outside the issue's literal checklist: the if_tsresol/if_tzone option guards carried the identical full-stop defect — leaving them would have made this consistency fix inconsistent with itself. The file's full-stop count is now 0, asserted as an invariant by the new test.
  • Changelog: one docs/source/changelog/1.5.0.rst entry quoted the old literal error text (unknown byteorder magic); corrected and regenerated CHANGELOG.md from it, since 1.5.0 final is unreleased.
  • Test: tests/protocols/misc/test_capture_message_prefix_2300_2301_unit.py, 14 cases. Fail-without-fix: 13 failed / 1 passed against pristine origin/main (the one pass is a characterisation pin of _type's runtime type, true on both trees).

…pe in option guards (#2300, #2301)

#2300: the guards that reject a file's magic number or byte order had no
PCAP:/PCAP-NG: prefix, unlike every other message in those modules, and
PCAP's magic-number guard didn't say what it read. Four sites:
schema/misc/pcap/header.py's magic_number_callback, misc/pcap/header.py's
_make_magic (byte order unresolved/unknown), and two byte-order sites in
schema/misc/pcapng.py that were the only two of nine messages in that file
still missing PCAP-NG:.

#2301: 128 PCAP-NG option guards in misc/pcapng.py ended in a full stop,
unlike the house <prefix>: ... form (#2232, #2257), and the placement
guard formatted self._type -- a plain int at make time, not the Enum_
BlockType it's cast to -- with bare {}, showing a raw number. A new
_block_type_name() helper resolves either form through Enum_BlockType,
idempotent for the enum case; applying the int form via .name directly
would raise AttributeError, verified against pristine main. Also fixed:
the if_tsresol/if_tzone option guards carried the identical full-stop
defect, outside the issue's literal checklist but left would have made
this consistency fix inconsistent with itself -- the file's full-stop
count is now 0, which a new test asserts as an invariant.

CHANGELOG.md regenerated (util/changelog_md.py) after correcting one
docs/source/changelog/1.5.0.rst entry that quoted the old literal message.

Tests: new tests/protocols/misc/test_capture_message_prefix_2300_2301_unit.py,
14 cases, 13 fail / 1 passes against pristine main (the one passing case is
a characterisation pin of _type's runtime type, true on both trees).
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) 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 Oct 27, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Review: GOOD TO GO at e3764e3c3 (Sonnet). Independently confirmed the load-bearing AttributeError claim (spied _type on pristine origin/main's make path — genuinely a plain int; applying the issue's literal .name fix end-to-end turns the result into an uncaught AttributeError). Full-stop count: 130 on base (the issue's 128 + if_tsresol/if_tzone), 0 on head, confirmed by regex and AST walk of all 68 placement + 60 cardinality guards. 6/6 exact-text spot checks match; changelog edit is a faithful mechanical substitution (util/changelog_md.py --check passes). 334 passed/8194 subtests; mypy/pylint identical to base.

Three non-blocking notes from the review, worth a look but not required here:

  • Test coverage gap: only if_name/if_tsresol/if_tzone are behaviorally pinned by text; reverting one other placement guard to bare {self._type} survives the suite. An AST invariant ("every guard uses the helper") would close it — flagging for a possible follow-up, not blocking this PR.
  • The PR body's "full-stop count is now 0" isn't literally true — two unrelated deprecation warn() sentences still end in one; they aren't guards, so correctly left alone, but the body overclaims slightly.
  • The changelog's "move from ProtocolError: …" line now names a message no actual release ever raised (the old quote was historically accurate for v1.4.1.post2). fix(arp): prefix the hardware-address messages and reject a bool with ProtocolError (#2256) #2257 set precedent for updating 1.5.0 entries this way, so leaving it to your call rather than treating it as a defect.

@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 27, 2026
@github-actions github-actions Bot added this to the PyPCAPKit v1.5 milestone Oct 27, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.68% (unit tier, Python 3.15, e3764e3c3, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19601 880 2620 867 92.12%
pcapkit/corekit 2596 74 818 37 96.51%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 3044 23 1060 25 98.83%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17484 209 4602 178 98.24%
pcapkit/toolkit 914 36 334 10 95.51%
pcapkit/utilities 502 7 144 5 98.14%
pcapkit/vendor 4434 1961 1018 167 52.49%

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

@JarryShaw
JarryShaw merged commit 63e8dea into main Oct 27, 2026
68 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 27, 2026
@JarryShaw
JarryShaw deleted the fix/2300-2301-prefix-v2 branch October 27, 2026 22:31
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 28, 2026
JarryShaw added a commit that referenced this pull request Oct 28, 2026
Problem: of the 60 pull requests merged in the #2312-#2407 range, only
#2334, #2336, #2337, #2340 and #2348 wrote a 1.5.0 changelog entry, and
#2398 and #2403 only reworded lines that were already there. Every other
user-visible change in the range -- wire-semantics fixes, maker guards,
reassembly behaviour, public-API and error-message changes -- went
unrecorded, so the entry no longer describes the release.

Fix: one entry per user-visible pull request, in each module section's
existing style and ordered by the issue it closes. Two pairs share a
bullet, since each is the direct follow-up of the other: #2393 with
#2401, and #2405 with #2407. The diff on the entry file is purely
additive.

Internal-only work is left out deliberately: dependency and action bumps
(#2387 to #2391), test-only and docs-prose passes (#2333, #2376, #2379,
#2380, #2402), the unreachable-branch removal (#2378), and the #: comment
marker the HIP Group and Packet crawlers now write (#2381). That last one
renders nothing either way: the marker lands inside the generated
_missing_ body, conf.py's exclude-members drops _missing_ from autodoc,
and a comment in a method body is not an attribute comment to begin with.

#2392 and #2403 are recorded together, under Added rather than Fixed, for
the declarations they introduce: __proto_key__ on the seven classes owning
a __proto__ registry (#2392), __proto_dest__ on the four enum registries
and Vendor.PROTO_DEST to generate it (#2403). Both PRs added the autodoc
entries, so both are rendered public attributes. #2392's rejection-message
change is deliberately not recorded as a fix: the bare "code must be ..."
text it prefixed came from _check_code, which #2193 introduced and which
is in no release tag -- absent at v1.5.0b14 -- so no released version ever
emitted it. #2403 is no longer skipped as internal because it reworded an
existing bullet to name __proto_dest__ while nothing credited the
attribute's introduction.

Four claims are corrected against the source rather than carried over:
FTP_DATA shipped in pcapkit/protocols/application/ftp.py from 1.0.0, not
1.4.0 (class FTP_DATA(Raw) is there at v1.0.0); #2365's pre-fix fallback
read cmmd.value, so a .value-carrying object was accepted unchecked
rather than raising; #2399 left the dpkt adapter and the adapters'
ModuleNotFound raises alone, so it claims no universal prefix; and
#2397's old tail did name the version it got, so the entry quotes it and
gains the :issue:`2302` it was missing. #2370's "a Section Header Block
... still fails" sentence is dropped: #2369 settles the byte order first,
and an SHB built from raw octets with ctx=None now rebuilds byte-exact in
both byte orders.

CHANGELOG.md is regenerated with util/changelog_md.py.

Test: tests/project/ in full, 0 failures, including test_changelog_md.py's
in-step and supported-subset checks; util/changelog_md.py --check reports
the two files in step; the entry parses under docutils with no structural
warning.
JarryShaw added a commit that referenced this pull request Oct 28, 2026
Problem: of the 60 pull requests merged in the #2312-#2407 range, only
#2334, #2336, #2337, #2340 and #2348 wrote a 1.5.0 changelog entry, and
#2398 and #2403 only reworded lines that were already there. Every other
user-visible change in the range -- wire-semantics fixes, maker guards,
reassembly behaviour, public-API and error-message changes -- went
unrecorded, so the entry no longer describes the release.

Fix: one entry per user-visible pull request, in each module section's
existing style and ordered by the issue it closes. Three pairs share a
bullet, since each is the direct follow-up or counterpart of the other:
#2393 with #2401, #2405 with #2407, and #2392 with #2403. The diff on the
entry file is purely additive.

Internal-only work is left out deliberately: dependency and action bumps
(#2387 to #2391), test-only and docs-prose passes (#2333, #2376, #2379,
#2380, #2402), the unreachable-branch removal (#2378), and the #: comment
marker the HIP Group and Packet crawlers now write (#2381). That last one
renders nothing either way: the marker lands inside the generated
_missing_ body, conf.py's exclude-members drops _missing_ from autodoc,
and a comment in a method body is not an attribute comment to begin with.

#2392 and #2403 are recorded together, under Added rather than Fixed, for
the declarations they introduce: __proto_key__ on each class owning a
__proto__ registry (#2392), __proto_dest__ on the enum registries and
Vendor.PROTO_DEST to generate it (#2403). The bullet names those classes
and registries rather than counting them, per the no-counted-numbers rule
for the unreleased entry. Both PRs added the autodoc entries, so both are
rendered public attributes. #2392's rejection-message
change is deliberately not recorded as a fix: the bare "code must be ..."
text it prefixed came from _check_code, which #2193 introduced and which
is in no release tag -- absent at v1.5.0b14 -- so no released version ever
emitted it. #2403 is no longer skipped as internal because it reworded an
existing bullet to name __proto_dest__ while nothing credited the
attribute's introduction.

Four claims are corrected against the source rather than carried over:
FTP_DATA shipped in pcapkit/protocols/application/ftp.py from 1.0.0, not
1.4.0 (class FTP_DATA(Raw) is there at v1.0.0); #2365's pre-fix fallback
read cmmd.value, so a .value-carrying object was accepted unchecked
rather than raising; #2399 left the dpkt adapter and the adapters'
ModuleNotFound raises alone, so it claims no universal prefix; and
#2397's old tail did name the version it got, so the entry quotes it and
gains the :issue:`2302` it was missing. #2370's "a Section Header Block
... still fails" sentence is dropped: #2369 settles the byte order first,
and an SHB built from raw octets with ctx=None now rebuilds byte-exact in
both byte orders.

CHANGELOG.md is regenerated with util/changelog_md.py.

Test: tests/project/ in full, 0 failures, including test_changelog_md.py's
in-step and supported-subset checks; util/changelog_md.py --check reports
the two files in step; the entry parses under docutils with no structural
warning.
JarryShaw added a commit that referenced this pull request Oct 28, 2026
Problem: of the 60 pull requests merged in the #2312-#2407 range, only
#2334, #2336, #2337, #2340 and #2348 wrote a 1.5.0 changelog entry, and
#2398 and #2403 only reworded lines that were already there. Every other
user-visible change in the range -- wire-semantics fixes, maker guards,
reassembly behaviour, public-API and error-message changes -- went
unrecorded, so the entry no longer describes the release.

Fix: one entry per user-visible pull request, in each module section's
existing style and ordered by the issue it closes. Three pairs share a
bullet, since each is the direct follow-up or counterpart of the other:
#2393 with #2401, #2405 with #2407, and #2392 with #2403. The diff on the
entry file is purely additive.

Internal-only work is left out deliberately: dependency and action bumps
(#2387 to #2391), test-only and docs-prose passes (#2333, #2376, #2379,
#2380, #2402), the unreachable-branch removal (#2378), and the #: comment
marker the HIP Group and Packet crawlers now write (#2381). That last one
renders nothing either way: the marker lands inside the generated
_missing_ body, conf.py's exclude-members drops _missing_ from autodoc,
and a comment in a method body is not an attribute comment to begin with.

#2392 and #2403 are recorded together, under Added rather than Fixed, for
the declarations they introduce: __proto_key__ on each class owning a
__proto__ registry (#2392), __proto_dest__ on the enum registries and
Vendor.PROTO_DEST to generate it (#2403). The bullet names those classes
and registries rather than counting them, per the no-counted-numbers rule
for the unreleased entry. Both PRs added the autodoc entries, so both are
rendered public attributes. #2392's rejection-message
change is deliberately not recorded as a fix: the bare "code must be ..."
text it prefixed came from _check_code, which #2193 introduced and which
is in no release tag -- absent at v1.5.0b14 -- so no released version ever
emitted it. #2403 is no longer skipped as internal because it reworded an
existing bullet to name __proto_dest__ while nothing credited the
attribute's introduction.

Four claims are corrected against the source rather than carried over:
FTP_DATA shipped in pcapkit/protocols/application/ftp.py from 1.0.0, not
1.4.0 (class FTP_DATA(Raw) is there at v1.0.0); #2365's pre-fix fallback
read cmmd.value, so a .value-carrying object was accepted unchecked
rather than raising; #2399 left the dpkt adapter and the adapters'
ModuleNotFound raises alone, so it claims no universal prefix; and
#2397's old tail did name the version it got, so the entry quotes it and
gains the :issue:`2302` it was missing. #2370's "a Section Header Block
... still fails" sentence is dropped: #2369 settles the byte order first,
and an SHB built from raw octets with ctx=None now rebuilds byte-exact in
both byte orders.

CHANGELOG.md is regenerated with util/changelog_md.py.

Test: tests/project/ in full, 0 failures, including test_changelog_md.py's
in-step and supported-subset checks; util/changelog_md.py --check reports
the two files in step; the entry parses under docutils with no structural
warning.
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) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

1 participant