fix(generators): spell TCP_BASE's keys the way TCP.make declares them (#602)

CHANGELOG.md

@@ -56,6 +56,8 @@ This is the resolution of #548, which reported `TransType.L2TP` (115) as registe
 - **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).
+- **Fixed** -- `examples/generators/options.py`'s `TCP_BASE` spelled three header fields with names `TCP.make` does not declare, so every option fixture in `examples/captures/options-tcp.pcap` was built from values the generator had not asked for. `make` ends in `**kwargs` and reads nothing out of it, so an undeclared keyword is accepted and discarded with no warning and no `TypeError`, which is why this survived unnoticed. `'seq': 1` was `seq_no`, and all 25 captured frames therefore carried sequence number 0; `'urgent_pointer'` was `urgent`, whose requested value and unused default both happened to be 0; and `'ack_flag': False` was `ack`, while `'ack': 0` bound to `ack` itself -- the acknowledgement *flag* rather than the number, which is `ack_no` -- so that pair was the wrong way round in both directions. Each is renamed to the parameter `make` declares, keeping the values the table always read as. Measured: `TCP(**TCP_BASE).info.seq` was 0 against a declared 1 and is now 1, and `options-tcp.pcap` changes in exactly 25 bytes -- the low octet of each frame's sequence number -- with no case status and no `EXPECTED_FAILURES` entry moving. The module's six other base mappings were audited the same way: five pass nothing their `make` does not declare, and HIP's `extension` is declared by `HIP.read` instead, which is this library's documented way of forwarding a read-path keyword through `**kwargs` rather than a sixth instance of the defect (#602).
+- **Added** -- `tests/protocols/test_option_generator_tcp_base_unit.py`, asserting that every `TCP_BASE` key is a parameter `TCP.make` declares -- derived from `inspect.signature`, so it catches a fourth misspelling nobody has made yet -- and that the segment the mapping builds carries the stated header in both the data model and the packed octets. Two of the three wrong names were invisible to any assertion about a *value*, the value asked for being equal to the default that was used instead, which is what the signature check is for (#602).
 - **Fixed** -- the ILNP Nonce option builders in `HOPOPT` and `IPv6_Opts` sized the option with `math.ceil(nonce.bit_length() // 8)`, which is floor division dressed up as a ceiling: `//` floors, and `math.ceil` of an `int` is a no-op, so the ceiling was never actually taken. The nonce is packed by a `NumberField` whose width *is* that declared `len`, so an under-declared length did not merely mis-state the option -- it silently truncated the nonce on the wire, with nothing raised. Every nonce whose bit length was not an exact multiple of eight was affected, and **small values were the worst case rather than boundary values**: any nonce below 256 was declared as *zero* octets and dropped from the packet altogether, so `nonce=9` packed to `b'\x8b\x00'` and parsed back as `0`, while `nonce=256` and `nonce=65536` each truncated to `0` as well. Fixed to `max(1, math.ceil(nonce.bit_length() / 8))`, the form already used at five other sizing sites across `hip.py` and `mh.py`. The one-octet floor is the `mh.py` convention and is load-bearing here because `nonce` defaults to `0`, whose bit length is `0`: without it the default argument builds an ILNP Nonce option carrying no Nonce Value field at all, collapsing "the nonce is 0" into "there is no nonce" when [RFC 6744](https://datatracker.ietf.org/doc/html/rfc6744) gives the option that field. The read path was never affected, since it takes the width from the `len` octet on the wire rather than recomputing it (#601).
 - **Added** -- ILNP nonce sizing coverage in `tests/protocols/internet/test_ipv6_extension_unit.py`, one test per protocol, asserting the declared length, the exact packed octets and the construct-pack-parse cycle over ten nonces. The reason the existing suite missed this is that the only ILNP nonce it ever exercised was `0xFFFFFF` -- bit length 24, an exact multiple of eight, precisely where floor division and the ceiling agree -- the same blind spot that hid the identical typo in `numbers.py` behind bit lengths 8, 16, 24, 32 and 64. Every new case bar two deliberate controls therefore has a bit length that is *not* a multiple of eight, several of them below 256. The table also guards itself: the test asserts that at least six of its own values stay non-byte-aligned and that one stays below 256, so rounding them off to convenient constants later cannot quietly disarm the regression (#601).
 - **Fixed** -- the string-keyed `get()` in `pcapkit.const.ftp.command` and `pcapkit.const.http.method` tested membership with the raw key but registered `key.upper()`, so the *first* lowercase or mixed-case token raised `TypeError: 'RETR' already in use` rather than resolving. Reachable from wire data for FTP: `pcapkit.protocols.application.ftp` compiles its request pattern with `re.I` and passes the match verbatim, and [RFC 959 Section 5.3](https://datatracker.ietf.org/doc/html/rfc959#section-5.3) makes FTP commands case-insensitive -- "Upper and lower case alphabetic characters are to be treated identically", listing `RETR Retr retr ReTr rETr` as the same command -- so `retr file.txt` was a valid request this library could not parse. Both `get()` and `_missing_` now look the key up under the same canonical upper-case name they register it under, so every casing resolves to the one member that already exists instead of colliding with it. Resolving rather than registering a second member matters beyond not crashing -- a duplicate `GET` would carry neither the `safe` nor the `idempotent` attribute of the real one (#582, #583).

docs/source/changelog/1.5.0.rst

@@ -688,6 +688,36 @@ pull requests between #326 and #509.
   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).
+* **Fixed** -- ``examples/generators/options.py``'s ``TCP_BASE`` spelled three
+  header fields with names ``TCP.make`` does not declare, so every option
+  fixture in ``examples/captures/options-tcp.pcap`` was built from values the
+  generator had not asked for. ``make`` ends in ``**kwargs`` and reads nothing
+  out of it, so an undeclared keyword is accepted and discarded with no warning
+  and no ``TypeError``, which is why this survived unnoticed. ``'seq': 1`` was
+  ``seq_no``, and all 25 captured frames therefore carried sequence number 0;
+  ``'urgent_pointer'`` was ``urgent``, whose requested value and unused default
+  both happened to be 0; and ``'ack_flag': False`` was ``ack``, while
+  ``'ack': 0`` bound to ``ack`` itself -- the acknowledgement *flag* rather than
+  the number, which is ``ack_no`` -- so that pair was the wrong way round in
+  both directions. Each is renamed to the parameter ``make`` declares, keeping
+  the values the table always read as. Measured:
+  ``TCP(**TCP_BASE).info.seq`` was 0 against a declared 1 and is now 1, and
+  ``options-tcp.pcap`` changes in exactly 25 bytes -- the low octet of each
+  frame's sequence number -- with no case status and no ``EXPECTED_FAILURES``
+  entry moving. The module's six other base mappings were audited the same way:
+  five pass nothing their ``make`` does not declare, and HIP's ``extension`` is
+  declared by ``HIP.read`` instead, which is this library's documented way of
+  forwarding a read-path keyword through ``**kwargs`` rather than a sixth
+  instance of the defect (#602).
+* **Added** --
+  ``tests/protocols/test_option_generator_tcp_base_unit.py``, asserting that
+  every ``TCP_BASE`` key is a parameter ``TCP.make`` declares -- derived from
+  ``inspect.signature``, so it catches a fourth misspelling nobody has made yet
+  -- and that the segment the mapping builds carries the stated header in both
+  the data model and the packed octets. Two of the three wrong names were
+  invisible to any assertion about a *value*, the value asked for being equal to
+  the default that was used instead, which is what the signature check is for
+  (#602).
 * **Fixed** -- the ILNP Nonce option builders in ``HOPOPT`` and ``IPv6_Opts`` sized
   the option with ``math.ceil(nonce.bit_length() // 8)``, which is floor division
   dressed up as a ceiling: ``//`` floors, and ``math.ceil`` of an ``int`` is a

examples/generators/options.py

@@ -372,11 +372,31 @@ def _named_registry(owner: 'type', enum: 'Any', prefix: 'str',
 #: Header fields shared by every constructed TCP segment. Only ``options``
 #: varies between cases, so a difference in the octets is a difference in the
 #: option and nothing else.
+#:
+#: Every key here has to be spelled the way :meth:`TCP.make
+#: <pcapkit.protocols.transport.tcp.TCP.make>` declares it, because ``make``
+#: takes ``**kwargs`` and *silently drops* whatever it does not declare -- no
+#: ``UnknownFieldWarning``, no ``TypeError``, nothing. GitHub issue #602 is
+#: three keys that were spelled wrong and therefore ignored:
+#:
+#: * ``'seq'`` is ``seq_no``. The mapping read as sequence number 1 and every
+#:   generated frame carried 0, which is what made the defect worth an issue:
+#:   the captures were not the packets this table describes.
+#: * ``'urgent_pointer'`` is ``urgent``. Harmless in effect, since the value
+#:   asked for and the default that was used are both 0.
+#: * ``'ack_flag'`` is ``ack``, and ``ack`` is the *acknowledgement flag*
+#:   rather than the acknowledgement number, which is ``ack_no``. So the two
+#:   were the wrong way round: ``'ack': 0`` set the flag (to a falsy 0) and
+#:   ``'ack_flag': False`` set nothing at all.
+#:
+#: The values are unchanged from what the mapping always claimed to mean: a
+#: SYN-only segment with sequence number 1, every other flag clear, and the
+#: acknowledgement number, checksum and urgent pointer at zero.
 TCP_BASE = {
-    'srcport': 50000, 'dstport': 80, 'seq': 1, 'ack': 0,
-    'ns': False, 'cwr': False, 'ece': False, 'urg': False, 'ack_flag': False,
+    'srcport': 50000, 'dstport': 80, 'seq_no': 1, 'ack_no': 0,
+    'ns': False, 'cwr': False, 'ece': False, 'urg': False, 'ack': False,
     'psh': False, 'rst': False, 'syn': True, 'fin': False,
-    'window': 8192, 'checksum': b'\x00\x00', 'urgent_pointer': 0,
+    'window': 8192, 'checksum': b'\x00\x00', 'urgent': 0,
     'payload': b'',
 }