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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ This is the resolution of #548, which reported `TransType.L2TP` (115) as registe
- **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).
- **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](https://datatracker.ietf.org/doc/html/rfc959#section-5) 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).
Expand Down
2 changes: 1 addition & 1 deletion docs/source/changelog/1.5.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ pull requests between #326 and #509.
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
:rfc:`959#section-5` 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_``
Expand Down
8 changes: 4 additions & 4 deletions docs/source/contributing/conventions/registry-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ about spelling**? The owner's answer, verbatim:
So the test a new registry has to pass has **two limbs**, and satisfying either one
justifies case-insensitivity:

1. **A comparison rule in the governing document.** :rfc:`959#section-4.1` for FTP
1. **A comparison rule in the governing document.** :rfc:`959#section-5` for FTP
command codes, :rfc:`5797#section-2` for FTP FEAT codes, :rfc:`6335#section-5.1`
for IANA service names.
2. **A documented spelling disagreement between the specification and the registry.**
Expand Down Expand Up @@ -278,7 +278,7 @@ That leaves the classes with something to decide:
- What it says
- Verdict
* - :class:`~pcapkit.const.ftp.command.Command`
- :rfc:`959#section-4.1`
- :rfc:`959#section-5`
- *"Upper and lower case alphabetic characters are to be treated
identically."* Limb 1.
- **case-insensitive** -- ``get``/``_missing_`` fold, correctly
Expand Down Expand Up @@ -334,7 +334,7 @@ That leaves the classes with something to decide:
are** service names.
- **unimplemented** -- no service-name lookup exists to fold; see below
* - ``CommandType``, ``ConformanceRequirement``
- :rfc:`959#section-4.1`, :rfc:`5797#section-2`
- :rfc:`959#section-4`, :rfc:`5797#section-2`
- Limb 2 holds on measurement: the RFC and registry pages present the kind and
conformance letters upper case (``A``/``P``/``S``, ``M``/``O``/``H``) while
the CSV columns the crawler reads are lower case in every row (``s`` 26,
Expand Down Expand Up @@ -440,7 +440,7 @@ rather than changing it.
fall-through can be measured on a class that defines no ``get``.
``Command.get`` upper-cases its key
before matching, which makes it look as though the base were case-insensitive --
deliberately, since :rfc:`959#section-4.1` treats FTP command codes identically
deliberately, since :rfc:`959#section-5` treats FTP command codes identically
regardless of case. ``Method.get`` used to fold case the same way, but
`#896 <https://github.com/JarryShaw/PyPCAPKit/issues/896>`__ made it
case-sensitive instead: :rfc:`9110#section-9.1` says the HTTP method token is
Expand Down
4 changes: 2 additions & 2 deletions pcapkit/const/ftp/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def _missing_(cls, value: 'str') -> 'FEATCode':


class CommandType(EnumLookup, IntFlag):
"""Type of "kind" of command, based on :rfc:`959#section-4.1`.
"""Type of "kind" of command, based on :rfc:`959#section-4`.

Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub
issue #930, finishing #877's phase 2. Pure re-parenting as far as
Expand Down Expand Up @@ -258,7 +258,7 @@ class Command(EnumRegistry, StrEnum):
feat: 'Optional[FEATCode]'
#: Brief description of command / extension.
desc: 'Optional[str]'
#: Type of "kind" of command, based on :rfc:`959#section-4.1`.
#: Type of "kind" of command, based on :rfc:`959#section-4`.
type: 'CommandType'
#: Expectation for support in modern FTP implementations.
conf: 'ConformanceRequirement'
Expand Down
2 changes: 1 addition & 1 deletion pcapkit/const/http/method.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ def get(cls, key: 'str', default: 'Optional[str]' = None) -> 'Method':
per :rfc:`9110#section-9.1` -- the method token is
case-sensitive, unlike :meth:`~pcapkit.const.ftp.command.
Command.get`'s equivalent override, which stays
case-insensitive because :rfc:`959#section-4.1` says FTP
case-insensitive because :rfc:`959#section-5` says FTP
command codes are not. Checked against both member names
and values, via the base's own precedence -- name before
value -- so a value-only match such as
Expand Down
4 changes: 2 additions & 2 deletions pcapkit/vendor/ftp/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ def _missing_(cls, value: 'str') -> 'FEATCode':


class CommandType(EnumLookup, IntFlag):
"""Type of "kind" of command, based on :rfc:`959#section-4.1`.
"""Type of "kind" of command, based on :rfc:`959#section-4`.

Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub
issue #930, finishing #877's phase 2. Pure re-parenting as far as
Expand Down Expand Up @@ -270,7 +270,7 @@ class {NAME}(EnumRegistry, StrEnum):
feat: 'Optional[FEATCode]'
#: Brief description of command / extension.
desc: 'Optional[str]'
#: Type of "kind" of command, based on :rfc:`959#section-4.1`.
#: Type of "kind" of command, based on :rfc:`959#section-4`.
type: 'CommandType'
#: Expectation for support in modern FTP implementations.
conf: 'ConformanceRequirement'
Expand Down
2 changes: 1 addition & 1 deletion pcapkit/vendor/http/method.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def get(cls, key: 'str', default: 'Optional[str]' = None) -> '{NAME}':
per :rfc:`9110#section-9.1` -- the method token is
case-sensitive, unlike :meth:`~pcapkit.const.ftp.command.
Command.get`'s equivalent override, which stays
case-insensitive because :rfc:`959#section-4.1` says FTP
case-insensitive because :rfc:`959#section-5` says FTP
command codes are not. Checked against both member names
and values, via the base's own precedence -- name before
value -- so a value-only match such as
Expand Down
2 changes: 1 addition & 1 deletion tests/const/test_const_enum_no_mint.py
Original file line number Diff line number Diff line change
Expand Up @@ -2108,7 +2108,7 @@ def test_registered_lookups_are_still_unaffected(self) -> None:
``Method.get`` is probed with its exact registered casing
(``'GET'``), not the lower-cased ``'get'`` this test used before
GitHub issue #896: ``Command``'s FTP command codes stay
case-insensitive per :rfc:`959#section-4.1`, but the HTTP method
case-insensitive per :rfc:`959#section-5`, but the HTTP method
token :rfc:`9110#section-9.1` covers is case-sensitive, so
``Method.get('get')`` no longer resolves to :attr:`Method.GET` --
see :class:`BespokeGetUnchangedTests`'s
Expand Down
Loading
Loading