pre_process sizes a value with floor division disguised as math.ceil, so every octet boundary raises OverflowError #599
Description
Activity
Two corrections to this issue, both established by the work in #600 and verified here.
1. The defect is far wider than this issue's framing. I wrote that "any value just past an octet boundary is sized one octet too small". That is only part of it: floor division is wrong for every bit length that is not a multiple of 8, and small values fare worst rather than best. Measured on
origin/main(13a75dfcd):value=1 bit_length=1 floor=0 correct=1 WRONG <- sized at ZERO octets value=2 bit_length=2 floor=0 correct=1 WRONG value=42 bit_length=6 floor=0 correct=1 WRONG value=127 bit_length=7 floor=0 correct=1 WRONG value=255 bit_length=8 floor=1 correct=1 value=256 bit_length=9 floor=1 correct=2 WRONG value=65536 bit_length=17 floor=2 correct=3 WRONGSo of the four examples in this issue's body,
255and65535are the only ones that worked, and they worked because 8 and 16 are exact multiples of 8. Every value from 1 to 127 was sized at zero octets.Why #591's suite missed it, which is the useful part: its repair-path values were
0xFF,0xFFFF,0xFFFFFFFF,0xFFFFFFFFFFFFFFFFand0x800001— bit lengths 8, 16, 32, 64 and 24. Every one an exact multiple of 8, precisely where floor division and the ceiling agree.2. My hypothesis about reachability was wrong. I briefed that the branch is reached by "a field constructed with no length at all". It is not —
NumberField()raisesIntError: Field has no length.andEnumField()raisesTypeError. The-1placeholder is assigned in exactly one place,pcapkit/corekit/fields/field.py:542, where a callable length is swapped for it, and the branch is then reached only on a field whose__call__never ran. It also requires__template__unset, soNumberFieldandEnumFieldonly — the eightInt/UIntsubclasses never enter it. And sinceSchema.packresolves every field first, it is not reachable through a protocol at all, only through the field-level API.3. One thing this issue's "Measured" block no longer describes accurately. Those figures were taken on
fa6d18e31, before #598 merged. #598 did not change the reachability or the sizes — both verified byte-identical across the two versions — but it did change the observable failure mode for three of the four boundaries:255now succeeds where it previously raisedstruct.error, and256/65536now raisestruct.errorwith a format-range message rather thanOverflowError. Only a mis-sized 3-octet width still raises theOverflowErrorthis issue is titled after. A fix should therefore assert widths and octets, not exception types.Separately, the same typo exists at two protocol-layer sites and is now filed as #601:
pcapkit/protocols/internet/hopopt.py:1889andpcapkit/protocols/internet/ipv6_opts.py:1892, bothmath.ceil(nonce.bit_length() // 8)in the ILNP nonce builder, against eight sibling sites inhip.pyandmh.pythat use/ 8correctly.
pre_process's width repair sizes a value with floor division dressed up as a ceiling, so any value just past an octet boundary is sized one octet too small andto_bytesraisesOverflowError.Mechanism
pcapkit/corekit/fields/numbers.py:198:math.ceilon anintis a no-op —//has already floored it — so the expression isvalue.bit_length() // 8. The intent was evidentlymath.ceil(value.bit_length() / 8).Measured
On
origin/main(fa6d18e31), CPython 3.14.7:So it fails at every octet boundary, not at one unlucky width: any value needing
8n + 1bits or more is sized atnoctets.255works and256does not;65535works and65536does not.The correct expression gives
sized=2for 256 andsized=3for 65536, both of which pack.Reachability
This is the repair path taken when a field's length is still the
-1placeholder at pack time — the branch guarded byif self._length < 0:atnumbers.py:197. It is therefore reached only when a width was never resolved, which makes it narrower than #591 but not unreachable.Notes
/for//— but it changes the packed width of any value past a boundary on that path, so it wants a test per boundary rather than a single case.