fix(dumpkit): JSON output of DPKT/Scapy frames grows without bound and fills the disk

pcapkit/dumpkit/common.py

@@ -14,10 +14,12 @@
 import datetime
 import decimal
 import enum
+import inspect
 import ipaddress
 import json
 import re
 import string
+import types
 import xml.sax.saxutils
 from typing import TYPE_CHECKING
 
@@ -268,6 +270,32 @@ def _iter_slots(value: 'Any') -> 'Iterator[str]':
             yield name
 
 
+#: Most objects :meth:`DictDumper._append_fallback` expands for one top-level
+#: object, counting an object once per time it is met. No fallback object in the
+#: ``foundation``, ``dumpkit`` or ``interface`` test legs expands into more than 7.
+FALLBACK_LIMIT = 10_000
+
+
+def _object_name(value: 'Any') -> 'str':
+    """Name a class, module or routine for dumping, in place of its attributes.
+
+    Args:
+        value: Object to name.
+
+    Returns:
+        The module-qualified name of ``value``, falling back to :func:`str`
+        where it carries none.
+
+    """
+    qualname = getattr(value, '__qualname__', None) or getattr(value, '__name__', None)
+    if not isinstance(qualname, str):
+        return str(value)
+    module = getattr(value, '__module__', None)
+    if isinstance(module, str) and module != 'builtins' and not isinstance(value, types.ModuleType):
+        return f'{module}.{qualname}'
+    return qualname
+
+
 def make_dumper(output: 'Type[ABCDumper]') -> 'Type[ABCDumper]':
     """Create a customised :class:`~dictdumper.dumper.Dumper` object.
 
@@ -356,6 +384,12 @@ def escape_key(key: 'Any') -> 'Any':
     class DictDumper(output):
         """Customised :class:`~dictdumper.dumper.Dumper` object."""
 
+        #: Objects :meth:`_append_fallback` is expanding, by :func:`id` -- the
+        #: one being written and every one enclosing it.
+        _fallback_path = None  # type: Optional[set[int]]
+        #: Objects expanded so far for the current top-level fallback object.
+        _fallback_count = 0
+
         def object_hook(self, o: 'Any') -> 'Any':
             """Convert content for function call.
 
@@ -543,18 +577,69 @@ def _append_fallback(self, value: 'Any', file: 'TextIO') -> 'None':
                 to find a suitable function for dumping and it should pair with
                 ``default`` for use.
 
+                A class, module or routine is written as its name rather than
+                expanded, since its attributes reach the whole type graph
+                (GitHub issue #1372). Every other object is expanded in full
+                each time it is met, except one that encloses itself: there
+                it is written as a ``<circular reference: ...>`` marker, so a
+                reference cycle terminates.
+
+            Raises:
+                UnsupportedCall: If a top-level fallback object expands into
+                    more than :data:`FALLBACK_LIMIT` objects.
+
             """
-            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]
+            if isinstance(value, (type, types.ModuleType)) or inspect.isroutine(value):
+                self._append_fallback_value(_object_name(value), file)
+                return
+
+            path = self._fallback_path
+            if path is None:
+                # NOTE: the outermost fallback object owns the bookkeeping, so
+                # each record dumped is tracked on its own.
+                self._fallback_path, self._fallback_count = set(), 0
+                try:
+                    self._append_fallback(value, file)
+                finally:
+                    self._fallback_path, self._fallback_count = None, 0
+                return
+
+            # NOTE: an object on the path is alive, so its id is not reused.
+            key = id(value)
+            if key in path:
+                marker = f'<circular reference: {_object_name(type(value))}>'
+                self._append_fallback_value(marker, file)
+                return
+            self._fallback_count += 1
+            if self._fallback_count > FALLBACK_LIMIT:
+                raise UnsupportedCall(f'{type(self).__name__}: fallback object expands into more '
+                                      f'than {FALLBACK_LIMIT} objects; stopped at '
+                                      f'{_object_name(type(value))}')
+            path.add(key)
+            try:
+                new_value = {name: getattr(value, name) for name 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]
+                self._append_fallback_value(new_value, file)
+            finally:
+                path.discard(key)
+
+        def _append_fallback_value(self, value: 'Any', file: 'TextIO') -> 'None':
+            """Write what :meth:`_append_fallback` made of an object.
 
+            Args:
+                self: Dumper instance.
+                value: Value to dump in place of the object.
+                file: File object to write.
+
+            """
             # 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)
+            value = self.object_hook(value)
+            func = self._encode_func(value)
+            func(value, file)
 
     return DictDumper

tests/dumpkit/test_fallback_cycle_unit.py

@@ -0,0 +1,133 @@
+# -*- coding: utf-8 -*-
+"""GitHub issue #1372: the fallback writer expands an object without bound.
+
+``_append_fallback`` dumps an object no other branch recognises through its
+attributes. A class among them was expanded too, through ``vars()``, which
+reaches the whole type graph, again along every path to each shared node. A
+frame's info object meeting the fallback -- as it does when its
+:class:`~pcapkit.corekit.infoclass.Info` class comes from another import of
+:mod:`pcapkit` -- carries its next layer's protocol class, and its JSON output
+grew until the disk filled. A class is now written as its name, a cycle as a
+marker, and an expansion past ``FALLBACK_LIMIT`` objects raises.
+
+:mod:`pcapkit` is imported inside each test, after
+:func:`~tests._support.reimport_once_per_class`.
+
+"""
+
+import importlib.util
+import json
+import os
+import plistlib
+import tempfile
+import unittest
+from unittest import mock
+
+from tests._support import reimport_once_per_class
+
+try:
+    import resource
+except ImportError:  # pragma: no cover - not POSIX
+    resource = None  # type: ignore[assignment]
+
+RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper')
+HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS)
+
+#: Output size no case here comes near, and a cap on the file that the
+#: unbounded expansion reaches within seconds.
+SIZE_CAP = 16 * 1024 * 1024
+
+
+class Node:
+    """A plain object, which only the fallback writer can dump."""
+
+    def __init__(self, **kwargs: 'object') -> None:
+        self.__dict__.update(kwargs)
+
+
+@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed')
+class TestFallbackBounded(unittest.TestCase):
+    """The fallback writer terminates, and writes each object once."""
+
+    def setUp(self) -> None:
+        reimport_once_per_class(self)
+        self.tmpdir = tempfile.TemporaryDirectory()  # pylint: disable=consider-using-with
+        self.addCleanup(self.tmpdir.cleanup)
+        if resource is not None:
+            # NOTE: without the fix, the class case writes without bound, so
+            # cap the file size rather than let it fill the disk.
+            limits = resource.getrlimit(resource.RLIMIT_FSIZE)
+            cap = SIZE_CAP if limits[1] == resource.RLIM_INFINITY else min(SIZE_CAP, limits[1])
+            resource.setrlimit(resource.RLIMIT_FSIZE, (cap, limits[1]))
+            self.addCleanup(resource.setrlimit, resource.RLIMIT_FSIZE, limits)
+
+    def dump(self, kind: 'str', value: 'dict[str, object]') -> 'bytes':
+        """Dump ``value`` through the customised ``dictdumper`` writer ``kind``."""
+        import dictdumper
+
+        from pcapkit.dumpkit.common import make_dumper
+
+        path = os.path.join(self.tmpdir.name, f'out.{kind.lower()}')
+        make_dumper(getattr(dictdumper, kind))(path)(value, name='x')
+        with open(path, 'rb') as file:
+            return file.read()
+
+    def test_reference_cycle_terminates(self) -> None:
+        """An object reaching itself is written once, then as a marker."""
+        node = Node(name='a')
+        node.me = node  # type: ignore[attr-defined]
+        record = json.loads(self.dump('JSON', {'node': node}))['x']
+        self.assertEqual(record['node'], {'name': 'a', 'me': f'<circular reference: {__name__}.Node>'})
+
+        record = plistlib.loads(self.dump('PLIST', {'node': node}))['x']
+        self.assertEqual(record['node'], {'name': 'a', 'me': f'<circular reference: {__name__}.Node>'})
+
+    def test_shared_object_is_expanded_each_time(self) -> None:
+        """An object reached by two paths, but enclosing neither, is written in full twice."""
+        shared = Node(leaf=0, more=Node(leaf=1))
+        record = json.loads(self.dump('JSON', {'node': Node(left=shared, right=shared)}))['x']
+        expected = {'leaf': 0, 'more': {'leaf': 1}}
+        self.assertEqual(record['node'], {'left': expected, 'right': expected})
+
+        record = plistlib.loads(self.dump('PLIST', {'node': Node(left=shared, right=shared)}))['x']
+        self.assertEqual(record['node'], {'left': expected, 'right': expected})
+
+    def test_expansion_past_the_limit_raises(self) -> None:
+        """An object expanding into more than ``FALLBACK_LIMIT`` objects raises."""
+        from pcapkit.dumpkit import common
+        from pcapkit.utilities.exceptions import UnsupportedCall
+
+        def chain(depth: 'int') -> 'Node':
+            node = Node(leaf=0)
+            for _ in range(depth):
+                node = Node(left=node, right=node)
+            return node
+
+        # 2 ** 15 - 1 expansions, past the default limit
+        with self.assertRaisesRegex(UnsupportedCall, 'more than 10000 objects'):
+            self.dump('JSON', {'node': chain(14)})
+
+        with mock.patch.object(common, 'FALLBACK_LIMIT', 7):
+            # seven objects: within the limit
+            record = json.loads(self.dump('JSON', {'node': chain(2)}))['x']
+            self.assertEqual(record['node']['right']['left'], {'leaf': 0})
+            with self.assertRaisesRegex(UnsupportedCall, 'more than 7 objects'):
+                self.dump('JSON', {'node': chain(3)})
+
+    def test_class_is_written_as_its_name(self) -> None:
+        """A class, module or function is named, not expanded through ``vars()``."""
+        from pcapkit.protocols.internet.ipv6 import IPv6
+
+        holder = Node(next_type=IPv6, module=json, func=json.dumps, method=json.JSONEncoder.encode)
+        data = self.dump('JSON', {'holder': holder})
+        self.assertLess(len(data), 4096)
+        self.assertEqual(json.loads(data)['x']['holder'], {
+            'next_type': 'pcapkit.protocols.internet.ipv6.IPv6',
+            'module': 'json',
+            'func': 'json.dumps',
+            'method': 'json.encoder.JSONEncoder.encode',
+        })
+
+
+if __name__ == '__main__':
+    unittest.main()

tests/foundation/engines/test_runtime_engines.py

@@ -114,7 +114,7 @@ def __init__(self, file) -> None:
                 self.assertEqual(sink.records[-1][1], 'Global Header')
                 extractor._open_output.assert_called_once_with(
                     protocol=LinkType.ETHERNET, byteorder='little', nanosecond=True,
-                    thiszone=-3600, sigfigs=1, snaplen=65535)
+                    version=VersionInfo(2, 4), thiszone=-3600, sigfigs=1, snaplen=65535)
 
         quiet, sink = make_extractor(_flag_q=True)
         engine = PCAP(quiet)