Skip to content

docs(pep,tests): correct the checksum protocol count and the registry-leak lineage #1007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JarryShaw merged 1 commit into from
Oct 3, 2026
Merged

docs(pep,tests): correct the checksum protocol count and the registry-leak lineage #1007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions docs/source/contributing/pep.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,18 +741,20 @@ is worth knowing before benchmarking against them: ``pyshark``, ``pypcap`` and
Checksum and Integrity Verification
-----------------------------------

Eight protocols parse a checksum or CRC field —
Ten protocols parse a checksum or CRC field —
:class:`~pcapkit.protocols.internet.hip.HIP`,
:class:`~pcapkit.protocols.internet.hopopt.HOPOPT`,
:class:`~pcapkit.protocols.internet.ipv4.IPv4`,
:class:`~pcapkit.protocols.internet.ipv6_opts.IPv6_Opts`,
:class:`~pcapkit.protocols.internet.ipx.IPX`,
:class:`~pcapkit.protocols.internet.mh.MH`,
:class:`~pcapkit.protocols.link.ospf.OSPF`,
:class:`~pcapkit.protocols.transport.sctp.SCTP`,
:class:`~pcapkit.protocols.transport.tcp.TCP` and
:class:`~pcapkit.protocols.transport.udp.UDP` — and exactly one of them checks
whether the value is *right*:
:attr:`SCTP.checksum_valid <pcapkit.protocols.transport.sctp.SCTP.checksum_valid>`.
For the other seven the field is recorded and never questioned, so a corrupted
For the other nine the field is recorded and never questioned, so a corrupted
capture parses as cleanly as an intact one.

**These are two different problems and they want separating**, because only one
Expand Down
4 changes: 2 additions & 2 deletions pcapkit/corekit/sentinels.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,8 @@ class NoDefaultType:
compares by value rather than identity, and reload staleness is a
*tracked* defect class here for other constructs -- see
:meth:`pcapkit.protocols.protocol.ProtocolBase._lookup_next_layer`'s own
docstring note citing GitHub issues :issue:`425` and :issue:`555`, and
:mod:`tests.protocols.test_dispatch_default_resolution_unit`'s own
docstring note citing GitHub issues :issue:`421`, :issue:`425` and
:issue:`555`, and :mod:`tests.protocols.test_dispatch_default_resolution_unit`'s own
``test_no_stale_class_survives_a_module_reload``, which reloads a module
deliberately to pin the fix for exactly that class of bug elsewhere. A
*future* comparison site written the vulnerable way -- a bare ``is
Expand Down
6 changes: 4 additions & 2 deletions pcapkit/protocols/protocol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1741,8 +1741,10 @@ def _lookup_next_layer(registry: 'DefaultDict[int, ModuleDescriptor[ProtocolBase
:func:`importlib.import_module` -- see :issue:`574`. Memoising the resolved
class here instead, whether under ``proto``, in ``registry``'s
default factory, or in a cache beside the registry, would retain a
class that :func:`importlib.reload` then makes stale; :issue:`425` at
this layer and :issue:`555` at the schema layer are all that same defect.
class that :func:`importlib.reload` then makes stale; :issue:`421` at
this layer, :issue:`425` for the option, chunk and block registries
beside it, and :issue:`555` at the schema layer are all that same
defect.

"""
protocol = ProtocolBase._lookup_registry(registry, proto)
Expand Down
9 changes: 5 additions & 4 deletions tests/protocols/test_dispatch_default_resolution_unit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
produces -- normally :class:`~pcapkit.protocols.misc.raw.Raw`. That resolution is
deliberately **not** written back into the registry, because the registry is a
class-level :class:`collections.defaultdict` and recording a miss in it is the
defect GitHub issue #425 reported and pull request #428 fixed at this layer, and #560 fixed at the schema
layer. The cost of not writing it back is that every unrecognised frame resolves
the same descriptor again: 48 of the 52 resolutions an extraction of
:file:`many_interfaces.pcapng` performs.
defect GitHub issue #421 reported and pull request #426 fixed at this layer,
pull request #428 extended to the option, chunk and block registries, and pull
request #560 fixed at the schema layer. The cost of not writing it back is that
every unrecognised frame resolves the same descriptor again: 48 of the 52
resolutions an extraction of :file:`many_interfaces.pcapng` performs.

Proposed by @Ts-Boom in GitHub pull request #563, which paid that cost with a
class-level cache of resolved classes and no invalidation -- so a
Expand Down
Loading