Repository navigation
fix(reassembly): make Deferred generic over what its analyser dispatches on - #2418
Merged
Merged
Conversation
JarryShaw
added
fix
JarryShaw
added
review: running
Contributor
|
Coverage: 91.73% (unit tier, Python 3.15,
Per-file detail: the |
Owner
Author
|
Review: GOOD TO GO at |
JarryShaw
added
review: good-to-go
Owner
Author
|
Can we use the actual signature for analyze then? Avoid using Any in the Callable |
Owner
Author
|
Tried, and it doesn't work cleanly — here's why, and the real alternative.
The clean alternative is making |
JarryShaw
added
review: needs-changes
Owner
Author
|
Yea let's make it generic |
Owner
Author
|
Making |
JarryShaw
force-pushed
the
fix/2417-deferred-proto-type
branch
from
October 28, 2026 20:12
f405533 to
2f51747
Compare
JarryShaw
added
review: running
JarryShaw
changed the title
Owner
Author
|
Review: NEEDS CHANGES at Two things to fix:
|
JarryShaw
added
review: needs-changes
…hes on (#2417) `Deferred.__init__` typed `proto` as `TransType` and documented it as the payload protocol type. That is what `IP.submit` passes, but `TCP.submit` passes a `(source, destination)` port pair, because `Transport.analyze` overrides `Protocol.analyze(proto: int, ...)` to `analyze(ports: tuple[int, int], ...)`. mypy 2.3.1 flagged the call site: pcapkit/foundation/reassembly/tcp.py:754:64: error: Argument 2 to "Deferred" has incompatible type "tuple[Any, Any]"; expected "TransType" * `Deferred` is now `Generic[_KT]`, constrained to `TransType` and `tuple[int, int]`. `proto` and the first parameter of `analyze` both name that variable, so the two track together instead of being a union beside an `Any`. `_KT` rather than `_PT`, which already means "packet" in this subpackage. * How far that is enforced is asymmetric, and the docstrings now say so instead of claiming every mismatched pair is rejected. The pairing #2417 was about -- a port pair handed to an analyser declared over `TransType` -- is rejected, which is the direction a real mismatch takes. The reverse pairing is not: `aenum` ships no type stubs, so under `ignore_missing_imports` the `aenum.IntEnum` base of `TransType` resolves to `Any`, a `TransType` is accepted where a `tuple[int, int]` is expected, and a `Deferred[TransType]` built over the port-pair analyser type-checks clean. That is a gap in the dependency's type coverage rather than in the constraint, which would bite both ways were `TransType` fully typed. * Both call sites are unchanged: mypy infers `Deferred[TransType]` at `ip.py:546` and `Deferred[tuple[int, int]]` at `tcp.py:754` from the arguments, so neither needs an explicit subscript. * The two `Datagram.__init__` stubs that hold a placeholder name the parameterization each really receives, `Deferred[TransType]` for IP and `Deferred[tuple[int, int]]` for TCP. `__deferred__` stays unsubscripted, since `__analyse__` tests against it with `isinstance`. * The attributes are annotated on their assignment: a slotted class declares no class-level attributes, and mypy cannot infer one's type from a type variable. * The test reads the type variable off `__parameters__` and requires each reassembler's value to be admitted by exactly one constraint, and the two by different ones -- which a union could not satisfy. It reads the declaration at runtime rather than resting on mypy, for the reason above. No runtime change: behaviour, `__slots__` and the deferred resolution are as they were; this is a typing refinement, though the class does gain a type parameter. mypy over `pcapkit/` drops that one error and adds none against mainline (99 -> 98); `pylint` with the Makefile flags reports the same message set as mainline over `pcapkit/foundation/reassembly/`; the reassembly and traceflow suites pass (184 tests, 991 subtests), as do `corekit` (693) and the isort, annotation-name and tier guards.
JarryShaw
force-pushed
the
fix/2417-deferred-proto-type
branch
from
October 28, 2026 21:03
2f51747 to
719f35c
Compare
JarryShaw
added
review: running
Owner
Author
|
Review: GOOD TO GO at |
JarryShaw
added
review: good-to-go
Owner
Author
|
Good catch — |
JarryShaw
added
review: needs-changes
4 of 5 tasks
JarryShaw
added a commit
that referenced
this pull request
Oct 29, 2026
… bare ports Follow-up to #2418, which made `Deferred` generic over what its analyser dispatches on but left the TCP side of that constraint as `tuple[int, int]`. The owner's note on it: we are still using int tuples for transport ports, so move to AppType tuples. * `TCP.submit` resolves `bufid[1]` / `bufid[3]` through `Transport._make_port` before building the `Deferred`, in a new `TCP._make_ports` helper. A deferred analysis now dispatches on a named service rather than on a number. * `_KT`'s second constraint is `tuple[AppType_TCP, AppType_TCP]` -- the per-transport registry `pcapkit.const.reg.apptype.tcp.TCP`, which is what the resolution really answers with (`type(...) is TCP`, measured), not the base `AppType`. `Transport.analyze` keeps the base, being the one method TCP, UDP and SCTP share, and says so. * `Transport.analyze` tests the chosen port against `__proto__` exactly as it arrived and unwraps it to the registry's `int` key only when it is a member, so a bare `int` caller keeps working -- the method is in v1.4.1 -- and `(None, None)` or `(54321, '80')` still fall back to `Raw` as on mainline rather than raising or dispatching. * `_make_ports` reaches `_make_port` through `TCP` rather than through `self.protocol`: `__protocol_type__` is an extension point whose analyser need only define `analyze`, so `self.protocol._make_port` would be an `AttributeError` for one that is not a `Transport`. Going through `_make_port` rather than calling `AppType.get` directly is what keeps #2287's bool rejection: `AppType.get(True, proto=tcp)` resolves `True` as port 1 and answers `<TCP.tcpmux: 1 [tcp]>`, measured. * `DatagramID`'s ports stay bare `int`s, and its docstring now says why: it is the flow identity split out of `BufferID`, a dict key every toolkit adapter builds from a plain port number and which is dumped as part of the report. * Docs entries for `TCP._make_ports` and for `AppType.__hash__`, both now named in public prose (#1940). All 65,536 ports resolve to a member -- an assignment to its canonical service, an IANA span to that span's label, anything else to an unregistered `unknown` -- none raise and the registry does not grow, so this adds no way for reassembly to raise. For a registered port the member is the one `TCP.src` reads off a parsed header; for an unassigned one the two are equal but not identical and named apart, which the docstrings record rather than overstate. UDP and SCTP need no change: neither has a reassembler, so `Transport.analyze` has no caller but `TCP.submit`. mypy over `pcapkit/` is byte-identical to mainline (98 errors, same set); `pylint` with the Makefile flags reports the same two pre-existing `_decode_next_layer` messages as mainline over the touched files; `isort -l100 -ppcapkit` is clean. Tests: `foundation` + `protocols/transport` + `interface` with the annotation-name, tier and docstring guards, 997 tests / 4906 subtests; the full CI selection was green on the pre-review revision at 6552 tests. Reassembly over the sample captures resolves the same next layer as before, port for port.
JarryShaw
added a commit
that referenced
this pull request
Oct 29, 2026
… bare ports Follow-up to #2418, which made `Deferred` generic over what its analyser dispatches on but left the TCP side of that constraint as `tuple[int, int]`. The owner's note on it: we are still using int tuples for transport ports, so move to AppType tuples. * `TCP.submit` resolves `bufid[1]` / `bufid[3]` through `Transport._make_port` before building the `Deferred`, in a new `TCP._make_ports` helper. A deferred analysis now dispatches on a named service rather than on a number. * `_KT`'s second constraint is `tuple[AppType_TCP, AppType_TCP]` -- the per-transport registry `pcapkit.const.reg.apptype.tcp.TCP`, which is what the resolution really answers with (`type(...) is TCP`, measured), not the base `AppType`. `Transport.analyze` keeps the base, being the one method TCP, UDP and SCTP share, and says so. * `Transport.analyze` tests the chosen port against `__proto__` exactly as it arrived and unwraps it to the registry's `int` key only when it is a member, so a bare `int` caller keeps working -- the method is in v1.4.1 -- and `(None, None)` or `(54321, '80')` still fall back to `Raw` as on mainline rather than raising or dispatching. * `_make_ports` reaches `_make_port` through `TCP` rather than through `self.protocol`: `__protocol_type__` is an extension point whose analyser need only define `analyze`, so `self.protocol._make_port` would be an `AttributeError` for one that is not a `Transport`. Going through `_make_port` rather than calling `AppType.get` directly is what keeps #2287's bool rejection: `AppType.get(True, proto=tcp)` resolves `True` as port 1 and answers `<TCP.tcpmux: 1 [tcp]>`, measured. * `DatagramID`'s ports stay bare `int`s, and its docstring now says why: it is the flow identity split out of `BufferID`, a dict key every toolkit adapter builds from a plain port number and which is dumped as part of the report. * Docs entries for `TCP._make_ports` and for `AppType.__hash__`, both now named in public prose (#1940). All 65,536 ports resolve to a member -- an assignment to its canonical service, an IANA span to that span's label, anything else to an unregistered `unknown` -- none raise and the registry does not grow, so this adds no way for reassembly to raise. For a registered port the member is the one `TCP.src` reads off a parsed header; for an unassigned one the two are equal but not identical and named apart, which the docstrings record rather than overstate. UDP and SCTP need no change: neither has a reassembler, so `Transport.analyze` has no caller but `TCP.submit`. Make an unregistered member survive pickling -------------------------------------------- Dispatching on members rather than numbers put them inside the `Deferred` that `TCP.submit` stores as a `Datagram`'s `packet`, and that made an unresolved datagram unpicklable for most ports. `AppType.get` answers a port it holds no assignment for through `AppType._unregistered_member`, which builds a member deliberately absent from every one of the registry's lookup tables so the lookup does not grow it (#860). `Enum.__reduce_ex__` reduces a member to `(cls, (value,))` -- a value-keyed lookup, i.e. the one table such a member is missing from -- so it pickled fine and then raised on read-back: ValueError: 'unknown [50000 - tcp]' is not a valid TCP Measured before the fix: 59,413 of the 65,536 possible TCP ports failed to round-trip, 90.7% of the space, and not merely the truly unregistered `unknown` ones but every IANA-span label too. All four sample captures with TCP flows failed, every datagram, because a client socket is ephemeral. * `EnumRegistry._make_reducible` installs a per-instance `__reduce_ex__` that rebuilds through `cls._unregistered_member(*args)` instead of through a value lookup, with module-level `_rebuild_unregistered_member` / `_reduce_unregistered_member` so that pickle protocols below 4 can name the callable. This is the technique `EnumField._unregistered_member` already uses for the same class of problem on the *parse* path (#575, #1774). * `AppType._unregistered_member` opts in, reducing to its own `(value, name, proto)` rather than to the crafted `'svc [port - proto]'` string it hands its base -- reducing to the latter would rebuild a member whose value had been formatted twice. It is opt-in per registry for exactly that reason; a registry whose `_missing_` calls the base method directly is unchanged. * Rebuilding re-enters `_unregistered_member` and never `cls.__new__`, so an unpickled member is unregistered just as the original was and no table grows -- `__members__`, `_value2member_map_` and `__registry__` all measured unchanged before and after, on every transport. A restored member is itself reducible, so a datagram read back from a dump can be written out again. After: all 65,536 ports round-trip on every transport (262,144 members), on every pickle protocol 0-5, under `copy.copy`/`copy.deepcopy`, and all four named captures load back -- 231 deferred datagrams across `http.pcap`, `stream.pcap`, `tcp.pcap` and `http6.cap`. A registered member still unpickles to the identical shared singleton, which pins that the reducer went on the unregistered path only. * `tests/const/test_const_apptype_unregistered_pickle_2417_unit.py` pins the member-level round trip over the whole port space, every protocol, and the no-growth property. * `tests/foundation/reassembly/test_datagram_deferred_pickle_2417_unit.py` pins the symptom end to end, synthesised and over the four captures. It scopes to pickle protocols 2-5: `Deferred` declares `__slots__` and no `__getstate__`, which protocols 0 and 1 cannot serialise at all. That is pre-existing and independent of these members -- it reproduces for an assigned flow and for a bare `Deferred(..., 42, b'x')` holding a plain `int` -- so it is recorded there rather than worked around, and wants its own issue. * The #1774 canary asserted that `get`'s miss member was *not* picklable, which was a defect rather than a distinction. It now asserts the round trip works and that the member's `name` is the only remaining difference from what a parse returns -- which is what still justifies the peek. `AppType.__hash__` gained the docstring its new `#1940` entry requires. mypy over `pcapkit/` is byte-identical to mainline (98 errors in 32 files, same set); `pylint` with the Makefile flags reports the same message set and counts over the touched files as the pre-review revision; `isort -l100 -ppcapkit` is clean. Tests: 876 tests / 25,888 subtests over `foundation/reassembly`, `protocols/transport`, the `const` registries, the field-pickling suites, the `#1940` docs guard and the docstring contract, with 4 skips that predate this branch (the local venv has sphinx-autodoc-typehints 3.13.6, not the 3.17.2 `docs/requirements.txt` pins). Reassembly over the sample captures resolves the same next layer as before, port for port.
JarryShaw
added a commit
that referenced
this pull request
Oct 29, 2026
… bare ports Follow-up to #2418, which made `Deferred` generic over what its analyser dispatches on but left the TCP side of that constraint as `tuple[int, int]`. The owner's note on it: we are still using int tuples for transport ports, so move to AppType tuples. * `TCP.submit` resolves `bufid[1]` / `bufid[3]` through `Transport._make_port` before building the `Deferred`, in a new `TCP._make_ports` helper. A deferred analysis now dispatches on a named service rather than on a number. * `_KT`'s second constraint is `tuple[AppType_TCP, AppType_TCP]` -- the per-transport registry `pcapkit.const.reg.apptype.tcp.TCP`, which is what the resolution really answers with (`type(...) is TCP`, measured), not the base `AppType`. `Transport.analyze` keeps the base, being the one method TCP, UDP and SCTP share, and says so. * `Transport.analyze` tests the chosen port against `__proto__` exactly as it arrived and unwraps it to the registry's `int` key only when it is a member, so a bare `int` caller keeps working -- the method is in v1.4.1 -- and `(None, None)` or `(54321, '80')` still fall back to `Raw` as on mainline rather than raising or dispatching. * `_make_ports` reaches `_make_port` through `TCP` rather than through `self.protocol`: `__protocol_type__` is an extension point whose analyser need only define `analyze`, so `self.protocol._make_port` would be an `AttributeError` for one that is not a `Transport`. Going through `_make_port` rather than calling `AppType.get` directly is what keeps #2287's bool rejection: `AppType.get(True, proto=tcp)` resolves `True` as port 1 and answers `<TCP.tcpmux: 1 [tcp]>`, measured. * `DatagramID`'s ports stay bare `int`s, and its docstring now says why: it is the flow identity split out of `BufferID`, a dict key every toolkit adapter builds from a plain port number and which is dumped as part of the report. * Docs entries for `TCP._make_ports` and for `AppType.__hash__`, both now named in public prose (#1940). All 65,536 ports resolve to a member -- an assignment to its canonical service, an IANA span to that span's label, anything else to an unregistered `unknown` -- none raise and the registry does not grow, so this adds no way for reassembly to raise. For a registered port the member is the one `TCP.src` reads off a parsed header; for an unassigned one the two are equal but not identical and named apart, which the docstrings record rather than overstate. UDP and SCTP need no change: neither has a reassembler, so `Transport.analyze` has no caller but `TCP.submit`. Make an unregistered member survive pickling -------------------------------------------- Dispatching on members rather than numbers put them inside the `Deferred` that `TCP.submit` stores as a `Datagram`'s `packet`, and that made an unresolved datagram unpicklable for most ports. `AppType.get` answers a port it holds no assignment for through `AppType._unregistered_member`, which builds a member deliberately absent from every one of the registry's lookup tables so the lookup does not grow it (#860). `Enum.__reduce_ex__` reduces a member to `(cls, (value,))` -- a value-keyed lookup, i.e. the one table such a member is missing from -- so it pickled fine and then raised on read-back: ValueError: 'unknown [50000 - tcp]' is not a valid TCP Measured before the fix: 59,413 of the 65,536 possible TCP ports failed to round-trip, 90.7% of the space, and not merely the truly unregistered `unknown` ones but every IANA-span label too. All four sample captures with TCP flows failed, every datagram, because a client socket is ephemeral. * `EnumRegistry._make_reducible` installs a per-instance `__reduce_ex__` that rebuilds through `cls._unregistered_member(*args)` instead of through a value lookup, with module-level `_rebuild_unregistered_member` / `_reduce_unregistered_member` so that pickle protocols below 4 can name the callable. This is the technique `EnumField._unregistered_member` already uses for the same class of problem on the *parse* path (#575, #1774). * `AppType._unregistered_member` opts in, reducing to its own `(value, name, proto)` rather than to the crafted `'svc [port - proto]'` string it hands its base -- reducing to the latter would rebuild a member whose value had been formatted twice. It is opt-in per registry for exactly that reason; a registry whose `_missing_` calls the base method directly is unchanged. * Rebuilding re-enters `_unregistered_member` and never `cls.__new__`, so an unpickled member is unregistered just as the original was and no table grows -- `__members__`, `_value2member_map_` and `__registry__` all measured unchanged before and after, on every transport. A restored member is itself reducible, so a datagram read back from a dump can be written out again. After: all 65,536 ports round-trip on every transport (262,144 members), on every pickle protocol 0-5, under `copy.copy`/`copy.deepcopy`, and all four named captures load back -- 231 deferred datagrams across `http.pcap`, `stream.pcap`, `tcp.pcap` and `http6.cap`. A registered member still unpickles to the identical shared singleton, which pins that the reducer went on the unregistered path only. * `tests/const/test_const_apptype_unregistered_pickle_2417_unit.py` pins the member-level round trip over the whole port space, every protocol, and the no-growth property. * `tests/foundation/reassembly/test_datagram_deferred_pickle_2417_unit.py` pins the symptom on a synthesised datagram, unassigned and assigned alike. `test_datagram_deferred_pickle_2417_runtime.py` pins the same thing end to end over the four sample captures, split into its own fixture-dependent module because CI's unit-tests leg never generates them -- the unit file reading a generated capture directly is what broke that leg. Both scope to pickle protocols 2-5: `Deferred` declares `__slots__` and no `__getstate__`, which protocols 0 and 1 cannot serialise at all. That is pre-existing and independent of these members -- it reproduces for an assigned flow and for a bare `Deferred(..., 42, b'x')` holding a plain `int` -- so it is recorded there rather than worked around, and wants its own issue. * The #1774 canary asserted that `get`'s miss member was *not* picklable, which was a defect rather than a distinction. It now asserts the round trip works and that the member's `name` is the only remaining difference from what a parse returns -- which is what still justifies the peek. `AppType.__hash__` gained the docstring its new `#1940` entry requires. mypy over `pcapkit/` is byte-identical to mainline (98 errors in 32 files, same set); `pylint` with the Makefile flags reports the same message set and counts over the touched files as the pre-review revision; `isort -l100 -ppcapkit` is clean. Tests: 876 tests / 25,888 subtests over `foundation/reassembly`, `protocols/transport`, the `const` registries, the field-pickling suites, the `#1940` docs guard and the docstring contract, with 4 skips that predate this branch (the local venv has sphinx-autodoc-typehints 3.13.6, not the 3.17.2 `docs/requirements.txt` pins). Reassembly over the sample captures resolves the same next layer as before, port for port.
JarryShaw
added a commit
that referenced
this pull request
Oct 29, 2026
… bare ports (#2421) Follow-up to #2418, which made `Deferred` generic over what its analyser dispatches on but left the TCP side of that constraint as `tuple[int, int]`. The owner's note on it: we are still using int tuples for transport ports, so move to AppType tuples. * `TCP.submit` resolves `bufid[1]` / `bufid[3]` through `Transport._make_port` before building the `Deferred`, in a new `TCP._make_ports` helper. A deferred analysis now dispatches on a named service rather than on a number. * `_KT`'s second constraint is `tuple[AppType_TCP, AppType_TCP]` -- the per-transport registry `pcapkit.const.reg.apptype.tcp.TCP`, which is what the resolution really answers with (`type(...) is TCP`, measured), not the base `AppType`. `Transport.analyze` keeps the base, being the one method TCP, UDP and SCTP share, and says so. * `Transport.analyze` tests the chosen port against `__proto__` exactly as it arrived and unwraps it to the registry's `int` key only when it is a member, so a bare `int` caller keeps working -- the method is in v1.4.1 -- and `(None, None)` or `(54321, '80')` still fall back to `Raw` as on mainline rather than raising or dispatching. * `_make_ports` reaches `_make_port` through `TCP` rather than through `self.protocol`: `__protocol_type__` is an extension point whose analyser need only define `analyze`, so `self.protocol._make_port` would be an `AttributeError` for one that is not a `Transport`. Going through `_make_port` rather than calling `AppType.get` directly is what keeps #2287's bool rejection: `AppType.get(True, proto=tcp)` resolves `True` as port 1 and answers `<TCP.tcpmux: 1 [tcp]>`, measured. * `DatagramID`'s ports stay bare `int`s, and its docstring now says why: it is the flow identity split out of `BufferID`, a dict key every toolkit adapter builds from a plain port number and which is dumped as part of the report. * Docs entries for `TCP._make_ports` and for `AppType.__hash__`, both now named in public prose (#1940). All 65,536 ports resolve to a member -- an assignment to its canonical service, an IANA span to that span's label, anything else to an unregistered `unknown` -- none raise and the registry does not grow, so this adds no way for reassembly to raise. For a registered port the member is the one `TCP.src` reads off a parsed header; for an unassigned one the two are equal but not identical and named apart, which the docstrings record rather than overstate. UDP and SCTP need no change: neither has a reassembler, so `Transport.analyze` has no caller but `TCP.submit`. Make an unregistered member survive pickling -------------------------------------------- Dispatching on members rather than numbers put them inside the `Deferred` that `TCP.submit` stores as a `Datagram`'s `packet`, and that made an unresolved datagram unpicklable for most ports. `AppType.get` answers a port it holds no assignment for through `AppType._unregistered_member`, which builds a member deliberately absent from every one of the registry's lookup tables so the lookup does not grow it (#860). `Enum.__reduce_ex__` reduces a member to `(cls, (value,))` -- a value-keyed lookup, i.e. the one table such a member is missing from -- so it pickled fine and then raised on read-back: ValueError: 'unknown [50000 - tcp]' is not a valid TCP Measured before the fix: 59,413 of the 65,536 possible TCP ports failed to round-trip, 90.7% of the space, and not merely the truly unregistered `unknown` ones but every IANA-span label too. All four sample captures with TCP flows failed, every datagram, because a client socket is ephemeral. * `EnumRegistry._make_reducible` installs a per-instance `__reduce_ex__` that rebuilds through `cls._unregistered_member(*args)` instead of through a value lookup, with module-level `_rebuild_unregistered_member` / `_reduce_unregistered_member` so that pickle protocols below 4 can name the callable. This is the technique `EnumField._unregistered_member` already uses for the same class of problem on the *parse* path (#575, #1774). * `AppType._unregistered_member` opts in, reducing to its own `(value, name, proto)` rather than to the crafted `'svc [port - proto]'` string it hands its base -- reducing to the latter would rebuild a member whose value had been formatted twice. It is opt-in per registry for exactly that reason; a registry whose `_missing_` calls the base method directly is unchanged. * Rebuilding re-enters `_unregistered_member` and never `cls.__new__`, so an unpickled member is unregistered just as the original was and no table grows -- `__members__`, `_value2member_map_` and `__registry__` all measured unchanged before and after, on every transport. A restored member is itself reducible, so a datagram read back from a dump can be written out again. After: all 65,536 ports round-trip on every transport (262,144 members), on every pickle protocol 0-5, under `copy.copy`/`copy.deepcopy`, and all four named captures load back -- 231 deferred datagrams across `http.pcap`, `stream.pcap`, `tcp.pcap` and `http6.cap`. A registered member still unpickles to the identical shared singleton, which pins that the reducer went on the unregistered path only. * `tests/const/test_const_apptype_unregistered_pickle_2417_unit.py` pins the member-level round trip over the whole port space, every protocol, and the no-growth property. * `tests/foundation/reassembly/test_datagram_deferred_pickle_2417_unit.py` pins the symptom on a synthesised datagram, unassigned and assigned alike. `test_datagram_deferred_pickle_2417_runtime.py` pins the same thing end to end over the four sample captures, split into its own fixture-dependent module because CI's unit-tests leg never generates them -- the unit file reading a generated capture directly is what broke that leg. Both scope to pickle protocols 2-5: `Deferred` declares `__slots__` and no `__getstate__`, which protocols 0 and 1 cannot serialise at all. That is pre-existing and independent of these members -- it reproduces for an assigned flow and for a bare `Deferred(..., 42, b'x')` holding a plain `int` -- so it is recorded there rather than worked around, and wants its own issue. * The #1774 canary asserted that `get`'s miss member was *not* picklable, which was a defect rather than a distinction. It now asserts the round trip works and that the member's `name` is the only remaining difference from what a parse returns -- which is what still justifies the peek. `AppType.__hash__` gained the docstring its new `#1940` entry requires. mypy over `pcapkit/` is byte-identical to mainline (98 errors in 32 files, same set); `pylint` with the Makefile flags reports the same message set and counts over the touched files as the pre-review revision; `isort -l100 -ppcapkit` is clean. Tests: 876 tests / 25,888 subtests over `foundation/reassembly`, `protocols/transport`, the `const` registries, the field-pickling suites, the `#1940` docs guard and the docstring contract, with 4 skips that predate this branch (the local venv has sphinx-autodoc-typehints 3.13.6, not the 3.17.2 `docs/requirements.txt` pins). Reassembly over the sample captures resolves the same next layer as before, port for port.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
make testpasses, and a test case covers the changeWhat is the purpose of your pull request?
fix— corrects a defectDescription
Fixes #2417.
Deferred.__init__(pcapkit/foundation/reassembly/data/data.py) typedproto: 'TransType', butTCP.submitpasses a port-pair tuple, not aTransType—Transport.analyzeoverridesProtocol.analyze(proto: int, ...)to takeports: tuple[int, int].Deferredis now generic over what its analyser dispatches on:_KT = TypeVar('_KT', 'TransType', 'tuple[int, int]'),class Deferred(Generic[_KT]), withanalyze: Callable[[_KT, bytes], ProtocolBase]andproto: _KTtracking together. Both real call sites (IP.submit,TCP.submit) are unchanged — mypy infersDeferred[TransType]andDeferred[tuple[int, int]]respectively without explicit subscripting.The generic catches the specific mismatch #2417 was about (a port-pair proto paired with a
TransType-typed analyser). The reverse direction isn't fully caught by mypy, becauseaenum(whichTransTypeis built on) ships no type stubs and the repo's mypy config ignores missing imports —TransType's base resolves toAny, so atuple[int, int]-typed analyser accepts aTransTypeproto without complaint. That's a gap in the dependency's type coverage, not a flaw in the constraint; the values the reassemblers actually pass are pinned by the tests either way.__deferred__ = Deferredstays unsubscripted (used withisinstance, which raisesTypeErroragainst a parameterized generic, andDeferredPacket.__init_subclass__rejects a subscripted alias). Explicit subscripting at the call sites was also rejected — mypy already infers correctly, and it costs ~15x a bare construction (measured).Test:
tests/foundation/reassembly/test_deferred_proto_type_2417_unit.py— checks each reassembler's value is admitted by exactly one constraint, the two differ, andanalyze's Callable first-arg uses the same TypeVar asproto(viaast). mypy overpcapkit/: 99 → 98 errors, exactly the one targeted error removed, zero new. pylint: message set identical tomain.tests/foundation/reassembly/+tests/foundation/traceflow/: 184 passed, 991 subtests.tests/corekit/: 693 passed, 6767 subtests. isort/annotation-name/tier-guard tests: 16 passed, 266 subtests.tests/project/+tests/toolkit/: 2323 passed, 97 skipped, 0 failed.