fix(httpv1): accept HTTP/1 messages with no header fields (#1041)

CHANGELOG.md

@@ -190,6 +190,7 @@ This resolves [#548](https://github.com/JarryShaw/PyPCAPKit/issues/548), which r
 - **a breaking change to** 8 more bespoke registries: step 2 of [#860](https://github.com/JarryShaw/PyPCAPKit/issues/860), PR 1 of 2 (`AppType` is PR 2, [#874](https://github.com/JarryShaw/PyPCAPKit/pull/874) below), bringing `StatusCode`, `ReturnCode`, `ResponseKind`, `GroupingInformation`, `OptionType`, `FEATCode`, `Command` and `Method` onto `EnumRegistry` and applying [#775](https://github.com/JarryShaw/PyPCAPKit/issues/775)'s mint/unmint ruling to their `get()` as well as `_missing_`. The first five convert 12 unambiguous placeholder branches (`Unassigned`, `Unknown`, `opt_unknown`) to `_unregistered_member`; the three with a custom `__new__` (`StatusCode`, `ReturnCode`, `OptionType`) get an override reconstructing the attributes the base's generic helper would leave unset, and `StatusCode`/`ReturnCode`'s hand-written `get()`, still on the retired `default == -1` convention, is replaced by the base's (no caller relied on the old form). `OptionType` keeps its own `get()` for its multi-namespace dispatch, but a round-2 review found it still minted on both its int/namespace and `str` paths, and the live pcapng parse path (`PCAPNG._make_pcapng_options`) calls it with wire bytes, so parsing an undeclared option code still registered a permanent member; both paths now build an unregistered member. `FEATCode`, `Command` and `Method` mint the literal wire value as its own name rather than any placeholder; per the owner's ruling (*"get will not have sufficient information to create new ones"*: `Command` needs `feat`/`desc`/`type`/`conf` and `Method` needs `safe`/`idempotent`, which a bare wire string lacks), both `_missing_` and each class's own `get()` (a second, independent mint site) now build an unregistered member. `FEATCode`'s crawler now declares all 15 real FEAT-code values from the live IANA table instead of minting 10 of them as a side effect of evaluating `Command`'s rows at import time, the import-time-mutation shape [#861](https://github.com/JarryShaw/PyPCAPKit/pull/861) removed from `FilterType`; pinned count-agnostically so a future IANA update cannot fail a correct regeneration. Untouched, per rulings: `CommandType` stays `IntFlag` (real `A|P` composites in the generated data), and `TransportProtocol`'s `auto()` renumbering and `AppType` are PR 2's. `pcapkit/protocols/schema/misc/pcapng.py`'s `OptionEnumField.post_process` docstring is corrected: it still bypasses `OptionType.get()` directly, now for `pickle` round-trip safety ([#860](https://github.com/JarryShaw/PyPCAPKit/issues/860) already stops the minting that used to be the reason), since neither of `OptionType`'s two paths gets both pickling and correct rendering right at once ([#869](https://github.com/JarryShaw/PyPCAPKit/pull/869)).
 - `register` on `ProtocolBase`, `Frame`, `PCAPNG`, `SCTP`, `Link`, `Internet` and the concrete `Transport` subclasses (`TCP`, `UDP`) now raises `RegistryError` for a non-class argument, where it raised a bare `TypeError` ("issubclass() arg 1 must be a class") from inside `abc`. Each guard gains an explicit `isinstance(protocol, type)` test ahead of its `issubclass`, after the `ModuleDescriptor` is unwrapped, so a descriptor naming a non-class attribute is rejected too. All seven `Raises:` clauses already promised `RegistryError` if `protocol` "is not a ... subclass", which a non-class satisfied in spirit but not in fact; they now read "is not a class, or not a ... subclass". No guard's target class changes, and a wrong class still raises `RegistryError` as before. `Transport.register` on `Transport` itself still raises `UnsupportedCall` first; only its subclasses reach the guard. Not breaking for exception handling: `RegistryError` subclasses `TypeError` through `BaseError`, so `except TypeError` still catches it, and the type and message text are the only things a handler can key on that change. Two observable side effects do come with it, because `BaseError` is loud by default: the non-class path now emits pcapkit's one `CRITICAL` log record, and outside devmode it installs `sys.excepthook` and `threading.excepthook`, neither of which the bare `TypeError` from `abc` did. That is the wrong-class path's existing behaviour, so the fix makes the two consistent rather than introducing something new. Follows the same fix for the foundation registrars ([#1021](https://github.com/JarryShaw/PyPCAPKit/issues/1021)) ([#1026](https://github.com/JarryShaw/PyPCAPKit/issues/1026)).
 - `PCAPNG`'s `ns_dnsname`, `ns_dnsIP4addr` and `ns_dnsIP6addr` scope guards, on both the read and the make side, now say the option "must be in Name Resolution Block", which is the block they check; they used to name the systemd(1) Journal Export Block, with a literal `:manpage:` role in the text. `register_record`'s `RegistryWarning` likewise now says "name resolution record already registered". The make-side `ns_dnsIP4addr`/`ns_dnsIP6addr` messages also drop the lowercase `ns_dnsip4addr`/`ns_dnsip6addr` spelling for the `OptionType` member name. Two more guards named the wrong option: `_make_option_if_rxspeed`'s scope and duplicate guards now say `[if_rxspeed]` rather than `[if_txspeed]`, and `_read_option_epb_queue`'s length guard says `[epb_queue]` rather than `[epb_packetid]`. Only message text changes; the exception and warning types are unchanged ([#1038](https://github.com/JarryShaw/PyPCAPKit/issues/1038)).
+- `httpv1.HTTP` refused every HTTP/1 message with no header fields, which [[RFC 9112 Section 2.1](https://datatracker.ietf.org/doc/html/rfc9112#section-2.1)] allows (`*( field-line CRLF )`): `b'GET / HTTP/1.0\r\n\r\n'` raised `ProtocolError: HTTP: invalid format`, because `_read_http_header` treated a header with no CRLF as malformed. So `HTTPv1(method='GET', uri='/', http_version='1.1')` failed to read back the bytes its own `make` built, and `HTTP._guess_version` answered `unknown HTTP version`, since `_test_start_line` mirrored the same check. A CRLF-less header is now a start line with zero fields, in the parser and the predicate alike. That check was also what refused the HTTP/2 connection preface, whose header is exactly `PRI * HTTP/2.0`; it is now refused by name instead, to the same message ([#1041](https://github.com/JarryShaw/PyPCAPKit/issues/1041)).
 
 ### pcapkit.toolkit
 

docs/source/changelog/1.5.0.rst

@@ -2163,6 +2163,17 @@ Fixed
   ``_read_option_epb_queue``'s length guard says ``[epb_queue]`` rather than
   ``[epb_packetid]``. Only message text changes; the exception and warning types are unchanged
   (:issue:`1038`).
+* ``httpv1.HTTP`` refused every HTTP/1 message with no header fields, which
+  [:rfc:`9112#section-2.1`] allows (``*( field-line CRLF )``):
+  ``b'GET / HTTP/1.0\r\n\r\n'`` raised ``ProtocolError: HTTP: invalid format``,
+  because ``_read_http_header`` treated a header with no CRLF as malformed. So
+  ``HTTPv1(method='GET', uri='/', http_version='1.1')`` failed to read back the bytes
+  its own ``make`` built, and ``HTTP._guess_version`` answered
+  ``unknown HTTP version``, since ``_test_start_line`` mirrored the same check. A
+  CRLF-less header is now a start line with zero fields, in the parser and the
+  predicate alike. That check was also what refused the HTTP/2 connection preface,
+  whose header is exactly ``PRI * HTTP/2.0``; it is now refused by name instead, to
+  the same message (:issue:`1041`).
 
 pcapkit.toolkit
 ---------------

docs/source/contributing/conventions/process.rst

@@ -101,7 +101,7 @@ commands down instead of a figure that will be stale by the next merge:
 The grouping scheme was settled on :issue:`918`: **a section per top-level module, with**
 ``Added``/``Changed``/``Fixed`` **nested inside each** -- module granularity, not
 per-file and not per-subpackage. The file carries **9** module-level sections holding
-164 entries, and no entry carries an inline kind label::
+165 entries, and no entry carries an inline kind label::
 
    $ grep -cE '^\* \*\*(Added|Changed|Fixed)\*\*' docs/source/changelog/1.5.0.rst
    0

pcapkit/protocols/application/httpv1.py

@@ -34,6 +34,7 @@
 from pcapkit.const.http.status_code import StatusCode as Enum_StatusCode
 from pcapkit.corekit.enum import EnumLookup
 from pcapkit.corekit.multidict import OrderedMultiDict
+from pcapkit.protocols.application.http import _HTTP2_PREFACE
 from pcapkit.protocols.application.http import HTTP as HTTPBase
 from pcapkit.protocols.data.application.httpv1 import HTTP as Data_HTTP
 from pcapkit.protocols.data.application.httpv1 import RequestHeader as Data_RequestHeader
@@ -72,6 +73,12 @@
 # the syntax.
 _RE_STATUS = re.compile(rb'\d{3}\Z')
 
+# The HTTP/2 connection preface's header block, as ``HTTP.read`` splits it off at
+# the first CRLFCRLF: ``PRI * HTTP/2.0``. A start line with zero field lines, so
+# the patterns above accept it; it is refused by name in ``_read_http_header``
+# and ``_test_start_line`` so that the preface never parses as HTTP/1.
+_HTTP2_PREFACE_HEADER = _HTTP2_PREFACE.split(b'\r\n\r\n', maxsplit=1)[0]
+
 
 def _test_start_line(data: 'bytes') -> 'bool':
     """Whether ``data`` opens with an HTTP/1.* start line.
@@ -102,17 +109,18 @@ def _test_start_line(data: 'bytes') -> 'bool':
         ``test_start_line_predicate_agrees_with_the_httpv1_parser`` pins that
         agreement.
 
-        Both of the unpackings the parser performs *before* those patterns are
-        mirrored too, and this is not pedantry -- the second of them is the whole
-        reason the HTTP/2 connection preface is not claimed here. ``PRI *
+        The checks the parser performs *before* those patterns are mirrored too,
+        and this is not pedantry -- they are the whole reason the HTTP/2
+        connection preface is not claimed here. ``PRI *
         HTTP/2.0\\r\\n\\r\\nSM\\r\\n\\r\\n`` is deliberately a well-formed
         HTTP/1.1 *request line* (:rfc:`9113#section-3.4`), so a predicate that
         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 why the
-        parser refuses it, and why this does. Without the separator split this
-        returns :data:`True` for the preface.
+        header is exactly ``PRI * HTTP/2.0`` (``_HTTP2_PREFACE_HEADER``), which
+        the parser refuses by name and so does this. Any *other* header with no
+        CRLF in it is a start line with zero field lines, which
+        :rfc:`9112#section-2.1` allows and both accept (:issue:`1041`).
 
         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.
@@ -122,10 +130,10 @@ def _test_start_line(data: 'bytes') -> 'bool':
     if header == data:  # no header/body separator -- ``read`` raises
         return False
 
-    startline = header.split(b'\r\n', maxsplit=1)[0]
-    if startline == header:  # header holds no CRLF -- ``_read_http_header`` raises
+    if header == _HTTP2_PREFACE_HEADER:  # the preface -- ``_read_http_header`` raises
         return False
 
+    startline = header.split(b'\r\n', maxsplit=1)[0]
     try:
         para1, para2, para3 = re.split(rb'\s+', startline, maxsplit=2)
     except ValueError:
@@ -369,15 +377,21 @@ def _read_http_header(self, header: 'bytes') -> 'tuple[Data_Header, OrderedMulti
             ProtocolError: If the packet is malformed.
 
         """
-        # NOTE: Both unpackings are short for input that is not an HTTP/1
-        # 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, with
-        # the same message this method uses below for a start line it cannot
-        # recognise.
+        # NOTE: A header with no CRLF is a start line with zero field lines,
+        # which :rfc:`9112#section-2.1` allows (``*( field-line CRLF )``). The
+        # one such header refused by name is the HTTP/2 connection preface's:
+        # ``PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n`` splits in ``read`` to exactly
+        # ``PRI * HTTP/2.0``, which ``_RE_METHOD`` and ``_RE_VERSION`` would
+        # otherwise accept as an HTTP/1 request (see ``_HTTP2_PREFACE_HEADER``).
+        if header == _HTTP2_PREFACE_HEADER:
+            raise ProtocolError('HTTP: invalid format')
+        startline, _, headerfield = header.partition(b'\r\n')
+
+        # NOTE: Short for a start line of fewer than three whitespace-separated
+        # tokens. 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)
         except ValueError as error:
             raise ProtocolError('HTTP: invalid format') from error
@@ -394,7 +408,7 @@ def _read_http_header(self, header: 'bytes') -> 'tuple[Data_Header, OrderedMulti
         # 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'):
+        for line in headerfield.split(b'\r\n') if headerfield else ():
             if line.startswith((b' ', b'\t')):
                 # A continuation with nothing to continue -- the first field line
                 # folded -- is malformed rather than unfoldable.