From 58da7d3778ede8b64061d4e5ed62457d172141b1 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Thu, 15 Oct 2026 19:39:15 -0400 Subject: [PATCH] test: run the PCAP-NG JSON report test now that keys are escaped (#1795) - Drop the "blocked on dictdumper" skip from PcapngUnescapedKeyTests.test_json_report_of_a_decryption_secrets_block_parses. #1158 made pcapkit.dumpkit escape JSON mapping keys itself, so the test.pcapng report parses although dictdumper 0.8.4.post6 still writes keys raw (JarryShaw/DictDumper#121). - Give the class the HAS_RUNTIME guard its siblings carry; _helpers.py to match. - Rewrite the class and test docstrings to say what the test now guards, citing symbols instead of the stale pcapng.py:2244 / common.py:351 lines, and fix the PCAPNG_SECTIONS comment that called the report unreadable. Bite: with escape_key's JSON branch returning the key unchanged, the test fails with "JSONDecodeError: Expecting ':' delimiter"; restored, it passes. The file, tests/project and tests/test_tier_guard.py pass. Closes #1795 --- tests/integration/_helpers.py | 15 +++--- tests/integration/test_pcapng_end_to_end.py | 58 +++++++-------------- 2 files changed, 27 insertions(+), 46 deletions(-) diff --git a/tests/integration/_helpers.py b/tests/integration/_helpers.py index bda0f80525..a847d4ab2d 100644 --- a/tests/integration/_helpers.py +++ b/tests/integration/_helpers.py @@ -82,14 +82,13 @@ def setUpClass(cls) -> None: the one imported here. The re-import is guarded because nothing depends on it, so it must not be - able to turn a test failure into a class error. Most - subclasses carry ``@skipUnless(HAS_RUNTIME, ...)`` and never reach this - method without the runtime dependencies installed, but - ``PlistRoundTripTests`` and ``PcapngUnescapedKeyTests`` do not, so on a - checkout without them an unguarded ``import`` here would raise - :exc:`ModuleNotFoundError` out of ``setUpClass`` and error the whole - class, where before it was the individual tests that failed. Swallowed, - the table simply stays cold and those tests fail exactly as they did. + able to turn a test failure into a class error. Every subclass carries + ``@skipUnless(HAS_RUNTIME, ...)`` and so never reaches this method + without the runtime dependencies installed, but one added without it + would: on a checkout without them an unguarded ``import`` here would + raise :exc:`ModuleNotFoundError` out of ``setUpClass`` and error the + whole class, where otherwise its individual tests would fail. Swallowed, + the table simply stays cold and those tests fail as they would anyway. :exc:`BaseException` is deliberately not caught, for the same reason as in :func:`tests._support._close_quietly`. diff --git a/tests/integration/test_pcapng_end_to_end.py b/tests/integration/test_pcapng_end_to_end.py index a14ad21fc6..e02c8a0f17 100644 --- a/tests/integration/test_pcapng_end_to_end.py +++ b/tests/integration/test_pcapng_end_to_end.py @@ -42,8 +42,8 @@ #: stand-in the generator synthesises when that download fails (#1478). UPSTREAM_ONLY_COUNTS = frozenset({'many_interfaces.pcapng'}) -#: Block inventory of each fixture whose report can be read back. The -#: ``test.pcapng`` report cannot; see :class:`PcapngUnescapedKeyTests`. +#: Block inventory of each fixture's ``json`` report. The ``test.pcapng`` report +#: is read back by :class:`PcapngUnescapedKeyTests` instead. PCAPNG_SECTIONS = { 'dhcp.pcapng': {'Section Header': 1, 'Interface Description': 1, 'Frame': 4}, 'dhcp_big_endian.pcapng': {'Section Header': 1, 'Interface Description': 1, 'Frame': 4}, @@ -166,48 +166,30 @@ def test_all_three_layouts_yield_the_same_protocol_chains(self) -> None: [['Ethernet:IPv4:UDP:Raw'] * 4] * 3) +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') class PcapngUnescapedKeyTests(EndToEndTestCase): - """The report of :file:`test.pcapng`, which carries a decryption secrets block. + """The ``json`` report of :file:`test.pcapng`, whose mapping keys need escaping. - Its tree report is fine -- :class:`PcapngTreeReportTests` covers it -- and - since #784 its ``plist`` report parses too. Its ``json`` report still comes - out unparseable, so that round trip lives here behind a skip rather than - being asserted either way. + The fixture's decryption secrets block keys its TLS key log entries by a raw + :class:`bytes` client random, and :mod:`dictdumper` writes a mapping key + into a ``json`` report without escaping it. The report parses only because + :mod:`pcapkit.dumpkit` escapes the keys itself (GitHub issue #1152). """ - @unittest.skip('blocked on dictdumper interpolating mapping keys into the report without ' - 'escaping them (dictdumper/json.py:224), which the bytes-keyed TLS key log ' - 'entries of a decryption secrets block then break') def test_json_report_of_a_decryption_secrets_block_parses(self) -> None: - """A ``json`` report should be readable by :func:`json.load`. - - For this fixture it is not, and two things stack up to make it so: - - 1. ``pcapkit/protocols/schema/misc/pcapng.py:2244`` keys the TLS key log - entries by ``bytes.fromhex(random)``, i.e. by a raw :class:`bytes` - client random rather than by its hex text, so the report's key is a - Python ``bytes`` repr: ``b' !"#$%&\\'()*+,-./0123456789:;<=>?'``. - 2. ``dictdumper/json.py:224`` writes a key as - ``'"{item}": '.format(item=item)``, with no escaping at all -- values - go through ``_encode_value`` and are escaped, keys do not. The quotes - inside that repr therefore land in the document verbatim. Reproduce - with:: - - >>> import dictdumper, json - >>> dictdumper.JSON('probe.json')({'a"b': 'c"d'}) - >>> json.load(open('probe.json')) - json.decoder.JSONDecodeError: Expecting ':' delimiter ... - - Measured on this fixture at ``ef859f776``: ``json.load`` fails with - ``Expecting ':' delimiter: line 1019 column 12`` -- quoted as what that - commit measured, not as a promise the fixture will keep producing it, - since a stale instance of exactly this number is what #785 was filed - over. The same key used to make the ``plist`` report invalid XML too; - #784 fixed that half by adding an ``escape_key`` helper in - ``pcapkit/dumpkit/common.py:351`` that XML-escapes mapping keys, leaving - only this ``json`` quoting defect, still open upstream as - JarryShaw/DictDumper#121. + """The report is readable by :func:`json.load` despite its keys. + + :meth:`TLSKeyLog.post_process + ` keys the + entries by ``bytes.fromhex(random)``, so the key the writer formats is a + :class:`bytes` repr carrying ``"`` and ``\\``: + ``b' !"#$%&\\'()*+,-./0123456789:;<=>?'``. + :meth:`dictdumper.json.JSON._append_object` writes a key into + ``'"{item}": '`` raw, so that ``"`` would end the JSON string early -- + still open upstream as JarryShaw/DictDumper#121. The ``escape_key`` + helper of :func:`~pcapkit.dumpkit.common.make_dumper` escapes it with + :func:`json.dumps` first; without that, this test fails. """ extractor = self.extract(fin=sample_path('test.pcapng'), fout=self.out('report'),