fix(corekit): bound FieldBase.unpack's rjust() padding to a sane ceiling (#554)

[JarryShaw]

Summary

FieldBase.unpack zero-padded straight up to a field's declared length with rjust(), regardless of how little data the buffer actually held. length is frequently wire-derived -- resolved by a Field's _length_callback against the packet under parse, or built by a schema selector from a value it just read off the wire (e.g. DecryptionSecretsBlock's secrets_data: BytesField(length=lambda pkt: pkt['__length__'])) -- so a corrupt or hostile capture could declare an arbitrarily large one. A ~40-octet PCAP-NG Decryption Secrets Block with a bogus inner length was enough to force a multi-gigabyte allocation.

The bound, and where it comes from. pcapkit/corekit/fields/field.py now raises FieldValueError when a declared length exceeds both (a) the buffer's actual size and (b) a 262144-octet (0x40_000) ceiling. That ceiling is not picked "by feel": it is libpcap's own MAXIMUM_SNAPLEN -- the point past which libpcap itself treats a capture's declared snapshot length as corrupt or byte-order-swapped -- and it is the exact figure this package's own PCAP writer already defaults snaplen to (Header.make's snaplen: int = 0x40_000).

FieldValueError (not BoolError, which means "must be a bool") is used because it already is this codebase's precedent for an invalid/insufficient field length -- see ListField.unpack's raise FieldValueError(f'Field {self.name} has invalid length.') in collections.py.

Why not "any declared length beyond what the buffer holds", full stop. That was my first attempt, and it is closer to the issue's literal wording, but the full test suite caught it: ListField.unpack/OptionField.unpack (collections.py) deliberately read a short, sometimes entirely empty, tail past a truncated option area and depend on it decoding as zero -- that is how an over-long ihl, or a capture cut short by its snapshot length, reads as end-of-option-list or Pad1 instead of wedging or raising (#431). An unconditional "any shortfall is bogus" guard broke 15 existing tests exercising exactly that mechanism (IPv4UnitTests::test_an_option_area_longer_than_the_datagram_still_parses, the HOPOPT/IPv6-Opts truncation tests, the TCP SACK overrun tests, several SchemaUnitTests). Every one of those fields is a handful of octets -- far under the 256 KiB ceiling -- so scoping the guard to fire only past it leaves that mechanism completely untouched while still stopping the actual reported danger, which is always multiple orders of magnitude past 256 KiB.

Test plan

• New tests/corekit/test_fields_field.py: exact-length and trailing-extra buffers still work; a small field short by one octet, and one over a completely empty buffer, still zero-pad (proving OptionField.unpack never returns for a well-formed HOPOPT header with an SMF_DPD option #431's mechanism survives); the ceiling boundary in both directions (length == ceiling still pads, length == ceiling + 1 with insufficient data raises); a field past the ceiling with a full buffer is still accepted (the guard fires on the padding, not on size alone); a 16 GiB declared length against a 2-octet buffer is rejected under a 5s deadline without ever allocating; the exception message names both counts.
• Verified each new test fails without the fix (checked against the pre-fix code via a monkeypatched FieldBase.unpack, using a safe 5 MiB/50 MiB surrogate for the magnitude case rather than the real 16 GiB, per the resource constraint below).
• Verified the tests are sensitive to a nearly-right bound, not just an absent one: mutated the comparison to length >= _MAX_ZERO_PAD_LENGTH (off-by-one at the ceiling) and to a wrong constant (0x40_001) -- both mutants were caught by the boundary tests.
• pytest tests/corekit/test_fields_field.py -- 11 passed.
• pytest tests (full tier, no coverage instrumentation) -- 1271 passed, 17 skipped, 2850 subtests passed, 0 failed. This is the run that caught the first, broken attempt at this fix (15 failures) and confirms the corrected version regresses nothing.
• coverage run -m pytest tests/corekit -- 101 passed, 120 subtests passed.

No test in this PR allocates anything close to the declared "huge" lengths it exercises -- the largest actual allocation any test performs is ~256 KiB (the ceiling boundary case itself).

[JarryShaw]

✅ GOOD TO MERGE

Cross-model review (Opus 5; the PR was authored on Sonnet), head fef728bc1. One commit on current main 8cfd6ab01, 4 files, +234/−1. Judged on its own merits.

The bound is genuinely derived, and I verified both halves of the derivation I could reach. _MAX_ZERO_PAD_LENGTH = 0x40_000 at field.py:58; pcapkit/protocols/misc/pcap/header.py:190 is exactly snaplen: int = 0x40_000, in Header.make's signature; and git grep over pcapkit/ returns only those two sites, so the figure is singular rather than scattered. The provenance is documented in a 20-line #: comment on the constant itself, covering the libpcap MAXIMUM_SNAPLEN origin and — crucially — why the rule is deliberately not "any shortfall", naming #431. That is exactly where a maintainer looks, and it is what separates a derived ceiling from a magic number.

The two-condition rule and its boundary, measured myself (nothing above 256 KiB materialised):

declared	available	outcome
1	0	ACCEPTED
2	1	ACCEPTED
262143	0	ACCEPTED
262144	0	ACCEPTED (256 KiB zero-pad)
262144	262144	ACCEPTED
262145	0	REJECTED
262145	262144	REJECTED — an off-by-one shortfall past the ceiling is still caught
262145	262145	ACCEPTED

#431 is preserved structurally, not incidentally, and I confirmed it against main rather than just checking it works. Identical output on both trees: UInt8Field().unpack(b'') → 0, UInt16Field().unpack(b'') → 0, UInt16Field().unpack(b'\x01') → 1, and the truncated IPv4 option area 4a00001800010000400600000a0000010a000002 parses to [(0, 'EOOLOption')] on both. The reason it cannot break is the right one: every such read is a one- or two-octet fixed-width field, five orders of magnitude under the ceiling, so length > 262144 is false and the guard cannot fire on that path at all. Given that the first attempt at this fix broke 15 tests by rejecting any shortfall, getting the rule's shape right rather than merely its threshold is the substance of this PR, and it is right.

The tests do fail a nearly-right bound — I applied the mutation myself rather than taking the claim:

_MAX_ZERO_PAD_LENGTH = 0x40_000  ->  0x40_001
1 failed, 10 passed   (rc 1, from a file)
FAILED …::FieldBaseUnpackBoundsTests::test_the_ceiling_is_262144_octets

So the standard is met. But it is worth saying plainly where the protection lives: that one test is load-bearing. The other ten read self.ceiling back from the module, so they move with the mutation and cannot catch a wrong constant — delete or weaken test_the_ceiling_is_262144_octets and any wrong ceiling ships with the boundary suite still green. The author anticipated exactly this (its docstring says "A test that only ever reads the constant back from the module would still pass if that figure were quietly changed"), which is good design rather than an oversight — I raise it so nobody later "simplifies" that test away.

FieldValueError is the right exception. It is (BaseError, ValueError) and a bogus declared length is a value fault; collections.py already raises it for the sibling length faults in the same layer, and strings.py/ipaddress.py follow the same convention. BoolError is a TypeError flavour meaning "must be a bool" and does not apply; FieldError is the TypeError-flavoured sibling and would be the wrong flavour.

Three non-blocking observations, the first worth a look before merge:

1. The guard sits after the read. buffer = buffer.read(length) is at field.py:271; the check is at :285. So on the file-backed path the unvalidated length reaches read() first, and the guard only constrains the subsequent rjust(). read(n) returns at most what is available so nothing is fabricated, but moving the check above the read would be strictly stronger at no cost and no behaviour change. I did not measure whether CPython's buffered read(n) pre-sizes a destination, so I am not claiming this is currently exploitable — only that the ordering is free to improve.
2. Two body/test wordings promise slightly more than they deliver. The body says both mutants "were caught by the boundary tests"; measurably, the >= mutant was, and the wrong-constant mutant was caught by test_the_ceiling_is_262144_octets alone, not by the boundary tests. And test_..._rejected_without_allocating proves rejection inside a 5 s deadline rather than non-allocation — its docstring is honest but the name over-promises, and since the deadline is signal.alarm-based it could not interrupt a single large C-level rjust() mid-flight anyway; the real protection there is the assertRaises.
3. test_rejection_message_names_the_declared_and_available_counts asserts assertIn('1', message) for the available count, which is satisfied by the 1 inside 262145 — so it would pass even if the available count were dropped from the message.

Green: tests/corekit/test_fields_field.py 11 passed (exit 0, from a file, run by me after restoring from a byte-identical copy with the md5sum confirmed); full tier 1271 passed / 17 skipped / 2850 subtests, exit 0, matching the claim exactly and consistent with the main baseline of 1260/2850 I measured elsewhere in this batch plus this PR's 11 new tests; util/changelog_md.py --check exit 0, with an entry that names the ceiling, its dual provenance, and explicitly records that #431's tolerance is untouched.

Detail and provenance of each measurement below.

[JarryShaw]

Detailed cross-review — #569 @ fef728bc1

Reviewer: Opus 5, per the standing rule that an agent-raised PR gets a cross-review from a different model than the one that wrote it. All runs with PYTHONSAFEPATH=1 and PYTHONPATH pinned to my worktree, asserting pcapkit.__file__ resolves inside it before importing anything else. Exit codes read from files, never from summary lines.

Provenance, stated plainly. I personally measured: the constant and its two-site derivation, the guard's exact code and its position relative to the read, the full boundary truth table, #431's tolerance on both trees, the 0x40_001 mutation, and the clean run of the new test file. The >= and 0x3F_FFF mutations, the guard-removal run, and the full-tier numbers came from a parallel Opus 5 run I dispatched; I cross-checked those against my own baselines and say below which is which. One earlier attempt at this delegation was cut short by a content safeguard on its resource-probing framing, so it was re-run with the framing narrowed to bounds correctness — flagging that because it means the resource-magnitude question was deliberately not measured by either of us (see "Could not verify").

---

1. The ceiling is derived, and singularly so — measured

pcapkit/corekit/fields/field.py:58            _MAX_ZERO_PAD_LENGTH = 0x40_000
pcapkit/protocols/misc/pcap/header.py:190     snaplen: int = 0x40_000,

git grep -n "0x40_000\|262144" over pcapkit/ at this head returns exactly those two lines and nothing else, so the figure is not scattered and the cited line number is correct. Line 190 sits in Header.make's signature, which is what the constant's own docstring points at.

The docstring is the part that makes this a derived bound rather than a magic one, and it is in the right place — on the constant, 20 lines, covering both why 262144 and why the rule is not "any shortfall":

This is libpcap's own MAXIMUM_SNAPLEN … and it is the same figure this package's own PCAP writer defaults snaplen to … No single field within one captured packet is legitimately larger than the largest packet libpcap itself is willing to believe …

The bound is deliberately not "any declared length beyond what the buffer holds": ListField.unpack and OptionField.unpack depend on reading a short, sometimes empty, tail past a truncated area and having it decode as zero … (see #431). Every such read is of a fixed-width, few-octet field, always far under this ceiling, so it is untouched.

2. The guard, and its position

# field.py:271
    buffer = buffer.read(length)
…
# field.py:285
        if length > _MAX_ZERO_PAD_LENGTH and len(buffer) < length:
            raise FieldValueError(
                f'Field {self.name!r} declares a length of {length} octet(s), '
                f'but only {len(buffer)} octet(s) are available.'
            )
# field.py:291
        value = struct.unpack(self.template, buffer[:length].rjust(length, b'\x00'))[0]

Note the ordering: the read at 271 precedes the check at 285. See §6.1.

3. Boundary truth table — measured by me

Using a BytesField with a packet-derived length. Nothing above 256 KiB was materialised.

declared length	available	outcome
1	0	ACCEPTED
2	1	ACCEPTED
262143	0	ACCEPTED
262144	0	ACCEPTED — a 256 KiB zero-pad from an empty buffer
262144	262144	ACCEPTED
262145	0	REJECTED
262145	262144	REJECTED
262145	262145	ACCEPTED

Two readings worth stating. First, the guard is not merely a magnitude check — row 7 shows a one-octet shortfall is still refused once the declared length is past the ceiling, so it is genuinely the conjunction it claims to be. Second, the largest single-field zero-pad still permitted is 256 KiB from an empty buffer (row 4).

My view on whether row 4 is a gap or a limit: a deliberate, documented limit. The docstring states the trade-off and names #431 as the cause, and #431's reads are 1–2 octet fixed-width fields — five orders of magnitude clear of the ceiling. A tighter number would not help; what would is a different shape of rule (per-field-kind, or the packet-context discrimination), and choosing that shape is a design question rather than a defect in this one. Whether many such fields can aggregate is a property of the option/list loop bounds, not of this guard, and is outside its remit.

4. #431 preserved — verified on both trees, identical output

I ran the same probe on main 8cfd6ab01 and on fef728bc1. Byte-identical results:

UInt8Field().unpack(b'')       -> 0
UInt16Field().unpack(b'')      -> 0
UInt16Field().unpack(b'\x01')  -> 1

IPv4(bytes.fromhex('4a00001800010000400600000a0000010a000002'))
  -> PARSED OK; options = [(0, 'EOOLOption')]

Preserved structurally: each of those is a one- or two-octet fixed-width read, so length > 262144 is false and the guard is unreachable on that path. That is a stronger guarantee than "the tests still pass", which is what I was asked to establish.

The parallel run additionally reproduced the TCP reproduction named in OptionField.unpack's own NOTE (data offset 10, four option octets → [(1, 'NoOperation'), (0, 'EndOfOptionList')]) identically on both trees. I did not re-run that one.

5. Mutation testing — the standard is met, with one caveat

Measured by me:

mutation	rc (from file)	caught by
0x40_000 → 0x40_001	1	test_the_ceiling_is_262144_octets only (10 others passed)

From the parallel run, not measured by me:

mutation	rc	caught by
> → >=	1	test_field_at_the_ceiling_still_zero_pads
0x40_000 → 0x3F_FFF	1	test_the_ceiling_is_262144_octets only
guard removed entirely	1	test_field_one_past_the_ceiling_with_insufficient_buffer_is_rejected, test_rejection_message_names_the_declared_and_available_counts

So a nearly-right bound is failed, not merely an absent one. The caveat: the wrong-constant protection is a single point of failure. Ten of the eleven tests read self.ceiling back from the module in setUp, so they move with the mutation and cannot detect it; only the one test that hard-codes 262144 can. The author saw this coming — that test's docstring says "A test that only ever reads the constant back from the module would still pass if that figure were quietly changed" — which is why I am flagging it as something to protect rather than as a defect. If that test is ever "tidied up" to use the constant, the suite silently stops guarding the number.

I restored field.py from a byte-identical copy after the mutation and proved it: md5 f2e28a7d489ee08ba80b2902022f47b9, git status empty, and the file back to 11 passed / exit 0.

Also worth crediting: the body's fails-without proof used a monkeypatched surrogate rather than a plain revert, and that was justified rather than lazy — a plain revert of field.py removes _MAX_ZERO_PAD_LENGTH, which setUp reads, so all eleven tests would error in setUp and prove nothing about the guard.

6. Non-blocking observations

6.1 The check is after the read. buffer.read(length) at :271, guard at :285. On the file-backed path an unvalidated length reaches read() before anything can reject it; the guard then only constrains rjust(). read(n) returns at most what is available, so no data is fabricated — and I want to be clear I did not measure whether CPython's buffered read(n) pre-sizes an n-octet destination, so I am not claiming a live problem. Only that hoisting the check above the read is strictly stronger, costs nothing and changes no behaviour.

6.2 Two wordings promise more than they deliver. The body says both mutants "were caught by the boundary tests" — measurably not so for the constant mutants (§5). And test_..._rejected_without_allocating asserts rejection inside a 5 s signal.alarm deadline, not non-allocation; signals land between bytecodes, so it could not interrupt one large C-level rjust() mid-flight even if the guard were gone. Its docstring is honest; the name is what over-promises. The real protection there is the assertRaises(FieldValueError).

6.3 test_rejection_message_names_the_declared_and_available_counts asserts assertIn('1', message) for the available count. '1' occurs inside 262145, so that assertion holds even if the available count were dropped from the message entirely. Asserting the full rendered substring would pin it.

6.4 Cosmetic. The message uses {self.name!r} where collections.py's sibling length errors use {self.name}. Mine rendered as Field '<bytes>' declares ….

7. Tests and changelog

• pytest tests/corekit/test_fields_field.py → exit 0, 11 passed. Measured by me, after restoration.
• Full tier pytest tests → exit 0, 1271 passed, 17 skipped, 2850 subtests passed. From the parallel run; it matches the body exactly and is consistent with baselines I measured myself in this batch (main at 1260 passed / 2850 subtests — I measured 1261/2850 on fix(corekit): stop a malformed TCP SACK's exception type depending on sys.modules state (#525) #562's branch, which is main plus one new test, and 1279/2850 on feat(protocols): opt-in code= registration with enum-type inference for Protocol/ProtocolBase #570's, which is main plus 19). 1260 + 11 new tests = 1271, and the subtest count is unchanged because the new tests add none. Both line up.
• python util/changelog_md.py --check → exit 0, in step. The entry names the ceiling, its dual provenance, and explicitly records that OptionField.unpack never returns for a well-formed HOPOPT header with an SMF_DPD option #431's tolerance is untouched — which, having verified that tolerance on both trees, I can say is accurate.

Could not verify

• libpcap's MAXIMUM_SNAPLEN == 262144 from any local source. No libpcap headers on this host (/usr/include/pcap* absent, no bindings in the venv). It matches my knowledge of pcap-int.h, but that is knowledge, not measurement — so of the two halves of the derivation, the internal one (header.py:190) is measured and the external one is not. Worth a maintainer's eye if that matters to you.
• The resource magnitude behind the issue — deliberately not measured by either run. Arithmetically the expressible ceiling is clear enough (DecryptionSecretsBlock.secrets_length is a UInt32Field, so up to ~4 GiB is declarable), but I am not putting a figure on peak cost.
• Whether CPython's buffered read(n) pre-allocates (§6.1).
• Any interpreter other than CPython 3.14.7.
• CI. Reached on local evidence only, per my brief; no claim about its tally.
• The >= and 0x3F_FFF mutations, the guard-removal run, the TCP #431 reproduction, and the full-tier numbers were measured by a delegated Opus 5 run rather than by me. I re-derived the 0x40_001 mutation and the #431 field-level behaviour independently and they agreed, which is why I am reporting the rest.

One cross-PR note, out of scope: this test file's setUp adds eleven more purge_modules(['pcapkit']) calls — the same sys.modules eviction #562's SchemaField hoist defends against, and which is still latent in ListField.pack at collections.py:105 (where it cannot be hoisted, since schema.py imports collections.py). Harmless here; worth knowing while both are in flight.

[JarryShaw]

Amended to f206ba428 — one character. The error message spells the field name plain rather than quoted, adopted from @lux-liang's #571 which proposed the same correction; plain is 8/8 across pcapkit/corekit/, including field.py:207 five lines above. Nothing else changed: git diff fef728bc1 f206ba428 is one line in pcapkit/corekit/fields/field.py. tests/corekit passes, 106 passed / 120 subtests / exit 0.

[JarryShaw]

✅ GOOD TO MERGE — re-pointed to f206ba428.

Cross-model review (Opus 5; PR authored on Sonnet). My earlier verdict was posted against fef728bc1, so this re-points it. The delta is exactly what the amend note claims, and I confirmed it as a real two-tree git diff between the two fetched objects rather than from the description: one line, one file, {self.name!r} → {self.name} at field.py:287. Nothing else moved.

Re-verified at this head rather than carried over:

• _MAX_ZERO_PAD_LENGTH is 262144 (0x40_000), and Header.make's snaplen default is the same 262144 — both halves of the derivation I can reach locally hold.
• The OptionField.unpack never returns for a well-formed HOPOPT header with an SMF_DPD option #431 truncated-data path is byte-identical to main. TCP data offset 7, kind 0x4f, declared length 12 against 6 octets → options=[('79', 12)] with data=b'\x00\x00\x00\x00\x01\x02\x03\x04\x05\x06' on both trees; same at declared length 32. This is the path corekit: reject short dynamic field buffers #571 broke, and this PR leaves it untouched.
• Full suite, exit code read from a file: 0 — 1271 passed, 17 skipped, 2850 subtests passed, 1189 s. Matches the body's claim exactly.
• No test asserts the quoted form, so dropping !r cannot break one: tests/corekit/test_fields_field.py:141 is the only test that names the message, and it asserts the counts, not the quoting.
• util/changelog_md.py --check: in step.

Two non-blocking notes in the detailed comment below: one stale line citation in the amend note, and one nearly-vacuous assertion in the message test.

[JarryShaw]

Detailed cross-review — #569 @ f206ba428 (delta from fef728bc1)

Reviewer: Opus 5, per the standing rule that an agent-raised PR gets a cross-review from a different model than the one that wrote it. All runs with PYTHONSAFEPATH=1 and PYTHONPATH pinned to my worktree, asserting pcapkit.__file__ resolves inside it before importing anything else. Exit codes read from files, never from a summary line.

The delta is one line — established, not accepted

gh api .../compare/fef728bc1...f206ba428 is useless for this question: it reports status: "diverged", ahead_by: 1, behind_by: 1, and its files list is the merge-base diff against main (the whole 234-line PR), because the amend rewrote the commit rather than adding one. Anyone checking the "one character" claim from the compare view would conclude the entire PR had changed.

The real answer needs both objects side by side:

git diff fef728bc177e555bcd7a9e78eff82d7fbab55c67 f206ba42837ccb71f4880cb27f0a5e6d11357ea1
 pcapkit/corekit/fields/field.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-                f'Field {self.name!r} declares a length of {length} octet(s), '
+                f'Field {self.name} declares a length of {length} octet(s), '

One line, one file. Confirmed.

The style claim is right; its supporting citation is wrong

The change is a genuine consistency fix. Across pcapkit/corekit/ the field-name-in-message style is plain in every instance, and after this amend there are zero !r forms left:

misc.py:319, misc.py:676, field.py:231, collections.py:116, collections.py:180,
collections.py:189, collections.py:193, collections.py:452      <- 8 pre-existing, all plain
field.py:287                                                     <- this PR's, now plain

So "plain is 8/8" is accurate, counting the eight that predate this line.

But the amend note cites field.py:207 as a precedent "five lines above", and that is wrong on both counts. field.py:207 is def pre_process(self, value: '_T', packet: ...) — not a message at all. The nearest in-file precedent is field.py:231, raise NoDefaultValue(f'Field {self.name} has no default value.'), which is 56 lines above the changed line, not five. The substance of the claim holds; only the pointer is stale. Flagging it because stale file:line citations have been a recurring problem on this board, and a wrong pointer in a commit note outlives the review.

Nothing depended on the quoted form

grep -rn "declares a length" tests/ pcapkit/ docs/ finds exactly one production site (the changed line) and no test asserting the string. The only test that inspects the message at all is tests/corekit/test_fields_field.py:141, and it asserts only the two numbers. So the amend is behaviour-neutral for the suite — which the runs then confirm.

Non-blocking note on that test, since the amend touches the very line it guards:

message = str(ctx.exception)
self.assertIn(str(self.ceiling + 1), message)   # '262145'
self.assertIn('1', message)                     # the "available" count

The second assertion is nearly vacuous: '262145' already contains '1', so it passes on the declared count alone and would still pass if the available-octet count were dropped from the message entirely. assertIn('but only 1 octet(s) are available', message) — or asserting on the message with the declared count removed — would actually pin what the test name promises. This predates the amend and was already present under the previous verdict, so it is not a re-review finding; it is just adjacent enough to be worth saying.

Measurements at this head

Ceiling derivation — two of the three figures verified locally:

claim	measured
_MAX_ZERO_PAD_LENGTH == 0x40_000	262144, True
Header.make's snaplen default equals the ceiling	262144, True
libpcap's MAXIMUM_SNAPLEN is 262144	could not verify — external to this repo, no libpcap headers on this host

That last one I am explicitly not vouching for. It is the load-bearing justification for the constant's value, and it is the one claim in the PR I could not independently obtain. It is consistent with my recollection of pcap-int.h, but recollection is not evidence and I am not presenting it as such.

Guard boundary, derived by direct BytesField.unpack calls rather than read off the diff:

length == ceiling,      1-octet buffer  -> pads, returns 262144 octets
length == ceiling + 1,  1-octet buffer  -> FieldValueError: Field <bytes> declares a
                                           length of 262145 octet(s), but only 1 octet(s)
                                           are available.
length == ceiling + 10, full buffer     -> ok, 262154 octets  (guard fires on the padding,
                                           not on size alone — as the PR claims)
length 10, 6-octet buffer               -> pads to 10   (#431 mechanism intact)
length 4,  empty buffer                 -> pads to 4    (#431 mechanism intact)

The #431 truncated-data path, the thing #572 exists to protect and the thing #571 broke. Constructed independently from the issue text, run on main (8cfd6ab01) and on this head:

declared=12 -> parses, options=[('79', 12)]
               data=b'\x00\x00\x00\x00\x01\x02\x03\x04\x05\x06'
declared=32 -> parses, options=[('79', 32)]
               data=b'\x00'*24 + b'\x01\x02\x03\x04\x05\x06'

Byte-identical on both trees. This is the strongest thing I can say for the PR and it is the clean discriminator against #571: the guard's length > 262144 conjunct means a 10- or 30-octet field never reaches it.

Worth recording precisely because it is easy to misread: the data comes back left-padded with zeros to the declared width, not as the six octets actually present. Anyone writing the #572 test should assert the padded value, not b'\x01\x02\x03\x04\x05\x06'.

Test runs (exit codes from files):

run	exit	result
pytest tests/corekit/test_fields_field.py	0	11 passed
pytest tests/corekit tests/protocols/internet/test_ipv4_unit.py -k "option or truncat or length or unpack or field"	0	93 passed, 144 subtests, 38 deselected
pytest tests (full, plain — no coverage instrumentation)	0	1271 passed, 17 skipped, 2850 subtests passed, 1189 s
util/changelog_md.py --check	0	CHANGELOG.md in step with docs/source/changelog/1.5.0.rst

The full tier reproduces the body's claimed tally exactly. The targeted run includes IPv4UnitTests::test_an_option_area_longer_than_the_datagram_still_parses, the existing #431 guard test.

One design observation, not a defect

The bound caps amplification rather than removing it: length == 262144 against a 1-octet buffer still allocates and zero-fills 256 KiB, a ~262,000× amplification, and it does so per field. That is three-plus orders of magnitude better than the multi-gigabyte case #554 reports and is the deliberate consequence of scoping the guard to preserve #431, so I am not asking for a change — the alternative is the unconditional guard that broke 15 tests. Recording it so nobody later reads this PR as having closed the amplification question entirely.

CI

Filtered to __typename == "CheckRun" (the pyup.io/safety-ci StatusContext excluded, as it is not a check run):

SUCCESS 9 | SKIPPED 4 | CANCELLED 21 | pending 13 | FAILURE 0

The 21 CANCELLED are the superseded fef728bc1 run, not failures. This is not the 22 ok / 0 fail / 2 skip clean state the rest of the board sits at — 13 checks were still in flight when I looked, and per standing practice I did not wait on Actions, since the ruleset grants an admin bypass and waiting costs ten minutes per round. Reporting the tally as observed rather than as clean.

Verdict

GOOD TO MERGE at f206ba428. The delta is one line in a message string, it moves the line onto the convention every other site in pcapkit/corekit/ already follows, nothing asserts the old form, the full tier is green with the exit code read from a file, and the #431 path I care most about is byte-identical to main.

Claims I could not verify: libpcap's MAXIMUM_SNAPLEN value (external). Claims I disputed: the field.py:207 citation in the amend note is wrong — the precedent is field.py:231.

[JarryShaw]

Correction to my amend note above, and to the commit message which is now merged as 4c0bcf9b7 and cannot be fixed.

I wrote that the plain-{self.name} precedent includes field.py:207, five lines away. Both halves are wrong. On main, field.py:207 is a blank line; the real precedent is field.py:231 — raise NoDefaultValue(f'Field {self.name} has no default value.') — which is 56 lines above the changed site at :287, not five.

The substantive claim still holds: plain is the house convention, 9 self.name sites in pcapkit/corekit/ with 8 plain, and @lux-liang's #571 proposed the right spelling. Only my citation was bad.

Provenance, since it matters more than the error: I took that line number from an earlier analysis rather than checking it, and repeated it in a commit message and a public comment. Caught by the cross-review, which enumerated all nine sites rather than accepting the count.