Repository navigation
fix(fields): #811 misreports a malformed template as a negative length, and never prevents the negative length #825
Description
Activity
Checkable blocker: #823 merged.
gh pr view 823 -R JarryShaw/PyPCAPKit --json state,mergedAtFixing (1) changes the message
FieldBase.lengthraises, and #823 pins that message verbatim attests/protocols/application/test_http_unit.py:662:self.assertEqual(str(ctx.exception), "Field debug resolved to a negative length; template='-1s'")
So landing this first would red the very test #823 exists to fix, and
mainis already red on #822. Once #823 is in, this change should relax that assertion toassertIn('negative length', str(ctx.exception))in the same commit — which is what #823's cross-review recommended as a follow-up anyway, and what makes the test survive the correction.Unblocked — #823 landed on
mainas21e9588af(direct push at the maintainer's instruction; #823 closed as delivered).gh pr view 823 -R JarryShaw/PyPCAPKit --json state,mergedAt # CLOSED git log --oneline -1 origin/main # 21e9588afOne consequence for whoever takes this: the assertion that pinned the current message is now on
main, attests/protocols/application/test_http_unit.py:662:self.assertEqual(str(ctx.exception), "Field debug resolved to a negative length; template='-1s'")
So fixing (1) reds
mainunless the same commit relaxes it toassertIn('negative length', str(ctx.exception))— which is what #823's cross-review recommended anyway, and is now a hard requirement rather than a suggestion. A worker is on it.- added 5 commits that reference this issue
on Sep 26, 2026
Two related defects in #811's negative-length guard, both surfaced by #823's cross-review and both confirmed here.
1. The message is a category error.
FieldBase.length(pcapkit/corekit/fields/field.py:284-290) catchesstruct.errorblanket and unconditionally reportsf'Field {self.name} resolved to a negative length; template={self.template!r}'. Butstruct.calcsizeraises the same error for a malformed template as for a negative count — measured on 3.14.7:So a schema with a typo'd template is reported as a negative length, which sends the reader to the wrong cause. The guard should distinguish the two — the negative case is detectable from the resolved length before
calcsizeis called at all.2. The negative length is reported, never prevented. A
SchemaWarning: packet length < 0: -1fires before theProtocolError, sopkt['__length__']still goes negative and the schema still builds a'-1s'template; #811 converts the resulting exception rather than stopping the arithmetic. #823's reviewer found 32 inputs across two different fields (debugandfragment) taking this path in a 1210-case sweep, so it is a class of input, not one packet. The real fix boundspkt['__length__']at the schema layer.Related and worth re-measuring in the same change:
http.py:318-319(except (ValueError, struct.error)in the preface arm) took zero hits across that 1210-case sweep and may now be unreachable, andhttp.py:416'scontextlib.suppress(ProtocolError, struct.error)is likely in the same position. Neither is safe to simply delete —StructErrorsubclassesstruct.error, and the comment at:401-402explains why removing a suppression from a non-final arm previously caused an HTTP/1→HTTP/2 mislabel.Note the test pinned by #823 asserts the current message verbatim, so fixing (1) will red it;
assertIn('negative length', …)there would survive.