Skip to content

fix(corekit): record Info's field order instead of reading it from __dict__ (#2003) - #2009

Closed
JarryShaw wants to merge 1 commit into
mainfrom
fix/2003-info-field-order
Closed

JarryShaw wants to merge 1 commit into
mainfrom
fix/2003-info-field-order

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Info field order no longer depends on the interpreter's attribute layout.

Before: Info read its order from self.__dict__. On PyPy, an instance's attributes follow the layout of an earlier instance of the same class. That reordered __iter__, __len__, __str__, __repr__, items(multi=True) and to_dict. It also moved the anchor for a repeated key.

Now: __update__ records each key the first time it is set, in a per-instance __order__, and every one of those methods reads from it.

  • A key set again keeps its place.
  • A declared class records its declared order.
  • __copy__ and pickling carry the record.
  • __order__ joins the reserved names, so a field with that name is renamed like the others.

On CPython the recorded order always equals __dict__ order, so output is unchanged there.

Tests. test_info_field_order_2003_unit.py simulates PyPy on CPython by reordering an instance's __dict__. It fails without the fix on CPython (8 failed) and on PyPy 8.0.1. With the fix, the three PyPy failures named in #2003 pass. The tests/corekit, top-level tests/protocols, tests/foundation and tests/dumpkit files pass on CPython.

Found separately: #2008 covers Schema, which has the same __dict__-order dependence in its display and iteration.

Closes #2003

…dict__ (#2003)

Info listed its fields in __dict__ iteration order, which on PyPy is the
attribute layout of the first instance of the class, so to_dict(), iteration,
repr and str could reorder a later instance's fields.

- Info.__update__ records each key in a per-instance __order__ dict as it is
  first set; __iter__, __len__, __str__, __repr__, items(multi=True) and
  to_dict read the order from there, and a repeated key anchors on the last
  recorded key. __copy__ copies the record; pickles carry it in the state.
- __order__ joins the bookkeeping names, so a field of that name is mangled.
- The multi-mapping subclasses keep their own storage and are unaffected.

CPython output is unchanged; the three PyPy 8.0.1 failures now pass.
@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 20, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 20, 2026
@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 20, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

The order of Info and Schema's attributes are not required i think. Nothing depends on them.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Understood: order isn't required, so I've stopped the order-recording work here and on #2008. I've also held this PR's review findings, the old-pickle bug among them.

Before closing #2003 and #2008, I'm checking the three PyPy failures #2003 cites, one by one. test_a_cut_frame_rebuilds_byte_exact is the one to settle: if PyPy changes its rebuilt bytes and not just its key order, something does depend on order. Each order-only assertion becomes order-insensitive. I'll report back here.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Something does depend on the order: rebuilding from to_dict() does. I ran the three PyPy failures with main's infoclass.py:

  • test_a_cut_frame_rebuilds_byte_exact: the rebuild is 902 octets against 14273 on the wire. An IPv6 header's opts lands after raw in __dict__, and Protocol._lookup_payload takes the last nested key (keys[-1]) as the payload. It therefore rebuilds opts as the payload and drops the rest of the frame.
  • test_to_dict_repeated_exthdr_1453_unit: IPv6.from_data(ip6.info.to_dict()) rebuilds three header chains short. For example, hop-dst-dst comes back at 56 octets, not 76. IPv6._lookup_exthdr walks items(multi=True) in order, so the header chain follows key order.
  • test_multiinfo_1484_unit: order only. Every value sits under the right key.

Schema (#2008) really is display-only, so it stays closed.

Option 1 (recommended): keep #2009. Info records its order, so the rebuilds work on any interpreter. I'd add the fix for older pickles that loaded empty (which the review found) and the deleted-key fallback. Cost: about +3% on the largest sample capture.
Option 2: leave Info as it is. Rework _lookup_payload and _lookup_exthdr to find the payload and header chain from protocol structure instead of key order. This is a larger change across protocols.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.54% (unit tier, Python 3.15, 9c485f933, 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 2543 71 808 37 96.54%
pcapkit/dumpkit 276 3 100 3 98.40%
pcapkit/foundation 2929 23 1020 25 98.78%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17468 222 4600 187 98.13%
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

Copy link
Copy Markdown
Owner Author

Closing as superseded: per the ruling on #2003, payload lookup will go by the protocol's name instead of recording Info's field order.

@JarryShaw JarryShaw closed this Oct 21, 2026
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 21, 2026
@JarryShaw JarryShaw removed the review: needs-changes Cross-review at the current head says changes are required; see the verdict comment label Oct 21, 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(protocols): Info field order follows PyPy's attribute layout, not the declared fields

1 participant