Repository navigation
docs: #719-style prose sweep after the #1090-1102 defect wave - #2408
Conversation
|
This PR's branch name and scope (maker-message wave #2296/#2404/#2405/#2407, the
Not fixed, flagged only (code-level, out of scope for a prose-only pass):
Full diff of the above (13 files, prose/comments only, AST-guard verified against this branch's current head): patchdiff --git a/pcapkit/foundation/reassembly/ip.py b/pcapkit/foundation/reassembly/ip.py
index 45b233061..05f864974 100644
--- a/pcapkit/foundation/reassembly/ip.py
+++ b/pcapkit/foundation/reassembly/ip.py
@@ -67,9 +67,9 @@ class IP(ReassemblyBase[Packet[_AT], Datagram[_AT], BufferID, Buffer[_AT]], Gene
<pcapkit.foundation.reassembly.data.ip.Datagram.conflict>`
(:issue:`477`). An incomplete buffer expires
after ``timeout`` seconds of capture time, 60 by default for both IPv4
- and IPv6. A datagram with fragment offset zero and **MF** clear is
- reported even when its BUFID is already buffered, rather than dropped
- (:issue:`2270`).
+ and IPv6. A datagram with fragment offset zero and **MF** (**M** in
+ IPv6) clear is reported even when its BUFID is already buffered,
+ rather than dropped (:issue:`2270`).
"""
if TYPE_CHECKING:
@@ -156,8 +156,10 @@ class IP(ReassemblyBase[Packet[_AT], Datagram[_AT], BufferID, Buffer[_AT]], Gene
# belong to.
self._dtgram.extend(self.expire(TS))
- # when non-fragmented (possibly discarded) packet received: it is
- # reassembled on its own, in a buffer of its own (#2270)
+ # a datagram with fragment offset zero and MF clear is reported even
+ # when its BUFID is already buffered, rather than dropped: isolated
+ # from that buffer (IPv6 atomic fragments) or after flushing it first
+ # (IPv4) (#2270)
isolated = False
if not FO and not MF:
if self.__isolate_atomic__:
diff --git a/pcapkit/foundation/registry/protocols.py b/pcapkit/foundation/registry/protocols.py
index 3a32851cf..3d5e0770e 100644
--- a/pcapkit/foundation/registry/protocols.py
+++ b/pcapkit/foundation/registry/protocols.py
@@ -366,9 +366,9 @@ def _register_name(protocol: 'Type[ProtocolBase]', *targets: 'tuple[type, Any]')
releasing whatever each target code held before, and followed by a lease
when the write changed the name, so that registering the code back undoes
it (:issue:`1505`). A name already showing ``protocol`` -- shipped, defined
- by a :class:`~pcapkit.protocols.protocol.Protocol` subclass, or registered
- directly -- is not leased, so no restore takes it away; one showing it
- through a lease extends that lease to ``targets``.
+ by a :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass, or
+ registered directly -- is not leased, so no restore takes it away; one
+ showing it through a lease extends that lease to ``targets``.
Args:
protocol: Protocol class just registered -- and so validated -- at ``targets``.
@@ -565,12 +565,15 @@ def register_protocol_code(protocol: 'Type[ProtocolBase]', code: 'Any') -> 'None
def _check_code(code: 'Any', kind: 'type', target: 'Type[ProtocolBase]', *,
number: 'bool' = False) -> 'None':
- """Reject a registration key that is not a member of the registrar's enum.
+ """Reject a registration key that is not a member of the enum its
+ destination registry is keyed by.
Each registrar typed on one enum calls this before touching any registry,
- so a bad ``code`` leaves every registry as it was (:issue:`2186`). The check is on the type,
- not the value: the registry enums compare equal to their values, so a
- member of another enum would key the entry of the member it equals, e.g.
+ as does :func:`register_protocol_code` for an inferred or explicitly
+ named destination, so a bad ``code`` leaves every registry as it was
+ (:issue:`2186`). The check is on the type, not the value: the registry
+ enums compare equal to their values, so a member of another enum would
+ key the entry of the member it equals, e.g.
:attr:`TransType.ICMP <pcapkit.const.reg.transtype.TransType.ICMP>` would
replace :attr:`LinkType.ETHERNET <pcapkit.const.reg.linktype.LinkType.ETHERNET>`
(:issue:`2224`).
@@ -580,7 +583,7 @@ def _check_code(code: 'Any', kind: 'type', target: 'Type[ProtocolBase]', *,
kind: Enum the registrar is typed on.
target: Protocol class whose registry ``code`` is destined for, named in
the error message so a rejection says *whose* registry rejected the
- key rather than only what it wanted -- the registrars share one
+ key rather than only what it wanted -- its callers share one
message, and several of them are typed on the same enum.
number: Whether a bare :class:`int` is a valid key too, as for the
registrars keyed on a port number or a payload protocol identifier.
diff --git a/pcapkit/protocols/application/httpv1.py b/pcapkit/protocols/application/httpv1.py
index f1c48b70e..ad0336555 100644
--- a/pcapkit/protocols/application/httpv1.py
+++ b/pcapkit/protocols/application/httpv1.py
@@ -224,7 +224,8 @@ class HTTP(HTTPBase[Data_HTTP, Schema_HTTP],
Raises:
ProtocolError: If the values are neither a request's nor a
- response's, or if ``raw_line`` or a ``raw_header`` entry that
+ response's, if they are a request's and ``uri`` is
+ :obj:`None`, or if ``raw_line`` or a ``raw_header`` entry that
reads back as the given values carries a CR or LF that the
values do not (other than the CRLF of an ``obs-fold``).
@@ -328,7 +329,8 @@ class HTTP(HTTPBase[Data_HTTP, Schema_HTTP],
Raises:
ProtocolError: If the values are neither a request's nor a
- response's.
+ response's, or if they are a request's and ``uri`` is
+ :obj:`None`.
"""
version = http_version.encode() if isinstance(http_version, str) else http_version
diff --git a/pcapkit/protocols/internet/hip.py b/pcapkit/protocols/internet/hip.py
index cad41bf60..163c328aa 100644
--- a/pcapkit/protocols/internet/hip.py
+++ b/pcapkit/protocols/internet/hip.py
@@ -1955,8 +1955,9 @@ class HIP(IPv6_Ext[Data_HIP, Schema_HIP], Internet[Data_HIP, Schema_HIP],
An explicit ``exponent`` is the octet and wins, as a parsed parameter's
is; otherwise ``lifetime`` is a duration and
- :meth:`_make_registration_lifetime` encodes it. Shared by the three
- :rfc:`8003` makers, which between them carry four such octets.
+ :meth:`_make_registration_lifetime` encodes it. Shared by the
+ :rfc:`8003` registration makers, wherever a ``Lifetime`` octet
+ appears.
Args:
code: parameter code
diff --git a/pcapkit/protocols/internet/internet.py b/pcapkit/protocols/internet/internet.py
index a64b5c769..5e986a7a8 100644
--- a/pcapkit/protocols/internet/internet.py
+++ b/pcapkit/protocols/internet/internet.py
@@ -182,7 +182,7 @@ class Internet(ProtocolBase[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=ab
Arguments:
code: protocol code as in :class:`~pcapkit.const.reg.transtype.TransType`
protocol: module descriptor or a
- :class:`~pcapkit.protocols.protocol.Protocol` subclass
+ :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass
Raises:
pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a
diff --git a/pcapkit/protocols/link/link.py b/pcapkit/protocols/link/link.py
index cc4d148af..228a38b71 100644
--- a/pcapkit/protocols/link/link.py
+++ b/pcapkit/protocols/link/link.py
@@ -138,7 +138,7 @@ class Link(ProtocolBase[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=abstra
Arguments:
code: protocol code as in :class:`~pcapkit.const.reg.ethertype.EtherType`
protocol: module descriptor or a
- :class:`~pcapkit.protocols.protocol.Protocol` subclass
+ :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass
Raises:
pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a
@@ -150,7 +150,7 @@ class Link(ProtocolBase[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=abstra
replacement so a caller can tell *what* was lost. Fires only
when the incumbent differs from the replacement -- see
:meth:`ProtocolBase.register
- <pcapkit.protocols.protocol.Protocol.register>` for the
+ <pcapkit.protocols.protocol.ProtocolBase.register>` for the
guard this shares with ``register_protocol``, and for the
shipped descriptor that a restore puts back (:issue:`1504`).
@@ -158,7 +158,7 @@ class Link(ProtocolBase[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=abstra
Takes the protocol as one ``protocol`` argument, a
:class:`~pcapkit.corekit.module.ModuleDescriptor` or a
- :class:`~pcapkit.protocols.protocol.Protocol` subclass, in place of
+ :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass, in place of
the ``module`` and ``class_`` names.
"""
diff --git a/pcapkit/protocols/misc/null.py b/pcapkit/protocols/misc/null.py
index fb689b4ca..4e57bbbc0 100644
--- a/pcapkit/protocols/misc/null.py
+++ b/pcapkit/protocols/misc/null.py
@@ -68,7 +68,17 @@ class NoPayload(ProtocolBase[Data_NoPayload, Schema_NoPayload],
@property
def protochain(self) -> 'None': # type: ignore[override]
"""Protocol chain of current instance, which an empty payload does not
- have."""
+ have.
+
+ .. versionchanged:: 1.5.0
+
+ Narrows :attr:`Protocol.protochain
+ <pcapkit.protocols.protocol.ProtocolBase.protochain>`'s declared
+ type from :class:`~pcapkit.corekit.protochain.ProtoChain` to
+ :obj:`None`, the value ``self._protos`` already held here; the fix
+ is to the declaration, not the runtime value (:issue:`2317`).
+
+ """
return None
##########################################################################
diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py
index 4eda240b0..05f883e27 100644
--- a/pcapkit/protocols/protocol.py
+++ b/pcapkit/protocols/protocol.py
@@ -661,7 +661,15 @@ class ProtocolBase(Generic[_PT, _ST], metaclass=ProtocolMeta):
# protocol chain of current instance
@property
def protochain(self) -> 'ProtoChain':
- """Protocol chain of current instance."""
+ """Protocol chain of current instance.
+
+ Note:
+ :class:`~pcapkit.protocols.misc.null.NoPayload` has no chain and
+ narrows this to :obj:`None`; :meth:`_decode_next_layer` passes it
+ on as ``basis``, which
+ :class:`~pcapkit.corekit.protochain.ProtoChain` accepts.
+
+ """
return self._protos
# packet data
@@ -1031,7 +1039,7 @@ class ProtocolBase(Generic[_PT, _ST], metaclass=ProtocolMeta):
Arguments:
code: protocol code
protocol: module descriptor or a
- :class:`~pcapkit.protocols.protocol.Protocol` subclass
+ :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass
Raises:
pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a
@@ -1271,9 +1279,12 @@ class ProtocolBase(Generic[_PT, _ST], metaclass=ProtocolMeta):
UnsupportedCall: When constructing (``file`` is :obj:`None`), if a
keyword names no parameter of this protocol's :meth:`make`,
:meth:`read`, :meth:`pack`, :meth:`unpack`,
- :meth:`__post_init__` or :meth:`__init__`, anywhere in the MRO,
- and is not listed in :attr:`__keywords__`. Parsing (``file`` is
- given) is unaffected.
+ :meth:`__post_init__` or :meth:`__init__`, anywhere in the MRO
+ -- except a ``make`` whose class sets
+ :attr:`__own_make_keywords__`, which counts only while it is
+ the ``make`` of the protocol being constructed -- and is not
+ listed in :attr:`__keywords__`. Parsing (``file`` is given) is
+ unaffected.
Note:
The ``_layer``, ``_protocol`` and ``__context__`` keywords are
diff --git a/pcapkit/protocols/transport/sctp.py b/pcapkit/protocols/transport/sctp.py
index 1c7086f60..45d217f73 100644
--- a/pcapkit/protocols/transport/sctp.py
+++ b/pcapkit/protocols/transport/sctp.py
@@ -694,6 +694,10 @@ class SCTP(Transport[Data_SCTP, Schema_SCTP],
Returns:
Constructed packet data.
+ Raises:
+ ProtocolError: If ``srcport`` or ``dstport`` is a :obj:`bool`
+ (:issue:`2287`).
+
Note:
There is no ``payload`` argument: SCTP carries its user data in the
``data`` field of a DATA chunk, so the payload is supplied as part
@@ -730,7 +734,7 @@ class SCTP(Transport[Data_SCTP, Schema_SCTP],
code: payload protocol identifier (PPID), as in
:class:`~pcapkit.const.sctp.payload_protocol_identifier.PayloadProtocolIdentifier`
protocol: module descriptor or a
- :class:`~pcapkit.protocols.protocol.Protocol` subclass
+ :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass
Important:
SCTP overrides :meth:`Transport.register
diff --git a/pcapkit/protocols/transport/tcp.py b/pcapkit/protocols/transport/tcp.py
index 86abc1e57..16cb60ee3 100644
--- a/pcapkit/protocols/transport/tcp.py
+++ b/pcapkit/protocols/transport/tcp.py
@@ -624,6 +624,10 @@ class TCP(Transport[Data_TCP, Schema_TCP],
Returns:
Constructed packet data.
+ Raises:
+ ProtocolError: If ``srcport`` or ``dstport`` is a :obj:`bool`
+ (:issue:`2287`).
+
.. versionchanged:: 1.5.0
An out-of-range ``srcport`` or ``dstport`` raises
diff --git a/pcapkit/protocols/transport/transport.py b/pcapkit/protocols/transport/transport.py
index e4b100874..0535c517a 100644
--- a/pcapkit/protocols/transport/transport.py
+++ b/pcapkit/protocols/transport/transport.py
@@ -79,7 +79,7 @@ class Transport(ProtocolBase[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=a
Arguments:
code: port number
protocol: module descriptor or a
- :class:`~pcapkit.protocols.protocol.Protocol` subclass
+ :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass
Important:
This method must be called from a non-abstract class, as the
@@ -99,7 +99,7 @@ class Transport(ProtocolBase[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=a
:class:`~pcapkit.corekit.module.ModuleDescriptor` counts as
different from the class it names. See
:meth:`ProtocolBase.register
- <pcapkit.protocols.protocol.Protocol.register>` for the
+ <pcapkit.protocols.protocol.ProtocolBase.register>` for the
guard this shares with ``register_protocol``, and for the
shipped descriptor that a restore puts back (:issue:`1504`).
@@ -115,7 +115,7 @@ class Transport(ProtocolBase[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=a
Takes the protocol as one ``protocol`` argument, a
:class:`~pcapkit.corekit.module.ModuleDescriptor` or a
- :class:`~pcapkit.protocols.protocol.Protocol` subclass, in place of
+ :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass, in place of
the ``module`` and ``class_`` names.
"""
diff --git a/pcapkit/protocols/transport/udp.py b/pcapkit/protocols/transport/udp.py
index 224d1c8ec..931420089 100644
--- a/pcapkit/protocols/transport/udp.py
+++ b/pcapkit/protocols/transport/udp.py
@@ -228,6 +228,10 @@ class UDP(Transport[Data_UDP, Schema_UDP],
Returns:
Constructed packet data.
+ Raises:
+ ProtocolError: If ``srcport`` or ``dstport`` is a :obj:`bool`
+ (:issue:`2287`).
+
.. versionchanged:: 1.5.0
An out-of-range ``srcport`` or ``dstport`` raises
diff --git a/tests/protocols/test_maker_invalid_format_messages_2406_unit.py b/tests/protocols/test_maker_invalid_format_messages_2406_unit.py
index aada9efb8..41562b964 100644
--- a/tests/protocols/test_maker_invalid_format_messages_2406_unit.py
+++ b/tests/protocols/test_maker_invalid_format_messages_2406_unit.py
@@ -20,8 +20,9 @@ wording:
The parse side is deliberately untouched, as in #2296: ``invalid format`` is the
right thing for a reader to say. :class:`ReadersKeepInvalidFormat` below pins
that division on the one pair where the reader and the maker guard the very same
-thing -- ``_read_opt_pad`` and ``_make_opt_pad`` both reject a ``Pad1`` carrying
-a non-zero length.
+thing -- ``_read_opt_pad`` and ``_make_opt_pad`` guard a ``Pad1`` carrying a
+non-zero length, though only the reader raises; the maker warns and makes a
+``PadN`` instead.
Every message is pinned whole, and every maker's valid input is exercised in the
same class, so a guard that starts refusing good input regresses here too.
@@ -211,9 +212,10 @@ class PCAPNGBlockMessage(MakerMessageTestCase):
class ReadersKeepInvalidFormat(MakerMessageTestCase):
"""The parse side still says ``invalid format``, as #2296 asks.
- ``_read_opt_pad`` and ``_make_opt_pad`` reject the same thing -- a ``Pad1``
- option carrying a non-zero length -- so the pair is where a sweep over the
- makers is most likely to catch a reader by accident.
+ ``_read_opt_pad`` and ``_make_opt_pad`` guard the same thing -- a ``Pad1``
+ option carrying a non-zero length -- though only the reader raises; the
+ maker warns and makes a ``PadN`` instead. The pair is where a sweep over
+ the makers is most likely to catch a reader by accident.
""" |
|
Coverage: 91.72% (unit tier, Python 3.15,
Per-file detail: the |
56c8fbc to
ac5016b
Compare
|
Review: NEEDS CHANGES at Three leftover narration spots missed by the last round, all docstring-only:
Non-blocking: |
ac5016b to
8c259cb
Compare
|
Review: NEEDS CHANGES at All four claimed fixes confirmed real and accurate (re-derived each from the actual code: Two things left:
Line 2 of |
8c259cb to
acdcae2
Compare
|
Review: NEEDS CHANGES at
Optional, outside this PR's touched files, not blocking: After these two fixes, this PR should be ready — no need for another full 25-minute review round. |
…egistry and registration prose (#1103) #1103 whole-repo prose sweep over the maker-message wave (#2296, #2404, #2405, #2407) and the __proto_dest__ registry refactor (#2392, #2403). Docstrings corrected where the prose did not match the code: * ``Protocol`` -> ``ProtocolBase`` in the ``register``/``register_protocol`` cross-references (protocol.py, internet.py, link.py, transport.py, sctp.py, registry/protocols.py). * ``Raises:`` entries for the ``bool`` port guard (#2287) in TCP, UDP and SCTP ``make``, and for HTTP/1's request-without-``uri`` guard. * ``protochain`` documented on both sides of its narrowing to ``None`` (protocol.py, misc/null.py, #2317), and ``__init__`` names the ``__own_make_keywords__`` exception to its keyword check. * IP reassembly names the IPv6 ``M`` bit and which buffer an atomic fragment is isolated from (#2270); ``_check_code`` names ``register_protocol_code`` among its callers (#2186). * HIP's shared lifetime helper says where the octet appears instead of counting the makers that carry it, and ``_make_opt_pad`` warns and makes a ``PadN`` rather than raising as its reader does. Before/after narration replaced with present tense, the pattern #1103's round 6 banned: * the ``__proto_dest__`` prose in the registration and vendor tests, and the ``register_protocol_code`` key-check narration; * the #2296 and #2406 maker-message module docstrings -- readers are exempt by design rather than "deliberately untouched", and the HTTP ``frame=None`` case is outside the guard rather than one the guard "has not widened to"; * the ``_NameLease`` rationale and the ``register_protocol`` name-collision note in registry/protocols.py, which described a fix and a redesign in the past tense; * TCP's ``_make_mptcp_unknown`` length comment, which contrasted the current expression with "the former" one. AST guard (``ast.dump`` via util/vendor_code_changed.py's ``code_of``, with docstrings and bare string literals stripped) confirms all 16 touched files are prose-only against e48062c. 129 tests pass (1 pre-existing skip) with 334 subtests across the four touched test modules, tests/test_docstring_contract.py, tests/project/test_conventions_doc_claims.py, tests/project/test_docs_issue_pr_roles_1103_unit.py and tests/project/test_issue_refs_in_comments.py.
acdcae2 to
4c569fa
Compare
|
Review: GOOD TO GO at |
Summary
Part of the #1103 whole-repo #719-style prose sweep, scoped this round to the maker-message wave (#2296, #2404, #2405, #2407) and the registry-destination refactor (#2392, #2403). 16 files, prose-only (docstrings and comments), no behaviour change, no renames.
1. Stale
Protocol→ProtocolBasecross-references in theregister/register_protocoldocs:protocols/protocol.py,protocols/internet/internet.py,protocols/link/link.py,protocols/transport/transport.py,protocols/transport/sctp.py,foundation/registry/protocols.py.2. Missing
Raises:sections: theboolport guard (#2287) onTCP.make,UDP.make,SCTP.make; HTTP/1's request-without-uriguard in twohttpv1.pymake docstrings.3. Behaviour the prose omitted:
misc/null.pygains a.. versionchanged:: 1.5.0forprotochain's narrowing toNone(#2317) andprotocol.pydocuments the other side of it;protocol.py's__init__names the__own_make_keywords__exception to its keyword check;foundation/reassembly/ip.pynames the IPv6 M bit and which buffer an atomic fragment is isolated from (#2270);_check_codenamesregister_protocol_codeamong its callers (#2186); HIP's shared lifetime helper says where theLifetimeoctet appears instead of counting the makers that carry it.4. Before/after narration → present tense (the #1103 round-6 rule): the
__proto_dest__prose andregister_protocol_codekey-check narration intests/protocols/test_protocol_code_registration_unit.pyandtests/vendor/test_vendor_proto_dest_2392_unit.py; the #2296 and #2406 maker-message module docstrings and theirReadersKeepInvalidFormatdocstrings;registry/protocols.py's_NameLeaserationale andregister_protocolname-collision note; TCP's_make_mptcp_unknownlength comment.Not in this PR: no
docs/source/changelog/1.5.0.rstentries for #2312–#2407 — #719 scopes the changelog to its own PR (tracked separately in #2409/#2412).docs/source/contributing/pep.rst's "_make_indexnot yet refactored" claim was verified against #2393/#2401 and still holds, so it's left as is.Test plan
util/vendor_code_changed.py'scode_of) — all 16 touched files prose-only againstmain, zero code-structure diffstests/protocols/test_maker_invalid_format_messages_2296_unit.py+_2406_unit.py,tests/protocols/test_protocol_code_registration_unit.py,tests/vendor/test_vendor_proto_dest_2392_unit.py— all passtests/test_docstring_contract.py+tests/project/test_conventions_doc_claims.py— passtests/project/test_docs_issue_pr_roles_1103_unit.py+tests/project/test_issue_refs_in_comments.py— pass (issue refs moved within docstrings/comments)