Repository navigation
refactor(analysis): give IP and TCP reassembly their own foundation.analysis modules #2581
Description
From the owner on #2531: should IP and TCP reassembly also get pcapkit.foundation.analysis modules?
Today, reassembly-integration.rst says point 3 ("a new engine's reassembled outcome needs a new foundation.analysis method") holds for DTLS alone. analysis/dtls.py has dtls_handshake. IP and TCP outcomes are instead parsed by the general Protocol.analyze dispatcher, through reassembly.data.data.Deferred, with no analysis method of their own.
Decision needed:
- Add them:
analysis/ip.py, for IPv4/IPv6 datagrams, andanalysis/tcp.py, for TCP streams. These would be thin public functions over the same machinery:Protocol.analyzefor a datagram's upper layer, andTransport.analyzeby ports for a stream.Deferredcalls them, so the package mirrorsfoundation.reassemblyengine by engine and point 3 holds for every engine. - Leave the asymmetry and keep the page's note.
Recommendation: 1. Callers get a public function to parse a reassembled outcome, the package's shape matches its rule, and behaviour is unchanged (the deferred packet is built the same way).
Blocked: #2531 is changing the same package and page, so this unblocks when #2531's PR merges.
Activity
JarryShaw commentedon Nov 1, 2026
Take 1
JarryShaw commentedon Nov 1, 2026
Owner chose 1, paraphrased: IP and TCP reassembly get their own analysis modules.
foundation/analysis/ip.pyparses a reassembled IPv4 or IPv6 datagram's upper layer throughProtocol.analyze.foundation/analysis/tcp.pyparses a reassembled TCP stream by its ports throughTransport.analyze.Deferredcalls these functions, and they are exported asdtls_handshakeis.Blocked: #2531 is changing the same package and page, so this unblocks when #2531's PR merges.
JarryShaw commentedon Nov 2, 2026
#2531's PR (#2605) has merged, so this is unblocked. A worker is adding
foundation/analysis/ip.pyandtcp.pyon the DTLS model, with a differential check that IP and TCP reassembly output is unchanged.JarryShaw commentedon Nov 2, 2026
The implementation is done locally, and a Sonnet review is running. It opens as a PR once the Actions queue drops below 20.
Needs deciding: the public function names. The worker named them
ip_datagramandtcp_datagram, after the reassembly modules'Datagramclasses. The issue says "TCP stream".ip_datagram/tcp_datagram: each call parses one reassembled payload buffer, which is oneDatagram.ip_datagram/tcp_stream: matches the issue's wording.Recommendation: 1. A TCP
Datagramis one buffer per ACK number, not the whole stream, sotcp_streamwould promise more than the function does. The PR goes ahead with option 1 unless you choose otherwise.JarryShaw commentedon Nov 2, 2026
take 1
JarryShaw commentedon Nov 2, 2026
The owner chose 1, paraphrased: the functions stay
ip_datagramandtcp_datagram. The branch already uses those names, so nothing changes. The PR opens once the Actions queue drops below 20, with the Sonnet review's verdict attached.