A nested schema's field callbacks receive a packet dict that does not contain the enclosing schema's fields by name. The enclosing packet is reachable, but only under a __packet__ key, and that convention is documented in exactly one comment in the whole tree. So the natural thing to write — length=lambda pkt: pkt['length'], exactly as every top-level schema writes it — raises KeyError when the schema is nested.
The live casualty
pcapkit/protocols/schema/internet/mh.py:515-521, the CGA Parameters option:
class CGAParameter(Schema):
...
extensions: 'list[CGAExtension]' = OptionField(
length=lambda pkt: pkt['length'] - 25 - len(pkt['public_key']),
...
)
CGAParameter has no length field. The length this wants belongs to the enclosing CGAParametersOption (:529-536), which reaches it via ListField(length=..., item_type=SchemaField(schema=CGAParameter)).
A well-formed 40-octet option therefore cannot be parsed on f50436a8a:
raw = bytes.fromhex('11040000123400000c1e'
'0000000000000000000000000000086f'
'0000000020010db8'
'00'
'3003010203') # len(raw) == 40
MH(io.BytesIO(raw), len(raw), extension=True)
-> KeyError: 'length'
Four SchemaWarning: packet length < 0 warnings (-17, -25, -26, -31) come out on the way, which is the same arithmetic going negative during the option scan.
Mechanism
SchemaField.pack and SchemaField.unpack (pcapkit/corekit/fields/misc.py) hand the nested schema a dict with the parent nested under a key rather than merged:
return value.pack({
'__packet__': packet,
})
So pkt['length'] is absent and pkt['__packet__']['length'] is what exists. Confirmed by fixing just that lookup in place, which turns the KeyError into a different error (see below) rather than another KeyError.
Why this is a design problem and not one bad lambda
The __packet__ contract is real but undiscoverable:
- It is explained in one place,
pcapkit/protocols/schema/misc/pcapng.py:155-157, in a docstring belonging to an unrelated helper.
- There are only two consumers of it in any schema module, both in
schema/misc/pcapng.py (:168-170 and :354), and both defensively spell out the fallback themselves: if 'byteorder' not in packet and '__packet__' in packet: / packet.get('__packet__', {}).get('type', 'N/A'). Each reinvents the lookup because there is no helper for it.
Schema.unpack's own docstring documents __length__ and __option_padding__ as reserved packet keys but says nothing about __packet__.
- A schema author cannot tell from the field definition whether their schema will ever be nested, and the same schema class can be used both ways — so "write
pkt['length'] at top level, pkt['__packet__']['length'] when nested" is not a rule that can be applied locally.
Worth deciding deliberately rather than patching mh.py. Options, roughly in increasing order of ambition:
- Give the nested dict a chained lookup, so a name missing locally falls through to
__packet__ automatically. Keeps every existing callback working and makes pkt['length'] mean the obvious thing.
- Provide a documented helper (e.g.
outer(pkt, 'length')) and use it at the three existing sites plus mh.py.
- Merge the parent's fields in directly, which is simplest but lets an inner field name be shadowed by an outer one silently — probably why it was not done.
Fixing this alone does not unblock CGA
With the lookup corrected to pkt['__packet__']['length'], the same packet then fails differently:
FieldValueError: Field parameters has invalid length.
which is the ForwardMatchField length-accounting fault filed separately. Both have to be fixed for the CGA Parameters option to parse, which is why #437 reverted its half-fix rather than shipping a change of exception type — see that PR's "One option deliberately left on the generic handler". #437 pins the current behaviour in test_mh_cga_parameters_option_is_unparsable_upstream so the day this starts working is visible.
Raised out of docs/source/pep.rst, which had been carrying this defect as prose. That page tracks feature requests, not defects.
A nested schema's field callbacks receive a packet dict that does not contain the enclosing schema's fields by name. The enclosing packet is reachable, but only under a
__packet__key, and that convention is documented in exactly one comment in the whole tree. So the natural thing to write —length=lambda pkt: pkt['length'], exactly as every top-level schema writes it — raisesKeyErrorwhen the schema is nested.The live casualty
pcapkit/protocols/schema/internet/mh.py:515-521, the CGA Parameters option:CGAParameterhas nolengthfield. Thelengththis wants belongs to the enclosingCGAParametersOption(:529-536), which reaches it viaListField(length=..., item_type=SchemaField(schema=CGAParameter)).A well-formed 40-octet option therefore cannot be parsed on
f50436a8a:Four
SchemaWarning: packet length < 0warnings (-17, -25, -26, -31) come out on the way, which is the same arithmetic going negative during the option scan.Mechanism
SchemaField.packandSchemaField.unpack(pcapkit/corekit/fields/misc.py) hand the nested schema a dict with the parent nested under a key rather than merged:So
pkt['length']is absent andpkt['__packet__']['length']is what exists. Confirmed by fixing just that lookup in place, which turns theKeyErrorinto a different error (see below) rather than anotherKeyError.Why this is a design problem and not one bad lambda
The
__packet__contract is real but undiscoverable:pcapkit/protocols/schema/misc/pcapng.py:155-157, in a docstring belonging to an unrelated helper.schema/misc/pcapng.py(:168-170and:354), and both defensively spell out the fallback themselves:if 'byteorder' not in packet and '__packet__' in packet:/packet.get('__packet__', {}).get('type', 'N/A'). Each reinvents the lookup because there is no helper for it.Schema.unpack's own docstring documents__length__and__option_padding__as reserved packet keys but says nothing about__packet__.pkt['length']at top level,pkt['__packet__']['length']when nested" is not a rule that can be applied locally.Worth deciding deliberately rather than patching
mh.py. Options, roughly in increasing order of ambition:__packet__automatically. Keeps every existing callback working and makespkt['length']mean the obvious thing.outer(pkt, 'length')) and use it at the three existing sites plusmh.py.Fixing this alone does not unblock CGA
With the lookup corrected to
pkt['__packet__']['length'], the same packet then fails differently:which is the
ForwardMatchFieldlength-accounting fault filed separately. Both have to be fixed for the CGA Parameters option to parse, which is why #437 reverted its half-fix rather than shipping a change of exception type — see that PR's "One option deliberately left on the generic handler". #437 pins the current behaviour intest_mh_cga_parameters_option_is_unparsable_upstreamso the day this starts working is visible.Raised out of
docs/source/pep.rst, which had been carrying this defect as prose. That page tracks feature requests, not defects.