fix(dumpkit)!: output writers drop or corrupt what the packets carried

pcapkit/dumpkit/common.py

@@ -16,6 +16,8 @@
 import enum
 import ipaddress
 import json
+import re
+import string
 import xml.sax.saxutils
 from typing import TYPE_CHECKING
 
@@ -26,6 +28,7 @@
 
 from pcapkit.corekit.infoclass import Info
 from pcapkit.corekit.multidict import MultiDict, OrderedMultiDict
+from pcapkit.corekit.sentinels import NoValueType, NullType
 from pcapkit.protocols.schema.schema import Schema
 from pcapkit.utilities.exceptions import UnsupportedCall
 from pcapkit.utilities.logging import get_logger
@@ -34,7 +37,7 @@
 
 
 if TYPE_CHECKING:
-    from typing import Any, DefaultDict, Optional, TextIO, Type
+    from typing import Any, DefaultDict, Iterator, Optional, TextIO, Type
 
     from dictdumper.dumper import Dumper as ABCDumper
     from typing_extensions import Literal
@@ -44,6 +47,16 @@
 #: :data:`pcapkit.utilities.logging.logger`.
 logger = get_logger(__name__)
 
+#: re.Pattern: A character XML 1.0 does not allow anywhere in a document, not
+#: even as a character reference: every C0 control other than tab, LF and CR,
+#: the surrogates, and ``U+FFFE``/``U+FFFF`` (XML 1.0, production ``[2] Char``).
+_XML_ILLEGAL = re.compile(r'[\x00-\x08\x0b\x0c\x0e-\x1f\ud800-\udfff\ufffe\uffff]')
+
+#: frozenset[str]: Characters :meth:`dictdumper.json.JSON._append_string`
+#: writes as themselves or through its escape table rather than as a
+#: ``\uXXXX`` escape.
+_JSON_PRINTABLE = frozenset(string.printable)
+
 
 class DumperBase(dictdumper.dumper.Dumper):
     """Base :class:`~dictdumper.dumper.Dumper` object.
@@ -221,6 +234,40 @@ def render_enum(o: 'enum.Enum | aenum.Enum') -> 'str':
     return f'{type(o).__name__}::{name} [{o.value}]'
 
 
+def _iter_slots(value: 'Any') -> 'Iterator[str]':
+    """Yield the names of every slot ``value`` has an assigned value for.
+
+    Args:
+        value: Object to inspect.
+
+    Yields:
+        Each slot name declared anywhere in ``type(value).__mro__``, base classes
+        first and each name once, skipping ``__dict__``, ``__weakref__`` and
+        any slot that is unset on ``value``.
+
+    Note:
+        ``__slots__`` on a class names only the slots *that* class adds, and
+        reading it from an instance finds the nearest class that declares one,
+        so a subclass declaring ``__slots__ = ()`` hides all of its bases'
+        slots. :class:`ipaddress.IPv6Network` is such a class.
+
+    """
+    seen = set()  # type: set[str]
+    for klass in reversed(type(value).__mro__):
+        names = klass.__dict__.get('__slots__', ())
+        if isinstance(names, str):
+            names = (names,)
+        for name in names:
+            if name in seen or name in ('__dict__', '__weakref__'):
+                continue
+            seen.add(name)
+            try:
+                getattr(value, name)
+            except AttributeError:
+                continue
+            yield name
+
+
 def make_dumper(output: 'Type[ABCDumper]') -> 'Type[ABCDumper]':
     """Create a customised :class:`~dictdumper.dumper.Dumper` object.
 
@@ -253,6 +300,12 @@ def make_dumper(output: 'Type[ABCDumper]') -> 'Type[ABCDumper]':
     # worked around here.
     escape_json_keys = issubclass(output, dictdumper.json.JSON)
     escape_keys = escape_strings or escape_json_keys
+    # NOTE: :meth:`~dictdumper.plist.PLIST._append_dict` and
+    # :meth:`~dictdumper.plist.PLIST._append_array` skip every :data:`None`
+    # value and item outright, so a ``plist`` report lacks keys the ``json``
+    # report has (GitHub issue #1257). The defect is :mod:`dictdumper`'s, so it
+    # is worked around here.
+    keep_none = issubclass(output, dictdumper.plist.PLIST)
 
     def escape_key(key: 'Any') -> 'Any':
         """Escape a mapping key on its way to the writer.
@@ -325,7 +378,10 @@ def object_hook(self, o: 'Any') -> 'Any':
                 them, so both branches that hand the writer a mapping -- a
                 :class:`~pcapkit.corekit.multidict.MultiDict` and a plain
                 :class:`dict` -- escape their own keys through
-                :func:`escape_key` instead.
+                :func:`escape_key` instead. An
+                :class:`~pcapkit.corekit.multidict.OrderedMultiDict` becomes a
+                :class:`list` of single-key :class:`dict` objects, each of
+                which comes back through the :class:`dict` branch.
 
             """
             if isinstance(o, decimal.Decimal):
@@ -334,9 +390,29 @@ def object_hook(self, o: 'Any') -> 'Any':
                 result = o.total_seconds()
             elif isinstance(o, (Info, Schema)):
                 result = o.to_dict()
-            elif isinstance(o, (ipaddress.IPv4Address, ipaddress.IPv6Address)):
+            elif isinstance(o, (ipaddress.IPv4Address, ipaddress.IPv6Address,
+                                ipaddress.IPv4Network, ipaddress.IPv6Network)):
                 result = str(o)
-            elif isinstance(o, (MultiDict, OrderedMultiDict)):
+            elif isinstance(o, datetime.timezone):
+                result = o.utcoffset(None).total_seconds()
+            elif isinstance(o, NullType):
+                result = '<NULL>'
+            elif isinstance(o, NoValueType):
+                result = '<NO_VALUE>'
+            elif isinstance(o, OrderedMultiDict):
+                # NOTE: one single-key mapping per entry, in insertion order --
+                # i.e. wire order for the option lists -- since a mapping keyed
+                # by name could only gather the repeats of a key under its first
+                # occurrence (GitHub issue #1263). The keys are escaped when the
+                # writer hands each mapping back to this hook, through the
+                # :class:`dict` branch below.
+                result = [
+                    {render_enum(key) if isinstance(key, (enum.Enum, aenum.Enum)) else key: val}
+                    for key, val in o.items(multi=True)
+                ]
+            elif isinstance(o, MultiDict):
+                # NOTE: a :class:`MultiDict` keeps no order across keys, only
+                # among the values of each, so grouping by key loses nothing.
                 temp = collections.defaultdict(list)  # type: DefaultDict[str, list[Any]]
                 for key, val in o.items(multi=True):
                     if isinstance(key, (enum.Enum, aenum.Enum)):
@@ -364,9 +440,76 @@ def object_hook(self, o: 'Any') -> 'Any':
                 result = super(type(self), self).object_hook(o)
 
             if escape_strings and isinstance(result, str):
+                # NOTE: XML 1.0 has no spelling at all for some characters, not
+                # even a character reference, so a string holding one is written
+                # as ``<data>``, the way :class:`bytes` already is (GitHub issue
+                # #1262).
+                if _XML_ILLEGAL.search(result) is not None:
+                    return result.encode('utf-8', 'surrogatepass')
                 return xml.sax.saxutils.escape(result)
             return result
 
+        if keep_none:
+            def _append_dict(self, value: 'dict[Any, Any]', file: 'TextIO') -> 'None':
+                """Call this function to write dict contents.
+
+                Args:
+                    self: Dumper instance.
+                    value: Content to be dumped.
+                    file: Output file.
+
+                Notes:
+                    Each :data:`None` value is first converted through
+                    :meth:`~dictdumper.plist.PLIST._encode_value`, whose own
+                    rendering of it is ``{'type': 'NoneType', 'value': 'None'}``,
+                    so that the writer no longer skips it.
+
+                """
+                value = {key: self._encode_value(val) if val is None else val for key, val in value.items()}
+                super()._append_dict(value, file)
+
+            def _append_array(self, value: 'list[Any]', file: 'TextIO') -> 'None':
+                """Call this function to write array contents.
+
+                Args:
+                    self: Dumper instance.
+                    value: Content to be dumped.
+                    file: Output file.
+
+                Notes:
+                    As :meth:`_append_dict`, for each :data:`None` item.
+
+                """
+                value = [self._encode_value(item) if item is None else item for item in value]
+                super()._append_array(value, file)
+
+        if escape_json_keys:
+            def _append_string(self, value: 'str', file: 'TextIO') -> 'None':
+                """Call this function to write string contents.
+
+                Args:
+                    self: Dumper instance.
+                    value: Content to be dumped.
+                    file: Output file.
+
+                Notes:
+                    :meth:`dictdumper.json.JSON._append_string` writes every
+                    other character as ``'\\u{0:04x}'.format(ord(char))``, which
+                    for a code point above ``U+FFFF`` yields five or six hex
+                    digits that a reader takes as one escape and literal text
+                    (GitHub issue #1261). This writes such a character as a
+                    UTF-16 surrogate pair instead, as :func:`json.dumps` does, and
+                    every other character exactly as the upstream writer does.
+
+                """
+                text = []  # type: list[str]
+                for char in value:
+                    if char in _JSON_PRINTABLE:
+                        text.append(dictdumper.json.ESCAPE_DCT.get(char, char))
+                    else:
+                        text.append(json.dumps(char)[1:-1] if ord(char) > 0xFFFF else f'\\u{ord(char):04x}')
+                file.write('"' + ''.join(text) + '"')
+
         def default(self, o: 'Any') -> 'Literal["fallback"]':  # pylint: disable=unused-argument
             """Check content type for function call.
 
@@ -401,14 +544,16 @@ def _append_fallback(self, value: 'Any', file: 'TextIO') -> 'None':
                 ``default`` for use.
 
             """
-            if hasattr(value, '__slots__'):
-                new_value = {key: getattr(value, key) for key in value.__slots__}
-            elif hasattr(value, '__dict__'):
-                new_value = vars(value)
-            else:
+            new_value = {key: getattr(value, key) for key in _iter_slots(value)}
+            if hasattr(value, '__dict__'):
+                new_value.update(vars(value))
+            if not new_value:
                 logger.warning('unsupported object type: %s', type(value))
                 new_value = str(value)  # type: ignore[assignment]
 
+            # NOTE: through :meth:`object_hook` as every other value is, so that
+            # the keys and strings built here are escaped like theirs.
+            new_value = self.object_hook(new_value)
             func = self._encode_func(new_value)
             func(new_value, file)
 

pcapkit/dumpkit/pcap.py

@@ -25,6 +25,7 @@
     from typing_extensions import Literal
 
     from pcapkit.const.reg.linktype import LinkType as Enum_LinkType
+    from pcapkit.corekit.version import VersionInfo
     from pcapkit.protocols.data.misc.pcap.frame import Frame as Data_Frame
 
 __all__ = [
@@ -57,8 +58,8 @@ class PCAPIO(DumperBase):
         protocol: data link type
         byteorder: header byte order
         nanosecond: nanosecond-resolution file flag
-        **kwargs: further global header fields, i.e. ``thiszone``, ``sigfigs``
-            and ``snaplen``, and arbitrary keyword arguments
+        **kwargs: further global header fields, i.e. ``version``, ``thiszone``,
+            ``sigfigs`` and ``snaplen``, and arbitrary keyword arguments
 
     """
     if TYPE_CHECKING:
@@ -90,8 +91,9 @@ def __init__(self, fname: 'str', *, protocol: 'Enum_LinkType | StdlibIntEnum | A
             protocol: data link type
             byteorder: header byte order
             nanosecond: nanosecond-resolution file flag
-            **kwargs: further global header fields, i.e. ``thiszone``,
-                ``sigfigs`` and ``snaplen``, and arbitrary keyword arguments
+            **kwargs: further global header fields, i.e. ``version``,
+                ``thiszone``, ``sigfigs`` and ``snaplen``, and arbitrary keyword
+                arguments
 
         """
         #: int: Frame counter.
@@ -129,6 +131,7 @@ def __call__(self, value: 'Data_Frame', name: 'Optional[str]' = None) -> 'PCAPIO
 
     def _dump_header(self, *, protocol: 'Enum_LinkType | StdlibIntEnum | AenumIntEnum | str | int',  # pylint: disable=arguments-differ,too-many-arguments
                      byteorder: 'Literal["big", "little"]' = sys.byteorder, nanosecond: 'bool' = False,
+                     version: 'tuple[int, int] | VersionInfo' = (2, 4),
                      thiszone: 'int' = 0, sigfigs: 'int' = 0, snaplen: 'int' = 0x40_000,
                      **kwargs: 'Any') -> 'None':  # pylint: disable=unused-argument
         """Initially dump file heads and tails.
@@ -137,6 +140,7 @@ def _dump_header(self, *, protocol: 'Enum_LinkType | StdlibIntEnum | AenumIntEnu
             protocol: data link type
             byteorder: header byte order
             nanosecond: nanosecond-resolution file flag
+            version: format version, as ``(major, minor)``
             thiszone: GMT to local correction
             sigfigs: accuracy of timestamps
             snaplen: max length of captured packets, in octets
@@ -147,6 +151,7 @@ def _dump_header(self, *, protocol: 'Enum_LinkType | StdlibIntEnum | AenumIntEnu
             network=protocol,
             byteorder=byteorder,
             nanosecond=nanosecond,
+            version=version,
             thiszone=thiszone,
             sigfigs=sigfigs,
             snaplen=snaplen,

pcapkit/foundation/engines/pcap.py

@@ -125,7 +125,7 @@ def run(self) -> 'None':
             return
 
         ext._open_output(protocol=self._dlink, byteorder=self._gbhdr.byteorder,
-                         nanosecond=self._nnsec, thiszone=self._gbhdr.info.thiszone,
+                         nanosecond=self._nnsec, version=self._vinfo, thiszone=self._gbhdr.info.thiszone,
                          sigfigs=self._gbhdr.info.sigfigs, snaplen=self._gbhdr.info.snaplen)
         if ext._flag_f:
             ofile = ext._ofile(f'{ext._ofnm}/Global Header.{ext._fext}')

tests/dumpkit/test_common_unit.py

@@ -199,7 +199,7 @@ def test_make_dumper_object_hook_conversions_and_fallbacks(self) -> None:
         self.assertEqual(converted['plain'], ['value'])
 
         ordered = OrderedMultiDict([('first', 1), ('first', 2)])
-        self.assertEqual(dumper.object_hook(ordered)['first'], [1, 2])
+        self.assertEqual(dumper.object_hook(ordered), [{'first': 1}, {'first': 2}])
 
         self.assertEqual(dumper.object_hook(PlainEnum.VALUE), 'PlainEnum::VALUE [1]')
         rich = dumper.object_hook(RichEnum.VALUE)
@@ -216,9 +216,11 @@ def test_make_dumper_object_hook_conversions_and_fallbacks(self) -> None:
         dumper._append_fallback(types.SimpleNamespace(name='dict'), file)
         self.assertEqual(file.getvalue(), "{'name': 'dict'}")
 
+        # The rendering goes back through ``object_hook``, which for this stub
+        # wraps whatever it does not convert itself.
         file = io.StringIO()
         dumper._append_fallback(123, file)
-        self.assertEqual(file.getvalue(), "'123'")
+        self.assertEqual(file.getvalue(), "{'base': '123'}")
 
     def test_make_dumper_escapes_strings_only_for_plist_like_output(self) -> None:
         """GitHub issue #772: no entity escaping anywhere in :mod:`dictdumper`.
@@ -291,15 +293,15 @@ def test_make_dumper_escapes_mapping_keys_for_plist_like_output(self) -> None:
         self.assertEqual(converted['a & b <c>'], 'value')
         self.assertEqual(converted[ESCAPED_BYTES_KEY], 'secret')
 
-        # A MultiDict: a plain-str key was interpolated raw until now, and the
-        # bytes key of the fixture arrives through here rather than through the
-        # dict branch -- its TLS key log entries are an OrderedMultiDict.
+        # An OrderedMultiDict: the bytes key of the fixture arrives through here
+        # -- its TLS key log entries are an OrderedMultiDict. Each entry becomes
+        # a single-key mapping, whose key is escaped when the writer hands that
+        # mapping back to the hook.
         multidict = OrderedMultiDict()
         multidict.add('a & b <c>', 'value')
         multidict.add(BYTES_KEY, 'secret')
-        converted = plist_dumper.object_hook(multidict)
-        self.assertEqual(list(converted), ['a &amp; b &lt;c&gt;', ESCAPED_BYTES_KEY])
-        self.assertEqual(converted[ESCAPED_BYTES_KEY], ['secret'])
+        converted = [plist_dumper.object_hook(entry) for entry in plist_dumper.object_hook(multidict)]
+        self.assertEqual(converted, [{'a &amp; b &lt;c&gt;': 'value'}, {ESCAPED_BYTES_KEY: 'secret'}])
 
         # An enum-derived key is still escaped exactly once: the escaping moved
         # out of this branch and into one shared helper, so escaping it twice is
@@ -336,7 +338,7 @@ def test_make_dumper_hands_other_output_the_mapping_it_was_given(self) -> None:
         multidict.add('a & b <c>', 'value')
         multidict.add(BYTES_KEY, 'secret')
         converted = plain_dumper.object_hook(multidict)
-        self.assertEqual(list(converted), ['a & b <c>', BYTES_KEY])
+        self.assertEqual(converted, [{'a & b <c>': 'value'}, {BYTES_KEY: 'secret'}])
 
     def test_an_unassigned_port_dumps_its_addon_keys_in_the_declared_order(self) -> None:
         """GitHub issue #575's fallback must not reorder what it renders.