From b2aab951a73a03f1bf58add96dd5d08f21d4bf08 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 6 Oct 2026 12:56:21 -0400 Subject: [PATCH] docs(foundation): second-round pass over the foundation prose (#719) Correct docstrings and comments the code contradicts: the reassembly strict flag, the Extractor make_name/record_header/_cleanup contracts, the flow-tracing callback, make_fout and per-ack datagram counts, the opt-in registration comments, and several Warns/Raises lists. Cut the remaining timed context and fix a dangling reST target. Docstrings and comments only; AST identical to main once docstrings are stripped. --- pcapkit/foundation/engines/__init__.py | 2 +- pcapkit/foundation/engines/engine.py | 11 +- pcapkit/foundation/engines/pcap.py | 4 +- pcapkit/foundation/engines/pcap_ct.py | 2 +- pcapkit/foundation/engines/pyshark.py | 2 +- pcapkit/foundation/extraction.py | 142 +++++++++++--------- pcapkit/foundation/reassembly/data/data.py | 23 ++-- pcapkit/foundation/reassembly/data/ip.py | 6 +- pcapkit/foundation/reassembly/data/tcp.py | 4 +- pcapkit/foundation/reassembly/ip.py | 14 +- pcapkit/foundation/reassembly/ipv4.py | 6 +- pcapkit/foundation/reassembly/ipv6.py | 6 +- pcapkit/foundation/reassembly/reassembly.py | 39 +++--- pcapkit/foundation/reassembly/tcp.py | 9 +- pcapkit/foundation/registry/protocols.py | 29 ++-- pcapkit/foundation/traceflow/__init__.py | 2 +- pcapkit/foundation/traceflow/data/data.py | 10 +- pcapkit/foundation/traceflow/data/tcp.py | 10 +- pcapkit/foundation/traceflow/tcp.py | 20 ++- pcapkit/foundation/traceflow/traceflow.py | 47 ++++--- 20 files changed, 205 insertions(+), 183 deletions(-) diff --git a/pcapkit/foundation/engines/__init__.py b/pcapkit/foundation/engines/__init__.py index 32ad7dc0aa..532603fa73 100644 --- a/pcapkit/foundation/engines/__init__.py +++ b/pcapkit/foundation/engines/__init__.py @@ -12,7 +12,7 @@ .. _pcap-ct: https://pypi.org/project/pcap-ct/ -.. _PCAPNG: https://wiki.wireshark.org/Development/PcapNg +.. _PCAP-NG: https://wiki.wireshark.org/Development/PcapNg """ # Base class diff --git a/pcapkit/foundation/engines/engine.py b/pcapkit/foundation/engines/engine.py index 8aa87eda3d..ea23b28ef8 100644 --- a/pcapkit/foundation/engines/engine.py +++ b/pcapkit/foundation/engines/engine.py @@ -262,7 +262,7 @@ def __init_subclass__(cls, /, engine: 'Optional[str]' = None, *args: 'Any', **kw rather than having to inherit :class:`EngineBase` to avoid it, and it matches :meth:`EnumSchema.__init_subclass__ `, which - has guarded on its own ``code`` keyword all along. + guards on its own ``code`` keyword the same way. Note: :attr:`__engine_name__ @@ -295,11 +295,10 @@ def __init_subclass__(cls, /, engine: 'Optional[str]' = None, *args: 'Any', **kw """ # NOTE: an unrecognised class keyword lands in ``**kwargs`` and is then # dropped by the bare ``super().__init_subclass__()`` below, since - # ``object.__init_subclass__`` takes none. Silently swallowing it is how - # ``class MyEngine(Engine, engnie='x')`` would register under its class - # name instead -- no exception, no warning. Since a missing keyword means - # "do not register", the same typo would silently skip registration - # altogether, which is quieter still. So reject it. + # ``object.__init_subclass__`` takes none. Since a missing keyword means + # "do not register", silently swallowing it would make + # ``class MyEngine(Engine, engnie='x')`` skip registration altogether -- + # no exception, no warning. So reject it. # # One typo this cannot catch is ``name=``, and only on Python 3.10: it is # one of the four names that collide with ``ABCMeta.__new__``, so it fails diff --git a/pcapkit/foundation/engines/pcap.py b/pcapkit/foundation/engines/pcap.py index 3862f8d909..d990c02da7 100644 --- a/pcapkit/foundation/engines/pcap.py +++ b/pcapkit/foundation/engines/pcap.py @@ -100,8 +100,8 @@ def run(self) -> 'None': The method will parse the PCAP global header and save the parsed result as :attr:`self.header
`. Information such as PCAP version, data - link layer protocol type, nanosecond flag and byteorder are also saved - on the current :class:`PCAP` engine instance. + link layer protocol type and nanosecond flag are also saved on the + current :class:`PCAP` engine instance. For output, the method will dump the parsed PCAP global header under the name of ``Global Header``. diff --git a/pcapkit/foundation/engines/pcap_ct.py b/pcapkit/foundation/engines/pcap_ct.py index 1b4493c944..e153a6192b 100644 --- a/pcapkit/foundation/engines/pcap_ct.py +++ b/pcapkit/foundation/engines/pcap_ct.py @@ -326,7 +326,7 @@ def run(self) -> 'None': Raises: FormatError: If the file format is not supported, i.e., not a PCAP - file. PCAP-NG is rejected explicitly even though the vendored + file. PCAP-NG is rejected explicitly even though :manpage:`libpcap(3)` can read one -- see the note below. UnsupportedCall: If the input is not a file on disk, as :c:func:`pcap_open_offline` can only open a savefile by name. diff --git a/pcapkit/foundation/engines/pyshark.py b/pcapkit/foundation/engines/pyshark.py index f0629e472e..d48e638288 100644 --- a/pcapkit/foundation/engines/pyshark.py +++ b/pcapkit/foundation/engines/pyshark.py @@ -137,7 +137,7 @@ def unsupported_reason(cls) -> 'Optional[str]': # reports that case in its own words, and answering here as well would # produce two warnings for one problem. An ImportError from a *renamed* # upstream helper lands here too, and the engine then simply proceeds - # as it did before this check existed. + # without the ``tshark`` check. return None try: diff --git a/pcapkit/foundation/extraction.py b/pcapkit/foundation/extraction.py index ce5cfce1f7..7de4f0ba6f 100644 --- a/pcapkit/foundation/extraction.py +++ b/pcapkit/foundation/extraction.py @@ -68,8 +68,8 @@ #: aliases are spellable for a type checker as they are accepted at runtime. Formats = Literal['pcap', 'cap', 'json', 'tree', 'text', 'txt', 'plist', 'xml'] # NOTE: this alias is duplicated verbatim in ``pcapkit.interface.misc``; both - # copies need updating when a new engine lands. The duplication predates the - # engines added here and is left as-is on purpose. + # copies need updating when a new engine lands. The duplication is left + # as-is on purpose. Engines = Literal['default', 'pcapkit', 'dpkt', 'scapy', 'pyshark', 'pypcap', 'pcap_ct', 'pypcapfile'] Layers = Literal['link', 'internet', 'transport', 'application', 'none'] @@ -191,7 +191,7 @@ class Extractor(Generic[_P]): #: :data:`None` before the first one. Comparing it against the position #: at the next end of stream is what tells a live capture that has paused #: -- retry, more may arrive -- from one that is finished, which is the - #: termination condition ``no_eof`` was missing (:issue:`620`). + #: termination condition for ``no_eof`` (:issue:`620`). _eof_mark: 'Optional[int]' #: Magic number. @@ -381,16 +381,16 @@ def register_dumper(cls, format: 'str', dumper: 'ModuleDescriptor[Dumper] | Type The overwrite guard fires only when the incumbent dumper differs from the replacement, so re-registering the exact same object is a silent no-op rather than a warning about nothing displaced -- - the identity guard GitHub issue :issue:`718` gave the code-keyed - registrars, extended here by GitHub issue :issue:`739`. ``__output__`` + the identity guard of the code-keyed registrars (:issue:`718`, + :issue:`739`). ``__output__`` maps each format to a ``(dumper, ext)`` pair, so the identity check compares the incumbent *dumper* (index ``0``), not the pair -- a re-registration that only changes ``ext`` is still identity-equal on the dumper and stays silent, since the dumper is what "the same object" means here, not the pair as a whole. ``__output__`` is also a :class:`collections.defaultdict`, - unlike the other three - registrars this issue touches; :meth:`dict.get` does not invoke + unlike the engine, reassembly and flow tracing registries; + :meth:`dict.get` does not invoke the default factory the way ``cls.__output__[format]`` would, so it stays non-inserting here as well. @@ -430,8 +430,8 @@ def register_engine(cls, name: 'str', engine: 'ModuleDescriptor[Engine] | Type[E The overwrite guard fires only when the incumbent differs from the replacement, so re-registering the exact same object is a silent no-op rather than a warning about nothing displaced -- - the identity guard GitHub issue :issue:`718` gave the code-keyed - registrars, extended here by GitHub issue :issue:`739`. + the identity guard of the code-keyed registrars (:issue:`718`, + :issue:`739`). Arguments: name: engine name @@ -457,7 +457,10 @@ def register_engine(cls, name: 'str', engine: 'ModuleDescriptor[Engine] | Type[E # meant to extend and the one carrying the ``engine=`` registration hook. The # built-ins derive from ``EngineBase`` directly, so this door refuses them; # they go through :meth:`_register_internal_engine` instead, which checks the - # base. Checking the base here is what #513 did, and #1016 narrowed it back. + # base. The wider ``EngineBase`` is deliberately not accepted here: it is + # the internal base of the built-ins, and a class deriving only from it + # lacks the ``engine=`` hook, so the public door admits the public base + # alone (#513, #1016). if not issubclass(engine, Engine): raise RegistryError(f'engine must be an Engine subclass, not {engine!r}') cls._register_internal_engine(name, engine) @@ -503,8 +506,8 @@ def register_reassembly(cls, protocol: 'str', reassembly: 'ModuleDescriptor[Reas The overwrite guard fires only when the incumbent differs from the replacement, so re-registering the exact same object is a silent no-op rather than a warning about nothing displaced -- - the identity guard GitHub issue :issue:`718` gave the code-keyed - registrars, extended here by GitHub issue :issue:`739`. + the identity guard of the code-keyed registrars (:issue:`718`, + :issue:`739`). Arguments: protocol: protocol name @@ -574,8 +577,8 @@ def register_traceflow(cls, protocol: 'str', traceflow: 'ModuleDescriptor[TraceF The overwrite guard fires only when the incumbent differs from the replacement, so re-registering the exact same object is a silent no-op rather than a warning about nothing displaced -- - the identity guard GitHub issue :issue:`718` gave the code-keyed - registrars, extended here by GitHub issue :issue:`739`. + the identity guard of the code-keyed registrars (:issue:`718`, + :issue:`739`). Arguments: protocol: protocol name @@ -637,7 +640,7 @@ def _register_internal_traceflow(cls, protocol: 'str', traceflow: 'ModuleDescrip def run(self) -> 'None': # pylint: disable=inconsistent-return-statements """Start extraction. - We uses :meth:`~pcapkit.foundation.extraction.Extractor.import_test` to check if + We use :meth:`~pcapkit.foundation.extraction.Extractor.import_test` to check if a certain engine is available or not. For supported engines, each engine has different driver method: @@ -655,8 +658,13 @@ def run(self) -> 'None': # pylint: disable=inconsistent-return-statements Warns: pcapkit.utilities.warnings.EngineWarning: If the extraction engine is not - available. This is either due to dependency not installed, or supplied - engine unknown. + available: its dependency is not installed, its + :meth:`~pcapkit.foundation.engines.engine.EngineBase.unsupported_reason` + rules it out, or the supplied engine is unknown. + + Raises: + FormatError: If the selected engine cannot read the input's format -- + for the default engine, if it is neither PCAP nor PCAP-NG. :rtype: None """ @@ -716,7 +724,7 @@ def run(self) -> 'None': # pylint: disable=inconsistent-return-statements @staticmethod def import_test(engine: 'str', *, name: 'Optional[str]' = None) -> 'Optional[ModuleType]': - """Test import for extractcion engine. + """Test import for extraction engine. Args: engine: Extraction engine module name. @@ -749,15 +757,13 @@ def make_name(cls, fin: 'str | IO[bytes]' = 'in.pcap', fout: 'str' = 'out', 1. sanitise ``fin`` as the input PCAP filename; ``in.pcap`` as default value and append ``.pcap`` extension if needed and ``extension`` is :data:`True`; as well - as test if the file exists; + as test if the file exists; for a binary IO object, use its ``name``; 2. if ``nofile`` is :data:`True`, skips following processing; - 3. if ``fmt`` provided, then it presumes corresponding output file extension; - 4. if ``fout`` not provided, it presumes the output file name based on the presumptive - file extension; the stem of the output file name is set as ``out``; should the file - extension is not available, then it raises :exc:`~pcapkit.utilities.exceptions.FormatError`; - 5. if ``fout`` provided, it presumes corresponding output format if needed; should the - presumption cannot be made, then it raises :exc:`~pcapkit.utilities.exceptions.FormatError`; - 6. it will also append corresponding file extension to the output file name if needed + 3. look up the output file extension registered for ``fmt``; should none be + registered, then it raises :exc:`~pcapkit.utilities.exceptions.FormatError`; + 4. create the parent directory of ``fout``, and with ``files`` the ``fout`` + directory itself; + 5. it will also append corresponding file extension to the output file name if needed and ``extension`` is :data:`True`. And the method returns the generated input and output filenames as follows: @@ -785,7 +791,7 @@ def make_name(cls, fin: 'str | IO[bytes]' = 'in.pcap', fout: 'str' = 'out', Raises: FileNotFound: If input file does not exists. - FormatError: If output format not provided and cannot be presumpted. + FormatError: If no file extension is registered for ``fmt``. """ if isinstance(fin, str): @@ -834,16 +840,18 @@ def make_name(cls, fin: 'str | IO[bytes]' = 'in.pcap', fout: 'str' = 'out', def record_header(self) -> 'EngineBase[_P]': """Read global header. - The method will parse the PCAP global header and save the parsed result - to its extraction context. Information such as PCAP version, data link - layer protocol type, nanosecond flag and byteorder are also saved on the - returned engine instance. + The method parses the global header with the matching built-in engine -- + :class:`~pcapkit.foundation.engines.pcap.PCAP` for a PCAP file, or + :class:`~pcapkit.foundation.engines.pcapng.PCAPNG` for the first section + header block of a PCAP-NG file -- rewinds the input to its start, and + returns that engine, which holds the parsed header. - If TCP flow tracing is enabled, the nanosecond flag and byteorder will - be used for the output PCAP file of the traced TCP flows. + Unless output is disabled, the engine also dumps the parsed header, under + the name of ``Global Header`` for PCAP and ``Section Header 1`` for + PCAP-NG. - For output, the method will dump the parsed PCAP global header under - the name of ``Global Header``. + Raises: + FormatError: If the input is neither a PCAP nor a PCAP-NG file. """ # pylint: disable=attribute-defined-outside-init,protected-access @@ -956,8 +964,8 @@ def __init__(self, trace_byteorder: output file byte order trace_nanosecond: output nanosecond-resolution file flag trace_bidirectional: whether both halves of a conversation are - traced as one flow, which is the default; :data:`False` restores - the older behaviour of a flow per direction + traced as one flow, which is the default; :data:`False` traces + a flow per direction trace_analyse: whether each traced flow reassembles its application layer, so that its ``packet`` can be read. Off by default, because it buffers every traced payload -- a cost tracing does @@ -995,9 +1003,13 @@ def __init__(self, Warns: pcapkit.utilities.warnings.FormatWarning: Warns under following circumstances: - * If using PCAP output for TCP flow tracing while the extraction engine is PyShark. + * If using PCAP output for TCP flow tracing while the extraction engine is + DPKT, Scapy, PyShark or PyPCAPFile. * If output file format is not supported. + pcapkit.utilities.warnings.AttributeWarning: If ``trace_analyse`` is + requested while the extraction engine is PyShark. + """ if fin is None: fin = 'in.pcap' @@ -1105,16 +1117,12 @@ def __init__(self, # ` # itself substitutes ``'pcap'`` for ``None``. # - # DPKT and Scapy belong on this list and were once left off it. Their - # adapters build the frame with ``packet2dict`` exactly as the other two - # do, so both crash the same way -- but only DPKT did so visibly. The - # Scapy engine imports just :mod:`scapy.sendrecv`, which leaves the L2 - # link types unregistered, so every frame dissects as ``Raw``, no TCP - # layer is ever found, and the tracer is never fed at all (#406). That - # hides this defect rather than avoiding it: register the link types -- - # as importing :mod:`scapy.all` does -- and the same ``AttributeError`` - # appears. So the guard is written from what the adapters produce, not - # from which engines happen to crash. + # DPKT and Scapy belong on this list too: their adapters build the frame + # with ``packet2dict`` exactly as the other two do, so both crash the + # same way. The guard is written from what the adapters produce, not + # from which engines are seen to crash, because an engine that + # dissects nothing never feeds the tracer and so hides the defect + # rather than avoiding it (#406). if (self._exnam in ('dpkt', 'scapy', 'pyshark', 'pypcapfile') and trace_format in ('pcap', 'cap', None)): warn(f"'Extractor(engine={self._exnam})' does not support 'trace_format={trace_format}'; " @@ -1161,8 +1169,7 @@ def __init__(self, # closes the stream underneath it, so it follows *ownership*: only a # handle this class opened itself is ours to release. A non-seekable # stream always came from the caller, so the polarity here is the - # same one ``_cleanup`` uses, and it was inverted in both places -- - # see #610. + # same one ``_cleanup`` uses (#610). self._ifile = SeekableReader(self._ifile, buffer_size, buffer_save, buffer_path, stream_closing=self._flag_s) @@ -1276,9 +1283,9 @@ def __del__(self) -> 'None': with ``auto=False`` the caller drives :meth:`__next__` itself, and one that stops before end of file never reaches :meth:`_cleanup` at all, so the ownership rule there never gets to run. The handle then survives until the - interpreter collects it, and CPython announces that with the - ``ResourceWarning`` :issue:`606` was tripping over -- from an unrelated test, in an - unrelated file, which is what made that flake so hard to place. + interpreter collects it, and CPython announces that with a + ``ResourceWarning`` raised wherever collection happens to run, which can be + far from the code that abandoned the handle (:issue:`606`). This is a backstop and not the recommended route: collection is not deterministic, and :class:`Extractor` takes part in a reference cycle @@ -1355,13 +1362,13 @@ def _owns_input(self) -> 'bool': def _note_eof_progress(self) -> 'bool': """Record this end of stream, and say whether the input advanced to reach it. - This is the termination condition ``no_eof`` was missing. The flag means + This is the termination condition for ``no_eof``. The flag means "end of stream is not necessarily the end of the capture" -- which is true of a live capture, and is why :mod:`pcapkit.__main__` sets it for - ``fin='-'`` -- so the extraction retries rather than stopping. What it had - no way to decide was when the stream is *genuinely* finished, and for an - exhausted one every retry raises end of stream again immediately, which is - the spin :issue:`620` reported. + ``fin='-'`` -- so the extraction retries rather than stopping. Without a + way to decide when the stream is *genuinely* finished, an exhausted one + raises end of stream again on every retry, which is the spin + :issue:`620` reported. The signal is the input's own position. End of stream is raised by :func:`~pcapkit.utilities.decorators.prepare` when the bytes remaining in @@ -1384,12 +1391,12 @@ def _note_eof_progress(self) -> 'bool': immediately consecutive probes, because such a file does not block -- it reports end of stream at once. So an extraction over a file that is still being appended to ends at the data present when it got there, rather than - following the writer. That is a deliberate narrowing of what ``no_eof`` - used to do, and it is measured: on ``6c3d1b0d9`` a file gaining its last - record 0.6s in yielded all six frames, and here it yields five. The - previous behaviour was unbounded by construction -- it is the defect :issue:`620` - reports -- so *some* stopping rule had to be chosen, and a timed grace - period would only make the cut-off intermittent rather than absent. + following the writer. That is a deliberate narrowing, and it is measured: + a file gaining its last record 0.6s in yields five of its six frames here, + where the unbounded retry of ``6c3d1b0d9`` yielded all six. Unbounded + retrying is the defect :issue:`620` reports, so *some* stopping rule had + to be chosen, and a timed grace period would only make the cut-off + intermittent rather than absent. Following a growing file wants a deliberate policy of its own; see the note in :file:`docs/source/changelog/1.5.0.rst`. @@ -1432,9 +1439,10 @@ def _cleanup(self) -> 'None': :meth:`TraceFlow.finish `. That is the point at which a traced flow nothing has superseded can be said to be over, so it is where such a flow is finalised and its callbacks - run. This method can be reached twice for one extraction -- the EOF path in - :meth:`_read_frame` and again from :meth:`run` -- so ``finish`` is required - to be idempotent rather than guarded here. + run. This method can run more than once for one extraction -- each call to + :meth:`__next__` or :meth:`__call__` that reaches the end of a + caller-supplied stream runs it again -- so ``finish`` is required to be + idempotent rather than guarded here. """ # pylint: disable=attribute-defined-outside-init diff --git a/pcapkit/foundation/reassembly/data/data.py b/pcapkit/foundation/reassembly/data/data.py index 967e0ecc9a..fad0ea8813 100644 --- a/pcapkit/foundation/reassembly/data/data.py +++ b/pcapkit/foundation/reassembly/data/data.py @@ -29,7 +29,7 @@ class Completion(EnumLookup, StrEnum): :attr:`Datagram.completed `, which widens a plain :obj:`bool` rather than adding a second channel beside it: reassembly has *three* outcomes to report, not two, since a buffer - abandoned under the :rfc:`791` / :rfc:`8200` reassembly timeout is a different + abandoned under the :rfc:`1122` / :rfc:`8200` reassembly timeout is a different event from one that simply had not finished when the capture did. An expired datagram says "these fragments are gone", a partial one says "these fragments had not arrived yet". @@ -49,8 +49,8 @@ class Completion(EnumLookup, StrEnum): field straight out -- and ``datagram.completed == 'timeout'`` works, so a state can be tested for without importing this class. :class:`pcapkit.const.pcapng.tls_key_label.TLSKeyLabel` is the same kind of - string enumeration, but GitHub issue :issue:`886` made it generated like its - :mod:`pcapkit.const.pcapng` siblings, so it derives from :class:`aenum`'s own + string enumeration, but it is generated like its :mod:`pcapkit.const.pcapng` + siblings (:issue:`886`), so it derives from :class:`aenum`'s own ``StrEnum`` (via :class:`~pcapkit.corekit.enum.EnumRegistry`). Both properties above hold for it, as ``aenum.StrEnum`` is a :class:`str` subclass, but ``isinstance``/``issubclass`` against @@ -60,7 +60,7 @@ class Completion(EnumLookup, StrEnum): Being a :class:`str` whose :attr:`PARTIAL` and :attr:`TIMEOUT` members are **falsy** makes this a non-empty string that tests false, so ``bool(x)`` and ``bool(str(x))`` disagree. That is deliberate -- the truthiness above - is the property callers of a former :obj:`bool` field rely on -- but code + is what lets ``if datagram.completed:`` read as a boolean test -- but code that takes this for an ordinary string and tests it for truth will read it backwards. @@ -113,10 +113,10 @@ class Deferred: *every* frame -- not only the fragmented ones, since a frame that is not fragmented in any sense still reaches :meth:`IP.reassembly ` and is - submitted there as a trivially complete datagram -- so the eager parse - re-parsed captures holding no fragments at all: :file:`http.pcap` has 1117 - IPv4 frames, none of them fragmented, and the parse was 86% of the cost of IP - reassembly over it. + submitted there as a trivially complete datagram -- so an eager parse would + re-parse captures holding no fragments at all: :file:`http.pcap` has 1117 + IPv4 frames, none of them fragmented, and the eager parse measured 86% of the + cost of IP reassembly over it. TCP reassembly defers its ``packet`` as well (:meth:`TCP.submit `), at a far @@ -125,8 +125,8 @@ class Deferred: Holding the call here defers it to the first read of :attr:`Datagram.packet`, so a caller that wants the parsed payload still gets - exactly the object the eager call produced, and one that does not never pays - for it. + exactly the object an eager call would produce, and one that does not never + pays for it. Args: analyze: The analyser to call, i.e. @@ -184,7 +184,8 @@ def __analyse__(self) -> 'Optional[ProtocolBase]': """Resolve a deferred analysis, at most once. Returns: - Parsed IP payload, or :data:`None` for an incomplete datagram. + Parsed payload, or :data:`None` for a datagram reported as the tuple + of its received runs. """ key = self.__map__.get('packet', 'packet') diff --git a/pcapkit/foundation/reassembly/data/ip.py b/pcapkit/foundation/reassembly/data/ip.py index 98bdcd0718..c61d9d764b 100644 --- a/pcapkit/foundation/reassembly/data/ip.py +++ b/pcapkit/foundation/reassembly/data/ip.py @@ -29,7 +29,7 @@ @info_final class Packet(Info, Generic[_AT]): """Data model for :term:`IPv4 ` and/or - :term:`IPv6 ` packet representation..""" + :term:`IPv6 ` packet representation.""" #: Buffer ID. bufid: 'BufferID' @@ -48,7 +48,7 @@ class Packet(Info, Generic[_AT]): #: Raw :obj:`bytearray` type payload. payload: 'bytearray' #: Capture timestamp of the fragment, in seconds since the Unix epoch. This - #: is the *capture's* clock, not the host's: it is what drives the :rfc:`791` + #: is the *capture's* clock, not the host's: it is what drives the :rfc:`1122#section-3.3.2` #: and :rfc:`8200#section-4.5` reassembly timeout, since an offline parser #: replaying a file has no other notion of time passing. timestamp: 'float' @@ -157,7 +157,7 @@ class Buffer(Info, Generic[_AT]): TDL: 'int' #: Fragment received bit table. RCVBT: 'bytearray' - #: List of reassembled packets. + #: Numbers of the packets merged into this buffer. index: 'list[int]' #: Header buffer. header: 'bytes' diff --git a/pcapkit/foundation/reassembly/data/tcp.py b/pcapkit/foundation/reassembly/data/tcp.py index c3761f3d02..ee6f17ceb5 100644 --- a/pcapkit/foundation/reassembly/data/tcp.py +++ b/pcapkit/foundation/reassembly/data/tcp.py @@ -89,7 +89,7 @@ class Datagram(DeferredPacket, Info, Generic[_AT]): #: How completely the datagram was reassembled, and why reassembly stopped; #: see :class:`~pcapkit.foundation.reassembly.data.data.Completion`. Only #: :attr:`Completion.COMPLETE` is truthy, so ``if datagram.completed:`` reads - #: as it did while this was a :obj:`bool`. + #: as it would for a :obj:`bool`. completed: 'Completion' #: Listing ``packet`` here is what makes it lazy -- see #: :class:`~pcapkit.foundation.reassembly.data.data.DeferredPacket`. @@ -157,7 +157,7 @@ def __init__(self, first: 'int', last: 'int') -> 'None': ... # pylint: disable= class Fragment(Info): """Data model for :term:`TCP ` ACK list fragment item.""" - #: List of reassembled packets. + #: Numbers of the packets merged into this fragment. ind: 'list[int]' #: Sequence number of the octet held in ``raw[0]``, i.e. the origin this #: buffer is indexed from: ``raw[n]`` holds the octet whose sequence number diff --git a/pcapkit/foundation/reassembly/ip.py b/pcapkit/foundation/reassembly/ip.py index 472dd50831..2276487f63 100644 --- a/pcapkit/foundation/reassembly/ip.py +++ b/pcapkit/foundation/reassembly/ip.py @@ -9,7 +9,7 @@ only, which reconstructs fragmented IP packets back to origin. The following algorithm implement is based on IP reassembly procedure introduced in :rfc:`791`, using -``RCVBT`` (fragment receivedbit table). Though another +``RCVBT`` (fragment received bit table). Though another algorithm is explained in :rfc:`815`, replacing ``RCVBT``, however, this implement still used the elder one. @@ -34,8 +34,10 @@ class IP(ReassemblyBase[Packet[_AT], Datagram[_AT], BufferID, Buffer[_AT]], Gene """Reassembly for IP payload. Args: - strict: if return all datagrams (including those not - implemented) when submit + strict: if :data:`True`, report a datagram that is not completely + reassembled as the tuple of its received runs; otherwise as one + contiguous payload, its holes zero-filled -- or, while the total + length is unknown, only the prefix up to the first hole store: if store reassembled datagram in memory, i.e., :attr:`self._dtgram ` (if not, datagram will be discarded after callback) @@ -147,7 +149,7 @@ def reassembly(self, info: 'Packet[_AT]') -> 'None': # TCP's first-write-wins (:rfc:`9293#section-3.10`) -- so the arriving # payload always overwrites here; ``conflicts`` above is what records # that it *disagreed* with what it overwrote, which is the part RFC 791 - # leaves unrecorded and this fix adds. + # leaves unrecorded. buf.datagram[start:stop] = info.payload # set RCVBT bits (in 8 octets) @@ -318,8 +320,8 @@ def submit(self, buf: 'Buffer[_AT]', *, bufid: 'tuple[_AT, _AT, int, TransType]' if TDL > 0: # The length is known. Report it, holes and all: the gaps read as # zeros, exactly as they do in the TCP reassembler's loose mode. - # This is unchanged behaviour, and the reason ``strict=False`` - # exists -- a caller who wants the gaps *marked* rather than + # This is the reason ``strict=False`` exists -- a caller who + # wants the gaps *marked* rather than # zero-filled uses ``strict=True`` and gets the runs. stop = TDL else: diff --git a/pcapkit/foundation/reassembly/ipv4.py b/pcapkit/foundation/reassembly/ipv4.py index ad1e7cd1d3..88858324cc 100644 --- a/pcapkit/foundation/reassembly/ipv4.py +++ b/pcapkit/foundation/reassembly/ipv4.py @@ -23,8 +23,10 @@ class IPv4(IP): """Reassembly for IPv4 payload. Args: - strict: if return all datagrams (including those not - implemented) when submit + strict: if :data:`True`, report a datagram that is not completely + reassembled as the tuple of its received runs; otherwise as one + contiguous payload, its holes zero-filled -- or, while the total + length is unknown, only the prefix up to the first hole store: if store reassembled datagram in memory, i.e., :attr:`self._dtgram ` (if not, datagram will be discarded after callback) diff --git a/pcapkit/foundation/reassembly/ipv6.py b/pcapkit/foundation/reassembly/ipv6.py index e89ba2e97c..184ca8acbf 100644 --- a/pcapkit/foundation/reassembly/ipv6.py +++ b/pcapkit/foundation/reassembly/ipv6.py @@ -75,8 +75,10 @@ class IPv6(IP): """Reassembly for IPv6 payload. Args: - strict: if return all datagrams (including those not - implemented) when submit + strict: if :data:`True`, report a datagram that is not completely + reassembled as the tuple of its received runs; otherwise as one + contiguous payload, its holes zero-filled -- or, while the total + length is unknown, only the prefix up to the first hole store: if store reassembled datagram in memory, i.e., :attr:`self._dtgram ` (if not, datagram will be discarded after callback) diff --git a/pcapkit/foundation/reassembly/reassembly.py b/pcapkit/foundation/reassembly/reassembly.py index 60e105420a..35ef8de0d4 100644 --- a/pcapkit/foundation/reassembly/reassembly.py +++ b/pcapkit/foundation/reassembly/reassembly.py @@ -110,8 +110,10 @@ class ReassemblyBase(Generic[_PT, _DT, _IT, _BT], metaclass=ReassemblyMeta): """Base class for reassembly procedure. Args: - strict: if return all datagrams (including those not - implemented) when submit + strict: if :data:`True`, report a datagram that is not completely + reassembled as the tuple of its received runs; otherwise as one + contiguous payload, its holes zero-filled -- or, for IP while the + total length is unknown, only the prefix up to the first hole store: if store reassembled datagram in memory, i.e., :attr:`self._dtgram <_dtgram>` (if not, datagram will be discarded after callback) @@ -401,7 +403,7 @@ def register(cls, callback: 'CallbackFn', *, index: 'Optional[int]' = None) -> ' callback: callback function, which will be called when reassembled datagram is obtained, with the list of reassembled datagrams as its only argument - index: index to be inserted in the callback list,; by + index: index to be inserted in the callback list; by default, the callback will be appended to the end of the list @@ -429,8 +431,10 @@ def __init__(self, *, strict: 'bool' = True, store: 'bool' = True, """Initialise packet reassembly. Args: - strict: if return all datagrams (including those not - implemented) when submit + strict: if :data:`True`, report a datagram that is not completely + reassembled as the tuple of its received runs; otherwise as one + contiguous payload, its holes zero-filled -- or, for IP while the + total length is unknown, only the prefix up to the first hole store: if store reassembled datagram in memory, i.e., :attr:`self._dtgram <_dtgram>` (if not, datagram will be discarded after callback) @@ -443,10 +447,11 @@ def __init__(self, *, strict: 'bool' = True, store: 'bool' = True, FieldValueError: If ``timeout`` is negative. """ - #: bool: Strict mode flag. If set to :data:`True`, all - #: data will be returned, including those not completely - #: reassembled; otherwise, only completely reassembled - #: data will be returned. + #: bool: Strict mode flag. If set to :data:`True`, a datagram + #: not completely reassembled is reported as the tuple of its + #: received runs; otherwise, as one contiguous payload, its + #: holes zero-filled -- or, for IP while the total length is + #: unknown, only the prefix up to the first hole. self._flag_s = strict #: bool: Store mode flag. If set to :data:`True`, all #: reassembled datagram will be stored in memory, i.e., @@ -526,8 +531,10 @@ class MyMixin(Reassembly): # not registered Extractor.register_reassembly('my_mixin', MyMixin) Arguments: - strict: if return all datagrams (including those not - implemented) when submit + strict: if :data:`True`, report a datagram that is not completely + reassembled as the tuple of its received runs; otherwise as one + contiguous payload, its holes zero-filled -- or, for IP while the + total length is unknown, only the prefix up to the first hole store: if store reassembled datagram in memory, i.e., :attr:`self._dtgram <_dtgram>` (if not, datagram will be discarded after callback) @@ -556,7 +563,7 @@ def __init_subclass__(cls, /, protocol: 'Optional[str]' = None, *args: 'Any', ** rather than having to inherit :class:`ReassemblyBase` to avoid it, and it matches :meth:`EnumSchema.__init_subclass__ `, which - has guarded on its own ``code`` keyword all along. + guards on its own ``code`` keyword the same way. Note: :attr:`__protocol_name__` is *not* an opt-in. It supplies the @@ -570,10 +577,10 @@ def __init_subclass__(cls, /, protocol: 'Optional[str]' = None, *args: 'Any', ** """ # NOTE: the keyword here is ``protocol``, but ``Engine`` spells the same - # idea ``name`` -- so guessing ``name=`` by analogy is the expected - # mistake, not a careless one. Left in ``**kwargs`` it would be - # dropped by the bare ``super().__init_subclass__()`` below, leaving the - # class registered under its own class name: no exception, no warning. + # idea ``engine`` -- so guessing the other base's keyword, or ``name=``, + # is the expected mistake, not a careless one. Left in ``**kwargs`` it + # would be dropped by the bare ``super().__init_subclass__()`` below, + # leaving the class silently unregistered: no exception, no warning. # See the sibling note in ``Engine.__init_subclass__``. if args or kwargs: unexpected = ', '.join([*map(repr, args), *sorted(kwargs)]) diff --git a/pcapkit/foundation/reassembly/tcp.py b/pcapkit/foundation/reassembly/tcp.py index 3b256bd1b3..38e1556faa 100644 --- a/pcapkit/foundation/reassembly/tcp.py +++ b/pcapkit/foundation/reassembly/tcp.py @@ -5,7 +5,7 @@ .. module:: pcapkit.foundation.reassembly.tcp :mod:`pcapkit.foundation.reassembly.tcp` contains -:class:`~pcapkit.foundation.reassembly.reassembly.Reassembly` only, +:class:`~pcapkit.foundation.reassembly.tcp.TCP` only, which reconstructs fragmented TCP packets back to origin. """ @@ -29,8 +29,9 @@ class TCP(ReassemblyBase[Packet, Datagram, BufferID, Buffer]): """Reassembly for TCP payload. Args: - strict: if return all datagrams (including those not - implemented) when submit + strict: if :data:`True`, report a datagram that is not completely + reassembled as the tuple of its received runs; otherwise as one + contiguous payload, its holes zero-filled store: if store reassembled datagram in memory, i.e., :attr:`self._dtgram ` (if not, datagram will be discarded after callback) @@ -393,7 +394,7 @@ def _merge_overlap(gap: 'list[tuple[int, int]]', old: 'bytes', new: 'bytes', new: the arriving segment's bytes over the same range. start: absolute sequence number of ``old[0]``/``new[0]``, which cover the same range by construction -- see the two call - sites in :meth:`reassembly`. + sites, :meth:`_reassemble_append` and :meth:`_reassemble_prepend`. Returns: The bytes to keep for the range, and any ``(first, last)`` diff --git a/pcapkit/foundation/registry/protocols.py b/pcapkit/foundation/registry/protocols.py index f218d7d22e..f9b59ac297 100644 --- a/pcapkit/foundation/registry/protocols.py +++ b/pcapkit/foundation/registry/protocols.py @@ -178,8 +178,7 @@ class rather than supplied by a caller, and this function is the funnel :exc:`~pcapkit.utilities.warnings.RegistryWarning` wholesale, and that filter would then hide the ``HTTP`` collision this warning exists to surface. The sibling ``register`` methods across the package, each keyed on a - caller-supplied ``code``, apply the same identity criterion as of GitHub - issue :issue:`718`. + caller-supplied ``code``, apply the same identity criterion (:issue:`718`). Making the key itself unique would resolve the collision rather than merely reporting it, but it is a registry-format change that the bare-name @@ -240,11 +239,11 @@ class rather than supplied by a caller, and this function is the funnel #: Enum type -> the class(es) owning the :attr:`ProtocolBase.__proto__ #: ` dispatch registry #: keyed by that enum type -- the "registry-of-registries" that lets -#: ``code=`` infer a destination from a key's own type, per :issue:`514`. This is -#: not an invention: it is exactly the targeting +#: ``code=`` infer a destination from a key's own type, per :issue:`514`. It is +#: the same targeting #: :func:`register_ethertype`, :func:`register_transtype`, -#: :func:`register_linktype` and :func:`register_sctp` already hard-code by -#: hand, moved into one table so :func:`register_protocol_code` can consult +#: :func:`register_linktype` and :func:`register_sctp` hard-code by +#: hand, in one table so :func:`register_protocol_code` can consult #: it. ``LinkType`` naming two classes is deliberate, not ambiguous -- #: :func:`register_linktype` already fans out to both. _CODE_DESTINATIONS: 'dict[type, tuple[Type[ProtocolBase], ...]]' = { @@ -892,9 +891,9 @@ def register_apptype(code: 'int | Enum_AppType', module: 'str | ModuleDescriptor # anything unrecognised, composite-spelled or not, and ``__getitem__`` # raises a bare :exc:`KeyError` on a miss instead of that -- both # ``TransportProtocol['tcp|udp']`` and ``TransportProtocol['bogus']`` do, - # since GitHub issue #808 dropped the ``IntFlag`` base, which made the - # first of those two silently compose into the value ``3`` rather than - # miss at all. ``__members__.get(...)`` lets this function + # because ``TransportProtocol`` is not an ``IntFlag``, under which the + # first would silently compose into the value ``3`` (#808). + # ``__members__.get(...)`` lets this function # raise its own exception on a miss instead of letting ``__getitem__``'s # propagate, and lowercasing does not turn ``'tcp|udp'`` into a member # name either way. Anything that is neither a ``str`` nor a @@ -1140,7 +1139,7 @@ def register_sctp(code: 'int | SCTP_PayloadProtocolIdentifier', module: 'str | M # NOTE: pcapkit.protocols.application.httpv2.HTTPv2.__frame__ def register_http_frame(code: 'HTTP_Frame', meth: 'str | tuple[HTTP_FrameParser, HTTP_FrameConstructor]', *, schema: 'Optional[Type[Schema_HTTP_FrameType]]' = None) -> 'None': - """Registered a frame parser. + """Register a frame parser. The function will register the given frame parser to the :data:`pcapkit.protocols.application.httpv2.HTTP.__frame__` registry. @@ -1170,13 +1169,13 @@ def register_http_frame(code: 'HTTP_Frame', meth: 'str | tuple[HTTP_FrameParser, # NOTE: pcapkit.protocols.misc.pcapng.PCAPNG.__block__ def register_pcapng_block(code: 'PCAPNG_BlockType', meth: 'str | tuple[PCAPNG_BlockParser, PCAPNG_BlockConstructor]', *, schema: 'Optional[Type[Schema_PCAPNG_BlockType]]' = None) -> 'None': - """Registered a block parser. + """Register a block parser. The function will register the given block parser to the :data:`pcapkit.protocols.misc.pcapng.PCAPNG.__block__` registry. Args: - code: :class:`HTTP/2 ` block type + code: :class:`PCAPNG ` block type code as in :class:`~pcapkit.const.pcapng.block_type.BlockType`. meth: Method name or callable to parse and/or construct the block. schema: :class:`~pcapkit.protocols.schema.schema.Schema` class for the block. @@ -1195,7 +1194,7 @@ def register_pcapng_block(code: 'PCAPNG_BlockType', meth: 'str | tuple[PCAPNG_Bl # NOTE: pcapkit.protocols.misc.pcapng.PCAPNG.__option__ def register_pcapng_option(code: 'PCAPNG_OptionType', meth: 'str | tuple[PCAPNG_OptionParser, PCAPNG_OptionConstructor]', *, schema: 'Optional[Type[Schema_PCAPNG_Option]]' = None) -> 'None': - """Registered a option parser. + """Register an option parser. The function will register the given option parser to the :data:`pcapkit.protocols.misc.pcapng.PCAPNG.__option__` registry. @@ -1220,7 +1219,7 @@ def register_pcapng_option(code: 'PCAPNG_OptionType', meth: 'str | tuple[PCAPNG_ # NOTE: pcapkit.protocols.misc.pcapng.PCAPNG.__record__ def register_pcapng_record(code: 'PCAPNG_RecordType', meth: 'str | tuple[PCAPNG_RecordParser, PCAPNG_RecordConstructor]', *, schema: 'Optional[Type[Schema_PCAPNG_NameResolutionRecord]]' = None) -> 'None': - """Registered a name resolution record parser. + """Register a name resolution record parser. The function will register the given name resolution record parser to the :data:`pcapkit.protocols.misc.pcapng.PCAPNG.__record__` registry. @@ -1246,7 +1245,7 @@ def register_pcapng_record(code: 'PCAPNG_RecordType', meth: 'str | tuple[PCAPNG_ # NOTE: pcapkit.protocols.misc.pcapng.PCAPNG.__secrets__ def register_pcapng_secrets(code: 'PCAPNG_SecretsType', meth: 'str | tuple[PCAPNG_SecretsParser, PCAPNG_SecretsConstructor]', *, schema: 'Optional[Type[Schema_PCAPNG_DSBSecrets]]' = None) -> 'None': - """Registered a decryption secrets parser. + """Register a decryption secrets parser. The function will register the given decryption secrets parser to the :data:`pcapkit.protocols.misc.pcapng.PCAPNG.__secrets__` registry. diff --git a/pcapkit/foundation/traceflow/__init__.py b/pcapkit/foundation/traceflow/__init__.py index 5cc34294ab..778c80cf09 100644 --- a/pcapkit/foundation/traceflow/__init__.py +++ b/pcapkit/foundation/traceflow/__init__.py @@ -38,7 +38,7 @@ class TraceFlowManager(Info): """TraceFlow Manager.""" - #: TCP reassembly. + #: TCP flow tracing. tcp: 'TCP_TraceFlow' if TYPE_CHECKING: diff --git a/pcapkit/foundation/traceflow/data/data.py b/pcapkit/foundation/traceflow/data/data.py index 90c16e2617..46e9fdcb0b 100644 --- a/pcapkit/foundation/traceflow/data/data.py +++ b/pcapkit/foundation/traceflow/data/data.py @@ -19,7 +19,9 @@ class Deferred: """A postponed reassembly of a traced flow's application layer. A traced flow's ``packet`` is the application-layer payload of the - conversation, one datagram per direction. Producing it means reassembling the + conversation, as reassembled datagrams -- one per direction and + acknowledgement number, see :class:`~pcapkit.foundation.traceflow.tcp.TCP`. + Producing it means reassembling the stream, which is neither free nor wanted by most callers of a *tracer* -- so the flow keeps the reassembler it was fed and this holds it until somebody reads @@ -31,7 +33,7 @@ class Deferred: with it. That one postpones a single ``analyze()`` call over bytes already in hand; this postpones a *submit* over a reassembler's buffers. The two subpackages are siblings and neither should depend on the other, so the - twenty lines are written twice rather than one importing the other -- the + few lines are written twice rather than one importing the other -- the same reason the two ``data/data.py`` modules mirror each other instead of merging. @@ -51,8 +53,8 @@ def __call__(self) -> 'tuple[TCP_Datagram, ...]': """Run the postponed reassembly. Returns: - One reassembled datagram per direction of the conversation. Each - carries its *own* postponed analysis in + The conversation's reassembled datagrams, one per direction and + acknowledgement number. Each carries its *own* postponed analysis in :attr:`Datagram.packet `, so parsing the payload as an application-layer protocol is still not paid for until that is read in turn. diff --git a/pcapkit/foundation/traceflow/data/tcp.py b/pcapkit/foundation/traceflow/data/tcp.py index 42d9590fe3..6511799903 100644 --- a/pcapkit/foundation/traceflow/data/tcp.py +++ b/pcapkit/foundation/traceflow/data/tcp.py @@ -142,9 +142,7 @@ class Buffer(Info, Generic[_AT]): # NOTE: one line, however long. ``# pylint: disable`` is *line*-scoped and # ``unused-argument`` is reported against the ``def``, so wrapping the # signature leaves every parameter on a continuation line outside the - # disable's reach -- which is why the shorter form this replaces leaked - # three ``unused-argument`` messages of its own. Every other data model in - # :mod:`pcapkit` writes these stubs on one line for the same reason. + # disable's reach. def __init__(self, fpout: 'Dumper', index: 'list[int]', label: 'str', origin: 'tuple[_AT, int]', forward: 'list[int]', reverse: 'list[int]', fin: 'set[tuple[_AT, int]]', reset: 'bool', reassembly: 'Optional[TCP_Reassembly]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long @@ -178,9 +176,9 @@ class Index(DeferredPacket, Info): #: Empty when tracing unidirectionally, in which case #: :attr:`forward` ``==`` :attr:`index`. reverse: 'tuple[int, ...]' - #: The conversation's **application layer**: one reassembled datagram per - #: direction, or :data:`None` when the tracer was not asked for it - #: (``analyse=False``, the default). + #: The conversation's **application layer**: its reassembled datagrams, one + #: per direction and acknowledgement number, or :data:`None` when the tracer + #: was not asked for it (``analyse=False``, the default). #: #: Reassembled on the first read, not when the flow is finalised, and each #: datagram's own diff --git a/pcapkit/foundation/traceflow/tcp.py b/pcapkit/foundation/traceflow/tcp.py index e655ca2b6a..d61474c02d 100644 --- a/pcapkit/foundation/traceflow/tcp.py +++ b/pcapkit/foundation/traceflow/tcp.py @@ -73,10 +73,8 @@ class TCP(TraceFlowBase[BufferID, Buffer[_AT], Index, Packet[_AT]], Generic[_AT] Finalising a flow is also when its callbacks run, so they run against the whole conversation rather than a truncated one. - Pass ``bidirectional=False`` for the older per-direction behaviour: a flow - is one direction, and closes on that direction's FIN. That mode reproduces - what flow tracing did before conversations became one flow, RST included -- - which is to say it ignores RST, as it always did. + Pass ``bidirectional=False`` to trace each direction as a flow of its + own, finalised on that direction's FIN. That mode ignores RST. Note: With ``analyse=True`` a flow also carries its **application layer**: @@ -147,9 +145,9 @@ class TCP(TraceFlowBase[BufferID, Buffer[_AT], Index, Packet[_AT]], Generic[_AT] # Defaults. ########################################################################## - #: Protocol name of current reassembly object. + #: Protocol name of current flow tracing object. __protocol_name__ = 'TCP' - #: Protocol of current reassembly object. + #: Protocol of current flow tracing object. __protocol_type__ = TCP_Protocol ########################################################################## @@ -226,11 +224,11 @@ def trace(self, packet: 'Packet[_AT]', *, output: 'bool' = False) -> 'Dumper | s .. code-block:: python - f'{packet.src}_{packet.srcport}-{packet.dst}_{info.dstport}-{packet.timestamp}' + f'{packet.src}_{packet.srcport}-{packet.dst}_{packet.dstport}-{packet.timestamp}' It is built from the packet that **opened** the flow, not from the - canonical buffer ID, so the label still names the initiator first and - reads the way it always did. The reverse half of a bidirectional + canonical buffer ID, so the label names the initiator first. The + reverse half of a bidirectional conversation joins that flow rather than minting a label of its own. """ @@ -302,7 +300,7 @@ def trace(self, packet: 'Packet[_AT]', *, output: 'bool' = False) -> 'Dumper | s label = buffer.label # A *unidirectional* flow is one direction, and a direction is over when - # its FIN goes out -- so that mode keeps closing on FIN exactly as it did. + # its FIN goes out -- so that mode closes on FIN. # # A bidirectional flow is a whole connection, and observing its teardown # is not the same as knowing nothing more will arrive. The four-way close @@ -337,7 +335,7 @@ def _make_reassembly(self) -> 'Optional[TCP_Reassembly]': shared instance would have to be asked for its datagrams by endpoint, and flushing it early would finalise flows that are still open. - It is constructed with ``strict=False`` so that each direction comes back + It is constructed with ``strict=False`` so that each datagram comes back as one contiguous payload, which is what an application-layer parse needs, and it inherits :attr:`TCP.__timeout__ ` diff --git a/pcapkit/foundation/traceflow/traceflow.py b/pcapkit/foundation/traceflow/traceflow.py index 3490b91eca..c4c72d6e57 100644 --- a/pcapkit/foundation/traceflow/traceflow.py +++ b/pcapkit/foundation/traceflow/traceflow.py @@ -128,12 +128,12 @@ class TraceFlowBase(Generic[_DT, _BT, _IT, _PT], metaclass=TraceFlowMeta): """ if TYPE_CHECKING: - #: Protocol name of current reassembly object. + #: Protocol name of current flow tracing object. __protocol_name__: 'str' - #: Protocol of current reassembly object. + #: Protocol of current flow tracing object. __protocol_type__: 'Type[ProtocolBase]' - #: List of callback functions upon reassembled datagram. + #: List of callback functions upon a finalised flow. __callback_fn__: 'list[CallbackFn]' # Internal data storage for cached properties. @@ -193,7 +193,7 @@ def protocol(self) -> 'Type[ProtocolBase]': @property def index(self) -> 'tuple[_IT, ...]': - """_IT table for traced flow.""" + """Index table for traced flows.""" if self._buffer: return self.submit() return tuple(self._stream) @@ -213,16 +213,16 @@ def register_dumper(cls, format: 'str', dumper: 'ModuleDescriptor[Dumper] | Type The overwrite guard fires only when the incumbent dumper differs from the replacement, so re-registering the exact same object is a silent no-op rather than a warning about nothing displaced -- - the identity guard GitHub issue :issue:`718` gave the code-keyed - registrars, extended here by GitHub issue :issue:`739`. ``__output__`` + the identity guard of the code-keyed registrars (:issue:`718`, + :issue:`739`). ``__output__`` maps each format to a ``(dumper, ext)`` pair, so the identity check compares the incumbent *dumper* (index ``0``), not the pair -- a re-registration that only changes ``ext`` is still identity-equal on the dumper and stays silent, since the dumper is what "the same object" means here, not the pair as a whole. ``__output__`` is also a :class:`collections.defaultdict`, - unlike the other three - registrars this issue touches; :meth:`dict.get` does not invoke + unlike the :class:`~pcapkit.foundation.extraction.Extractor` engine, + reassembly and flow tracing registries; :meth:`dict.get` does not invoke the default factory the way ``cls.__output__[format]`` would, so it stays non-inserting here as well. @@ -250,9 +250,11 @@ def register_callback(cls, callback: 'CallbackFn', *, index: 'Optional[int]' = N Arguments: callback: callback function, which will be called - when reassembled datagram is obtained, with the - list of reassembled datagrams as its only argument - index: index of datagram to be called + when a traced flow is finalised, with the flow's + index entry (:term:`trace.tcp.index`) as its only argument + index: index to be inserted in the callback list; by + default, the callback will be appended to the end + of the list """ if index is not None: @@ -273,10 +275,12 @@ def make_fout(cls, fout: 'str' = './tmp', fmt: 'str' = 'pcap') -> 'tuple[Type[Du Warns: FormatWarning: If ``fmt`` is not supported. - FileWarning: If ``fout`` exists and ``fmt`` is :data:`None`. + FileWarning: If ``fout`` exists but is not a directory, and ``fmt`` + is not supported. Raises: - FileExists: If ``fout`` exists and ``fmt`` is **NOT** :data:`None`. + FileExists: If ``fout`` exists but is not a directory, and ``fmt`` + is supported. """ output, ext = cls.__output__[fmt] @@ -347,8 +351,7 @@ def finish(self) -> 'None': superseded can be said to be over. The base implementation does nothing, so a tracer that has no such notion - -- or an existing third-party subclass that predates this method -- keeps - working unchanged. :meth:`submit` must remain able to report a flow that + need not override it. :meth:`submit` must remain able to report a flow that was never finalised, since nothing guarantees this is called: a tracer driven directly rather than through an :class:`~pcapkit.foundation.extraction.Extractor` never sees an end of @@ -387,8 +390,8 @@ def __init__(self, fout: 'Optional[str]', format: 'Optional[str]', # pylint: di bidirectional: whether the two halves of a conversation are one flow. :data:`True` -- the default -- keys a flow on the *pair* of endpoints rather than on (source, destination), so a connection - is traced as the one thing it is; pass :data:`False` for the - older per-direction behaviour. + is traced as the one thing it is; pass :data:`False` to trace + each direction as a flow of its own. analyse: whether to reassemble each flow's application layer, so that its ``packet`` can be read. Off by default: it buffers every traced payload, a cost tracing does not otherwise pay. @@ -510,7 +513,7 @@ def __init_subclass__(cls, /, protocol: 'Optional[str]' = None, *args: 'Any', ** rather than having to inherit :class:`TraceFlowBase` to avoid it, and it matches :meth:`EnumSchema.__init_subclass__ `, which - has guarded on its own ``code`` keyword all along. + guards on its own ``code`` keyword the same way. Note: :attr:`__protocol_name__` is *not* an opt-in. It supplies the @@ -524,10 +527,10 @@ def __init_subclass__(cls, /, protocol: 'Optional[str]' = None, *args: 'Any', ** """ # NOTE: the keyword here is ``protocol``, but ``Engine`` spells the same - # idea ``name`` -- so guessing ``name=`` by analogy is the expected - # mistake, not a careless one. Left in ``**kwargs`` it would be - # dropped by the bare ``super().__init_subclass__()`` below, leaving the - # class registered under its own class name: no exception, no warning. + # idea ``engine`` -- so guessing the other base's keyword, or ``name=``, + # is the expected mistake, not a careless one. Left in ``**kwargs`` it + # would be dropped by the bare ``super().__init_subclass__()`` below, + # leaving the class silently unregistered: no exception, no warning. # See the sibling note in ``Engine.__init_subclass__``. if args or kwargs: unexpected = ', '.join([*map(repr, args), *sorted(kwargs)])