[JarryShaw]

• Searched for similar pull requests

• Followed the coding style (make pylint, make mypy, make isort): run on the 4 source files with the Makefile flags; isort clean, no pylint/mypy finding on a changed line

• make test passes, and a test case covers the change: the 4 new modules, 28 neighbouring modules, run_unittest_leg.py protocols/internet (503 OK), protocols --exclude protocols/internet (857 OK), foundation (483 OK), pytest tests/project (389 passed)

• Added a changelog entry — N/A, added centrally

• fix — corrects a defect

---

Closes #1274
Closes #1275
Closes #1265
Closes #1285

#1274 — signed NumberField decodes as unsigned

post_process masked with _bit_mask and returned, so once __call__ had set that mask — as it does for every field inside a Schema — a signed field came back as the unsigned pattern. It now sign-extends after masking, mirroring pre_process.

Crafted captures carrying a negative value, against origin/main at 11ae68ebf:

Capture	origin/main	this PR
PCAP-NG if_tzone = -3600	ValueError: offset must be a timedelta strictly between -timedelta(hours=24) and timedelta(hours=24)	parses, timestamp renders -01:00
PCAP-NG if_tsoffset = -5	OverflowError: timestamp out of range for platform time_t	parses, epoch 1 → -4
PCAP thiszone = -3600	parses, but reports thiszone=4294963696	reports -3600, rebuilds byte-exact

The two PCAP-NG cases also crash origin/main in json, plist and tree dumps, and all three dump cleanly here. The PCAP case does not crash on either side — it is a wrong-value bug, not a crash.

The fixture captures do not cover this path at all, so they are not evidence either way: all 23 carry thiszone, if_tzone and if_tsoffset of 0, and section_length of -1 reached through the all-ones value that schema/misc/pcapng.py:959 already special-cased. Per-frame info digests being unchanged (below) is therefore vacuously true for this issue.

#1275 — a computed PayloadField length of 0 swallows the rest

payload_length = length or packet['__length__'] read a computed 0 as "unset". Only a negative length is unset now, and it is bounded by max(__length__, 0).

	origin/main	this PR
n=0, then a 2-octet field	body=b'ab' tail=b'\x00\x00'	body=b'' tail=b'ab'
unset payload, __length__=3, 8 octets in the stream	body=b'abcdefgh', stream at 9, __length__ left at 3 — larger than the 2 it held before the read	body=b'ab', stream at 3, __length__ left at 0

The change is one-directional: it can only truncate. The new read count is max(__length__, 0), which is never more than the read(-1) it replaces, so no input can be made to read further than before. The one behaviour change for an unset payload is that it now stops at __length__ instead of running to the end of the stream, and yields b'' when __length__ is 0 or less. No protocol in the tree hits that: across the layers nothing moved (see the digests below).

Earlier drafts of this description said a negative length other than -1 used to raise ValueError. That is wrong for in-memory streams — BytesIO.read(-2) reads everything and raises nothing. It raised only on a real file object, which is #1265's case.

#1265 — 3 trailing bytes crash extract

Frame.unpack now raises a quiet StreamEOFError when fewer than 16 octets are left, the floor __length_hint__ already reports, so extraction ends with the frames it has — the same signal the PCAP-NG reader has used since #678. examples/captures/test.pcap plus b'\x00\x01\x02': ValueError: read length must be non-negative or -1 before, 34 frames after.

Both files are load-bearing. With only the schema.py half applied, a tail of 1, 3 or 15 octets yields unbounded phantom frames — still producing at a probe cap of 200. A tail of 16 or more is above the floor and parses as an ordinary empty record, unchanged from origin/main.

#1285 — Raw(b"")

Raw now declares its buffer's length instead of leaving it to be derived, since a derived 0 is prepare's end-of-stream signal. Raw(b"") raised StreamEOFError; it now parses, and agrees with Raw(packet=b"").

Digests, and what they do and do not show

Per-frame info digests plus the PCAP global header over all 23 examples/captures files: 0 of 23 differ, after each fix alone and after all four. That rules out collateral damage to the parsed output of every fixture, which is what it is cited for. It says nothing about #1274, for the coverage reason above.

Each new test fails with its fix reverted — for #1265, with either half reverted.

Notes

• schema/misc/pcapng.py:959 (section_length == 0xFFFF… → -1) is now redundant, since the field decodes to -1 itself. Left alone: that file belongs to another worker in this wave.
• Not a public-contract change in signature terms, but a resolved signed field now returns a negative int where it returned the unsigned pattern, and Schema.unpack leaves a correctly decremented __length__ for an unset payload.
• Out of scope and pre-existing: out-of-range if_tzone still raises a raw ValueError, and out-of-range if_tsoffset a raw OSError/OverflowError from fromtimestamp, which catches only ValueError. This change shifts if_tsoffset = -2**63 from OverflowError to OSError. Being filed separately.

[JarryShaw]

Cross-review verdict on 8bfaffa49: GOOD TO GO (Sonnet reviewed; the author ran on Opus). No defect found in the code. Three of the PR's claims are overstated and should be reworded in the body — none changes the verdict.

• fix(corekit): signed NumberField values decode as unsigned once the field is resolved #1274: ffffffff → -1 and fffff1f0 (thiszone 4294963696) → -3600, both rebuilding byte-exactly; unsigned on main. The signed-field inventory is exactly four — PCAP thiszone, PCAP-NG section_length, if_tzone, if_tsoffset — and the reviewer swept the tree for others rather than taking the list on trust. No dumper reads the first two. The latter two are strictly improved: if_tzone = -3600 raised ValueError on main and now yields -01:00; if_tsoffset = -5 raised OverflowError and now yields -4. json, plist and tree dumps of crafted negative captures work here and crash on main.
• fix(protocols): a PCAP file with 3 trailing bytes after the last record crashes extract with ValueError #1265 phantom frames: closed, measured across three trees with every probe capped at 50 frames. A 1-, 3-, 15- or 17-octet tail gives a clean stop here, ValueError on main, and 50 frames (cap hit) with only the schema.py half — so both halves are load-bearing. The issue's own repro gives 34 frames in all three formats.
• fix(schema): a computed PayloadField length of 0 swallows the rest of the block #1275 PCAP-NG half — which the author had not reproduced — does reproduce: an EPB with captured_len=0 plus a comment drops the comment and swallows 16 octets into packet on main; here the comment parses and packet is empty.
• fix(protocols): Raw(b"") raises StreamEOFError while Raw(packet=b"") works #1285: Raw(b"") and Raw(io.BytesIO(b"")) now succeed; Raw(b"abc") and Raw(b"", length=0) unchanged. Internal callers never reach Raw with empty data — _import_next_layer returns NoPayload first.
• Tests: each of the four new modules, run against this tree with the four source files taken from main, gives 4 / 1 / 3 / 16 failures. No existing test was edited. protocols leg 1173 tests, 0 failures; the 5 corekit failures are test(corekit): test_sentinel_exports_unit fails 5 tests in the corekit unittest leg #1332's and are present on the base.

Three corrections for the PR body. (1) The identical-digest claim is vacuously true and should not be cited as evidence: per-frame info never holds the global header, and every fixture has thiszone, if_tzone and if_tsoffset of 0 with section_length of -1, which the old schema already mapped. No fixture exercises a negative value — the crafted captures are the real evidence. (2) "A negative length other than -1 used to raise ValueError" is wrong for in-memory streams: BytesIO.read(-2) reads everything. The old path read to the end of the stream ignoring __length__ and left __length__ mis-decremented; it only raised on real file objects. (3) #1275 truncates in one direction only — an unset payload used to swallow octets past __length__ and now stops at it — and no over-read was constructible. Ethernet trailer padding and UDP/IPv4 length variations are unaffected.

Confirmed as flagged: schema/misc/pcapng.py:959 is now dead code. Follow-up to file, out of scope here: out-of-range if_tzone still raises a raw ValueError, and out-of-range if_tsoffset a raw OSError/OverflowError from fromtimestamp, which catches only ValueError; this PR shifts -2**63 from OverflowError to OSError.

[JarryShaw]

review: good-to-go re-pointed to 51d3e724c. I diffed 8bfaffa49...51d3e724c myself: one file, tests/protocols/test_schema_payload_zero_length_1275_unit.py, +27/−1. No source file changed, so the reviewed code is byte-identical and the verdict carries.

The added subtest closes the coverage gap the review named — it pins the unset-length case and fails without the fix (3 SUBFAILED, b'abcdefgh' != b'ab' and twice b'abcdefgh' != b''). The protocols --exclude protocols/internet leg re-ran green at 857 tests, one more than before.

Two corrections to my own earlier comment, both from re-derivation rather than relay:

• The dump claim was too broad. The two PCAP-NG cases do crash json, plist and tree on main and work here, but PCAP thiszone dumps succeed on both sides — it is a wrong-value bug (4294963696 against -3600), not a crash.
• The phantom frames are unbounded, not 50. With only the schema.py half, tails of 1, 3 and 15 octets were still producing frames at a 200-frame cap. And a tail of 16 or 17 octets sits above the floor and parses as an ordinary empty record on both trees, so the 16-octet floor is exactly where the behaviour divides and that case is not a regression.

The one-directional widening now rests on a derivation rather than a failed search: the new read count is max(__length__, 0), which is never greater than the read(-1) it replaces, so no input can read further than before.