fix(corekit): size the width repair with a real ceiling, not a floored one (#599)

CHANGELOG.md

@@ -54,6 +54,8 @@ This is the resolution of #548, which reported `TransType.L2TP` (115) as registe
 - **Fixed** -- a `NumberField` whose `length` was a callable could not pack or parse at any width `struct` has a native integer code for. `length` is a placeholder of `-1` until the callable is resolved, `-1` has no native code, and the template builder raised `_need_process` for it and never put it back -- so the flag was a latch. Resolving the real width rebuilt the template and left the latch set, and `pre_process` then handed `bytes` to a template that had become `>Q`, raising `struct.error: required argument is not an integer`; parsing failed in the mirror direction, calling `int.from_bytes` on the integer that `struct.unpack` had already produced. The flag is now recomputed from the width actually in force rather than only ever raised, which is also what keeps a callable resolving to a width with no native code -- 3 octets, say -- byte-packed as it must be. **All four native widths were affected, not only the 8 that was reported**: the latch has nothing to do with the width it latches into, so 1, 2 and 4 failed identically, on `NumberField` and on `EnumField`, both of which leave `__template__` unset. This is what made every extended 8-octet MPTCP DSS form unbuildable, since those widths are chosen at runtime from the DSS flags and so must come from a callable; #585 worked around it in the TCP schema alone, leaving every other caller exposed (#591).
 - **Fixed** -- building any `MP_JOIN` option raised `AttributeError: 'TCP' object has no attribute '_flags'`. `TCP.make` constructed the options *before* it assigned the `self._flags` that the option makers read, and `_make_mptcp_join` branches on that attribute to choose between the three layouts [RFC 8684](https://datatracker.ietf.org/doc/html/rfc8684) section 3.2 gives for MP_JOIN -- figure 5 for SYN at 12 octets, figure 6 for SYN/ACK at 16, figure 7 for ACK at 24. The parse path was never affected: `read` assigns the flags before it parses the options, so the identical branches in `_read_mptcp_join` always had them. Fixed by hoisting the flag resolution above the `_make_tcp_options` call, leaving only the header's data-offset computation -- which genuinely needs the options' total length -- after it. **Not** fixed by giving `_flags` a zero default, which would have been worse: `Protocol.pack` is public and calls `make`, so an instance that had already parsed a segment always had the attribute set, and it silently built the option for the segment it had *read* rather than the one it was asked to write. Measured pre-fix, a parsed MP_JOIN-SYN instance asked to pack an MP_JOIN-ACK segment emitted an ACK header carrying figure 5's 12-octet SYN option, with the caller's 20-octet HMAC -- the whole authentication payload of figure 7's form -- replaced by an all-zero phantom token and nonce, and nothing raised. The crash was the benign symptom; a default value fixes only that and leaves the silent corruption. Hoisting also made one branch reachable for the first time, an MP_JOIN asked for on a segment with neither SYN nor ACK set: the accumulator was seeded with `cast('Enum_Flags', 0)`, and `typing.cast` being a runtime no-op, `self._flags` stayed a plain `int` on which the first membership test raised `TypeError` rather than the `ProtocolError` the method documents. It is now seeded with `Enum_Flags(0)`, a real flagless member that compares equal to `0` and ORs identically. The read path's own seed is deliberately unchanged -- a flagless MP_JOIN is rejected by `mptcp_data_selector` before `_read_mptcp_join` runs, so it cannot reach those branches (#587).
 - **Added** -- `tests/protocols/transport/test_tcp_mptcp_join_flag_ordering_unit.py`, covering all three MP_JOIN layouts through the *public* constructor, the construct-pack-parse cycle for each, the stale-flags case that rules out a zero-valued default, the statement order itself, and controls that the parse path and the flag-independent options are unaffected. The gap it closes is why 100% statement and branch coverage of the two changed modules coexisted with a completely broken public path: the pre-existing cases reach `_make_mptcp_join` by assigning a Python `set` to `_flags` on a bare `TCP.__new__(TCP)`, which executes every branch while bypassing both the ordering and the accumulator's type. The now-stale `tcp-mptcp/MP_JOIN` entry is deleted from `EXPECTED_FAILURES`, and the MP_JOIN exclusion in `test_tcp_mptcp_subtype_unit.py` is lifted (#587).
+- **Fixed** -- `NumberField.pre_process` sized a value with floor division dressed up as a ceiling. When a field is packed while its `length` is still the `-1` placeholder, the width is derived from the value, and it was derived with `math.ceil(value.bit_length() // 8)`. `math.ceil` of an integer is that integer, so the `//` had already floored the quotient and the outer call did nothing at all; the expression was plain floor division and the width came out one octet short. `256` was sized at one octet, `65536` at two, `16777216` at three, and both `int.to_bytes` and `struct.pack` refuse a value that does not fit the width they are given. **The reach is wider than "just past a boundary"**: floor division is wrong for every bit length that is not an exact multiple of eight, so `1` -- bit length 1, floored to *zero* octets -- failed too, and every value from 1 to 127 with it. Now written as the ceiling it was meant to be, matching the `math.ceil(n / 8)` idiom used elsewhere in the package. The repair is reached only by packing a field the caller never resolved, since a schema resolves every field before packing it and `__call__` installs a real width; that narrowness is why the defect survived the suite added for #591, whose five repair-path values -- `0xFF`, `0xFFFF`, `0xFFFFFFFF`, `0xFFFFFFFFFFFFFFFF` and `0x800001` -- have bit lengths of 8, 16, 32, 64 and 24 and so sat exactly where floor division and the ceiling agree. #591's own fix neither caused nor masked this, but it did change what the failure looks like: with `_need_process` now recomputed from the width in force, a mis-sized 1, 2 or 4 octets surfaces from `struct.pack` as `'B' format requires 0 <= number <= 255` where it used to surface from `int.to_bytes` as `OverflowError`, which is why the exception named in the report is no longer the one a mis-sized octet boundary raises. Two things on this path are deliberately left alone, both independent of the arithmetic: a signed field is sized without room for its sign bit, so an unresolved signed field still cannot pack `128`; and an unresolved field's bit mask is `-1`, which makes the masking and the sign remap above no-ops. The identical `math.ceil(x.bit_length() // 8)` expression also survives at the two ILNP nonce option builders in `hopopt.py` and `ipv6_opts.py`, which are a separate change (#599).
+- **Added** -- `tests/corekit/test_fields_numbers_width_repair.py`, covering each octet boundary in its own method rather than one parametrised sweep, since the defect is a pattern and a single case would pass against a fix that special-cased the reported width. Each boundary is asserted as a pair -- the value below it, which always packed, and the value above it, which did not -- so that a width shifted by one in the other direction fails too. Also swept over all eight boundaries, pinned as the `ceil(bit_length / 8)` invariant, checked for the smallest mis-sized value being `1`, round-tripped through pack and unpack, and given controls for the bit lengths that divide by eight and for the reachability of the repair at all. The suite deliberately asserts widths and octets rather than exception types, because the exception depends on whether the mis-sized width happens to have a native `struct` code (#599).
 
 Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` and `1.5.0b2` (both 2026-09-18) and `1.5.0b3` (2026-09-19), all published as prereleases and so resolved only by `pip install --pre`. `1.5.0b1` half-shipped: the tag, the GitHub release and the Conda deployments landed, but PyPI rejected the wheel because `twine check` found a Sphinx-only `:mod:` role in `README.rst`, which `pyproject.toml` declares as the dynamic long description. `1.5.0b2` is what reshipped it -- the release workflow is version-driven, so an existing version cannot republish -- and `1.5.0b3` followed the CI change that stops a TestPyPI outage from costing a release its wheels (#497, #498).
 

docs/source/changelog/1.5.0.rst

@@ -644,6 +644,50 @@ pull requests between #326 and #509.
   type. The now-stale ``tcp-mptcp/MP_JOIN`` entry is deleted from
   ``EXPECTED_FAILURES``, and the MP_JOIN exclusion in
   ``test_tcp_mptcp_subtype_unit.py`` is lifted (#587).
+* **Fixed** -- ``NumberField.pre_process`` sized a value with floor division
+  dressed up as a ceiling. When a field is packed while its ``length`` is still
+  the ``-1`` placeholder, the width is derived from the value, and it was derived
+  with ``math.ceil(value.bit_length() // 8)``. ``math.ceil`` of an integer is
+  that integer, so the ``//`` had already floored the quotient and the outer call
+  did nothing at all; the expression was plain floor division and the width came
+  out one octet short. ``256`` was sized at one octet, ``65536`` at two,
+  ``16777216`` at three, and both ``int.to_bytes`` and ``struct.pack`` refuse a
+  value that does not fit the width they are given. **The reach is wider than
+  "just past a boundary"**: floor division is wrong for every bit length that is
+  not an exact multiple of eight, so ``1`` -- bit length 1, floored to *zero*
+  octets -- failed too, and every value from 1 to 127 with it. Now written as the
+  ceiling it was meant to be, matching the ``math.ceil(n / 8)`` idiom used
+  elsewhere in the package. The repair is reached only by packing a field the
+  caller never resolved, since a schema resolves every field before packing it
+  and ``__call__`` installs a real width; that narrowness is why the defect
+  survived the suite added for #591, whose five repair-path values -- ``0xFF``,
+  ``0xFFFF``, ``0xFFFFFFFF``, ``0xFFFFFFFFFFFFFFFF`` and ``0x800001`` -- have bit
+  lengths of 8, 16, 32, 64 and 24 and so sat exactly where floor division and the
+  ceiling agree. #591's own fix neither caused nor masked this, but it did change
+  what the failure looks like: with ``_need_process`` now recomputed from the
+  width in force, a mis-sized 1, 2 or 4 octets surfaces from ``struct.pack`` as
+  ``'B' format requires 0 <= number <= 255`` where it used to surface from
+  ``int.to_bytes`` as ``OverflowError``, which is why the exception named in the
+  report is no longer the one a mis-sized octet boundary raises. Two things on
+  this path are deliberately left alone, both independent of the arithmetic: a
+  signed field is sized without room for its sign bit, so an unresolved signed
+  field still cannot pack ``128``; and an unresolved field's bit mask is ``-1``,
+  which makes the masking and the sign remap above no-ops. The identical
+  ``math.ceil(x.bit_length() // 8)`` expression also survives at the two ILNP
+  nonce option builders in ``hopopt.py`` and ``ipv6_opts.py``, which are a
+  separate change (#599).
+* **Added** -- ``tests/corekit/test_fields_numbers_width_repair.py``, covering
+  each octet boundary in its own method rather than one parametrised sweep, since
+  the defect is a pattern and a single case would pass against a fix that
+  special-cased the reported width. Each boundary is asserted as a pair -- the
+  value below it, which always packed, and the value above it, which did not --
+  so that a width shifted by one in the other direction fails too. Also swept
+  over all eight boundaries, pinned as the ``ceil(bit_length / 8)`` invariant,
+  checked for the smallest mis-sized value being ``1``, round-tripped through
+  pack and unpack, and given controls for the bit lengths that divide by eight
+  and for the reachability of the repair at all. The suite deliberately asserts
+  widths and octets rather than exception types, because the exception depends on
+  whether the mis-sized width happens to have a native ``struct`` code (#599).
 
 Preceded by ``1.5.0a1`` (2026-09-15), ``1.5.0b1`` and ``1.5.0b2`` (both
 2026-09-18) and ``1.5.0b3`` (2026-09-19), all published as prereleases and so

pcapkit/corekit/fields/numbers.py

@@ -223,13 +223,24 @@ def pre_process(self, value: 'int', packet: 'dict[str, Any]') -> 'int | bytes':
             second is how the template and the value being returned came to
             disagree in the first place. C.f. #591.
 
+            That width is a **ceiling** of the bit length over eight, and it is
+            written as one. It used to read
+            ``math.ceil(value.bit_length() // 8)``, which is not a ceiling at
+            all: :func:`math.ceil` of an :obj:`int` is that :obj:`int`, so the
+            ``//`` had already floored the quotient and the outer call did
+            nothing. Every value whose bit length is not an exact multiple of
+            eight was therefore sized one octet short -- ``256`` at one octet,
+            ``65536`` at two, and ``1`` itself at *zero* -- which
+            :meth:`int.to_bytes` and :func:`struct.pack` both refuse. See GitHub
+            issue #599.
+
         """
         value = value & self._bit_mask
         if self._signed and value > self._bit_mask >> 1:
             value -= self._bit_mask + 1
 
         if self._need_process and self._length < 0:
-            self._length = math.ceil(value.bit_length() // 8)
+            self._length = math.ceil(value.bit_length() / 8)
 
             endian = '>' if self._byteorder == 'big' else '<'
             struct_fmt = self.build_template(self._length, self._signed)