fix(protocols): resolve a str enum name when its argument defaults to None (#2356)

pcapkit/protocols/application/httpv1.py

@@ -348,8 +348,13 @@ def _make_start_line(self, http_version: 'str | bytes', *,
 
             header_line = b'%s %s HTTP/%s\r\n' % (meth, uri_val, version)
         elif method is None and status is not None:
-            status_code = self._make_index(status, status_default, namespace=status_namespace,
-                                           reversed=status_reversed, pack=False)
+            # NOTE: ``status`` defaults to :data:`None`, so ``_make_index`` has no
+            # enum default to infer the namespace of a :obj:`str` member name from;
+            # name it here rather than leave the name unresolvable (#2356).
+            status_code = self._make_index(
+                status, status_default,
+                namespace=status_namespace if status_namespace is not None else Enum_StatusCode,
+                reversed=status_reversed, pack=False)
             status_code_val = int(status_code)
 
             if message is None:

pcapkit/protocols/internet/hopopt.py

@@ -1746,8 +1746,13 @@ def _make_opt_smf_dpd(self, code: 'Enum_Option', opt: 'Optional[Data_SMFIdentifi
             tid_len = len(tid_bytes)
 
             if tid_type is not None:
-                tid_kind = self._make_index(tid_type, tid_type_default, namespace=tid_type_namespace,
-                                            reversed=tid_type_reversed, pack=False)
+                # NOTE: ``tid_type`` defaults to :data:`None`, so ``_make_index`` has no
+                # enum default to infer the namespace of a :obj:`str` member name from;
+                # name it here rather than leave the name unresolvable (#2356).
+                tid_kind = self._make_index(
+                    tid_type, tid_type_default,
+                    namespace=tid_type_namespace if tid_type_namespace is not None else Enum_TaggerID,
+                    reversed=tid_type_reversed, pack=False)
             elif tid_len == 0:
                 tid_kind = Enum_TaggerID.NULL
             elif tid_len == 4:
@@ -2056,8 +2061,13 @@ def _make_opt_mpl(self, code: 'Enum_Option', opt: 'Optional[Data_MPLOption]' = N
             seed = opt.seed_id
 
         if seed_type is not None:
-            kind = self._make_index(seed_type, seed_type_default, namespace=seed_type_namespace,
-                                    reversed=seed_type_reversed, pack=False)
+            # NOTE: ``seed_type`` defaults to :data:`None`, so ``_make_index`` has no
+            # enum default to infer the namespace of a :obj:`str` member name from;
+            # name it here rather than leave the name unresolvable (#2356).
+            kind = self._make_index(
+                seed_type, seed_type_default,
+                namespace=seed_type_namespace if seed_type_namespace is not None else Enum_SeedID,
+                reversed=seed_type_reversed, pack=False)
         elif seed is None or isinstance(seed, ipaddress.IPv6Address):
             kind = Enum_SeedID.IPV6_SOURCE_ADDRESS
         else:

pcapkit/protocols/internet/ipv6_opts.py

@@ -1734,8 +1734,13 @@ def _make_opt_smf_dpd(self, code: 'Enum_Option', opt: 'Optional[Data_SMFIdentifi
             tid_len = len(tid_bytes)
 
             if tid_type is not None:
-                tid_kind = self._make_index(tid_type, tid_type_default, namespace=tid_type_namespace,
-                                            reversed=tid_type_reversed, pack=False)
+                # NOTE: ``tid_type`` defaults to :data:`None`, so ``_make_index`` has no
+                # enum default to infer the namespace of a :obj:`str` member name from;
+                # name it here rather than leave the name unresolvable (#2356).
+                tid_kind = self._make_index(
+                    tid_type, tid_type_default,
+                    namespace=tid_type_namespace if tid_type_namespace is not None else Enum_TaggerID,
+                    reversed=tid_type_reversed, pack=False)
             elif tid_len == 0:
                 tid_kind = Enum_TaggerID.NULL
             elif tid_len == 4:
@@ -2044,8 +2049,13 @@ def _make_opt_mpl(self, code: 'Enum_Option', opt: 'Optional[Data_MPLOption]' = N
             seed = opt.seed_id
 
         if seed_type is not None:
-            kind = self._make_index(seed_type, seed_type_default, namespace=seed_type_namespace,
-                                    reversed=seed_type_reversed, pack=False)
+            # NOTE: ``seed_type`` defaults to :data:`None`, so ``_make_index`` has no
+            # enum default to infer the namespace of a :obj:`str` member name from;
+            # name it here rather than leave the name unresolvable (#2356).
+            kind = self._make_index(
+                seed_type, seed_type_default,
+                namespace=seed_type_namespace if seed_type_namespace is not None else Enum_SeedID,
+                reversed=seed_type_reversed, pack=False)
         elif seed is None or isinstance(seed, ipaddress.IPv6Address):
             kind = Enum_SeedID.IPV6_SOURCE_ADDRESS
         else:

tests/protocols/test_make_index_str_unit.py

@@ -9,6 +9,12 @@
 enumeration class of the argument's default value, and builds the same bytes
 as the member itself and as its integer value.
 
+GitHub issue #2356 is the other half: an argument carrying an optional field
+defaults to :data:`None`, which is no enumeration member, so there is nothing
+for that inference to read. Such a maker names its enumeration at the
+:meth:`~pcapkit.protocols.protocol.ProtocolBase._make_index` call instead, and
+:data:`NONE_DEFAULT_CASES` carries one case per argument that does.
+
 Every case builds its own packet in memory and reads no capture. Classes are
 imported inside each test, after
 :func:`~tests._support.reimport_once_per_class`, so they belong to the live
@@ -38,6 +44,27 @@ def _tcp_alternate_checksum(**kwargs: 'Any') -> 'Any':
     )
 
 
+def _ipv6_option(protocol: 'str', option: 'str') -> 'Any':
+    """A builder putting one option into an IPv6 extension header.
+
+    Args:
+        protocol: import path of the extension header class
+        option: :class:`~pcapkit.const.ipv6.option.Option` member name
+
+    Returns:
+        A callable taking the option's own keywords and returning the built
+        header, so that an option argument is reached the way ``make()``
+        reaches it.
+
+    """
+    def build(**kwargs: 'Any') -> 'Any':
+        code = getattr(_attr('pcapkit.const.ipv6.option.Option'), option)
+        transtype = _attr('pcapkit.const.reg.transtype.TransType')
+        return _attr(protocol)(options=[(code, kwargs)],
+                               next=transtype.IPv6_NoNxt, payload=b'')
+    return build
+
+
 #: One ``make()`` per protocol family:
 #: (protocol class or builder, argument, enum class, non-default member).
 CASES = {
@@ -56,6 +83,27 @@ def _tcp_alternate_checksum(**kwargs: 'Any') -> 'Any':
 }
 
 
+#: One ``make()`` per argument whose own default is :data:`None`, so that no enum
+#: default is there for the namespace to be inferred from (#2356): (builder,
+#: argument, enum class, member, the other keywords the option needs). Each
+#: member is one the maker would not have arrived at on its own, so a name that
+#: went unresolved cannot pass as a name that resolved.
+NONE_DEFAULT_CASES = {
+    'internet/hopopt MPL': (_ipv6_option('pcapkit.protocols.internet.hopopt.HOPOPT', 'MPL_Option'),
+                            'seed_type', 'pcapkit.const.ipv6.seed_id.SeedID',
+                            'SEEDID_64_BIT_UNSIGNED_INTEGER', {'seed': 5, 'seq': 7}),
+    'internet/ipv6_opts MPL': (_ipv6_option('pcapkit.protocols.internet.ipv6_opts.IPv6_Opts', 'MPL_Option'),
+                               'seed_type', 'pcapkit.const.ipv6.seed_id.SeedID',
+                               'SEEDID_64_BIT_UNSIGNED_INTEGER', {'seed': 5, 'seq': 7}),
+    'internet/hopopt SMF_DPD': (_ipv6_option('pcapkit.protocols.internet.hopopt.HOPOPT', 'SMF_DPD'),
+                                'tid_type', 'pcapkit.const.ipv6.tagger_id.TaggerID',
+                                'DEFAULT', {'tid': b'\x0a\x0b\x0c\x0d', 'id': b'\xaa\xbb'}),
+    'internet/ipv6_opts SMF_DPD': (_ipv6_option('pcapkit.protocols.internet.ipv6_opts.IPv6_Opts', 'SMF_DPD'),
+                                   'tid_type', 'pcapkit.const.ipv6.tagger_id.TaggerID',
+                                   'DEFAULT', {'tid': b'\x0a\x0b\x0c\x0d', 'id': b'\xaa\xbb'}),
+}
+
+
 class TestMakeIndexStr(unittest.TestCase):
     """Pin member-name resolution in ``_make_index`` across families."""
 
@@ -74,12 +122,54 @@ def test_member_name_matches_member_and_int(self) -> None:
                 # the member is not the default, so the name was not ignored
                 self.assertNotEqual(build().data, expected)
 
+    def test_member_name_resolves_where_the_argument_defaults_to_none(self) -> None:
+        """An argument whose own default is :data:`None` resolves a name too. See #2356.
+
+        The namespace is inferred from the enumeration class of the argument's
+        default value, which an argument defaulting to :data:`None` -- because
+        the field it carries is optional -- does not have. Such a maker names
+        its enumeration at the call instead, so that a member name resolves
+        without the caller also passing a ``*_namespace``.
+        """
+        for case, (build, arg, enum_path, member, extra) in NONE_DEFAULT_CASES.items():
+            with self.subTest(case=case):
+                value = getattr(_attr(enum_path), member)
+
+                expected = build(**extra, **{arg: value}).data
+                self.assertEqual(build(**extra, **{arg: member}).data, expected)
+                self.assertEqual(build(**extra, **{arg: int(value)}).data, expected)
+                # the maker derives the field from its siblings when the argument
+                # is not given, and derives a different value here
+                self.assertNotEqual(build(**extra).data, expected)
+
+    def test_http_status_member_name_resolves(self) -> None:
+        """An HTTP/1.* status code resolves from its member name. See #2356.
+
+        ``status`` defaults to :data:`None`, since a response has a status where
+        a request has a method and a URI, so its namespace is named at the call.
+        :meth:`~pcapkit.protocols.protocol.ProtocolBase._make_index` yields the
+        member's integer, and a start line built from the integer carries no
+        reason phrase; only the member itself brings one along.
+        """
+        http = _attr('pcapkit.protocols.application.httpv1.HTTP')
+        status_code = _attr('pcapkit.const.http.status_code.StatusCode')
+
+        self.assertEqual(http(status='CODE_404').data, http(status=404).data)
+        self.assertEqual(http(status='CODE_404').data, b'HTTP/1.1 404 \r\n\r\n')
+        self.assertEqual(http(status=status_code.CODE_404).data, b'HTTP/1.1 404 Not Found\r\n\r\n')
+
     def test_unknown_name_still_raises(self) -> None:
         exc = _attr('pcapkit.utilities.exceptions.ProtocolNotImplemented')
         ethernet = _attr('pcapkit.protocols.link.ethernet.Ethernet')
         with self.assertRaises(exc):
             ethernet(type='No_Such_EtherType')
 
+        # a named namespace resolves a member name, not an arbitrary string: an
+        # HTTP status code member is spelled ``CODE_404``, so ``'OK'`` is none
+        http = _attr('pcapkit.protocols.application.httpv1.HTTP')
+        with self.assertRaises(exc):
+            http(status='OK')
+
     def test_unknown_name_falls_back_to_default(self) -> None:
         ethernet = _attr('pcapkit.protocols.link.ethernet.Ethernet')
         proto = ethernet(type='No_Such_EtherType', type_default=0x1234)
@@ -138,10 +228,14 @@ def test_without_inferable_enum_still_raises(self) -> None:
         with self.assertRaises(exc):
             protocol._make_index('Internet_Protocol_version_6')
 
-        # ``status`` defaults to ``None``, so its enum cannot be inferred
-        http = _attr('pcapkit.protocols.application.httpv1.HTTP')
+        # an argument defaulting to ``None`` carries no enum to infer from, so a
+        # maker that names none at the call leaves the name unresolvable
+        class OptionalArgument(protocol):  # type: ignore[misc,valid-type]
+            def make_optional(self, arg: 'Any' = None, arg_namespace: 'Any' = None) -> 'int':
+                return self._make_index(arg, namespace=arg_namespace)
+
         with self.assertRaises(exc):
-            http(status='OK')
+            OptionalArgument.make_optional(OptionalArgument, arg='Internet_Protocol_version_6')
 
 
 if __name__ == '__main__':