docs(protocols): tighten application, link and root protocol prose (#719)

pcapkit/protocols/application/__init__.py

@@ -28,7 +28,7 @@
 from pcapkit.protocols.application.ospf import OSPF
 from pcapkit.protocols.application.rarp import RARP, DRARP
 
-# Deprecated / Base Classes
+# Base Classes
 from pcapkit.protocols.application.http import HTTP
 
 # Transport Layer Protocol Numbers

pcapkit/protocols/application/ftp.py

@@ -41,10 +41,9 @@
 class Type(EnumLookup, StrEnum):
     """FTP packet type.
 
-    Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub
-    issue :issue:`877`'s ruling that every non-registry enumeration shares that
-    lookup contract -- pure re-parenting, since this class defines neither
-    ``get`` nor ``_missing_`` of its own to reconcile with the base.
+    Built on :class:`~pcapkit.corekit.enum.EnumLookup`, the lookup contract
+    shared by every non-registry enumeration (:issue:`877`). The class defines
+    neither ``get`` nor ``_missing_`` of its own.
 
     """
 

pcapkit/protocols/application/httpv1.py

@@ -78,10 +78,9 @@ def _test_start_line(data: 'bytes') -> 'bool':
 
     This is a *classification* predicate and parses nothing: it answers "is this
     HTTP/1?" for :meth:`HTTP._guess_version
-    <pcapkit.protocols.application.http.HTTP._guess_version>`, which until :issue:`800`
-    answered that question by trial-parsing every version in the family and
-    keeping whichever one did not object -- so a payload that is not HTTP at all
-    was classified by which parser happened to fail less loudly.
+    <pcapkit.protocols.application.http.HTTP._guess_version>`. Classifying by
+    trial-parsing every version in the family would decide a payload that is
+    not HTTP at all by which parser happened to fail less loudly.
 
     Args:
         data: Payload to classify.
@@ -111,9 +110,9 @@ def _test_start_line(data: 'bytes') -> 'bool':
         tested only the first line would answer :data:`True` for it. Split at the
         header/body separator first, as :meth:`HTTP.read
         <pcapkit.protocols.application.httpv1.HTTP.read>` does, and the preface's
-        header is ``PRI * HTTP/2.0`` with no CRLF left in it -- which is exactly
-        why the parser refuses it, and now why this does. Measured: without the
-        separator split this returned :data:`True` for the preface.
+        header is ``PRI * HTTP/2.0`` with no CRLF left in it -- which is why the
+        parser refuses it, and why this does. Without the separator split this
+        returns :data:`True` for the preface.
 
         An HTTP/0.9 request line carries only two tokens and so is not recognised
         here either, matching the parser, which raises on fewer than three.
@@ -141,10 +140,9 @@ def _test_start_line(data: 'bytes') -> 'bool':
 class Type(EnumLookup, StrEnum):
     """HTTP packet type.
 
-    Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub
-    issue :issue:`877`'s ruling that every non-registry enumeration shares that
-    lookup contract -- pure re-parenting, since this class defines neither
-    ``get`` nor ``_missing_`` of its own to reconcile with the base.
+    Built on :class:`~pcapkit.corekit.enum.EnumLookup`, the lookup contract
+    shared by every non-registry enumeration (:issue:`877`). The class defines
+    neither ``get`` nor ``_missing_`` of its own.
 
     """
 
@@ -214,14 +212,14 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_HTTP':
         packet = schema.data
 
         # NOTE: A payload carrying no header/body separator at all unpacks short
-        # here, and the bare ``ValueError`` that used to escape is what made
-        # ``HTTP._guess_version``'s HTTP/2 arm unreachable: that dispatcher falls
-        # through on ``ProtocolError`` alone, so an HTTP/1 attempt on HTTP/2 wire
-        # bytes aborted the guess rather than failing it, and the HTTP/2 attempt
-        # never ran (#787). ``ProtocolError`` is what the ``Raises:`` section
-        # above already promises for a malformed packet, and the same conversion
-        # the explicit ``version=`` path performs at ``http.py:119``; chained, so
-        # the underlying unpacking error stays reachable as ``__cause__``.
+        # here. It must surface as ``ProtocolError``, not a bare ``ValueError``:
+        # ``HTTP._guess_version`` falls through to its HTTP/2 arm on
+        # ``ProtocolError`` alone, so any other exception from an HTTP/1 attempt
+        # on HTTP/2 wire bytes would abort the guess instead of failing it.
+        # ``ProtocolError`` is also what the ``Raises:`` section above promises,
+        # and the same conversion the explicit ``version=`` path of
+        # ``HTTP.read`` performs; chained, so the unpacking error stays
+        # reachable as ``__cause__``.
         try:
             header, body = packet.split(b'\r\n\r\n', maxsplit=1)
         except ValueError as error:
@@ -375,9 +373,9 @@ def _read_http_header(self, header: 'bytes') -> 'tuple[Data_Header, OrderedMulti
         # message: a header of one line with no CRLF -- the HTTP/2 connection
         # preface, ``PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n``, splits to exactly that
         # -- and a start line of fewer than three whitespace-separated tokens.
-        # Raised as ``ProtocolError`` for the reason ``read`` gives above, and
-        # to the same message this method already uses below for a start line it
-        # cannot recognise (#787).
+        # Raised as ``ProtocolError`` for the reason ``read`` gives above, with
+        # the same message this method uses below for a start line it cannot
+        # recognise.
         try:
             startline, headerfield = header.split(b'\r\n', 1)
             para1, para2, para3 = re.split(rb'\s+', startline, maxsplit=2)
@@ -387,17 +385,14 @@ def _read_http_header(self, header: 'bytes') -> 'tuple[Data_Header, OrderedMulti
         # NOTE: A field line beginning with SP or HTAB is an ``obs-fold``
         # continuation of the line before it (:rfc:`9112#section-5.2`), and is
         # unfolded here -- the RFC's own remedy -- rather than treated as a field
-        # line of its own. Deprecated, but present in real captures, and the two
-        # ways it used to come out were both wrong: a continuation carrying no
-        # colon left the split below one element long and ``item[1]`` raised
+        # line of its own. Deprecated, but present in real captures. Treating a
+        # continuation as a field line of its own goes wrong twice: one carrying
+        # no colon leaves the split below one element long, so ``item[1]`` raises
         # :exc:`IndexError`, which is neither a :exc:`ValueError` nor a
-        # ``ProtocolError`` and so escaped ``HTTP._guess_version``'s suppression
-        # exactly as the bare :exc:`ValueError` of #787 did; a continuation that
-        # happened to contain one was worse, parsing silently into a spurious
-        # extra field (``X-Long: a`` plus ``b: c``, for a folded ``X-Long: a b``)
-        # with nothing raised at all. Unfolded, a folded message parses to the
-        # field it actually carries, so this input class stops reaching the
-        # HTTP/2 arm by accident instead of merely failing more politely.
+        # ``ProtocolError`` and escapes ``HTTP._guess_version``'s suppression;
+        # one that happens to contain a colon parses silently into a spurious
+        # extra field (``X-Long: a`` plus ``b: c``, for a folded ``X-Long: a b``).
+        # Unfolded, a folded message parses to the field it actually carries.
         fields = []  # type: list[bytes]
         for line in headerfield.split(b'\r\n'):
             if line.startswith((b' ', b'\t')):

pcapkit/protocols/application/httpv2.py

@@ -83,10 +83,10 @@
     Flags = Schema_FrameType.Flags
 
     FrameParser = Callable[[Schema_FrameType, NamedArg(Schema_HTTP, 'header')], Data_HTTP]
-    # NB: ``Flags`` is bound just above, so it goes in unquoted. Quoting it left a
-    # bare ``ForwardRef('Flags')`` inside the alias, which every *other* module
-    # that spells ``FrameConstructor`` in an annotation then had to resolve in its
-    # own namespace -- where the name does not exist.
+    # NB: ``Flags`` is bound just above, so it goes in unquoted. Quoted, it would
+    # leave a bare ``ForwardRef('Flags')`` inside the alias, which every *other*
+    # module that spells ``FrameConstructor`` in an annotation would have to
+    # resolve in its own namespace -- where the name does not exist.
     FrameConstructor = Callable[[DefaultArg(Optional[Data_HTTP]),
                                  KwArg(Any)], Tuple[Schema_FrameType, Flags]]
 
@@ -97,7 +97,7 @@ class HTTP(HTTPBase[Data_HTTP, Schema_HTTP],
            schema=Schema_HTTP, data=Data_HTTP):
     """This class implements Hypertext Transfer Protocol (HTTP/2).
 
-    This class currently supports parsing of the following HTTP/2 frames,
+    This class supports parsing of the following HTTP/2 frames,
     which are directly mapped to the :class:`pcapkit.const.http.frame.Frame`
     enumeration:
 
@@ -204,49 +204,40 @@ def unpack(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_HTTP'
         Notes:
             This guards ahead of :meth:`Schema.unpack
             <pcapkit.protocols.schema.schema.Schema.unpack>` rather than
-            leaving the same check to :meth:`read`, because a buffer shorter
-            than the fixed 9-octet header is not merely invalid -- it can
-            crash the schema layer outright. ``pkt['__length__']`` there is
-            decremented by each field's *nominal* width regardless of how
-            many octets the buffer actually had, so it goes negative, and the
-            frame payload's length-derived fields (e.g. an unpadded ``DATA``
-            frame's ``data: BytesField(length=lambda pkt: pkt['__length__'])``)
-            resolve to that negative number. A negative field length becomes
-            a struct template such as ``'-5s'``, and :func:`struct.calcsize`
-            raises :exc:`struct.error` for it -- uncaught, since nothing
-            downstream expects a :exc:`ProtocolError` to be spelled that way.
-            Rejecting here, before :meth:`Schema.unpack` ever runs, keeps a
-            frame too short to hold a header from reaching that arithmetic at
-            all. This closes the *outer*-header class only -- an inner payload
-            field can still drive ``pkt['__length__']`` negative on a buffer
-            that clears nine (e.g. a ``GOAWAY`` frame at 9-16 octets, whose
-            fixed ``stream`` and ``error`` fields alone consume eight); that
-            residual is why :meth:`HTTP._guess_version
-            <pcapkit.protocols.application.http.HTTP._guess_version>` still
-            suppresses :exc:`struct.error` on its last arm. See :issue:`799`.
+            leaving the check to :meth:`read`, because a buffer shorter than the
+            fixed 9-octet header cannot be parsed meaningfully.
+            ``pkt['__length__']`` there is decremented by each field's
+            *nominal* width regardless of how many octets the buffer had, so it
+            goes negative and the frame payload's length-derived fields (e.g. an
+            unpadded ``DATA`` frame's ``data: BytesField(length=lambda pkt:
+            pkt['__length__'])``) resolve to a negative width. Rejecting here
+            keeps such a frame from reaching that arithmetic. An inner field can
+            still drive it negative on a buffer that clears nine (e.g. a
+            ``GOAWAY`` frame at 9-16 octets, whose fixed ``stream`` and
+            ``error`` fields alone consume eight); the schema layer raises
+            :exc:`ProtocolError` for that itself.
 
             ``length`` is resolved against :func:`len` only for *this*
             method's own check, and the *original* argument -- ``None``
-            included -- is what is actually forwarded to :meth:`Protocol.unpack
+            included -- is what is forwarded to :meth:`Protocol.unpack
             <pcapkit.protocols.protocol.ProtocolBase.unpack>`. Collapsing
-            ``None`` to a concrete ``0`` before forwarding would change what
-            :meth:`Schema.unpack`'s own ``prepare`` decorator does with a
-            now-exhausted stream: it raises
+            ``None`` to a concrete ``0`` would change what
+            :meth:`Schema.unpack`'s own ``prepare`` decorator does with an
+            exhausted stream: it raises
             :exc:`~pcapkit.utilities.exceptions.StreamEOFError` (an
             :exc:`EOFError`, the documented "no more packets" signal) only when
-            the *caller* left ``length`` unspecified, and forwarding a resolved
-            ``0`` instead would report an exhausted stream as a malformed
-            packet. A *non-zero* short buffer is unambiguously malformed either
-            way, so it is rejected here regardless of whether ``length`` was
-            given explicitly.
+            the *caller* left ``length`` unspecified, and a resolved ``0``
+            would report an exhausted stream as a malformed packet. A
+            *non-zero* short buffer is malformed either way, so it is rejected
+            here whether or not ``length`` was given.
 
         """
         if length is None:
             # ``0 < ... < 9`` rather than ``... < 9``: an exhausted stream
-            # (``len(self) == 0``) is left alone here, so ``length`` reaches
+            # (``len(self) == 0``) is left alone, so ``length`` reaches
             # :meth:`Protocol.unpack <pcapkit.protocols.protocol.ProtocolBase.unpack>`
             # still ``None`` and its ``prepare`` decorator raises
-            # :exc:`StreamEOFError` as documented above -- only a *non-empty*
+            # :exc:`StreamEOFError` as documented above; only a *non-empty*
             # short stream is rejected as malformed here.
             if 0 < len(self) < 9:
                 raise ProtocolError(f'HTTP/2: invalid format, packet ({len(self)} octet(s)) '
@@ -293,21 +284,16 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_HTTP':
         schema = self.__header__
 
         # NOTE: ``schema.length`` is the *declared* length off the wire -- the
-        # 24-bit field a hostile or truncated capture controls outright -- and
-        # checking only it lets a frame whose buffer holds far fewer octets than
-        # it claims sail through this guard and report a length nothing backs.
-        # ``length`` is the actual number of octets available for this frame
-        # (``Protocol.__len__`` returns ``len(self._data)``, and a caller that
-        # supplies ``length`` explicitly means the same thing by it), so both
-        # have to clear the minimum header size for the frame to be viable at
-        # all, *and* the declared value must not exceed what is actually
-        # available -- this library's convention (see ``_make_http_length``)
-        # is that ``length`` counts the whole frame, header included, so the
-        # two are directly comparable. Without the third clause a frame that
-        # declares far more than its buffer holds -- the headline case in
-        # #799, e.g. a nine-octet buffer declaring 16777215 -- still passed
-        # this guard and reported the declared, attacker-controlled length as
-        # if the capture actually contained it. See #799.
+        # 24-bit field a hostile or truncated capture controls outright -- so
+        # checking only it would let a frame whose buffer holds far fewer octets
+        # than it claims report a length nothing backs (e.g. a nine-octet buffer
+        # declaring 16777215). ``length`` is the number of octets actually
+        # available for this frame (``Protocol.__len__`` returns
+        # ``len(self._data)``, and an explicit ``length`` means the same), so
+        # both have to clear the minimum header size *and* the declared value
+        # must not exceed what is available. This library's convention (see
+        # ``_make_http_length``) is that ``length`` counts the whole frame,
+        # header included, so the two are directly comparable.
         if schema.length < 9 or length < 9 or schema.length > length:
             raise ProtocolError(f'HTTP/2: [Type {schema.type}] invalid format')
         if schema.type in (Enum_Frame.SETTINGS, Enum_Frame.PING) and schema.stream['sid'] != 0: