protocols: bind HTTP.make to a real instance, and wrap SETTINGS' item schema

pcapkit/protocols/application/http.py

@@ -131,7 +131,23 @@ def make(self,
             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}")
-        return protocol.make(**kwargs)  # type: ignore[return-value]
+
+        # NOTE: ``protocol.make`` is an ordinary instance method (the abstract
+        # declaration at ``ProtocolBase.make`` takes ``self``, and the
+        # versioned overrides use instance-bound helpers such as
+        # ``self._make_index``/``self.__frame__``), so calling it on the
+        # class itself -- as this used to -- left ``self`` unfilled and raised
+        # ``TypeError`` for every real call; see GH-452. There is no ``file``
+        # or ``length`` to construct with here, since building a packet from
+        # keyword arguments is the inverse of parsing one, so a bare instance
+        # via ``protocol.__new__`` -- bypassing ``__init__``'s parse/pack
+        # machinery entirely -- is what the versioned ``make`` needs to be
+        # called on. This is safe only as long as the versioned ``make`` never
+        # reads state that ``__init__``/``__post_init__`` would otherwise have
+        # established (neither ``HTTPv1.make`` nor ``HTTPv2.make`` does today);
+        # a future ``make`` override that reaches for such state would need a
+        # different dispatch here.
+        return protocol.__new__(protocol).make(**kwargs)  # type: ignore[return-value]
 
     ##########################################################################
     # Utilities.

pcapkit/protocols/schema/application/httpv2.py

@@ -282,7 +282,7 @@ class Flags(FrameType.Flags):
     #: Settings.
     settings: 'list[SettingPair]' = ListField(
         length=lambda pkt: pkt['__length__'],
-        item_type=SettingPair,  # type: ignore[arg-type]
+        item_type=SchemaField(schema=SettingPair),
     )
 
     if TYPE_CHECKING:

tests/protocols/application/test_http_unit.py

@@ -144,6 +144,74 @@ def make(**kwargs):
             with self.assertRaises(ProtocolError):
                 HTTP._guess_version(http, 9)
 
+    def test_http_make_dispatches_to_real_versioned_classes(self) -> None:
+        """Regression test for GH-452.
+
+        ``HTTP.make`` used to dispatch with ``protocol.make(**kwargs)``, where
+        ``protocol`` is the imported *class* -- an unbound call, since neither
+        ``HTTPv1.make`` nor ``HTTPv2.make`` is declared ``staticmethod`` or
+        ``classmethod`` on the real class (both are plain ``def make(self,
+        ...)``, matching the abstract ``ProtocolBase.make``). Every real call
+        therefore raised ``TypeError: make() missing 1 required positional
+        argument: 'self'``.
+
+        This deliberately exercises the **real** ``HTTPv1``/``HTTPv2``
+        classes rather than a fake with a ``staticmethod`` ``make`` --
+        ``test_http_read_make_and_guess_version_delegation_paths`` above uses
+        exactly such a fake, which is what let the original defect through:
+        a ``staticmethod`` absorbs an unbound call the same as a bound one,
+        so a fake-based assertion passes whether or not ``self`` is actually
+        threaded through.
+        """
+        from pcapkit.protocols.application.http import HTTP
+        from pcapkit.protocols.schema.application.httpv1 import HTTP as Schema_HTTPv1
+        from pcapkit.protocols.schema.application.httpv2 import HTTP as Schema_HTTPv2
+
+        http = object.__new__(HTTP)
+
+        schema_v1 = http.make(version=1, method='GET', uri='/index.html')
+        self.assertIsInstance(schema_v1, Schema_HTTPv1)
+        self.assertIn(b'GET /index.html HTTP/1.1\r\n', schema_v1.data)
+
+        schema_v2 = http.make(version=2, sid=1, frame=b'payload')
+        self.assertIsInstance(schema_v2, Schema_HTTPv2)
+        self.assertEqual(schema_v2.stream['sid'], 1)
+        self.assertEqual(schema_v2.frame, b'payload')
+
+    def test_http_construction_reaches_the_versioned_make_callee(self) -> None:
+        """Regression test for GH-452, using the corrected reproduction.
+
+        The issue's original reproduction called ``HTTP.make(version=1,
+        ...)`` directly on the class -- but the *outer* ``HTTP.make`` is
+        itself an ordinary instance method, so that call fails at the outer
+        method and never demonstrates anything about the inner
+        ``protocol.make(**kwargs)`` dispatch this issue is actually about.
+
+        The real, supported entry point is construction:
+        :meth:`ProtocolBase.__init__` (``protocol.py:519``, the ``**kwargs``-
+        only overload at ``:517``) calls ``self.pack(**kwargs)`` when built
+        with no ``file``, and :meth:`ProtocolBase.pack` (``protocol.py:284``)
+        is ``self.__header__ = self.make(**kwargs)`` -- a *bound* call on the
+        outer ``HTTP`` instance, which is what actually reaches the inner,
+        previously-unbound ``protocol.make(**kwargs)``. So the reachable
+        reproduction is ``HTTP(version=1, ...)``, not ``HTTP.make(...)``:
+
+        - on the unfixed code, this raises ``TypeError: HTTP.make() missing
+          1 required positional argument: 'self'`` -- the *same* exception
+          text as the wrong reproduction, but reached legitimately this time.
+        - on the fixed code, deliberately minimal keyword arguments (no
+          ``method``/``status``) reach ``HTTPv1.make`` and are rejected
+          *there*, as ``ProtocolError: HTTP/1: invalid format`` -- proving
+          the callee was actually reached, which "no ``TypeError``" alone
+          would not.
+        """
+        from pcapkit.protocols.application.http import HTTP
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        with self.assertRaises(ProtocolError) as ctx:
+            HTTP(version=1, http_version='1.1', method='GET', uri='/')
+        self.assertEqual(str(ctx.exception), 'HTTP/1: invalid format')
+
     def test_http_read_explicit_version_uses_same_buffer_as_guess(self) -> None:
         """Regression test for GH-447.
 
@@ -762,6 +830,61 @@ def test_httpv2_frame_constructors_cover_all_frame_types_and_branches(self) -> N
         with self.assertRaises(ProtocolError):
             proto.make(type=Frame.DATA, frame=object())
 
+    def test_settings_frame_settings_field_wraps_item_schema(self) -> None:
+        """Regression test for GH-459.
+
+        ``SettingsFrame.settings`` used to pass the bare ``SettingPair``
+        *class* as ``ListField``'s ``item_type``, where every sibling
+        (``tcp.py``'s ``SACK.sack``, ``hip.py``, ``mh.py``'s
+        ``CGAParametersOption.parameters``, ``sctp.py``'s
+        ``gap_blocks``/``dup_tsn``) wraps its schema item in a
+        :class:`~pcapkit.corekit.fields.misc.SchemaField`. A bare
+        ``SchemaMeta`` is not a field instance, so ``ListField.unpack``'s
+        schema branch -- ``field = self._item_type(packet)`` -- constructed a
+        ``SettingPair`` from the packet *dict* instead of configuring a
+        per-item field, and the ``isinstance(self._item_type, SchemaField)``
+        check that picks the schema branch was ``False`` for a bare class in
+        the first place, so it fell through to the plain-field branch and
+        failed there instead: ``field.length`` does not exist on a
+        ``SettingPair`` instance.
+
+        This is a unit-level check on the field wiring, not an end-to-end
+        ``HTTPv2`` round trip: the SETTINGS frame's pack path still dies
+        earlier on ``KeyError: 'flags'``, raised by ``FrameType.post_process``
+        at ``schema/application/httpv2.py:144`` where it reaches the enclosing
+        header's ``flags`` field through a nested packet context that cannot
+        see it (GH-445, fixed by the still-open PR #457), so a real
+        ``SettingsFrame.pack()``/``HTTPv2(...).make()`` round trip through this
+        field remains unreachable until that lands. The
+        ``httpv2-frame/SETTINGS`` entry in ``EXPECTED_FAILURES`` records that
+        same ``KeyError: 'flags'``.
+        """
+        from pcapkit.corekit.fields.misc import SchemaField
+        from pcapkit.protocols.schema.application.httpv2 import SettingPair, SettingsFrame
+
+        field = SettingsFrame.__fields__['settings']
+
+        # The type-check the ``# type: ignore[arg-type]`` used to silence:
+        # ``item_type`` must be a field instance, not the schema class itself.
+        self.assertIsInstance(field._item_type, SchemaField)
+        self.assertIs(field._item_type.schema, SettingPair)
+
+        # Two SETTINGS pairs, 6 octets each: HEADER_TABLE_SIZE=4096,
+        # ENABLE_PUSH=0.
+        raw = (1).to_bytes(2, 'big') + (4096).to_bytes(4, 'big') \
+            + (2).to_bytes(2, 'big') + (0).to_bytes(4, 'big')
+        packet = {'__length__': len(raw)}
+
+        settings = field(packet).unpack(raw, packet)
+
+        self.assertEqual(len(settings), 2)
+        self.assertIsInstance(settings[0], SettingPair)
+        self.assertIsInstance(settings[1], SettingPair)
+        self.assertEqual(settings[0].id, 1)
+        self.assertEqual(settings[0].value, 4096)
+        self.assertEqual(settings[1].id, 2)
+        self.assertEqual(settings[1].value, 0)
+
     def test_httpv2_callable_frame_registry_paths(self) -> None:
         from pcapkit.const.http.frame import Frame
         from pcapkit.protocols.application.httpv2 import HTTP as HTTPv2