fix(httpv1): rebuild a message whose body is empty (#1050)

CHANGELOG.md

@@ -192,6 +192,7 @@ This resolves [#548](https://github.com/JarryShaw/PyPCAPKit/issues/548), which r
 - `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)).
+- `httpv1.HTTP.from_data` could not rebuild an HTTP/1 message whose body is empty: `HTTP.from_data(info)` for `b'GET / HTTP/1.1\r\nHost: a\r\n\r\n'` raised `TypeError: can't concat NoneType to bytes`. `read` records an empty body as `None`, `_make_data` passed it through, and `make` concatenated it. `_make_data` now hands `make` `b''` for a `None` body; `info.body` is still `None`, so readers see no change. `httpv2` is unaffected: its empty-payload frames already round-trip ([#1050](https://github.com/JarryShaw/PyPCAPKit/issues/1050)).
 
 ### pcapkit.toolkit
 

docs/source/changelog/1.5.0.rst

@@ -2187,6 +2187,13 @@ Fixed
   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`).
+* ``httpv1.HTTP.from_data`` could not rebuild an HTTP/1 message whose body is
+  empty: ``HTTP.from_data(info)`` for ``b'GET / HTTP/1.1\r\nHost: a\r\n\r\n'``
+  raised ``TypeError: can't concat NoneType to bytes``. ``read`` records an empty
+  body as ``None``, ``_make_data`` passed it through, and ``make`` concatenated it.
+  ``_make_data`` now hands ``make`` ``b''`` for a ``None`` body; ``info.body`` is
+  still ``None``, so readers see no change. ``httpv2`` is unaffected: its
+  empty-payload frames already round-trip (:issue:`1050`).
 
 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
-166 entries, and no entry carries an inline kind label::
+167 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

@@ -345,14 +345,17 @@ def _make_data(cls, data: 'Data_HTTP') -> 'dict[str, Any]':  # type: ignore[over
             Key-value pairs for protocol construction.
 
         """
+        # NOTE: ``read`` records an empty body as :obj:`None`, which is the
+        # public ``info.body`` value; ``make`` takes ``bytes`` and concatenates
+        # it, so the read-side ``None`` is translated back here (#1050).
         return {
             'http_version': data.receipt.version,  # type: ignore[attr-defined]
             'method': getattr(data.receipt, 'method', None),
             'uri': getattr(data.receipt, 'uri', None),
             'status': getattr(data.receipt, 'status', None),
             'message': getattr(data.receipt, 'message', None),
             'headers': data.header,
-            'body': data.body,
+            'body': b'' if data.body is None else data.body,
         }
 
     def _read_http_header(self, header: 'bytes') -> 'tuple[Data_Header, OrderedMultiDict[str, str]]':

tests/protocols/application/test_http_unit.py

@@ -1093,9 +1093,9 @@ def test_httpv1_make_read_round_trips_a_message_with_no_header_fields(self) -> N
         constructing ``HTTPv1(method='GET', uri='/', http_version='1.1')`` raised
         from its own ``read``. The response is also round-tripped through
         ``_make_data``, so an empty parsed header dict is shown to rebuild the
-        same bytes; the request is not, because ``_make_data`` hands an empty
-        body back as :obj:`None`, which ``make`` cannot concatenate -- with or
-        without header fields, so a separate defect from this one.
+        same bytes. The empty-bodied request's ``from_data`` round trip is
+        :meth:`test_httpv1_from_data_round_trips_a_message_with_an_empty_body`
+        (#1050).
 
         """
         import io
@@ -1118,6 +1118,35 @@ def test_httpv1_make_read_round_trips_a_message_with_no_header_fields(self) -> N
         parsed = HTTPv1(io.BytesIO(wire), len(wire))
         self.assertEqual(HTTPv1(**HTTPv1._make_data(parsed.info)).data, wire)
 
+    def test_httpv1_from_data_round_trips_a_message_with_an_empty_body(self) -> None:
+        """``from_data(info)`` rebuilds a message whose body is empty (#1050).
+
+        ``read`` records an empty body as :obj:`None`, and that stays the public
+        ``info.body`` value; ``_make_data`` used to pass it straight to ``make``,
+        which concatenated it onto the header bytes and raised ``TypeError``.
+        The response with a body is a regression guard: it round-tripped before
+        the fix and must keep doing so.
+
+        """
+        import io
+
+        from pcapkit.protocols.application.httpv1 import HTTP as HTTPv1
+
+        cases = {
+            'request with headers': (b'GET / HTTP/1.1\r\nHost: a\r\n\r\n', True),
+            'headerless request': (b'GET / HTTP/1.1\r\n\r\n', True),
+            'response with headers': (b'HTTP/1.1 204 No Content\r\nServer: x\r\n\r\n', True),
+            'response with a body': (b'HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nhi', False),
+        }
+        for name, (raw, empty) in cases.items():
+            with self.subTest(name):
+                parsed = HTTPv1(io.BytesIO(raw), len(raw))
+                if empty:
+                    self.assertIsNone(parsed.info.body)
+                rebuilt = HTTPv1.from_data(parsed.info)
+                self.assertEqual(rebuilt.data, raw)
+                self.assertEqual(rebuilt.info.body, parsed.info.body)
+
     def test_httpv1_still_refuses_the_http2_connection_preface(self) -> None:
         """Accepting zero fields must not let the HTTP/2 preface parse as HTTP/1 (#1041).