fix(reg)!: reject an out-of-range port in AppType.get, and claim each span by transport - #764
Merged
Conversation
JarryShaw
added
fix
… span by transport * `get`'s `except ValueError` caught the range rejection `_missing_` raises and minted regardless, so `AppType.get(-1, proto='tcp')` returned `PORT_-1_tcp` -- a `__members__` key no attribute access can reach -- while `TCP(-1)` raised. It now tests `_missing_` for `None`, which is its "no row for this port" answer, and lets the rejection through, so both entry points raise the same `ValueError`. A valid but unassigned port still mints, which is what `get` is for. Caller-visible: `Transport._make_port` passes an unvalidated `int`, so `TCP.make(srcport=99999)` now raises where it minted junk. * the crawler rendered one `_missing_` branch per registry row, so the two spans IANA registers on more than one transport emitted the same condition twice and the second copy was unreachable. Every registry answered with the first: `AppType.get(6010, proto='udp')` gave a UDP member whose `proto` read `tcp`, `6666/udp` answered `ircu` rather than `reserved`, and `proto='sctp'` minted `x11` where IANA assigns nothing. A branch naming a transport now tests `cls.__transport__`; the 762 naming none answer every registry as before. * regenerated `pcapkit/const/reg/apptype/` from the crawler: 4 conditions changed in `apptype.py`, and `tcp.py`, `udp.py`, `sctp.py`, `dccp.py` byte-identical. 3 pins added, each red on 932cb48 (12, 3 and 4 failures) and green here. tests/const 55 passed, transport 66, vendor + tier guard 143, registry callers 36, 2,519 subtests; pylint 9.90/10 unchanged, mypy clean, isort clean.
JarryShaw
force-pushed
the
fix-apptype-get-range-and-span-transport-758-760
branch
from
September 25, 2026 00:14
8395f08 to
2c2dcab
Compare
JarryShaw
added
review: pending
Owner
Author
|
Cross-review verdict: GOOD TO GO (sonnet, a different model from the author). I re-derived the behavioural claims myself rather than relaying them. On the fixed tree, against The regression-safety claim is exhaustive, not sampled. A full 262,144-cell sweep (65,536 ports × 4 registries × both trees) found exactly 207 changed pairs, all confined to Determinism is established, with one framing correction. There is no pinned snapshot artifact in the repo — that wording is not literally reproducible. What was done: IANA's CSV fetched once, further network access blocked, the same input reused throughout. The load-bearing check is that diffing
Two description defects found and fixed rather than left: the #706 precedent was cited in error (it moves the opposite way — resolving an unassigned value instead of raising), and an "8,478 unguarded ports" figure I quoted in the review brief turns out to appear nowhere in this PR; it came from a working note, so it is not a claim this PR makes and should not be attributed to it. Unpublished and awaiting the owner. |
JarryShaw
deleted the
fix-apptype-get-range-and-span-transport-758-760
branch
September 25, 2026 01:36
This was referenced Sep 25, 2026
Closed
JarryShaw
removed
the
review: good-to-go
JarryShaw
added a commit
that referenced
this pull request
Sep 25, 2026
…ting (#575) - TCP/UDP/SCTP's PortEnumField and PCAP-NG's OptionEnumField called their registry's .get() directly on every parsed value, which mints a fresh member via aenum.extend_enum for any value no row or documented span covers. On http.pcap that is 111 calls and ~14% of extraction self time, entirely from unassigned ephemeral ports. - All four now peek at what .get() would consult -- the registry's own rows, then AppType's declared IANA spans via _missing_, or OptionType's per-namespace membership -- and only call .get() once one of those already holds. A genuine miss falls through to the bounded, nameless pseudo-member EnumField.post_process already builds for #701, carrying .port/.svc/.proto (or .opt_name/.opt_value) so downstream attribute access keeps working. - A bare ValueError (aenum's, or #764's out-of-range guard) is absorbed as before; a pcapkit.utilities.exceptions rejection still propagates unchanged. AppType.get() and every _missing_ are unmodified, so direct get() callers still mint. extend_enum calls on http.pcap: 111 (13.6-14.1%) -> 0. many_interfaces.pcapng: 15 -> 2, both from the untouched _missing_ mechanism. New unit tests cover the no-mint path, the in-library-rejection path, and the round trip; each fails without this change. coverage run -m pytest and python -m unittest agree; make isort clean.
12 tasks
JarryShaw
added a commit
that referenced
this pull request
Sep 25, 2026
… calls
- apptype.py:717 dropped `: {self.NAME!r}` from each member's assignment.
Every annotated member put a forward-ref string into AppType's
__annotations__, and Sphinx's default autodoc re-resolves the whole
class once per member -- 378.8s on CI, the largest stall in the docs
build. Also drops the now-redundant `# mypy: disable-error-code=
assignment` pragma that annotation forced on all five generated files.
- apptype.py's flag() now emits `TransportProtocol.tcp` instead of
`TransportProtocol.get('tcp')` -- 21x cheaper per call, 23,941 of the
23,942 call sites on this checkout rewritten (the 23,942nd, in
_dispatch, resolves a caller-supplied string and stays). Also covers
the 4 range-row conditions #764 added to records(). get() itself is
untouched; other callers still use it.
- Regenerated all five files under pcapkit/const/reg/apptype/ from one
pinned IANA snapshot. The unmodified crawler reproduced the current
files byte-identically first; the diff after both changes is
mechanical throughout.
- Updated the #760 regression test's regex for the new attribute form
and tightened its trailing match, and added generator-level pins for
both changes.
Fixes #744. Fixes #768.
Regenerated from one pinned IANA snapshot; unmodified crawler reproduced
the committed files byte-identically before either change landed.
Exhaustive member-set comparison (name/port/svc/proto) across all 12,391
members in AppType/TCP/UDP/SCTP/DCCP: identical. make isort clean.
JarryShaw
added a commit
that referenced
this pull request
Sep 25, 2026
…ting (#575) - TCP/UDP/SCTP's PortEnumField and PCAP-NG's OptionEnumField called their registry's .get() directly on every parsed value, which mints a fresh member via aenum.extend_enum for any value no row or documented span covers. On http.pcap that is 111 calls and ~14% of extraction self time, entirely from unassigned ephemeral ports. - All four now peek at what .get() would consult -- the registry's own rows, then AppType's declared IANA spans via _missing_, or OptionType's per-namespace membership -- and only call .get() once one of those already holds. A genuine miss falls through to the bounded, nameless pseudo-member EnumField.post_process already builds for #701, carrying .port/.svc/.proto (or .opt_name/.opt_value) so downstream attribute access keeps working. - The three port fields check the value against the field's own declared byte width (self.length) before consulting the registry at all, so a port outside that width still raises #764's deliberate rejection instead of being absorbed alongside a genuine in-range miss -- both are a bare ValueError with no way to tell them apart by type alone. A pcapkit.utilities.exceptions rejection still propagates unchanged. AppType.get() and every _missing_ are unmodified, so direct get() callers still mint. - dictdumper.plist.PLIST (which 'xml' also maps to) writes <string>/<key> content with no entity escaping at all (JarryShaw/DictDumper#125, tracked as #772), so the pseudo-member's <unknown>/<unassigned> naming broke plist/xml reports. make_dumper's object_hook now escapes &, < and > for PLIST-rooted output only, leaving json/tree/text untouched. extend_enum calls on http.pcap: 111 (13.6-14.1%) -> 0. many_interfaces.pcapng: 15 -> 2, both from the untouched _missing_ mechanism. New unit tests cover the no-mint, out-of-width, in-library-rejection, round-trip and escaping paths; each fails without its fix. coverage run -m pytest and python -m unittest agree; make isort clean. Fixes #575.
JarryShaw
added a commit
that referenced
this pull request
Sep 25, 2026
… calls (#769) - apptype.py:717 dropped `: {self.NAME!r}` from each member's assignment. Every annotated member put a forward-ref string into AppType's __annotations__, and Sphinx's default autodoc re-resolves the whole class once per member -- 378.8s on CI, the largest stall in the docs build. Also drops the now-redundant `# mypy: disable-error-code= assignment` pragma that annotation forced on all five generated files. - apptype.py's flag() now emits `TransportProtocol.tcp` instead of `TransportProtocol.get('tcp')` -- 21x cheaper per call, 23,941 of the 23,942 call sites on this checkout rewritten (the 23,942nd, in _dispatch, resolves a caller-supplied string and stays). Also covers the 4 range-row conditions #764 added to records(). get() itself is untouched; other callers still use it. - Regenerated all five files under pcapkit/const/reg/apptype/ from one pinned IANA snapshot. The unmodified crawler reproduced the current files byte-identically first; the diff after both changes is mechanical throughout. - Updated the #760 regression test's regex for the new attribute form and tightened its trailing match, and added generator-level pins for both changes. Fixes #744. Fixes #768. Regenerated from one pinned IANA snapshot; unmodified crawler reproduced the committed files byte-identically before either change landed. Exhaustive member-set comparison (name/port/svc/proto) across all 12,391 members in AppType/TCP/UDP/SCTP/DCCP: identical. make isort clean.
JarryShaw
added a commit
that referenced
this pull request
Sep 25, 2026
#575) - TCP/UDP/SCTP's PortEnumField and PCAP-NG's OptionEnumField called their registry's .get() directly on every parsed value, which mints a fresh member via aenum.extend_enum for any value no row or documented span covers. On http.pcap that is 111 calls and ~14% of extraction self time, entirely from unassigned ephemeral ports. - All four now peek at what .get() would consult -- the registry's own rows, then AppType's declared IANA spans via _missing_, or OptionType's per-namespace membership -- and only call .get() once one already holds. A genuine miss gets EnumField._unregistered_member: an instance built by calling the registry's own storage base's __new__ (str or int, picked per namespace) directly, skipping the registry's own __new__ and the registration inside it, so isinstance holds and nothing is ever added to any lookup table. Per the owner's ruling, tracked more broadly as #775 and applied here only to these four call sites. - The three port fields check the value against the field's own declared byte width (self.length) before consulting the registry at all, so a port outside that width still raises #764's deliberate rejection instead of being absorbed alongside a genuine in-range miss. A pcapkit.utilities.exceptions rejection still propagates unchanged. AppType.get() and every _missing_ are unmodified, so direct get() callers still mint. - dictdumper.plist.PLIST (which 'xml' also maps to) writes <string>/<key> content with no entity escaping at all (JarryShaw/DictDumper#125, tracked as #772), so the unregistered member's rendering broke plist/xml reports. make_dumper's object_hook now escapes &, < and > for PLIST-rooted output only, leaving json/tree/text untouched. extend_enum calls on http.pcap: 111 (13.6-14.1%) -> 0. many_interfaces.pcapng: 15 -> 2, both from the untouched _missing_ mechanism. New unit tests cover no-mint, isinstance, out-of-width, equality-without-identity, value-lookup still raising, copy/deepcopy, in-library-rejection, round-trip and escaping; each fails without its fix. coverage run -m pytest and python -m unittest agree; make isort clean. Fixes #575.
JarryShaw
added a commit
that referenced
this pull request
Sep 25, 2026
#575) - TCP/UDP/SCTP's PortEnumField and PCAP-NG's OptionEnumField called their registry's .get() directly on every parsed value, which mints a fresh member via aenum.extend_enum for any value no row or documented span covers. On http.pcap that is 111 calls and ~14% of extraction self time, entirely from unassigned ephemeral ports. - All four now peek at what .get() would consult -- the registry's own rows, then AppType's declared IANA spans via _missing_, or OptionType's per-namespace membership -- and only call .get() once one already holds. A genuine miss gets EnumField._unregistered_member: an instance built by calling the registry's own storage base's __new__ (str or int, picked per namespace) directly, skipping the registry's own __new__ and the registration inside it, so isinstance holds and nothing is ever added to any lookup table. Per the owner's ruling, tracked more broadly as #775 and applied here only to these four call sites. - The three port fields check the value against the field's own declared byte width (self.length) before consulting the registry at all, so a port outside that width still raises #764's deliberate rejection instead of being absorbed alongside a genuine in-range miss. A pcapkit.utilities.exceptions rejection still propagates unchanged. AppType.get() and every _missing_ are unmodified, so direct get() callers still mint. - dictdumper.plist.PLIST (which 'xml' also maps to) writes <string>/<key> content with no entity escaping at all (JarryShaw/DictDumper#125, tracked as #772), so the unregistered member's rendering broke plist/xml reports. make_dumper's object_hook now escapes &, < and > for PLIST-rooted output only, leaving json/tree/text untouched. extend_enum calls on http.pcap: 111 (13.6-14.1%) -> 0. many_interfaces.pcapng: 15 -> 2, both from the untouched _missing_ mechanism. New unit tests cover no-mint, isinstance, out-of-width, equality-without-identity, value-lookup still raising, in-library-rejection, round-trip and escaping; each fails without its fix. Also measured, not tested: copy/deepcopy of the returned member work on Python 3.11+ (stdlib Enum.__copy__/__deepcopy__ return self there) but raise on 3.10 exactly as pickle does on every version, since neither is reachable from any reader in this package. coverage run -m pytest and python -m unittest agree; make isort clean. Fixes #575.
JarryShaw
added a commit
that referenced
this pull request
Sep 25, 2026
#575) - TCP/UDP/SCTP's PortEnumField and PCAP-NG's OptionEnumField called their registry's .get() directly on every parsed value, which mints a fresh member via aenum.extend_enum for any value no row or documented span covers. On http.pcap that is 111 calls and ~14% of extraction self time, entirely from unassigned ephemeral ports. - All four now peek at what .get() would consult -- the registry's own rows, then AppType's declared IANA spans via _missing_, or OptionType's per-namespace membership -- and only call .get() once one already holds. A genuine miss gets EnumField._unregistered_member: an instance built by calling the registry's own storage base's __new__ (str or int, picked per namespace) directly, skipping the registry's own __new__ and the registration inside it, so isinstance holds and nothing is ever added to any lookup table. Per the owner's ruling, tracked more broadly as #775 and applied here only to these four call sites. - The three port fields check the value against the field's own declared byte width (self.length) before consulting the registry at all, so a port outside that width still raises #764's deliberate rejection instead of being absorbed alongside a genuine in-range miss. A pcapkit.utilities.exceptions rejection still propagates unchanged. AppType.get() and every _missing_ are unmodified, so direct get() callers still mint. - The extra attributes are passed in the order the registry's own __new__ sets them -- svc, port, proto for AppType and opt_name, opt_value for OptionType -- because make_dumper renders a member's addon keys straight out of its __dict__ in insertion order. Any other order dumps an unassigned value's keys the opposite way round from every declared one's, in all four formats, while leaving every value correct: 1117 rendered port blocks of each kind on http.pcap alone. - Enum.__reduce_ex__ reduces a member to (cls, (value,)), the one lookup an unregistered member is absent from, so _unregistered_member installs a __reduce_ex__ of its own that rebuilds an equivalent unregistered member. Without it, removing the mint would have taken away a pickle round-trip that worked before -- and taken it away on read-back, since dumps succeeds either way. copy/deepcopy reduce through it too on 3.10, where Enum.__copy__/__deepcopy__ do not exist. - dictdumper.plist.PLIST (which 'xml' also maps to) writes <string>/<key> content with no entity escaping at all (JarryShaw/DictDumper#125, tracked as #772), so the unregistered member's rendering broke plist/xml reports. make_dumper's object_hook now escapes &, < and > for PLIST-rooted output only, leaving json/tree/text untouched. extend_enum calls on http.pcap: 111 (13.6-14.1%) -> 0. many_interfaces.pcapng: 15 -> 2, both from the untouched _missing_ mechanism. Wall-clock extract(nofile=True), 7 fresh processes per tree at host load 1.29 -> 1.27: http.pcap 0.952 -> 0.679 s, many_interfaces.pcapng 0.130 -> 0.103 s. New unit tests cover no-mint, isinstance, out-of-width, equality-without-identity, value-lookup still raising, in-library-rejection, round-trip, escaping, addon key order, _unregistered_member's int and TypeError branches, and the pickle/copy round-trip; each fails without its fix. coverage run -m pytest and python -m unittest agree. Fixes #575.
JarryShaw
added a commit
that referenced
this pull request
Sep 25, 2026
…k included `main` moved from 73f09ae to 4530424 while this PR sat open, and the 1.5.0 section cited none of the 25 commits in between. Ten new bullets cover thirteen of them, appended in merge order, with the file's own `**a breaking change**` lead sentence on the three that are breaking: - #754 -- AppType split into per-transport registries; the 1,004 portless and 704 transportless rows stop being members. Breaking. - #764 -- an out-of-range port in `AppType.get` is refused, not minted, so `TCP.make(srcport=99999)` raises; per-transport `_missing_` spans. Breaking. - #778 -- `@final` enforced at runtime on `Info`/`Schema`: bare `@final` raises `InfoError`/`SchemaError` at first construction, deriving from a finalised class raises, and `SchemaError` is a `ValueError` where a caller may have been catching `TypeError`. Breaking. - #772 (with #790's docstring reword), #766, #759, #787, #794 (with #791's citation repoint), #792/#798 and #802 -- the remaining seven. Also re-ran the citation sweep against `origin/main` rather than the checkout. One stale line number fixed: the `httpv2.py` `header.length != 9` guard the `#692` entry calls out moved from `:562` to `:650` under #789 and #802. The preamble's "reaching #726" becomes #805, the new maximum reference. Verified unmoved on 4530424: `protocol.py:1411`, `schema/internet/ipv4.py:336`, `traceflow.py` 146/149/162/424, the four `:type:` fields in `engine.rst`, `reassembly.rst` and `traceflow.rst`, and `EXPECTED_FAILURES` at 43 entries. Carries the previous round's #651/#646 corrections unchanged. Two literals were reflowed so no ``literal`` wraps a line, which the generator's residual guard refuses. `changelog_md.py --check` exit 0; `test_changelog_md.py` 47 passed, 37 subtests.
JarryShaw
added a commit
that referenced
this pull request
Sep 26, 2026
`main` moved from 73f09ae to 3cbdf89 while this PR sat open. This commit (relative to its parent, 4233555) now names all 20 bullets it carries, not just the 8 this session added on top of the 10 already there -- a first draft of this message named only its own 8 and left the other 10 silent, which a cross-review caught. Four are breaking: - #754 -- AppType split into per-transport registries; the 1,004 portless and 704 transportless rows stop being members. - #764 -- an out-of-range port in `AppType.get` is refused, not minted. - #778 -- `@final` enforced at runtime on `Info`/`Schema`. - #575 -- four `.get()`-backed enum fields fall through to `_unregistered_member` instead of minting; 14 of 23 sample captures change output. - #772 (with #790's docstring reword), #766, #759, #787, #794 (with #791's citation repoint), #792/#798, #802, #782 (a further #745-hazard instance), #704, #723, #739, #743/#746 (cross-dependent, one bullet each), #805 (closes #802's own filed-as-out-of-scope), #796, #800 -- the other 16, non-breaking. Also restores a measurement an earlier round in this same diff dropped while updating an adjacent one: the #692 entry's "mypy is unmoved at 112 errors" silently lost its "and pylint ... 364 messages" half when `EXPECTED_FAILURES` was corrected 44 to 43 elsewhere in the same sentence. Restored to the last value earlier rounds signed off on rather than re-measured, since this branch's own `pcapkit/` tree predates several since-merged PRs and a fresh run would not be measuring the same thing the original round measured. Unmoved, and not silently dropped this time: `93 of the 95 sites` corrected to `94 of the 95` and `EXPECTED_FAILURES` 44 to 43 in the several other places that already carried the fix. On which PRs get a bullet: there is no clean "user-facing only" rule -- #766 and #791 are pure CI/test/lint-comment entries that are in, while #773 and #763 are the same kind of thing and are out. The real pattern across this file's 46 commits is closer to "each round's author judged it worth a reader's time," which is inconsistent by construction. This round leaves that inconsistency as found rather than trying to retrofit a rule, but did add #782 on reconsideration -- its own PR body names it as sharing #766's hazard, and pre-existing precedent already treats that hazard's instances as bullet-worthy. `changelog_md.py` regenerated `CHANGELOG.md`; `--check` exit 0. `test_changelog_md.py` 47 passed.
JarryShaw
added a commit
that referenced
this pull request
Sep 26, 2026
`main` moved from 73f09ae to 3cbdf89 while this PR sat open. This commit (relative to its parent, 4233555) now names all 20 bullets it carries, not just the 8 this session added on top of the 10 already there -- a first draft of this message named only its own 8 and left the other 10 silent, which a cross-review caught. Six are breaking, matching the crediting PRs' own `breaking` label in each case: - #754 -- AppType split into per-transport registries; the 1,004 portless and 704 transportless rows stop being members. - #764 -- an out-of-range port in `AppType.get` is refused, not minted. - #778 -- `@final` enforced at runtime on `Info`/`Schema`. - #575 -- four `.get()`-backed enum fields fall through to `_unregistered_member` instead of minting; 14 of 23 sample captures change output. - #759 -- `AppType._dispatch` on a multi-transport `proto` now raises `ProtocolError` instead of silently resolving to whichever transport owns the lowest set bit. - #805 -- `FieldBase.length` on a negative resolved length now raises `ProtocolError` instead of letting a bare `struct.error` escape. A second cross-review caught both: their crediting PRs (#783, #811) both carry GitHub's own `breaking` label, and neither bullet said so. - #772 (with #790's docstring reword), #766, #787, #794 (with #791's citation repoint), #792/#798, #802, #779 (via #782, a further #745-hazard instance), #704, #723, #739, #743/#746 (cross-dependent, one bullet each), #796, #800 -- the other 14, non-breaking. Also restores a measurement an earlier round in this same diff dropped while updating an adjacent one: the #692 entry's "mypy is unmoved at 112 errors" silently lost its "and pylint ... 364 messages" half when `EXPECTED_FAILURES` was corrected 44 to 43 elsewhere in the same sentence. Restored to the last value earlier rounds signed off on rather than re-measured, since this branch's own `pcapkit/` tree predates several since-merged PRs and a fresh run would not be measuring the same thing the original round measured. Unmoved, and not silently dropped this time: `93 of the 95 sites` corrected to `94 of the 95` and `EXPECTED_FAILURES` 44 to 43 in the several other places that already carried the fix. On which PRs get a bullet: there is no clean "user-facing only" rule -- #766 and #791 are pure CI/test/lint-comment entries that are in, while #773 and #763 are the same kind of thing and are out. The real pattern across this file's 46 commits is closer to "each round's author judged it worth a reader's time," which is inconsistent by construction. This round leaves that inconsistency as found rather than trying to retrofit a rule, but did add #779 (via #782) on reconsideration -- its own PR body names it as sharing #766's hazard, and pre-existing precedent already treats that hazard's instances as bullet-worthy. `changelog_md.py` regenerated `CHANGELOG.md`; `--check` exit 0. `test_changelog_md.py` 47 passed.
JarryShaw
added a commit
that referenced
this pull request
Sep 26, 2026
`main` moved from 73f09ae to 3cbdf89 while this PR sat open. This commit (relative to its parent, 4233555) now names all 20 bullets it carries, not just the 8 this session added on top of the 10 already there -- a first draft of this message named only its own 8 and left the other 10 silent, which a cross-review caught. Six are breaking, matching the crediting PRs' own `breaking` label in each case: - #754 -- AppType split into per-transport registries; the 1,004 portless and 704 transportless rows stop being members. - #764 -- an out-of-range port in `AppType.get` is refused, not minted. - #778 -- `@final` enforced at runtime on `Info`/`Schema`. - #575 -- four `.get()`-backed enum fields fall through to `_unregistered_member` instead of minting; 14 of 23 sample captures change output. - #759 -- `AppType._dispatch` on a multi-transport `proto` now raises `ProtocolError` instead of silently resolving to whichever transport owns the lowest set bit. - #805 -- `FieldBase.length` on a negative resolved length now raises `ProtocolError` instead of letting a bare `struct.error` escape. A second cross-review caught both: their crediting PRs (#783, #811) both carry GitHub's own `breaking` label, and neither bullet said so. - #772 (with #790's docstring reword), #766, #787, #794 (with #791's citation repoint), #792/#798, #802, #779 (via #782, a further #745-hazard instance), #704, #723, #739, #743/#746 (cross-dependent, one bullet each), #796, #800 -- the other 14, non-breaking. Also restores a measurement an earlier round in this same diff dropped while updating an adjacent one: the #692 entry's "mypy is unmoved at 112 errors" silently lost its "and pylint ... 364 messages" half when `EXPECTED_FAILURES` was corrected 44 to 43 elsewhere in the same sentence. Restored to the last value earlier rounds signed off on rather than re-measured, since this branch's own `pcapkit/` tree predates several since-merged PRs and a fresh run would not be measuring the same thing the original round measured. Unmoved, and not silently dropped this time: `93 of the 95 sites` corrected to `94 of the 95` and `EXPECTED_FAILURES` 44 to 43 in the several other places that already carried the fix. On which PRs get a bullet: there is no clean "user-facing only" rule -- #766 and #791 are pure CI/test/lint-comment entries that are in, while #773 and #763 are the same kind of thing and are out. The real pattern across this file's 46 commits is closer to "each round's author judged it worth a reader's time," which is inconsistent by construction. This round leaves that inconsistency as found rather than trying to retrofit a rule, but did add #779 (via #782) on reconsideration -- its own PR body names it as sharing #766's hazard, and pre-existing precedent already treats that hazard's instances as bullet-worthy. `changelog_md.py` regenerated `CHANGELOG.md`; `--check` exit 0. `test_changelog_md.py` 47 passed.
JarryShaw
added a commit
that referenced
this pull request
Sep 26, 2026
`main` moved from 73f09ae to 3cbdf89 while this PR sat open. This commit (relative to its parent, 4233555) now names all 20 bullets it carries, not just the 8 this session added on top of the 10 already there -- a first draft of this message named only its own 8 and left the other 10 silent, which a cross-review caught. Six are breaking, matching the crediting PRs' own `breaking` label in each case: - #754 -- AppType split into per-transport registries; the 1,004 portless and 704 transportless rows stop being members. - #764 -- an out-of-range port in `AppType.get` is refused, not minted. - #778 -- `@final` enforced at runtime on `Info`/`Schema`. - #575 -- four `.get()`-backed enum fields fall through to `_unregistered_member` instead of minting; 14 of 23 sample captures change output. - #759 -- `AppType._dispatch` on a multi-transport `proto` now raises `ProtocolError` instead of silently resolving to whichever transport owns the lowest set bit. - #805 -- `FieldBase.length` on a negative resolved length now raises `ProtocolError` instead of letting a bare `struct.error` escape. A second cross-review caught both: their crediting PRs (#783, #811) both carry GitHub's own `breaking` label, and neither bullet said so. - #772 (with #790's docstring reword), #766, #787, #794 (with #791's citation repoint), #792/#798, #802, #779 (via #782, a further #745-hazard instance), #704, #723, #739, #743/#746 (cross-dependent, one bullet each), #796, #800 -- the other 14, non-breaking. Also restores a measurement an earlier round in this same diff dropped while updating an adjacent one: the #692 entry's "mypy is unmoved at 112 errors" silently lost its "and pylint ... 364 messages" half when `EXPECTED_FAILURES` was corrected 44 to 43 elsewhere in the same sentence. Restored to the last value earlier rounds signed off on rather than re-measured, since this branch's own `pcapkit/` tree predates several since-merged PRs and a fresh run would not be measuring the same thing the original round measured. Unmoved, and not silently dropped this time: `93 of the 95 sites` corrected to `94 of the 95` and `EXPECTED_FAILURES` 44 to 43 in the several other places that already carried the fix. On which PRs get a bullet: there is no clean "user-facing only" rule -- #766 and #791 are pure CI/test/lint-comment entries that are in, while #773 and #763 are the same kind of thing and are out. The real pattern across this file's 46 commits is closer to "each round's author judged it worth a reader's time," which is inconsistent by construction. This round leaves that inconsistency as found rather than trying to retrofit a rule, but did add #779 (via #782) on reconsideration -- its own PR body names it as sharing #766's hazard, and pre-existing precedent already treats that hazard's instances as bullet-worthy. `changelog_md.py` regenerated `CHANGELOG.md`; `--check` exit 0. `test_changelog_md.py` 47 passed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
make pylint,make mypy,make isort)make testpasses, and a test case covers the change — bounded selections instead, counts belowWhat is the purpose of your pull request?
fix— corrects a defectfeat— adds a featureperf— changes performance, not behaviourrefactor— changes neither behaviour nor performancetest— tests onlydocs— documentation onlyci— workflows or build toolingchore— anything elseDescription of your pull request and other information
Fixes #758. Fixes #760. One PR: both live in
_missing_and the crawler that emits it.AppType.get(-1, proto='tcp')PORT_-1_tcp,len(TCP)6147→6148ValueError: -1 is not a valid TCPAppType.get(6010, proto='udp')<UDP.x11: 6010 [tcp]><UDP.x11: 6010 [udp]>AppType.get(6666, proto='udp')<UDP.ircu: 6666 [tcp]><UDP.reserved: 6666 [udp]>AppType.get(6010, proto='sctp')<SCTP.x11: 6010 [tcp]>PORT_6010_sctp#758 —
getraises rather than returns._missing_'sNonemeans "no row for this port"; itsValueErrormeans "not a port".getcaught the second and minted anyway, so it was more permissive thanTCP(...). Dropping theexceptand testing forNonemakesgetpropagate_missing_'s own error verbatim — exact parity, not an approximation — and leaves the mint for a valid unassigned port alone, which is whatgetis documented for.#760 — claim by transport, not merge. Merging needs one combined transport, and #732's
TransportProtocolretype would then want a named combination for it: work to be undone. It is also wrong on the merits —6665-6669isircuon TCP and IANA'sreservedon UDP, two different services, so one merged branch has to discard one. So each branch naming a transport testscls.__transport__. The 762 naming none are unchanged, and SCTP/DCCP are fixed too, which the duplicate only made visible on TCP/UDP.breaking: yes.Transport._make_port(pcapkit/protocols/transport/transport.py:187) passes an unvalidatedint, soTCP.make(srcport=99999)now raises where it minted junk. Parsing cannot reach it — the port fields unpack unsigned 16-bit.SCTP(6010)also raises now, exactly asSCTP(51)already did. Precedent: #677 carries the label for the same shape -- a registry gaining a guard that makes a previously-minting call raise. (#706 was cited here in error: it moves the opposite way, resolving an unassigned value instead of raising.)Regeneration. Crawler re-run from one pinned IANA snapshot, and on
932cb48d1the unmodified crawler reproduces all five const files byte-identically from it — so the entire diff is this change: 4 conditions inapptype.py, withtcp.py,udp.py,sctp.pyanddccp.pybyte-identical.Evidence. 3 pins, each red on
932cb48d1(12, 3 and 4 failures) and green here; the third asserts the invariant over all 766 branches rather than the 2 spans that collide today, which is what survives the next crawl.tests/const55, transport 66,tests/vendor+ tier guard 143, registry callers 36 — 300 tests / 2,519 subtests,pytestandunittestagreeing. Coverage ofpcapkit/const/reg/apptype/apptype.py51.516% → 51.752% from the same file. pylint 9.90/10 (unchanged; itsduplicate-codereports are pre-existing), mypy and isort clean. Merged--no-ffonto2146a54a6in a scratch worktree: 274 tests / 2,436 subtests green.One deliberate omission: a crawler-side unit test would want a new file under
tests/vendor/plus a gate intests/_dependency_gates.py, which #755 owns, so the invariant is pinned on the generated output instead.