fix(http): give HTTP's version, data, start-line and settings errors the HTTP: prefix (#2298)

pcapkit/protocols/application/http.py

@@ -298,7 +298,7 @@ def read(self, length: 'Optional[int]' = None, *,
             elif version == 2:
                 from pcapkit.protocols.application.httpv2 import HTTP as protocol  # type: ignore[assignment] # isort: skip # pylint: disable=line-too-long,import-outside-toplevel
             else:
-                raise ProtocolError(f"invalid HTTP version: {version}")
+                raise ProtocolError(f'HTTP: invalid version: {version!r}')
 
             try:
                 http = protocol(self._data, length, **kwargs)
@@ -358,7 +358,7 @@ def make(self,
         elif version == 2:
             from pcapkit.protocols.application.httpv2 import HTTP as protocol  # type: ignore[assignment] # isort: skip # pylint: disable=line-too-long,import-outside-toplevel
         else:
-            raise ProtocolError(f"invalid HTTP version: {version}")
+            raise ProtocolError(f'HTTP: invalid version: {version!r}')
 
         # NOTE: ``protocol.make`` is an ordinary instance method (the abstract
         # declaration at ``ProtocolBase.make`` takes ``self``, and the
@@ -449,7 +449,8 @@ def _make_data(cls, data: '_PT') -> 'dict[str, Any]':  # type: ignore[override]
             return {'version': 1, **HTTPv1._make_data(data)}
         if isinstance(data, Data_HTTPv2):
             return {'version': 2, 'preface': bool(data.get('preface')), **HTTPv2._make_data(data)}
-        raise ProtocolError(f"invalid HTTP data: {type(data).__name__}")
+        raise ProtocolError(f'HTTP: invalid data: must be HTTP/1.* or HTTP/2 data, '
+                            f'not {type(data).__name__}')
 
     def _test_preface(self, length: 'int') -> 'bool':
         """Test whether the first ``length`` octets open with the HTTP/2 connection preface.
@@ -632,10 +633,10 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP':
         # test rejects only headers that no conforming sender writes, and it
         # gates this guess alone: an explicit ``version=2`` is not affected.
         if not _test_http2_frame(self._data[:length]):
-            raise ProtocolError("unknown HTTP version")
+            raise ProtocolError('HTTP: unknown version')
 
         from pcapkit.protocols.application.httpv2 import HTTP as HTTPv2  # isort: skip # pylint: disable=line-too-long,import-outside-toplevel
         with contextlib.suppress(ProtocolError, struct.error):
             return HTTPv2(self._data, length, **kwargs)
 
-        raise ProtocolError("unknown HTTP version")
+        raise ProtocolError('HTTP: unknown version')

pcapkit/protocols/application/httpv1.py

@@ -334,7 +334,7 @@ def _make_start_line(self, http_version: 'str | bytes', *,
         version = http_version.encode() if isinstance(http_version, str) else http_version
         if method is not None and status is None:
             if uri is None:
-                raise ProtocolError('HTTP request must have URI.')
+                raise ProtocolError('HTTP: invalid start line: a request needs a URI')
 
             if isinstance(method, Enum_Method):
                 meth = method.value.encode()
@@ -362,7 +362,8 @@ def _make_start_line(self, http_version: 'str | bytes', *,
             # status-code is 3DIGIT (RFC 9112, section 4), so keep a leading zero
             header_line = b'HTTP/%s %03d %s\r\n' % (version, status_code_val, msg)
         else:
-            raise ProtocolError('HTTP packet must be either request or response.')
+            raise ProtocolError('HTTP: invalid start line: must have either a method or a status, '
+                                f'not method={method!r} and status={status!r}')
         return header_line
 
     @classmethod

pcapkit/protocols/application/httpv2.py

@@ -1342,7 +1342,8 @@ def _make_http_settings(self, frame: 'Optional[Data_SettingsFrame]' = None, *,
                     )
                 settings_val.append(temp)
         else:
-            raise ProtocolError(f'HTTP/2 : [Type {Enum_Frame.SETTINGS}] invalid settings')
+            raise ProtocolError(f'HTTP/2: [Type {Enum_Frame.SETTINGS}] invalid settings: '
+                                f'{settings!r}')
 
         return Schema_SettingsFrame(
             settings=settings_val,

tests/protocols/application/test_http_from_data_1154_unit.py

@@ -106,7 +106,7 @@ def test_dispatcher_rejects_data_of_no_http_version(self) -> None:
         from pcapkit.protocols.application.http import HTTP
         from pcapkit.utilities.exceptions import ProtocolError
 
-        with self.assertRaisesRegex(ProtocolError, 'invalid HTTP data'):
+        with self.assertRaisesRegex(ProtocolError, '^HTTP: invalid data: '):
             HTTP._make_data({'version': 1})  # type: ignore[arg-type]
 
 

tests/protocols/application/test_http_guess_http2_1141_unit.py

@@ -108,7 +108,7 @@ def test_nine_zero_octets_are_not_guessed_as_http2(self) -> None:
         """The #1141 reproduction: a stream-0 ``DATA`` frame is not HTTP/2 by guess."""
         from pcapkit.utilities.exceptions import ProtocolError
 
-        with self.assertRaisesRegex(ProtocolError, 'unknown HTTP version'):
+        with self.assertRaisesRegex(ProtocolError, 'HTTP: unknown version'):
             self._guess(bytes(9))
 
     def test_explicit_version_still_parses_nine_zero_octets(self) -> None:
@@ -125,7 +125,7 @@ def test_headers_no_sender_writes_are_not_guessed(self) -> None:
 
         for name, octets in REJECTED_FRAMES:
             with self.subTest(frame=name):
-                with self.assertRaisesRegex(ProtocolError, 'unknown HTTP version'):
+                with self.assertRaisesRegex(ProtocolError, 'HTTP: unknown version'):
                     self._guess(bytes.fromhex(octets))
 
     def test_every_legal_frame_type_is_still_guessed_as_http2(self) -> None:

tests/protocols/application/test_http_message_prefix_2298_unit.py

@@ -0,0 +1,124 @@
+# -*- coding: utf-8 -*-
+"""HTTP's version, data, start-line and ``SETTINGS`` errors carry the ``HTTP:`` prefix. C.f. #2298.
+
+The rest of HTTP's messages read ``HTTP: invalid format``, the house
+``<prefix>: <lowercase description>: … {value!r}`` form (#2232, #2257). Before
+the fix these read ``invalid HTTP version: 3``, ``invalid HTTP data: dict`` and
+``unknown HTTP version`` from :class:`~pcapkit.protocols.application.http.HTTP`,
+and ``HTTP request must have URI.`` and
+``HTTP packet must be either request or response.`` from
+:meth:`httpv1.HTTP._make_start_line
+<pcapkit.protocols.application.httpv1.HTTP._make_start_line>`, capitalised, with
+a closing full stop and without the values that were rejected. The
+``SETTINGS`` one read ``HTTP/2 : [Type 4] invalid settings`` from
+:meth:`httpv2.HTTP._make_http_settings
+<pcapkit.protocols.application.httpv2.HTTP._make_http_settings>`, a space before
+the colon and no rejected value either.
+
+Every case builds its own octets in memory and reads no capture. Classes are
+imported inside each test, after
+:func:`~tests._support.reimport_once_per_class`, so they match the live
+:mod:`pcapkit` import.
+
+"""
+
+import io
+import unittest
+
+from tests._support import reimport_once_per_class
+
+
+class TestHTTPDispatcherMessages(unittest.TestCase):
+    """Pin the dispatcher's version and data messages."""
+
+    def setUp(self) -> None:
+        reimport_once_per_class(self)
+
+    def test_make_names_the_rejected_version(self) -> None:
+        from pcapkit.protocols.application.http import HTTP
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        with self.assertRaises(ProtocolError) as ctx:
+            HTTP.make(HTTP.__new__(HTTP), version=3)  # type: ignore[arg-type]
+        self.assertEqual(str(ctx.exception), 'HTTP: invalid version: 3')
+
+    def test_read_names_the_rejected_version(self) -> None:
+        from pcapkit.protocols.application.http import HTTP
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        raw = b'GET / HTTP/1.1\r\n\r\n'
+        with self.assertRaises(ProtocolError) as ctx:
+            HTTP(io.BytesIO(raw), len(raw), version='3')
+        self.assertEqual(str(ctx.exception), "HTTP: invalid version: '3'")
+
+    def test_make_data_names_the_rejected_type(self) -> None:
+        from pcapkit.protocols.application.http import HTTP
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        with self.assertRaises(ProtocolError) as ctx:
+            HTTP._make_data({'version': 1})  # type: ignore[arg-type]
+        self.assertEqual(str(ctx.exception),
+                         'HTTP: invalid data: must be HTTP/1.* or HTTP/2 data, not dict')
+
+    def test_guess_version_has_the_prefix(self) -> None:
+        from pcapkit.protocols.application.http import HTTP
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        raw = b'\x00' * 3
+        with self.assertRaises(ProtocolError) as ctx:
+            HTTP(io.BytesIO(raw), len(raw))
+        self.assertEqual(str(ctx.exception), 'HTTP: unknown version')
+
+
+class TestHTTPv1StartLineMessages(unittest.TestCase):
+    """Pin ``httpv1.HTTP._make_start_line``'s messages."""
+
+    def setUp(self) -> None:
+        reimport_once_per_class(self)
+
+    def test_request_without_uri(self) -> None:
+        from pcapkit.protocols.application.httpv1 import HTTP
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        with self.assertRaises(ProtocolError) as ctx:
+            HTTP(method='GET', http_version='1.1')
+        self.assertEqual(str(ctx.exception), 'HTTP: invalid start line: a request needs a URI')
+
+    def test_neither_or_both_of_method_and_status(self) -> None:
+        from pcapkit.protocols.application.httpv1 import HTTP
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        cases = (
+            ({}, 'not method=None and status=None'),
+            ({'method': 'GET', 'status': 200}, "not method='GET' and status=200"),
+        )
+        for kwargs, tail in cases:
+            with self.subTest(**kwargs):
+                with self.assertRaises(ProtocolError) as ctx:
+                    HTTP.__new__(HTTP)._make_start_line(http_version='1.1', **kwargs)
+                self.assertEqual(str(ctx.exception),
+                                 'HTTP: invalid start line: must have either a method or a '
+                                 f'status, {tail}')
+
+
+class TestHTTPv2SettingsMessage(unittest.TestCase):
+    """Pin ``httpv2.HTTP._make_http_settings``'s rejection message."""
+
+    def setUp(self) -> None:
+        reimport_once_per_class(self)
+
+    def test_settings_names_the_rejected_value(self) -> None:
+        from pcapkit.const.http.frame import Frame
+        from pcapkit.protocols.application.httpv2 import HTTP
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        for settings, tail in ((42, '42'), (None, 'None'), ('x', "'x'"), (4.5, '4.5')):
+            with self.subTest(settings=settings):
+                with self.assertRaises(ProtocolError) as ctx:
+                    bytes(HTTP(type=Frame.SETTINGS, frame={'settings': settings}))
+                self.assertEqual(str(ctx.exception),
+                                 f'HTTP/2: [Type 4] invalid settings: {tail}')
+
+
+if __name__ == '__main__':
+    unittest.main()

tests/protocols/application/test_http_unit.py

@@ -509,7 +509,7 @@ def test_guess_version_identifies_the_http2_connection_preface(self) -> None:
         preface's own ASCII as a frame header, ``b'PRI'`` becoming a declared
         length of 5,263,945, which the consistency check of #799/#802 refuses --
         so a real HTTP/2 connection opening, bare preface or
-        preface-plus-SETTINGS, would raise ``unknown HTTP version``.
+        preface-plus-SETTINGS, raises ``HTTP: unknown version``.
 
         Three things are asserted, because "it answers 2" alone would also be
         true of a parse that consumed the preface as framing:
@@ -683,7 +683,7 @@ def test_guess_version_commits_to_http1_once_the_start_line_says_so(self) -> Non
 
         Asserted on the message, because the exception *type* is
         ``ProtocolError`` either way: a payload both arms declined comes back
-        ``unknown HTTP version``, while these come back ``HTTP: invalid format``
+        ``HTTP: unknown version``, while these come back ``HTTP: invalid format``
         from the version that was actually identified.
 
         """
@@ -871,7 +871,7 @@ def test_guess_version_still_prefers_http1_for_http1_bytes(self) -> None:
                 self.assertEqual(guessed.info, explicit.info)
 
     def test_guess_version_still_reports_an_unknown_http_version(self) -> None:
-        """Bytes neither arm accepts must raise ``unknown HTTP version``.
+        """Bytes neither arm accepts must raise ``HTTP: unknown version``.
 
         ``_guess_version``'s closing ``raise`` is only reachable when *both*
         arms fail with :class:`~pcapkit.utilities.exceptions.ProtocolError`, so
@@ -899,7 +899,7 @@ def test_guess_version_still_reports_an_unknown_http_version(self) -> None:
 
         with self.assertRaises(ProtocolError) as ctx:
             HTTP(io.BytesIO(raw), len(raw))
-        self.assertEqual(str(ctx.exception), 'unknown HTTP version')
+        self.assertEqual(str(ctx.exception), 'HTTP: unknown version')
 
     def test_guess_version_reports_unknown_version_for_a_short_payload(self) -> None:
         """A payload too short for an HTTP/2 frame header must not leak :exc:`struct.error`.
@@ -1004,7 +1004,7 @@ def test_httpv1_reads_a_request_with_no_header_fields(self) -> None:
         legal. The header block is then the start line alone, with no CRLF in
         it, which ``_read_http_header`` must not refuse as malformed. Asserted
         through the dispatcher too: were ``test_start_line`` to mirror such a
-        refusal, ``HTTP`` would answer ``unknown HTTP version`` for the same
+        refusal, ``HTTP`` would answer ``HTTP: unknown version`` for the same
         bytes.
 
         """
@@ -1129,8 +1129,8 @@ def test_httpv1_still_refuses_the_http2_connection_preface(self) -> None:
 
         cases = (
             ('preface', b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n', 'HTTP/2: connection preface with no frame'),
-            ('truncated preface', b'PRI * HTTP/2.0\r\n\r\n', 'unknown HTTP version'),
-            ('preface, corrupt tail', b'PRI * HTTP/2.0\r\n\r\nXX\r\n\r\n', 'unknown HTTP version'),
+            ('truncated preface', b'PRI * HTTP/2.0\r\n\r\n', 'HTTP: unknown version'),
+            ('preface, corrupt tail', b'PRI * HTTP/2.0\r\n\r\nXX\r\n\r\n', 'HTTP: unknown version'),
         )
 
         for label, raw, dispatched in cases:
@@ -1398,7 +1398,7 @@ def test_guess_version_does_not_suppress_struct_error_on_the_http1_arm(self) ->
         and the HTTP/2 arm accepts any payload of at least nine octets -- so a
         swallowed error on arm 1 becomes a confident ``version='2'`` for a
         message that is plainly HTTP/1, which is the mislabel #787 exists to
-        stop rather than a tidier failure. ``unknown HTTP version`` is only the
+        stop rather than a tidier failure. ``HTTP: unknown version`` is only the
         best case, needing arm 2 to decline as well.
 
         The *outer*-header slice of the class this suppression exists for is