diff --git a/pcapkit/dumpkit/common.py b/pcapkit/dumpkit/common.py
index bae86ff194..9d82f30bf7 100644
--- a/pcapkit/dumpkit/common.py
+++ b/pcapkit/dumpkit/common.py
@@ -15,11 +15,13 @@
import decimal
import enum
import ipaddress
+import json
import xml.sax.saxutils
from typing import TYPE_CHECKING
import aenum
import dictdumper.dumper
+import dictdumper.json
import dictdumper.plist
from pcapkit.corekit.infoclass import Info
@@ -244,6 +246,12 @@ def make_dumper(output: 'Type[ABCDumper]') -> 'Type[ABCDumper]':
# use it directly -- so :class:`~dictdumper.plist.PLIST` is the only
# concrete writer this applies to.
escape_strings = issubclass(output, dictdumper.plist.PLIST)
+ # NOTE: :class:`~dictdumper.json.JSON` escapes every string *value* it
+ # writes, but interpolates a mapping key into ``'"{item}": '`` raw, so a key
+ # carrying a ``"`` or a ``\`` closes the JSON string early (GitHub issue
+ # #1152). The defect is :mod:`dictdumper`'s, so it is worked around here.
+ escape_json_keys = issubclass(output, dictdumper.json.JSON)
+ escape_keys = escape_strings or escape_json_keys
def escape_key(key: 'Any') -> 'Any':
"""Escape a mapping key on its way to the writer.
@@ -257,33 +265,39 @@ def escape_key(key: 'Any') -> 'Any':
Note:
:meth:`~dictdumper.plist.PLIST._append_dict` writes a key straight
- into ``'{item}'`` and calls
- :meth:`~dictdumper.dumper.Dumper._encode_value` on the *value* two
- lines later, never on the key -- so :meth:`DictDumper.object_hook`
- is handed every value the writer will interpolate but no key at all,
- and cannot escape one on the way out the way it does a value. Each
- branch below that builds a mapping therefore escapes its own keys
- through here.
+ into ``'{item}'``, and
+ :meth:`~dictdumper.json.JSON._append_object` into ``'"{item}": '``;
+ both call :meth:`~dictdumper.dumper.Dumper._encode_value` on the
+ *value* two lines later, never on the key -- so
+ :meth:`DictDumper.object_hook` is handed every value the writer will
+ interpolate but no key at all, and cannot escape one on the way out
+ the way it does a value. Each branch below that builds a mapping
+ therefore escapes its own keys through here.
A non-:class:`str` key is rendered with :func:`format`, which is the
very conversion ``'{item}'.format(item=key)`` already applies to it,
- so the ```` text is unchanged apart from the escaping.
+ so the key text is unchanged apart from the escaping.
Rendering such a key rather than passing it over is deliberate:
:file:`examples/captures/test.pcapng` keys the TLS key log entries
of its decryption secrets block by a raw :class:`bytes` client
random (:meth:`TLSKeyLog.post_process
`), and
- the ``bytes`` repr of that one carries ``&``, ``<`` *and* ``>``.
- Unescaped, that makes the fixture's ``plist`` report unparseable:
- :func:`xml.etree.ElementTree.parse` stops at the key's ``&``. The
- same key also breaks the fixture's ``json`` report, but on the quotes
- in that repr rather than on these three characters, so that half is
- :mod:`dictdumper`'s to fix and is left alone.
+ the ``bytes`` repr of that one carries ``&``, ``<``, ``>``, ``"``
+ *and* ``\\``. Unescaped, that makes the fixture's ``plist`` report
+ unparseable at the key's ``&``, and its ``json`` report at the
+ key's ``"``.
+
+ For ``json`` the key is escaped by :func:`json.dumps` with the
+ surrounding quotes stripped, since the writer supplies its own. Like
+ :meth:`~dictdumper.json.JSON._append_string`, that spells every
+ non-ASCII character as a ``\\uXXXX`` escape.
"""
- if not escape_strings:
- return key
- return xml.sax.saxutils.escape(format(key, ''))
+ if escape_strings:
+ return xml.sax.saxutils.escape(format(key, ''))
+ if escape_json_keys:
+ return json.dumps(format(key, ''))[1:-1]
+ return key
class DictDumper(output):
"""Customised :class:`~dictdumper.dumper.Dumper` object."""
@@ -332,7 +346,7 @@ def object_hook(self, o: 'Any') -> 'Any':
# NOTE: rebuilt only where the keys need escaping, so every other
# output is still handed the caller's own mapping rather than a
# copy of it.
- if escape_strings:
+ if escape_keys:
result = {escape_key(key): val for key, val in o.items()}
else:
result = o
diff --git a/tests/dumpkit/test_json_key_escaping_regression.py b/tests/dumpkit/test_json_key_escaping_regression.py
new file mode 100644
index 0000000000..a4c835c5bf
--- /dev/null
+++ b/tests/dumpkit/test_json_key_escaping_regression.py
@@ -0,0 +1,61 @@
+# -*- coding: utf-8 -*-
+"""The JSON report of :file:`test.pcapng` parses (GitHub issue #1152).
+
+The fixture's decryption secrets block keys its TLS key log entries by a raw
+:class:`bytes` client random, whose repr holds both a ``"`` and a ``\\``.
+:class:`dictdumper.json.JSON` wrote that key unescaped, and :func:`json.load`
+stopped at line 1019 with ``Expecting ':' delimiter``.
+
+This module is fixture-tier by its ``_regression.py`` name -- ``test.pcapng`` is
+generated rather than committed (see :mod:`tests._tiers`).
+
+"""
+from __future__ import annotations
+
+import importlib.util
+import json
+import pathlib
+import tempfile
+import unittest
+
+from tests._support import close_extractor, reimport_once_per_class, sample_path
+
+RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper')
+HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS)
+
+#: The fixture's client random as the writer renders it, i.e. the ``bytes`` repr
+#: of every character from ``0x20`` to ``0x3F``.
+RAW_BYTES_KEY = """b' !"#$%&\\'()*+,-./0123456789:;<=>?'"""
+
+
+@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed')
+class TestPcapngJSONReportTests(unittest.TestCase):
+ """The ``json`` report of :file:`test.pcapng`."""
+
+ def setUp(self) -> None:
+ reimport_once_per_class(self)
+
+ tmpdir = tempfile.TemporaryDirectory(prefix='pcapkit-1152-')
+ self.addCleanup(tmpdir.cleanup)
+ self.tmp_path = pathlib.Path(tmpdir.name)
+
+ def test_the_json_report_parses_and_keeps_the_bytes_key(self) -> None:
+ """The report loads, and the client random reads back unchanged."""
+ from pcapkit.interface import extract
+
+ output = self.tmp_path / 'test.pcapng.json'
+ extractor = extract(fin=sample_path('test.pcapng'), fout=str(output), format='json',
+ store=False, extension=False)
+ self.addCleanup(close_extractor, extractor)
+
+ with output.open(encoding='utf-8') as file:
+ report = json.load(file)
+
+ entries = [frame['secrets_data']['entries'] for frame in report.values()
+ if isinstance(frame, dict) and 'secrets_data' in frame]
+ self.assertEqual(len(entries), 1)
+ self.assertIn(RAW_BYTES_KEY, entries[0]['CLIENT_RANDOM'])
+
+
+if __name__ == '__main__':
+ unittest.main()
diff --git a/tests/dumpkit/test_json_key_escaping_unit.py b/tests/dumpkit/test_json_key_escaping_unit.py
new file mode 100644
index 0000000000..d356bcb031
--- /dev/null
+++ b/tests/dumpkit/test_json_key_escaping_unit.py
@@ -0,0 +1,84 @@
+# -*- coding: utf-8 -*-
+"""JSON reports of a mapping whose keys need escaping.
+
+:class:`dictdumper.json.JSON` escapes every string *value* it writes, but
+interpolates a mapping key into ``'"{item}": '`` raw, so a key holding a ``"``
+or a ``\\`` closes the JSON string early and the report stops parsing (GitHub
+issue #1152). :func:`~pcapkit.dumpkit.common.make_dumper` escapes the keys on
+the writer's behalf; this module pins that the file on disk parses and that the
+key reads back as the writer's own rendering of it.
+
+"""
+from __future__ import annotations
+
+import importlib.util
+import json
+import pathlib
+import tempfile
+import unittest
+
+HAS_RUNTIME = importlib.util.find_spec('dictdumper') is not None
+
+#: Keys that each break an unescaped ``'"{item}": '``, or would if escaped wrongly.
+AWKWARD_KEYS = (
+ b' !"#$%&\'()*+,-./0123456789:;<=>?', # test.pcapng's client random
+ 'quote " and backslash \\',
+ 'tab\tnewline\ncontrol\x01',
+ 'non-ascii é中',
+)
+
+
+@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed')
+class JSONKeyEscapingTests(unittest.TestCase):
+ """Dump one mapping per container type and read the report back."""
+
+ def setUp(self) -> None:
+ tmpdir = tempfile.TemporaryDirectory(prefix='pcapkit-1152-')
+ self.addCleanup(tmpdir.cleanup)
+ self.tmp_path = pathlib.Path(tmpdir.name)
+
+ def dump(self, fmt: 'str', value: 'object') -> 'pathlib.Path':
+ """Write ``{'entries': value}`` through the customised ``fmt`` writer."""
+ import dictdumper
+
+ from pcapkit.dumpkit.common import make_dumper
+
+ output = self.tmp_path / f'report.{fmt}'
+ writer = make_dumper({'json': dictdumper.JSON, 'tree': dictdumper.Tree}[fmt])
+ writer(str(output))({'entries': value}, name='Frame 1')
+ return output
+
+ def test_a_multidict_with_awkward_keys_parses(self) -> None:
+ """The #1152 path: ``TLSKeyLog`` keys an ``OrderedMultiDict`` by bytes."""
+ from pcapkit.corekit.multidict import OrderedMultiDict
+
+ entries = OrderedMultiDict() # type: OrderedMultiDict[object, int]
+ for index, key in enumerate(AWKWARD_KEYS):
+ entries.add(key, index)
+
+ report = json.loads(self.dump('json', entries).read_text(encoding='utf-8'))
+
+ self.assertEqual(report['Frame 1']['entries'],
+ {format(key, ''): [index] for index, key in enumerate(AWKWARD_KEYS)})
+
+ def test_a_plain_dict_with_awkward_keys_parses(self) -> None:
+ """A plain :class:`dict` goes through a separate branch of the hook."""
+ entries = {key: index for index, key in enumerate(AWKWARD_KEYS)}
+
+ report = json.loads(self.dump('json', {'nested': entries}).read_text(encoding='utf-8'))
+
+ self.assertEqual(report['Frame 1']['entries']['nested'],
+ {format(key, ''): index for index, key in enumerate(AWKWARD_KEYS)})
+
+ def test_the_tree_report_keeps_the_key_verbatim(self) -> None:
+ """``tree`` takes the characters literally, so nothing is escaped there."""
+ key = 'quote " and backslash \\'
+
+ text = self.dump('tree', {key: 0}).read_text(encoding='utf-8')
+
+ self.assertIn(key, text)
+ self.assertNotIn('\\"', text)
+
+
+if __name__ == '__main__':
+ unittest.main()
diff --git a/tests/dumpkit/test_plist_escaping_regression.py b/tests/dumpkit/test_plist_escaping_regression.py
index e08081f4c4..df6fb8bacb 100644
--- a/tests/dumpkit/test_plist_escaping_regression.py
+++ b/tests/dumpkit/test_plist_escaping_regression.py
@@ -23,6 +23,7 @@
from __future__ import annotations
import importlib.util
+import json
import pathlib
import tempfile
import unittest
@@ -109,17 +110,17 @@ def test_the_json_and_tree_reports_keep_the_key_verbatim(self) -> None:
``json``, ``tree`` and ``text`` accept ``&``, ``<`` and ``>`` as
themselves, so escaping them there would corrupt every report in those
- formats. The ``json`` report of this fixture is separately unparseable --
- ``dictdumper`` writes a key as ``'"{item}": '`` and this one's repr holds
- a quote -- and that is deliberately still true here: it is
- JarryShaw/DictDumper#125's half, not this one's.
+ formats. ``json`` does escape the key's ``"`` and ``\\`` as a JSON
+ string must (GitHub issue #1152), so it is checked against the key's JSON
+ spelling rather than its raw text.
"""
for fmt in ('json', 'tree', 'text'):
with self.subTest(format=fmt):
text = self.report('test.pcapng', fmt).read_text(encoding='utf-8')
- self.assertIn(RAW_BYTES_KEY, text)
+ expected = json.dumps(RAW_BYTES_KEY) if fmt == 'json' else RAW_BYTES_KEY
+ self.assertIn(expected, text)
self.assertNotIn('&', text)
self.assertNotIn('<', text)