Skip to content

fix(ipv6): to_dict() collapses repeated extension headers, so the dict rebuild silently shortens the packet #1453

Closed

Description

@JarryShaw

Bug. Info.to_dict() keys IPv6 extension headers by name, so a second extension header of the same type overwrites the first. Rebuilding from the dict then silently produces a shorter packet. It does not raise.

Repro on #1452's head 2a6d4fba5; on main it is worse, since every extension header is lost:

dst  = bytes([60,0,1,4,0,0,0,0])        # Destination Options -> Destination Options
dst2 = bytes([17,0,1,4,0,0,0,0])        # Destination Options -> UDP
ud   = struct.pack('!HHHH', 1, 2, 8, 0)
body = dst + dst2 + ud
raw  = bytes([0x60,0,0,0]) + struct.pack('!HBB', len(body), 60, 64) + b'\x00'*32 + body
q = IPv6(raw, len(raw))
len(IPv6.from_data(q.info).data)            # 64, exact
len(IPv6.from_data(q.info.to_dict()).data)  # 56 of 64

The Info object keeps the full chain in __exthdr__. Only the dict form collapses it.

Any fix has to change what to_dict() emits, so it changes the dump output. That makes it a separate decision from #1447.

Found by the #1452 cross-review.

Activity

  1. added
    fixPull requests that fix a defect (fix: subject prefix)
    pendingAccepted and unstarted - nobody is on it and nothing blocks it
    on Oct 9, 2026
  2. moved this to Pending in PyPCAPKiton Oct 9, 2026
  3. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    Needs a decision: this fix has to change what the dumps contain, so I would like your call before a worker starts.

    Options:

    1. Suffix repeats. The second ipv6_opts becomes ipv6_opts_2, and so on. The dump layout is unchanged for every packet without a repeat, so real captures are unaffected (none of the 23 samples repeats a header).
    2. Emit the chain as a list under one key, e.g. extension_headers: [...]. This is cleaner, but it changes the dump shape for every IPv6 packet that carries an extension header.
    3. Leave the dict lossy and raise on rebuild. from_data(dict) would detect a chain it cannot recover and raise instead of shortening the packet. That breaks byte-exactness for the dict form, but no longer fails silently.

    My recommendation is (1): it keeps the dict form byte-exact and changes nothing for packets that don't repeat a header.

  4. added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    and removed
    pendingAccepted and unstarted - nobody is on it and nothing blocks it
    on Oct 9, 2026
  5. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    what about return a MultiDict?

  6. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    Recorded: you suggested having to_dict() return a MultiDict so that repeated keys survive. I measured it on main (b14d676c5) with pcapkit.corekit.multidict.OrderedMultiDict, and it works.

    • OrderedMultiDict subclasses dict. items() yields the first value per key, and items(multi=True) yields every value in order.
    • json.dumps, and dictdumper's JSON and Tree writers, all see one value per key. The dump output is therefore unchanged for every packet, so none of the options I listed above is needed.
    • The dict rebuild can walk items(multi=True) to recover a repeated chain, so from_data(info.to_dict()) becomes byte-exact.
    • One consequence: dumps keep showing only the first of two repeated headers, exactly as they do today. Showing both is a separate change to the dumpers.

    I'll go with this. It touches the same files as #1452 (infoclass.py, protocol.py, ipv6.py), so this is blocked until #1452 merges. A worker starts from main after that.

  7. added
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    and removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 9, 2026
  8. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    Unblocked: #1452 merged as ca5f6dba1. A worker is on it now, on branch fix/1453-to-dict-multidict, using the OrderedMultiDict approach recorded above.

  9. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Oct 9, 2026
  10. moved this from Blocked to WIP in PyPCAPKiton Oct 9, 2026
  11. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    Correction to my earlier measurement above. I said the dumps would see one value per key. That held for dictdumper's writers called directly, which is what I tested. But pcapkit's own dumper object_hook (pcapkit/dumpkit/common.py) writes every OrderedMultiDict as a list of single-key entries, the #1263 handling. Returning one from to_dict() therefore turned every nested layer into an array: all 92 sample dumps changed. #1466 adds an InfoDict subclass that the dumper writes as a mapping, and all 92 dumps are byte-identical to main again.

  12. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

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

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions