[JarryShaw]

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change (narrow selections only, see below)
• Changelog entry: N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

• fix — corrects a defect

---

Description of your pull request and other information

On PyPy, Info.__dict__ key order follows an earlier instance's attribute layout. A rebuild from to_dict() that recovered the payload or the IPv6 extension-header chain from key order therefore picked the wrong layer (#2003). Following the owner's rulings, no key order is used any more:

• IPv6 keeps main's layout: top-level per-header keys, plus __exthdr__ as a tuple of (parser class, info) in wire order. The fix(ipv6): from_data(info.to_dict()) crashes when an IPv6_Ext stands in for a dedicated extension header #1473 fallback travels with the parser class. _lookup_exthdr is deleted, and the chain is rebuilt only from __exthdr__.
• The payload is dispatched only from __next_name__/__next_type__, and the name-based lookup is deleted. A hand-written dict has to carry these keys, plus __short_read__ and, for IPv6, __exthdr__. Without them nothing is rebuilt, and the docstrings say so.
• There are now two class-level lists, both merged across subclasses:
  • __included__ names the dunders that to_dict(), iteration and len() carry; any other dunder is dropped.
  • __excluded__ now only means "left out of repr/str and the dumps".
• Data does not need to round-trip from dumps.
• Breaking: to_dict() now carries the dispatch dunders and __exthdr__. A custom Info must list its own dunder keys in __included__ (HTTP/2 Flags and the PCAP-NG block model now do).

Only narrow test selections were run (tests/protocols, tests/corekit, tests/dumpkit, tests/foundation, tests/toolkit). PyPy is not installed here, so its key reordering was simulated by reordering __dict__.

Closes #2003

[JarryShaw]

GOOD TO GO at 0ae6a5460. A Sonnet review:

• Unchanged on CPython. It rebuilt all 6833 layers of 1616 frames in the sample captures, from info and from to_dict(), in a main tree and a PR tree: 13666 rebuilds, byte-identical between the trees.
• Edge cases. A cut header, an unknown next code, next=59, and a next loop all finish, with no hang or new exception. A raw decoy on IPv6 now rebuilds the real payload where main raised.
• Tests. Overwriting repeated keys in Info.to_dict fails both the fix(ipv6): to_dict() collapses repeated extension headers, so the dict rebuild silently shortens the packet #1453 and refactor(corekit)!: Info.to_dict() returns an OrderedMultiDict; add MultiInfo and OrderedMultiInfo to infoclass #1484 tests, and the new test fails on main. The listed test files pass.

Caveat: the payload is the first registry name present, so a hand-built dict with a decoy key named after an earlier-registered protocol (e.g. tcp beside udp) still picks the decoy. Parsers never write such keys, and no capture hits it.

[github-actions]

Coverage: 91.55% (unit tier, Python 3.15, 755249666, Unit Tests run success)

Package	Statements	Missed	Branches	Partial	Cover
pcapkit (top level)	104	4	20	2	95.16%
pcapkit/const	19597	881	2618	867	92.11%
pcapkit/corekit	2567	71	816	37	96.57%
pcapkit/dumpkit	282	3	102	3	98.44%
pcapkit/foundation	2930	23	1022	25	98.79%
pcapkit/interface	138	4	52	3	96.32%
pcapkit/protocols	17436	219	4582	185	98.15%
pcapkit/toolkit	872	36	298	10	95.21%
pcapkit/utilities	523	2	158	4	99.12%
pcapkit/vendor	4431	1995	1018	167	51.73%

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

[JarryShaw]

Why don't we use a dunder name to save the payload key name so that the lookup can be way much easier?

[JarryShaw]

And for bookkeeping the IPv6 extension headers, i say we use an OrderedMultiInfo or OrderedMultiDict there as those headers are not exactly presented and stored in nested format in the initial design - they are supposed to be stored at the same level as an OrderedMultiDict.

[JarryShaw]

The payload key already has a dunder: __next_name__. Every _decode_next_layer writes it next to __next_type__. When either is present, _make_payload reads the payload key from it and never calls _lookup_payload.

The name match only runs on data rebuilt from Info.to_dict(), or from the JSON/plist dumps written from it. Those drop both dunders, because Data.__excluded__ lists them. That was #1447's choice: it keeps the dispatch keys out of the dumps, and test_to_dict_still_leaves_the_dispatch_keys_out pins it.

A new dunder would be dropped in exactly the same place. The one way a dunder could replace the fallback is to un-exclude __next_name__, the way __short_read__ is not excluded. That would make to_dict round trips exact, but it would change every dump, and hand-written dicts would still need the name match. Do you want that? If not, I'll keep __next_name__ excluded.

Extension headers: I'll rework the IPv6 half as you ruled. The headers will sit at the same level, in an ordered multi container, so the chain is read from the stored order instead of being recovered from the next fields. I'm moving this PR to review: needs-changes until the rework lands.

[JarryShaw]

I prototyped the OrderedMultiInfo change, then held off, because it needs a dump-format decision from you.

Result so far. With Data_IPv6 as an OrderedMultiInfo, the header order is stored explicitly and from_data(info.to_dict()) rebuilds byte-exact. The IPv6 tests pass.

Where it breaks.

• The JSON dump. The dumper writes every multi-dict as an ordered list of one-key mappings, so IPv6 stops being a JSON object, and the refactor(corekit)!: Info.to_dict() returns an OrderedMultiDict; add MultiInfo and OrderedMultiInfo to infoclass #1484/fix(ipv6): to_dict() collapses repeated extension headers, so the dict rebuild silently shortens the packet #1453 dump tests fail. The current object shape groups repeated headers by key. That means it cannot record order across header types: Destination, Routing, Destination comes out as two opts and one route.
• The data model. Data_IPv6 would become the only field-declaring data model whose fields live in a multi-mapping. Copy and pickle would need overrides to keep the dunders, and so would the parent repr.

Which do you want?

1. IPv6 dumps as the ordered list. Wire order survives every path, but the dump format changes and is breaking.
2. Keep the object dump. The order lives in OrderedMultiInfo in memory, and the next-field walk stays as the fallback for data loaded from a dump.
3. Land fix(protocols): find the payload by protocol name and follow IPv6 next headers, not key order (#2003) #2014 as it is (fixes fix(protocols): Info field order follows PyPy's attribute layout, not the declared fields #2003 without depending on key order) and do the OrderedMultiInfo move in its own issue with (1) or (2).

My recommendation is (3): it keeps this PR small, and the data-model change gets its own review.

[JarryShaw]

My thoughts are that we dont have to roundtrip from JSON or other format dumps.

[JarryShaw]

Understood: data loaded back from a dump does not need to rebuild. Here is what I'm doing on this PR:

• IPv6 data: Data_IPv6 becomes an OrderedMultiInfo, so the extension headers sit at the same level and their order is stored explicitly. The chain is read from that order, and the next-field walk and _exthdr_parser are deleted.
• Dumps: they keep their current JSON object shape, so the refactor(corekit)!: Info.to_dict() returns an OrderedMultiDict; add MultiInfo and OrderedMultiInfo to infoclass #1484/fix(ipv6): to_dict() collapses repeated extension headers, so the dict rebuild silently shortens the packet #1453 dump tests hold. Order is guaranteed in memory and through to_dict(), not through a dump.
• Data model: copy and pickle keep the dunders, and the parent repr stays short.
• Payload lookup: __next_name__ stays excluded from to_dict(). The lookup by info_name stays, because to_dict() drops the dunders.

The PR stays at review: needs-changes until the rework is pushed and reviewed.

[JarryShaw]

Data_IPv6 should still be Info, only its ext_hdr (or the actual name) attribute is an OderedMultiInfo holding the extension headers. And it's __next_name__ point to the actual next layer (non-extension header).

[JarryShaw]

Other dunders like __short_read__ should probably be excluded from dumps and repr as well?

[JarryShaw]

Corrected plan, now the worker's brief:

• Data_IPv6 stays an Info. One attribute holds the extension headers in wire order as an OrderedMultiInfo: ext_hdr, or a name the code already uses if there is one.
• __next_name__ and __next_type__ point to the actual next layer after the extension headers. The chain is read from that attribute, so the next-field walk goes.
• to_dict() nests the headers under that attribute, and the refactor(corekit)!: Info.to_dict() returns an OrderedMultiDict; add MultiInfo and OrderedMultiInfo to infoclass #1484/fix(ipv6): to_dict() collapses repeated extension headers, so the dict rebuild silently shortens the packet #1453 tests will pin the new shape.

__short_read__: leaving it out of __excluded__ was deliberate (#1458, #1451, via #1465). It keeps from_data(info.to_dict()) cutting a truncated header where the capture did, and test_short_read_roundtrip_unit checks that. So it has to stay in to_dict().

Dumps and repr are different. Since dumps don't need to round-trip, hiding it there is safe. My recommendation is to drop __short_read__ (and any other non-excluded dunder) from dumps and repr but keep it in to_dict(). I'd do that in its own issue, so this PR stays on #2003. Shall I file it?

[JarryShaw]

if we must keep __short_read__ in to_dict, why not keeping __next_name/type__ in to_dict as well? so that we can easily and properly roundtrip the Info objects.

[JarryShaw]

Yes, that works, and it fixes #2003's payload half at its source. What it involves:

• to_dict(): Data.__excluded__ stops listing __next_name__ and __next_type__, so from_data(info.to_dict()) dispatches the way from_data(info) does.
• Dumps and repr: these have to drop every dunder, including __short_read__. They need to anyway, because __next_type__ is a class and JSON cannot hold one.
• fix(protocols): from_data(info.to_dict()) drops the payload because to_dict omits __next_type__/__next_name__ #1447's test: it asserts that to_dict() leaves those keys out, so it gets reversed. The dumps-stay-clean half of that test moves to the dumper.
• Breaking: code that iterates to_dict() will see the new keys, so the PR gets the breaking label.
• The name match in _lookup_payload: it stays only as the fallback for hand-written dicts.

I'll put this in #2014 as a separate commit after the ext_hdr rework, so it is easy to drop if you would rather not.

[JarryShaw]

The name match in _lookup_payload: I say we should add a note in the code documents that hand rolled dict should also add the dunder names as meta data to properly process and construct a Data object. We do not automatically guess and lookup. Prefer explicitness.

[JarryShaw]

Agreed, no guessing. In the dunder commit:

• The info_name match in _lookup_payload is deleted, so the payload is dispatched only from __next_type__ and __next_name__.
• The data-model, from_data and _make_payload docstrings will say that a hand-written dict must carry those dunders, plus ext_hdr for IPv6, for the payload to be rebuilt. Without them, no payload layer is dispatched.
• The fix(protocols): Info field order follows PyPy's attribute layout, not the declared fields #2003 tests will check two things: from_data(info.to_dict()) is exact even with reordered keys, and a dict without the dunders is not guessed at.

[JarryShaw]

Yes, that's cleaner. The plan becomes two class-level lists, both extended per subclass the way __excluded__ is today:

• __included__: to_dict() drops dunder keys unless this list names them. Data lists __next_name__, __next_type__ and __short_read__, and Data_IPv6 adds __exthdr__. Non-dunder keys are unaffected.
• __excluded__: the keys left out of dumps and repr. It lists the same dunders.

Both are documented on Info. The worker has the amendment.

[JarryShaw]

NEEDS CHANGES at 8db96cc3f, for docs only. __included__ is missing from the rendered API docs. docs/source/pcapkit/corekit/infoclass.rst (for Info) and docs/source/pcapkit/protocols/index.rst (for Data) list __excluded__ with autoattribute but not __included__. That leaves the new contract, that a custom Info lists its own dunder keys, documented only in docstrings.

A Sonnet cross-review confirmed everything else:

• Rebuilds: Hop/Dest/Dest/UDP and Dest/Route/Dest/UDP rebuild byte-exact through to_dict() with every nested __dict__ reversed or sorted.
• __exthdr__: it is keyed by the Enum, the fix(ipv6): from_data(info.to_dict()) crashes when an IPv6_Ext stands in for a dedicated extension header #1473 IPv6_Ext fallback rebuilds, and a dict without it rebuilds no extension header and does not raise.
• Removed names: none of the removed names remain.
• Hidden keys: dunders are absent from repr, str and the JSON, plist and tree dumps.
• Declarations: HTTP/2 Flags and PCAP-NG are declared, and no other Info dunder key was found.
• Tests: all selections pass.

Noted, not blocking:

• len() and iteration now include the included dunders, which the PR already calls breaking.
• A dict without __exthdr__ raises a harmless SchemaWarning.
• The XML dump was not checked.

[JarryShaw]

GOOD TO GO at 2a9539ae9.

A Sonnet review of the docs commit found:

• Placement: __included__ now sits in the Info and Data autoattribute blocks, formatted like its __excluded__ siblings.
• Accuracy: the #: comments match the code. to_dict, iteration and len hide unlisted dunders, indexing still reads them, and subclasses extend the list.
• Scope: the .py edits are comments only.
• Tests: test_infoclass and test_docstring_contract gave 15 passed.

The earlier round-3 review covered the rest at 8db96cc3f. The Sphinx render was not built locally.

[JarryShaw]

okay we should probably keep __exthdr__: 'tuple[tuple[Type[ProtocolBase], Data], ...]' so that's the real preserved order.

and for those __included__ and __excluded__ updates, verify if anything else should go into __ex/included__ as well.

[JarryShaw]

Will do:

• __exthdr__ goes back to tuple[tuple[Type[ProtocolBase], Data], ...] in wire order, the same as on main. The tuple order is the preserved order, and the parser class still carries the fix(ipv6): from_data(info.to_dict()) crashes when an IPv6_Ext stands in for a dedicated extension header #1473 fallback. to_dict() carries it and the chain is rebuilt from it alone.
• __included__/__excluded__ audit: every Info/Data/Schema subclass in pcapkit/, plus every key written with __update__ or a dict literal, gets checked for metadata that should be listed. The worker will post the list, including the entries it decides to leave out, with a reason for each.

The PR is at review: needs-changes until then.

[JarryShaw]

Pushed 755249666. __exthdr__ is back to main's tuple[tuple[Type[ProtocolBase], Data], ...], and the chain is rebuilt from it alone. to_dict() carries it unchanged. A reordered __dict__ still rebuilds byte-exact, and so does the #1473 IPv6_Ext fallback.

__included__ / __excluded__ audit:

Key	Where	Included	Excluded	Why
__next_name__, __next_type__	every Data (_decode_next_layer)	yes	yes	payload dispatch; holds a class
__short_read__	any Data (keep_short_read)	yes	yes	rebuild-only metadata
__exthdr__	Data_IPv6	yes	yes	chain rebuild; holds classes
__value__	HTTP/2 Flags	yes	no	rebuild reads it; already dumped on main
__truncated_raw__	PCAP-NG block data	yes	no	rebuild reads it; already dumped on main
__map__, __map_reverse__, __multi__	every Info (bookkeeping)	no	no	always hidden

How the audit was done: the Info subclasses were found by walking Info.__subclasses__() after importing every module, and the dunder writes by grepping for __update__, dict literals, keyword arguments and __dict__ writes. None of the other Info subclasses write a dunder key, so they inherit Data's lists or need neither. Schema.__excluded__ is a separate mechanism and is untouched. A dunder key built dynamically would not have been caught by the grep.

[JarryShaw]

GOOD TO GO at 755249666.

A Sonnet cross-review confirmed:

• __exthdr__: it is a plain tuple of (parser class, Info) in wire order, and Data_IPv6.from_dict(info.to_dict()) gives it back equal to the parsed one.
• Order independence: Hop/Dest/Dest/UDP and Dest/Route/Dest/UDP rebuild byte-exact from to_dict() with every nested __dict__ reversed or sorted. So does the fix(ipv6): from_data(info.to_dict()) crashes when an IPv6_Ext stands in for a dedicated extension header #1473 IPv6_Ext fallback.
• The audit: its own grep of dunder keys written into an Info matches the table. HTTP/2 __value__ and PCAP-NG __truncated_raw__ are still dumped and still rebuild.
• No leftovers: nothing Enum-keyed is left for __exthdr__.
• Tests: tests/protocols/internet (511), tests/corekit (671), tests/dumpkit (73), tests/protocols/application (244), and the four fix(protocols): from_data(info.to_dict()) drops the payload because to_dict omits __next_type__/__next_name__ #1447/fix(ipv6): to_dict() collapses repeated extension headers, so the dict rebuild silently shortens the packet #1453/fix(protocols): Info field order follows PyPy's attribute layout, not the declared fields #2003/short-read files (30) all pass.

Not checked, and not blocking:

• No before/after dump comparison was made against main.
• A dynamically built dunder key would escape the grep.
• docs/source/changelog/1.5.0.rst still says __exthdr__ is absent from to_dict, which belongs to docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657.