Skip to content

dumpkit's object_hook interpolates o.name unguarded, so a nameless flag member renders as Type::None [0] #648

Description

@JarryShaw

The default dumper's object_hook interpolates o.name into f'{type(o).__name__}::{o.name} [{o.value}]' with no guard. A flag pseudo-member carrying no declared bits has name is None, so the literal four-character string None lands in the output and a flagless value renders as Flags::None [0].

Measured on 375e9d411, CPython 3.14.7, tree asserted against the worktree rather than the editable install (pcapkit.__file__ = …/pcapkit/__init__.py printed on every run). pcapkit/dumpkit/common.py is byte-identical on current origin/main (da381f259).

The sites — three, not one

pcapkit/dumpkit/common.py:216, inside object_hook (common.py:181) on the DictDumper subclass that make_dumper (common.py:168) builds:

            if isinstance(o, (enum.Enum, aenum.Enum)):
                addon = {key: val for key, val in o.__dict__.items() if not key.startswith('_')}
                if addon:
                    return {
                        'enum': f'{type(o).__name__}::{o.name} [{o.value}]',   # 213
                        **addon,
                    }
                return f'{type(o).__name__}::{o.name} [{o.value}]'             # 216

The identical unguarded interpolation appears at common.py:204 (the MultiDict/OrderedMultiDict key path, key.name) and common.py:213 (the addon branch, for an enum carrying public instance attributes). All three produce the literal None:

line 204: hook(MultiDict) -> {'Flags::None [0]': ['x'], 'Flags::ACK [2048]': ['y']}
line 213: hook(Sub(0))    -> {'enum': 'Sub::None [0]', 'extra': 'payload'}
line 216: hook(Flags(0))  -> 'Flags::None [0]'

There are zero occurrences of is None anywhere in make_dumper's body (common.py:177-264), so the line cannot behave otherwise. Fixing only :216 would leave the other two emitting None.

Measured

repr(Flags(0))        = <Flags: 0>
Flags(0).name         = None
Flags(0).name is None = True
Flags(0).value        = 0
Flags(0).__dict__     = {'_value_': 0, '_name_': None}
repr(Flags.ACK)       = <Flags.ACK: 2048> | .name = 'ACK' | .value = 2048

All three dumpers, via object_hook directly and again as full dumps:

JSON:  "connection_flagless": "Flags::None [0]",
Tree:  |-- connection_flagless -> Flags::None [0]
PLIST: <key>connection_flagless</key>
       <string>Flags::None [0]</string>

Extractor.__output__ (pcapkit/foundation/extraction.py:202-214) maps json→dictdumper.JSON, tree/text/txt→dictdumper.Tree, plist/xml→dictdumper.PLIST — six user-facing format names over three classes, all wrapped by make_dumper at extraction.py:979 and again at pcapkit/foundation/traceflow/traceflow.py:277. So traceflow output is affected too, not only extraction. pcap/cap route to PCAPIO (binary) and do not use the hook.

Two corrections to how this was first reported

1. It is not an aenum quirk. A stdlib enum.IntFlag built from the identical members behaves the same on 3.14.7:

repr(StdFlags(0))        = <Flags: 0>
StdFlags(0).name         = None
StdFlags(0).name is None = True
rendered as line 216 would: 'Flags::None [0]'

So the issue is how both aenum.Flag and stdlib enum.Flag spell a bitless pseudo-member. Attributing it to aenum would misdirect anyone who tried to fix it by changing enum libraries.

2. It is not limited to value 0. Any flag value composed entirely of undeclared bits is nameless:

Flags            (0)          name=None -> 'Flags::None [0]'
Flags            (1)          name=None -> 'Flags::None [1]'
Flags            (8)          name=None -> 'Flags::None [8]'
Flags            (65536)      name=None -> 'Flags::None [65536]'
TransportProtocol(256)        name=None -> 'TransportProtocol::None [256]'
CommandType      (1073741824) name=None -> 'CommandType::None [1073741824]'

Mix in one declared bit and the residue is named numerically instead — Flags(2049).name == 'ACK|1' → 'Flags::ACK|1 [2049]'.

This line's own defect, not #634's

git blame -L 200,220 pcapkit/dumpkit/common.py:

dfac5d1767 (Jarry Shaw 2023-04-28 204)  key = f'{type(key).__name__}::{key.name} [{key.value}]'
dfac5d1767 (Jarry Shaw 2023-04-28 213)  'enum': f'{type(o).__name__}::{o.name} [{o.value}]',
dfac5d1767 (Jarry Shaw 2023-04-28 216)  return f'{type(o).__name__}::{o.name} [{o.value}]'

dfac5d1767468e26debea04afb3d6ebc1a0bbb70, 2023-04-28, "revised default dumper object_hook". git log -S over the file returns that commit and no other, so it is both the earliest and only introduction — three years and four months before #616/#634.

One honest scoping note. Sweeping every enumeration under pcapkit/const/, seven are Flag/IntFlag, and pcapkit.const.tcp.flags.Flags is the only one nameless at zero:

pcapkit.const.reg.apptype.TransportProtocol  (0) name='undefined' -> 'TransportProtocol::undefined [0]'
pcapkit.const.ftp.command.CommandType        (0) name='undefined' -> 'CommandType::undefined [0]'
pcapkit.const.tcp.flags.Flags                (0) name=None        -> 'Flags::None [0]'
pcapkit.const.mh.*_flag.*                    (0) -- four registries, see #623

The other two declare an explicit undefined = 0. Explicitly named zeros render correctly (Integrity(0).name == 'NONE' → 'Integrity::NONE [0]'). So "any zero-valued flag enumeration would render the same way" is true of the code but currently hypothetical for every registry except Flags.

Consequence

  • The [0] half is correct. Flags(0).value == 0, and the wire bytes are untouched. Only the name half is wrong.
  • Nothing crashes. JSON, Tree and PLIST all emit a well-formed string. This is cosmetic-plus-semantic, not an availability problem.
  • But the encoding is lossy for a consumer. The format is Type::name [value]. A consumer splitting on :: and [ recovers the name as the string "None", indistinguishable from a member genuinely so named. Across all 10,143 distinct member names under pcapkit/const/ none is literally None (the near misses are NONE and none_included), so the collision is latent rather than actual — but "None" is precisely the token a JSON or plist consumer in another language will not recognise as "no flags set".
  • Affected: anyone using Extractor or TraceFlow output in json, tree, text, txt, plist or xml.

Today, effectively nobody — no wire-reachable path on main produces a nameless member. TCP.read only ORs declared members and leaves a plain int when none is set (#616). Once #634 lands, every flagless TCP segment does; an nmap NULL scan is the canonical example. examples/captures/in.pcap carries no flagless segment, so the committed example dumps are unaffected either way.

A note on #634, which this makes smaller than it looks

#634 (for #616) removes a type inconsistency rather than adding one. On main today the connection field's JSON type switches with the flag bits, because a flagless segment leaves _flags a plain int:

pre-#634  0x00 flagless  JSON   "connection": 0,
pre-#634  0x00 flagless  PLIST  <integer>0</integer>
pre-#634  0x10 ACK       JSON   "connection": "Flags::ACK [2048]",
pre-#634  0x12 SYN|ACK   JSON   "connection": "Flags::ACK|SYN [18432]",

Simulating Enum_Flags(0) in memory only — which is what _flag = Enum_Flags(0) would leave — gives:

post-#634 0x00 flagless  JSON   "connection": "Flags::None [0]",
post-#634 0x00 flagless  PLIST  <string>Flags::None [0]</string>

So after #634 the field is a string unconditionally: a number-or-string inconsistency is replaced by a consistently-typed string whose name half is wrong for one case. The None is this line's pre-existing defect, surfaced rather than caused. #634's own changelog already identifies it as "a separate rendering defect… left to its own change".

Suggested fix

All three sites should fall back to something unambiguous when name is None — the value's own repr, or an explicit empty-set token — and should be fixed together, since :204 and :213 carry the identical bug.

Notes

Activity

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

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions