Skip to content

fix(dumpkit): JSON output for test.pcapng does not parse - #1158

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1152-json-pcapng
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1152-json-pcapng

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — ran pylint/mypy/isort on the touched files directly; no new mypy errors in common.py
  • make test passes, and a test case covers the change — ran only the modules listed below
  • Added a changelog entry — N/A, added centrally after the wave

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Closes #1152. dictdumper.JSON._append_object writes a mapping key as '"{item}": ' with no escaping. test.pcapng's TLS key log is keyed by a bytes client random whose repr holds " and \. make_dumper's escape_key now JSON-escapes keys for JSON output (json.dumps(...)[1:-1]), alongside its existing XML escaping for plist. The dictdumper defect is still there upstream.

Probe (extract(format='json') + json.load): before Expecting ':' delimiter: line 1019 column 12; after OK. A sweep of all 23 examples/captures/* now gives json 23/23 parsing (was 22/23) and plist 23/23 parsing with ElementTree. Tree writes all 23.

Tests: new tests/dumpkit/test_json_key_escaping_unit.py (3 tests) and ..._regression.py (1 test). test_plist_escaping_regression.py asserted the raw key in the JSON text, i.e. the broken output, so it now asserts the key's JSON spelling. With the fix reverted: unit 2 failed / 1 passed (the tree guard), regression 1 failed at line 1019, plist module 1 failed. With the fix, all pass, plus test_common_unit 14, test_nameless_enum_rendering_unit 6, integration/test_output_formats 13 + 1 skipped, integration/test_pcapng_end_to_end 6 + 1 skipped, and tests/project 385 + 1 skipped.

dictdumper.JSON escapes string values but writes a mapping key into
'"{item}": ' raw, so the bytes-repr client random keying test.pcapng's
TLS key log entries (it holds '"' and '\') closed the JSON string early.
make_dumper's escape_key now JSON-escapes keys for JSON output, in both
the MultiDict and plain-dict branches.

Closes #1152
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 82e085b21: GOOD TO GO (ran on Sonnet; author Opus)

  • Root cause confirmed upstream. In dictdumper/json.py:224, _append_object interpolates keys raw; only values are encoded. On main, test.pcapng's JSON fails json.load at line 1019. On this PR it parses.
  • Every capture checked. I extracted all 23 captures as json, tree and plist on both trees. JSON now parses 23/23 (22/23 on main). 68 of the 69 outputs are byte-identical to main, so there is no double escaping. The one difference is the fixed line.
  • Tests. With common.py reverted, 3 of the 4 new tests fail. The edited plist regression now asserts the correct JSON spelling and keeps its negative checks. The test-tier guard passes.

Unflagged change: a plain str key with non-ASCII characters is now written as \uXXXX. It parses back to the same value, and no capture has such a key.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.75% (unit tier, Python 3.14, 82e085b21, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1035 2342 846 90.23%
pcapkit/corekit 1873 91 578 22 94.33%
pcapkit/dumpkit 142 0 42 0 100.00%
pcapkit/foundation 2425 145 842 34 92.56%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15659 188 3944 163 98.18%
pcapkit/toolkit 487 65 144 1 85.42%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit 2c7a97c into main Oct 6, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1152-json-pcapng branch October 6, 2026 22:40
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 6, 2026
JarryShaw added a commit that referenced this pull request Oct 6, 2026
- 22 new 1.5.0 entries for the 30 PRs merged since b10e795
  (#1106-#1158), with related fixes grouped per the #918 ruling;
  three marked breaking to match their labels (#1116, #1138, #1145)
- regenerate CHANGELOG.md with util/changelog_md.py
- process.rst: entry-count pin re-measured, 167 -> 189
- pep.rst: note the plist round-trip test skipped on the upstream
  dictdumper fractional-second <date> defect (DictDumper#122)

tests/project: conventions_doc_claims, changelog_md and
documentation_claims pass; changelog_md.py --check is in step.
JarryShaw added a commit that referenced this pull request Oct 16, 2026
…) (#1809)

- 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
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(dumpkit): JSON output for test.pcapng does not parse

1 participant