fix(ipv6_ext): stop the base make's keywords leaking into subclasses that ignore them (#2286)

docs/source/pcapkit/protocols/protocol.rst

@@ -67,6 +67,8 @@ Internal Definitions below.
       :no-value:
    .. autoattribute:: __keywords__
       :no-value:
+   .. autoattribute:: __own_make_keywords__
+      :no-value:
 
    .. autoattribute:: __schema__
       :no-value:
@@ -234,6 +236,7 @@ Internal Definitions
    .. py:attribute:: __layer__
                      __proto__
                      __keywords__
+                     __own_make_keywords__
                      __schema__
                      __header__
                      _data
@@ -252,7 +255,8 @@ Internal Definitions
       :no-contents-entry:
 
       :attr:`~Protocol.__layer__`, :attr:`~Protocol.__proto__`,
-      :attr:`~Protocol.__keywords__`, :attr:`~Protocol.__schema__`,
+      :attr:`~Protocol.__keywords__`, :attr:`~Protocol.__own_make_keywords__`,
+      :attr:`~Protocol.__schema__`,
       :attr:`~Protocol.__header__`, :attr:`~Protocol._data`,
       :attr:`~Protocol._file`, :attr:`~Protocol._info`,
       :attr:`~Protocol._next`, :attr:`~Protocol._protos`,

pcapkit/protocols/internet/ipv6_ext.py

@@ -236,6 +236,12 @@ class keywords implement it.
 
     """
 
+    #: Every subclass overrides :meth:`make` with its own fields and forwards
+    #: nothing to this one, so this class's ``len`` and ``data`` are accepted
+    #: only by this class itself. Without the marker, a subclass whose ``make``
+    #: does not declare them would accept and then drop them (:issue:`2286`).
+    __own_make_keywords__ = True
+
     ##########################################################################
     # Properties.
     ##########################################################################
@@ -500,10 +506,14 @@ def make(self, *,
             construction**: a plain
             ``IPv6_Ext(next=..., len=..., data=..., payload=...)`` raises
             ``UnsupportedCall: IPv6_Ext: unexpected keyword(s)``. The
-            ``__keywords__`` escape hatch would cover that too, with no
-            difference for subclasses: signatures and ``__keywords__`` are
-            both unioned down the MRO, so every subclass's allowlist takes
-            these names either way.
+            ``__keywords__`` escape hatch would cover that too, but a set
+            there is unioned down the MRO, so every subclass would accept
+            these names and drop them. A ``make`` signature can be kept from
+            subclasses instead: this class sets
+            :attr:`ProtocolBase.__own_make_keywords__
+            <pcapkit.protocols.protocol.ProtocolBase.__own_make_keywords__>`,
+            so a subclass that overrides ``make`` accepts ``len`` or ``data``
+            only where its own ``make`` declares it (:issue:`2286`).
 
             They are keyword-*only* because this class is also a base and each
             subclass's ``make`` puts different names in the same positions. As

pcapkit/protocols/protocol.py

@@ -281,7 +281,11 @@ def _declared_keywords(cls: 'type') -> 'Optional[frozenset[str]]':
 
     The walk covers the whole MRO rather than the most derived override of each
     method, so that a subclass declaring its own keyword and forwarding the rest
-    to its parent does not make the parent's keywords unreachable.
+    to its parent does not make the parent's keywords unreachable. The one
+    exception is a class that sets :attr:`ProtocolBase.__own_make_keywords__
+    <pcapkit.protocols.protocol.ProtocolBase.__own_make_keywords__>`: its
+    ``make`` counts only while it is the ``make`` of ``cls``, not once a
+    subclass overrides it (:issue:`2286`).
 
     """
     try:
@@ -291,6 +295,9 @@ def _declared_keywords(cls: 'type') -> 'Optional[frozenset[str]]':
 
     unchecked = False
     names = set(OUT_OF_BAND_KEYWORDS)
+    # NOTE: The class whose ``make`` is the one ``cls`` resolves to; any other
+    # ``make`` in the MRO has been overridden.
+    make_owner = next((klass for klass in cls.__mro__ if 'make' in klass.__dict__), None)
     for klass in cls.__mro__:
         # NOTE: A keyword read out of ``**kwargs`` by name is invisible to
         # :func:`inspect.signature`, so the class lists it in ``__keywords__``.
@@ -323,6 +330,13 @@ def _declared_keywords(cls: 'type') -> 'Optional[frozenset[str]]':
             if func is None:
                 continue
 
+            # NOTE: A ``make`` whose keywords its class keeps to itself is read
+            # only while it is the one ``cls`` uses. Read from ``__dict__`` so
+            # the marker, like the :obj:`None` opt-out above, is not inherited.
+            if (method == 'make' and klass is not make_owner
+                    and klass.__dict__.get('__own_make_keywords__', False)):
+                continue
+
             try:
                 signature = inspect.signature(func)
             except (TypeError, ValueError):  # pragma: no cover
@@ -556,6 +570,18 @@ class ProtocolBase(Generic[_PT, _ST], metaclass=ProtocolMeta):
     #: classes that can have it.
     __keywords__: 'Optional[frozenset[str]]' = frozenset()
 
+    #: Whether the parameters of this class's own :meth:`make` are accepted only
+    #: while that ``make`` is the one in use. By default
+    #: :func:`_declared_keywords` takes every ``make`` in the MRO, so a subclass
+    #: that overrides ``make`` and forwards the rest to its parent keeps the
+    #: parent's keywords. A base whose subclasses override ``make`` *without*
+    #: forwarding sets this, so that its keywords are refused for them rather than
+    #: accepted and dropped -- :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`
+    #: does, for ``len`` and ``data`` (:issue:`2286`). Read from the defining
+    #: class alone, so it is not inherited, and it leaves :attr:`__keywords__`
+    #: and the other construction methods unaffected.
+    __own_make_keywords__: 'bool' = False
+
     #: Whether this instance is being rebuilt by :meth:`from_data` from a parsed
     #: data model, as against constructed from keywords somebody wrote. It governs
     #: only whether the construction keyword check of :meth:`__init__` raises or

tests/protocols/internet/test_ipv6_ext_unit.py

@@ -757,6 +757,36 @@ def test_an_absent_or_unregistered_alias_uses_the_generic_rule(self) -> None:
                 # ``read`` with no explicit length falls back to ``len(self)``
                 self.assertEqual(inst.read().length, 16)
 
+    def test_base_make_keywords_are_refused_where_a_subclass_ignores_them(self) -> None:
+        """#2286: without ``__own_make_keywords__``, the base ``make``'s ``len``
+        and ``data`` stay in every subclass's construction allowlist, so
+        ``IPv6_Route(len=99)`` builds the same octets as ``IPv6_Route()``. With
+        the marker, a subclass accepts each only if its own ``make`` declares
+        it, and accepts the ``next`` they all declare; the base keeps all three.
+        """
+        import inspect
+
+        from pcapkit.const.reg.transtype import TransType
+        from pcapkit.protocols.internet.ipv6_ext import IPv6_Ext
+        from pcapkit.protocols.protocol import _declared_keywords
+        from pcapkit.utilities.exceptions import UnsupportedCall
+
+        self.assertTrue({'next', 'len', 'data'} <= _declared_keywords(IPv6_Ext))
+        self.assertEqual(bytes(IPv6_Ext(next=TransType.TCP, len=0, data=b'\x00' * 6,
+                                        version=6))[:2], bytes([int(TransType.TCP), 0]))
+
+        for klass in self._family():
+            own = inspect.signature(klass.make).parameters
+            for name, value in (('len', 99), ('data', b'zzzz')):
+                with self.subTest(klass=klass.__name__, keyword=name):
+                    if name in own:
+                        self.assertIn(name, _declared_keywords(klass))
+                        continue
+                    with self.assertRaisesRegex(UnsupportedCall, f"unexpected keyword\\(s\\): '{name}'"):
+                        klass(**{name: value})
+            with self.subTest(klass=klass.__name__, keyword='next'):
+                self.assertIsInstance(bytes(klass(next=TransType.TCP)), bytes)
+
     def test_length_hint_is_the_eight_octet_rfc_6564_minimum(self) -> None:
         """Eight: :rfc:`6564#section-4`'s Hdr Ext Len excludes the first 8
         octets, so no conforming header is shorter (:rfc:`8200#section-4`).

tests/protocols/test_construction_keyword_check_unit.py

@@ -426,6 +426,51 @@ def make(self, ham: 'bytes' = b'ab', **kwargs: 'Any') -> 'Any':
             with self.subTest(keyword=name):
                 self.assertIn(name, accepted)
 
+    def test_a_base_may_keep_its_make_keywords_to_itself(self) -> None:
+        """``__own_make_keywords__`` drops a base's ``make`` once it is overridden.
+
+        For a base whose subclasses replace ``make`` without forwarding to it, as
+        :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`'s do (#2286). The
+        base's ``make`` counts for the base itself and for a subclass that does
+        not override it, and the marker is not inherited: an overriding
+        subclass's own ``make`` is handed down as usual. ``__keywords__`` is
+        unaffected.
+
+        """
+        from pcapkit.protocols.protocol import _declared_keywords
+        from pcapkit.utilities.exceptions import UnsupportedCall
+
+        base = _protocol_class(__own_make_keywords__=True,
+                               __keywords__=frozenset({'eggs'}))
+
+        class Inheriting(base):  # type: ignore[misc,valid-type]
+            pass
+
+        class Overriding(base):  # type: ignore[misc,valid-type]
+            def make(self, ham: 'bytes' = b'ab', **kwargs: 'Any') -> 'Any':
+                return super().make(spam=ham)  # the base's name, never the caller's
+
+        class Grandchild(Overriding):  # type: ignore[misc,valid-type]
+            def make(self, beans: 'bytes' = b'ab', **kwargs: 'Any') -> 'Any':
+                return super().make(ham=beans, **kwargs)
+
+        for klass in (base, Inheriting):
+            with self.subTest(klass=klass.__name__):
+                self.assertIn('spam', _declared_keywords(klass))
+        self.assertEqual(bytes(Inheriting(spam=b'xy')), b'xy')
+
+        for klass, own in ((Overriding, {'ham'}), (Grandchild, {'ham', 'beans'})):
+            with self.subTest(klass=klass.__name__):
+                accepted = _declared_keywords(klass)
+                self.assertNotIn('spam', accepted)
+                self.assertTrue(own | {'eggs'} <= accepted)
+
+        with self.assertRaisesRegex(UnsupportedCall, r"unexpected keyword\(s\): 'spam'"):
+            Overriding(spam=b'xy')
+        # Its own keyword, and the base's ``__keywords__``.
+        self.assertEqual(bytes(Overriding(ham=b'xy', eggs=1)), b'xy')
+        self.assertEqual(bytes(Grandchild(beans=b'xy')), b'xy')
+
 
 @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed')
 class ScopeTests(unittest.TestCase):