Skip to content

fix(dumpkit): dump a key renamed for clashing with a builtin under its own name (#2733) - #2736

Open
JarryShaw wants to merge 1 commit into
mainfrom
fix/2733-dumpkit-renamed-keys
Open

JarryShaw wants to merge 1 commit into
mainfrom
fix/2733-dumpkit-renamed-keys

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect

dumpkit dropped every Info key that had been renamed for clashing with a builtin name, such as a field named items or a reassembled datagram's packet. fields() checked __excluded__ against the original name, and to_dict() carried the same list.

  • A new helper, _dump_excluded, returns __excluded__ minus the keys an instance renamed. to_dict() and dumpkit's excluded() both use it. Hidden dunders, excluded names and a renamed __doc__ stay out. The helper costs nothing when an Info renamed nothing.
  • A protocol object is dumped as its .info, the way frame layers already appear in dumps, so a resolved packet writes UDP's or HTTP's fields and not the object's internals. Dumps stay deterministic: the same datagrams dumped in two fresh processes give identical JSON, plist and tree bytes, and a test pins that.
  • A deferred packet is resolved once, when it is dumped. Building, iterating, len() and in leave it deferred.

The extractor's own JSON, plist and tree outputs for the shipped captures are byte-identical to main. The new tests fail on main.

Closes #2733

…s own name (#2733)

Info stores a key named like one of its builtin names, such as items or a
reassembled datagram's packet, under a renamed key, and finalisation adds
every builtin name to __excluded__. repr and str match __excluded__ against
the renamed key and write the field; the dumps matched it against the name
iterated, so every json, plist and tree dump left the field out, of the info
and of its to_dict() export alike.

- corekit.infoclass: _dump_excluded gives an Info's __excluded__ less each
  non-dunder key it renamed. to_dict() records that as the export's
  __excluded__, so an export dumps the field too.
- dumpkit: excluded() takes _dump_excluded for an Info, so fields() and the
  nested-export branch of object_hook write the field under its own name.
- An excluded name, a dunder key __excluded__ names, a dunder key iteration
  hides and a renamed dunder key stay out.
- The dump resolves a deferred packet through indexing, once, as to_dict(),
  str and repr do; constructing, iterating and len() leave it deferred.
- fields() writes a protocol object, such as that packet, as its info, as a
  frame's layers are written, at the top level and nested. Written by its
  attributes it carried its BytesIO's address, so a datagram dumped
  differently in each process.

The new tests fail on ab7be2f; the protocol and fresh-process ones also
fail on the first revision of this fix. dumpkit, corekit, foundation and
project pass module by module.

Closes #2733
@JarryShaw JarryShaw added this to the PyPCAPKit v1.6 milestone Nov 3, 2026
@JarryShaw JarryShaw added bug Issues reporting a defect (set by the bug report template; a default, not an assessment) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Nov 3, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Nov 3, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at f177f1ada. A Sonnet review of ca7d457f4 passed the renamed-key fix, the dunder exclusions and Deferred laziness. It asked for one change: dump a protocol object deterministically. That is now done.

  • A protocol object is written as its .info. The same reassembled datagrams dumped in two fresh processes give identical JSON, plist and tree bytes, on dhcp.pcapng and in memory, with no object at 0x lines. The new fresh-process test fails against ca7d457f4.
  • I re-ran the module on the final commit: 8 passed and 29 subtests passed, including the two-process comparison.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Coverage: 94.45% (unit tier, Python 3.15, f177f1ada, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 21062 175 2914 156 98.58%
pcapkit/corekit 2866 76 918 37 96.80%
pcapkit/dumpkit 286 3 102 3 98.45%
pcapkit/foundation 3496 26 1214 32 98.77%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 22613 150 5862 123 99.03%
pcapkit/toolkit 961 26 342 8 96.62%
pcapkit/utilities 533 7 144 5 98.23%
pcapkit/vendor 5590 2286 1280 214 55.75%

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issues reporting a defect (set by the bug report template; a default, not an assessment) review: good-to-go Cross-review at the current head says ready; CI state is separate

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

fix(dumpkit): a key renamed for clashing with a builtin is dropped from the dump

1 participant