Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ 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** -- 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).
- **Fixed** -- `httpv1`'s `_RE_METHOD` was unanchored and `re.match` anchors only at the start, so it prefix-matched, and the request-line reader then passed the whole `para1` to `Method.get` rather than the captured `method` group. Together those meant `b'Get'` matched on the single character `G`, satisfied the guard that decides a start-line is a request, and handed the entire mixed-case token to a lookup that raised on it. Fixing either half alone still gives a wrong answer -- normalising the lookup would parse `b'Get'` as `GET` off a one-character match, and passing the group would parse it as a method named `G`. The pattern is now anchored at both ends and the captured group is what is looked up, so a token that is not a method is a malformed request line rather than a mis-parsed one. Method tokens are case-sensitive per [RFC 9110 Section 9.1](https://datatracker.ietf.org/doc/html/rfc9110#section-9.1), so no `re.I` was added: `GET` parses, `Get` and `get` are rejected (#583).
- **Fixed** -- `_RE_STATUS` in the same reader carried the same unanchored prefix defect, found by auditing `_RE_METHOD`'s siblings, and it escaped as the wrong exception type. That pattern is only a guard -- the value is taken from `int(para2)` on the raw token -- so a prefix match let a malformed status past the guard and then out of `int()` uncaught, where `_read_http_header` documents `ProtocolError`. Measured: a status of `200x` raised `ValueError: invalid literal for int() with base 10: b'200x'`, and one of `2000` raised `ValueError: 2000 is not a valid StatusCode`; both are now `ProtocolError`. [RFC 9112 Section 4](https://datatracker.ietf.org/doc/html/rfc9112#section-4) gives `status-code = 3DIGIT`, exactly three, so the anchor is what the grammar already said -- the production lives in HTTP/1.1 because `status-code` is part of its `status-line`, while [RFC 9110 Section 15](https://datatracker.ietf.org/doc/html/rfc9110#section-15) covers the code semantics and the IANA registry rather than the syntax. `_RE_VERSION` was audited at the same time and is safe as it stands, because both of its call sites read the captured group rather than the raw token (#583).
- **Fixed** -- `get()`'s documented `default` was ignored on the integer path throughout the generated `pcapkit.const` tree, because `get` delegated the lookup to the enum call and `_missing_` has no access to the caller's `default` -- so `Hardware.get(99999, 0)` raised `ValueError: 99999 is not a valid Hardware` instead of returning the fallback it was handed. The integer path now consults `default` before letting the lookup error escape. `-1`, the placeholder the generated signature already carried, is what separates "no default was supplied" from "a default was supplied and should be used", so a caller that asked for no fallback still gets the error rather than a silent substitution. The sweep #584 asked for puts the scope at 110 of the 118 integer registries, not the three the issue named; the two carrying a bespoke integer fallback of their own, `pcapng` `OptionType` and `reg` `AppType`, are deliberately left alone, since neither drops a default by raising. Not reachable from wire data -- every value a wire field can carry already resolves -- so this is a contract fix rather than a parse fix. Applied to the nine vendor templates as well as the 113 generated modules, and a new test renders the shared template and compares it against the module generated from it, so a regeneration cannot quietly undo it (#584).

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).

Expand Down
64 changes: 64 additions & 0 deletions docs/source/changelog/1.5.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,70 @@ 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** -- 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` 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).
* **Fixed** -- ``httpv1``'s ``_RE_METHOD`` was unanchored and ``re.match``
anchors only at the start, so it prefix-matched, and the request-line reader
then passed the whole ``para1`` to ``Method.get`` rather than the captured
``method`` group. Together those meant ``b'Get'`` matched on the single
character ``G``, satisfied the guard that decides a start-line is a request,
and handed the entire mixed-case token to a lookup that raised on it. Fixing
either half alone still gives a wrong answer -- normalising the lookup would
parse ``b'Get'`` as ``GET`` off a one-character match, and passing the group
would parse it as a method named ``G``. The pattern is now anchored at both
ends and the captured group is what is looked up, so a token that is not a
method is a malformed request line rather than a mis-parsed one. Method tokens
are case-sensitive per :rfc:`9110#section-9.1`, so no ``re.I`` was added:
``GET`` parses, ``Get`` and ``get`` are rejected (#583).
* **Fixed** -- ``_RE_STATUS`` in the same reader carried the same unanchored
prefix defect, found by auditing ``_RE_METHOD``'s siblings, and it escaped as
the wrong exception type. That pattern is only a guard -- the value is taken
from ``int(para2)`` on the raw token -- so a prefix match let a malformed
status past the guard and then out of ``int()`` uncaught, where
``_read_http_header`` documents ``ProtocolError``. Measured: a status of
``200x`` raised
``ValueError: invalid literal for int() with base 10: b'200x'``, and one of
``2000`` raised ``ValueError: 2000 is not a valid StatusCode``;
both are now ``ProtocolError``.
:rfc:`9112#section-4` gives ``status-code = 3DIGIT``, exactly three, so the
anchor is what the grammar already said -- the production lives in HTTP/1.1
because ``status-code`` is part of its ``status-line``, while
:rfc:`9110#section-15` covers the code semantics and the IANA registry rather
than the syntax. ``_RE_VERSION`` was audited at the same time and is safe as
it stands, because both of its call sites read the captured group rather than
the raw token (#583).
* **Fixed** -- ``get()``'s documented ``default`` was ignored on the integer
path throughout the generated ``pcapkit.const`` tree, because ``get``
delegated the lookup to the enum call and ``_missing_`` has no access to the
caller's ``default`` -- so ``Hardware.get(99999, 0)`` raised
``ValueError: 99999 is not a valid Hardware`` instead of returning the
fallback it was handed. The integer path now consults ``default`` before
letting the lookup error escape. ``-1``, the placeholder the generated
signature already carried, is what separates "no default was supplied" from
"a default was supplied and should be used", so a caller that asked for no
fallback still gets the error rather than a silent substitution. The sweep
#584 asked for puts the scope at 110 of the 118 integer registries, not the
three the issue named; the two carrying a bespoke integer fallback of their
own, ``pcapng`` ``OptionType`` and ``reg`` ``AppType``, are deliberately left
alone, since neither drops a default by raising. Not reachable from wire data
-- every value a wire field can carry already resolves -- so this is a
contract fix rather than a parse fix. Applied to the nine vendor templates as
well as the 113 generated modules, and a new test renders the shared template
and compares it against the module generated from it, so a regeneration
cannot quietly undo it (#584).

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
Expand Down
11 changes: 9 additions & 2 deletions pcapkit/const/arp/hardware.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,19 @@ def get(key: 'int | str', default: 'int' = -1) -> 'Hardware':

Args:
key: Key to get enum item.
default: Default value if not found.
default: Default value if not found. The placeholder ``-1`` stands
for *no default*, in which case an unresolvable key propagates
the lookup error instead of falling back.

:meta private:
"""
if isinstance(key, int):
return Hardware(key)
try:
Comment thread
JarryShaw marked this conversation as resolved.
return Hardware(key)
except ValueError:
if default == -1:
raise
return Hardware(default)
if key not in Hardware._member_map_: # pylint: disable=no-member
return extend_enum(Hardware, key, default)
return Hardware[key] # type: ignore[misc]
Expand Down
11 changes: 9 additions & 2 deletions pcapkit/const/arp/operation.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,19 @@ def get(key: 'int | str', default: 'int' = -1) -> 'Operation':

Args:
key: Key to get enum item.
default: Default value if not found.
default: Default value if not found. The placeholder ``-1`` stands
for *no default*, in which case an unresolvable key propagates
the lookup error instead of falling back.

:meta private:
"""
if isinstance(key, int):
return Operation(key)
try:
return Operation(key)
except ValueError:
if default == -1:
raise
return Operation(default)
if key not in Operation._member_map_: # pylint: disable=no-member
return extend_enum(Operation, key, default)
return Operation[key] # type: ignore[misc]
Expand Down
11 changes: 9 additions & 2 deletions pcapkit/const/esp/cipher.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,19 @@ def get(key: 'int | str', default: 'int' = -1) -> 'Cipher':

Args:
key: Key to get enum item.
default: Default value if not found.
default: Default value if not found. The placeholder ``-1`` stands
for *no default*, in which case an unresolvable key propagates
the lookup error instead of falling back.

:meta private:
"""
if isinstance(key, int):
return Cipher(key)
try:
return Cipher(key)
except ValueError:
if default == -1:
raise
return Cipher(default)
if key not in Cipher._member_map_: # pylint: disable=no-member
return extend_enum(Cipher, key, default)
return Cipher[key] # type: ignore[misc]
Expand Down
11 changes: 9 additions & 2 deletions pcapkit/const/esp/integrity.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,19 @@ def get(key: 'int | str', default: 'int' = -1) -> 'Integrity':

Args:
key: Key to get enum item.
default: Default value if not found.
default: Default value if not found. The placeholder ``-1`` stands
for *no default*, in which case an unresolvable key propagates
the lookup error instead of falling back.

:meta private:
"""
if isinstance(key, int):
return Integrity(key)
try:
return Integrity(key)
except ValueError:
if default == -1:
raise
return Integrity(default)
if key not in Integrity._member_map_: # pylint: disable=no-member
return extend_enum(Integrity, key, default)
return Integrity[key] # type: ignore[misc]
Expand Down
18 changes: 12 additions & 6 deletions pcapkit/const/ftp/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,21 +289,27 @@ def get(key: 'str', default: 'Optional[str]' = None) -> 'Command':
"""Backport support for original codes.

Args:
key: Key to get enum item.
key: Key to get enum item. Looked up case-insensitively, since
member names are canonicalised to upper case on registration.
default: Default value if not found.

:meta private:
"""
if key not in Command._member_map_: # pylint: disable=no-member
return extend_enum(Command, key.upper(), default if default is not None else key)
return Command[key] # type: ignore[misc]
name = key.upper()
if name not in Command._member_map_: # pylint: disable=no-member
return extend_enum(Command, name, default if default is not None else key)
return Command[name] # type: ignore[misc]

@classmethod
def _missing_(cls, value: 'str') -> 'Command':
"""Lookup function used when value is not found.

Args:
value: Value to get enum item.
value: Value to get enum item. Matched case-insensitively against
the canonical upper-case member names.

"""
return extend_enum(cls, value.upper(), value)
name = value.upper()
if name in cls._member_map_:
return cls._member_map_[name] # type: ignore[return-value]
return extend_enum(cls, name, value)
Loading
Loading